SteNicholas commented on code in PR #3801:
URL: https://github.com/apache/celeborn/pull/3801#discussion_r3780892807
##########
cpp/celeborn/network/MessageDispatcher.cpp:
##########
@@ -163,6 +186,14 @@ folly::Future<std::unique_ptr<Message>>
MessageDispatcher::operator()(
}
}
+ // Fast path: the connection is already closed. Fail with a retriable error
+ // rather than asserting, so the caller's retry/failover logic can recover.
+ if (closed_.load()) {
+ return folly::makeFuture<std::unique_ptr<Message>>(
+ makeConnectionClosedException(fmt::format(
Review Comment:
**This retriable classification is discarded at the next public API layer.**
`TransportClient::sendRpcRequestSync` catches the exception and rethrows via
`CELEBORN_FAIL` (`isRetriable=false`), while the push and fetch `thenError`
handlers convert it to `std::runtime_error`. Consequently callers cannot
observe the `isRetriable=true` value asserted by these dispatcher-only tests.
Please preserve the `CelebornException` classification through
`TransportClient` and add API-level tests for the sync, push, and fetch paths.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
