SteNicholas commented on code in PR #3724:
URL: https://github.com/apache/celeborn/pull/3724#discussion_r3410790644
##########
cpp/celeborn/client/ShuffleClient.cpp:
##########
@@ -1096,6 +1176,22 @@ std::optional<std::unordered_map<int, int>>
ShuffleClientImpl::reviveBatch(
case protocol::StatusCode::SUCCESS: {
partitionLocationMap->set(
partitionInfo.partitionId, partitionInfo.partition);
+ // Revive moved this partition off the failed worker(s); drop the
+ // push exclusion on both the old and new locations.
+ if (pushExcludeWorkerOnFailureEnabled_) {
+ if (auto oldIter = oldLocationMap.find(partitionInfo.partitionId);
+ oldIter != oldLocationMap.end() && oldIter->second) {
+ pushExcludedWorkers_.erase(oldIter->second->hostAndPushPort());
Review Comment:
Good catch — fixed in e5320e1ef. Moved the old-location push-exclusion
removal out of the `SUCCESS` case to the top of the loop, so it runs for every
revive status (e.g. `SLOT_NOT_AVAILABLE` / `RESERVE_SLOTS_FAILED`) and before
the early returns, matching Java's removal that sits before the status check.
New-location + peer removal stays under `SUCCESS`.
--
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]
