SteNicholas opened a new pull request, #3724:
URL: https://github.com/apache/celeborn/pull/3724

   ### What changes were proposed in this pull request?
   
   This PR aligns the C++ shuffle client's push/merge data path with the Java 
`ShuffleClient`:
   
   - Refactor `pushData`/`mergeData`/`pushMergedData` to share one push 
prologue (`prepareBatch`) and a uniform synchronous-failure route 
(`pushWithFailureRouting`), and move throttling (`limitMaxInFlight`) to the 
caller before the initial push so the retry path never re-throttles.
   - Exclude push workers on connection/timeout failures 
(`celeborn.client.push.excludeWorkerOnFailure.enabled`), routing pushes away 
from them until a successful revive or re-assignment lifts the exclusion, and 
clear it wholesale on shutdown.
   - Track push-failed batches and report them at `MapperEnd`; the reader 
dedups duplicate batches for the adaptive skewed-partition read optimization.
   - Derive specific failure causes from transport error messages 
(`getPushDataFailCause`) and propagate them through revive/retry; add 
`StatusCode::toString`.
   
   ### Why are the changes needed?
   
   The C++ shuffle client lacked the push-side resiliency the Java 
`ShuffleClient` already provides: excluding failed push workers and tracking 
data-push failures so the reader can dedup duplicate batches for the adaptive 
skewed-partition read optimization. Bringing the C++ push/merge data path to 
parity keeps the native and JVM clients behaviorally consistent and honors the 
existing `celeborn.client.push.excludeWorkerOnFailure.enabled` / adaptive 
skewed-partition read configs.
   
   ### Does this PR resolve a correctness bug?
   
   <!-- Check if yes. The `correctness` label will be added/removed 
automatically. -->
   - [ ] Yes
   
   ### Does this PR introduce _any_ user-facing change?
   
   <!-- Check if yes. -->
   - [ ] Yes
   
   
   ### How was this patch tested?
   
   Covered by new/extended C++ unit tests:
   
   - `PushDataCallbackTest`, `PushMergedDataCallbackTest` — push / merged-push 
callback failure routing and cause propagation.
   - `ShuffleClientImplTest` — push worker exclusion and data-push-failure 
tracking.
   - `CelebornInputStreamRetryTest` — reader-side dedup of duplicate 
push-failed batches.
   - `PushStateTest`, `ControlMessagesTest` — failed-batch state and 
`MapperEnd` serialization round-trip.
   
   Run via `ctest` under `cpp/celeborn/client/tests` and 
`cpp/celeborn/protocol/tests`.
   


-- 
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]

Reply via email to