-
Notifications
You must be signed in to change notification settings - Fork 738
Replace verify by ensure #30142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Replace verify by ensure #30142
Conversation
|
🟢 |
|
⚪
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR replaces Y_ABORT_UNLESS macro calls with AFL_ENSURE macro across multiple actor classes in the PersQueue service. The change aims to provide better exception handling and logging capabilities through the AFL_ENSURE macro which throws exceptions instead of aborting the process.
Key changes:
- Added
IActorExceptionHandlerinterface to actor classes to handle unhandled exceptions gracefully - Replaced all
Y_ABORT_UNLESScalls withAFL_ENSUREcalls across partition, commit offset, and direct read actors - Implemented
OnUnhandledExceptionmethods that log exceptions and gracefully shut down actors - Refactored logging macros in
actor.hto use a commonNPQ_LOG_PREFIXdefine
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/services/persqueue_v1/actors/partition_actor.h | Added IActorExceptionHandler interface and OnUnhandledException method declaration |
| ydb/services/persqueue_v1/actors/partition_actor.cpp | Replaced 84 instances of Y_ABORT_UNLESS with AFL_ENSURE and implemented exception handler |
| ydb/services/persqueue_v1/actors/direct_read_actor.h | Added IActorExceptionHandler interface and OnUnhandledException method declaration |
| ydb/services/persqueue_v1/actors/direct_read_actor.cpp | Replaced 7 instances of Y_ABORT_UNLESS with AFL_ENSURE and implemented exception handler |
| ydb/services/persqueue_v1/actors/commit_offset_actor.h | Added IActorExceptionHandler interface and OnUnhandledException method declaration |
| ydb/services/persqueue_v1/actors/commit_offset_actor.cpp | Replaced 5 instances of Y_ABORT_UNLESS with AFL_ENSURE and implemented exception handler |
| ydb/core/persqueue/common/actor.h | Added DoLogUnhandledException function declaration and refactored logging macros |
| ydb/core/persqueue/common/actor.cpp | Implemented DoLogUnhandledException helper function for centralized exception logging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| NPQ::DoLogUnhandledException(NKikimrServices::PQ_READ_PROXY, "", exc); | ||
|
|
||
| this->Die(ActorContext()); | ||
|
|
||
| return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А как мы узнаем, что что-то сломалось? Метрика числа исключений ведь не увидет, и падений тоже не будет.
IncrementUnhandledExceptionCounter звать может
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
обобщу в другом ПР
|
|
||
| bool TPartitionActor::OnUnhandledException(const std::exception& exc) { | ||
| NPQ::DoLogUnhandledException(NKikimrServices::PQ_READ_PROXY, TStringBuilder() << "[" << Session <<"][" << Partition << "] ", exc); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тоже до алерта дотянуть сигнал надо
Co-authored-by: ubyte <[email protected]>
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Changelog entry
...
Changelog category
Description for reviewers
...