shlomitubul commented on code in PR #3721:
URL: https://github.com/apache/celeborn/pull/3721#discussion_r3373568054
##########
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:
Good catch — fixed. The cancel branch no longer special-cases empty
committed lists: `COMMIT_FILE_EXCEPTION` is now returned only when nothing
committed **and** nothing is empty, and the failed list is always `requested −
committed − empty`, so empty-file partitions are never reported as failed
(matching the success path's treatment of empties). Added a unit test for the
empty-only case.
--
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]
