[NBS] Disk Agent: use Ask method to send Acquire/Release requests to the actor#6592
[NBS] Disk Agent: use Ask method to send Acquire/Release requests to the actor#6592sharpeye wants to merge 1 commit into
Conversation
sharpeye
added
the
blockstore
|
Note This is an automated comment that will be appended during run. Note All workloads for linux-x86_64-relwithdebinfo have completed. Tip Planned checks for linux-x86_64-relwithdebinfo. 🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1412s): all tests PASSED for commit ac4d9a2.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac4d9a214d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| [](const auto& future) | ||
| { | ||
| NCloud::NProto::TAcquireDevicesResponse response; | ||
| const auto& ev = future.GetValue(); |
There was a problem hiding this comment.
When an in-flight acquire/release is interrupted during actor shutdown, the old TPromiseActor converted TEvPoisonPill into an E_REJECTED protobuf response, but TActorSystem::Ask completes with an exception for non-matching events such as poison pills (contrib/ydb/library/actors/core/ask.cpp:46-52). This future.GetValue() then turns that into an exceptional backend future, and the TCP server subscribers also call GetValue() without an exception fallback (cloud/storage/core/libs/journalled_device_tcp_server/server.cpp:327/:347), so clients can hang or lose the protocol response instead of receiving the previous error response; please translate future.HasException() back into an error response here and in the release path.
Useful? React with 👍 / 👎.
2 participants
Replace the custom
TPromiseActorwithActorSystem::AskforAcquireDevicesandReleaseDevicesrequests.This reduces boilerplate without changing behavior.