Copilot commented on code in PR #3721:
URL: https://github.com/apache/celeborn/pull/3721#discussion_r3370756072
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/Controller.scala:
##########
@@ -898,3 +905,48 @@ private[deploy] class Controller(
}
}
}
+
+private[deploy] object Controller {
+
+ def buildCommitFilesResponseOnCancel(
+ primaryIds: jList[String],
+ replicaIds: jList[String],
+ committedPrimaryIds: jSet[String],
+ committedReplicaIds: jSet[String],
+ emptyFilePrimaryIds: jSet[String],
+ emptyFileReplicaIds: jSet[String],
+ committedPrimaryStorageInfos: java.util.Map[String, StorageInfo],
+ committedReplicaStorageInfos: java.util.Map[String, StorageInfo],
+ committedMapIdBitMap: java.util.Map[String, RoaringBitmap],
+ partitionSizeList: java.util.Collection[Long]): CommitFilesResponse = {
+ if (committedPrimaryIds.isEmpty && committedReplicaIds.isEmpty) {
+ CommitFilesResponse(
+ StatusCode.COMMIT_FILE_EXCEPTION,
+ List.empty.asJava,
+ List.empty.asJava,
+ primaryIds,
+ replicaIds)
+ } else {
+ // Tasks still queued when cancellation fired reached no terminal set,
so failed must
+ // be requested - committed - empty: a partition absent from both
committed and failed
+ // is treated by the driver as empty-and-valid, which would silently
drop its data.
+ val failedPrimaryIds = new jArrayList[String](primaryIds)
+ failedPrimaryIds.removeAll(committedPrimaryIds)
+ failedPrimaryIds.removeAll(emptyFilePrimaryIds)
+ val failedReplicaIds = new jArrayList[String](replicaIds)
+ failedReplicaIds.removeAll(committedReplicaIds)
+ failedReplicaIds.removeAll(emptyFileReplicaIds)
+ CommitFilesResponse(
+ StatusCode.PARTIAL_SUCCESS,
+ new jArrayList[String](committedPrimaryIds),
Review Comment:
In `buildCommitFilesResponseOnCancel`, the `COMMIT_FILE_EXCEPTION` branch is
chosen when `committed*Ids` are empty, but empty partitions are tracked
separately in `emptyFile*Ids` (they’re a successful terminal state, just not
present in `committed*Ids`). If cancellation happens after some/all partitions
became empty, the current condition will incorrectly return
`COMMIT_FILE_EXCEPTION` and mark those empty partitions failed, forcing
unnecessary recompute.
--
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]
