Ngone51 commented on a change in pull request #33617:
URL: https://github.com/apache/spark/pull/33617#discussion_r688198216



##########
File path: 
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java
##########
@@ -513,12 +511,17 @@ public MergeStatuses 
finalizeShuffleMerge(FinalizeShuffleMerge msg) {
       msg.shuffleId, msg.shuffleMergeId, msg.appId, msg.appAttemptId);
     AppShuffleInfo appShuffleInfo = validateAndGetAppShuffleInfo(msg.appId);
     if (appShuffleInfo.attemptId != msg.appAttemptId) {
-      // If this Block belongs to a former application attempt, it is 
considered late,
-      // as only the blocks from the current application attempt will be merged
-      // TODO: [SPARK-35548] Client should be updated to handle this error.
+      // If finalizeShuffleMerge from a former application attempt, it is 
considered late,
+      // as only the finalizeShuffleMerge request from the current application 
attempt will be merged.
+      // Too old app attempt only being seen by an already failed app attempt, 
and no need use callback to
+      // return to client now, because the finalizeShuffleMerge in 
DAGScheduler has no retry policy,
+      // and don't use the BlockPushNonFatalFailure because it's the 
finalizeShuffleMerge related case,
+      // not the block push case, just throw it in server side now.
+      // TODO we may use a new exception class to include the 
finalizeShuffleMerge related case
+      // just as the BlockPushNonFatalFailure contains the block push cases.

Review comment:
       @zhuqi-lucas  Oh, by "revert", I meant that we don't need to handle the 
`BlockPushNonFatalFailure` specifically but throwing the 
`BlockPushNonFatalFailure` here is fine.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to