yugan95 commented on code in PR #3801:
URL: https://github.com/apache/celeborn/pull/3801#discussion_r3781648102
##########
cpp/celeborn/network/MessageDispatcher.cpp:
##########
@@ -17,10 +17,34 @@
#include "celeborn/network/MessageDispatcher.h"
+#include <folly/ExceptionWrapper.h>
+
#include "celeborn/protocol/TransportMessage.h"
namespace celeborn {
namespace network {
+namespace {
+// Builds a retriable "connection closed" error. Sending on a closed connection
+// is a normal recoverable condition -- the peer closed the socket because of a
+// worker restart or an idle timeout -- not an invariant violation. Reporting
it
+// through the promise instead of asserting mirrors the Java client, where a
+// send on an inactive channel surfaces as a retriable IOException via
+// TransportResponseHandler#channelInactive -> failOutstandingRequests, which
+// CelebornInputStream then retries or fails over to the replica.
+folly::exception_wrapper makeConnectionClosedException(
+ const std::string& detail) {
+ return folly::make_exception_wrapper<utils::CelebornRuntimeError>(
+ __FILE__,
+ static_cast<size_t>(__LINE__),
+ __FUNCTION__,
+ /*expression=*/"connection closed",
+ /*message=*/detail,
+ utils::error_source::kErrorSourceRuntime.c_str(),
+ utils::error_code::kInvalidState.c_str(),
+ /*isRetriable=*/true);
+}
Review Comment:
Valid — every failure reported the helper's own location, so all of them
collapsed onto one line.
Fixed: the helper now takes `file`/`line`/`function` from the caller, and
each failing site (`operator()`, `sendFetchChunkRequest`, `failPendingRequest`,
`failPendingFetch`, `cleanup()`) passes its own
`__FILE__`/`__LINE__`/`__FUNCTION__`.
--
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]
