You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/06/03 06:43:31 UTC
[GitHub] [spark] mridulm commented on a diff in pull request #36734: [SPARK-38987][SHUFFLE] Throw FetchFailedException when merged shuffle blocks are corrupted and spark.shuffle.detectCorrupt is set to true
mridulm commented on code in PR #36734:
URL: https://github.com/apache/spark/pull/36734#discussion_r888654676
##########
core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala:
##########
@@ -1885,6 +1885,16 @@ private[spark] class DAGScheduler(
mapOutputTracker.
unregisterMergeResult(shuffleId, reduceId, bmAddress, Option(mapIndex))
}
+ } else {
+ // Unregister the merge result of <shuffleId, reduceId> if there is a FetchFailed event
+ // and is not a MetaDataFetchException which is signified by bmAddress being null
+ if (bmAddress != null
+ && bmAddress.executorId.equals(BlockManagerId.SHUFFLE_MERGER_IDENTIFIER)) {
Review Comment:
nit: Move the `&&` to the previous line.
##########
core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala:
##########
@@ -1885,6 +1885,16 @@ private[spark] class DAGScheduler(
mapOutputTracker.
unregisterMergeResult(shuffleId, reduceId, bmAddress, Option(mapIndex))
}
+ } else {
+ // Unregister the merge result of <shuffleId, reduceId> if there is a FetchFailed event
+ // and is not a MetaDataFetchException which is signified by bmAddress being null
+ if (bmAddress != null
+ && bmAddress.executorId.equals(BlockManagerId.SHUFFLE_MERGER_IDENTIFIER)) {
+ assert(pushBasedShuffleEnabled, "Pushed based shuffle needs to " +
+ "be enabled so that merge results are present.")
Review Comment:
`Pushed based shuffle` -> `Push based shuffle`
Change message to something like `Push based shuffle expected to be enabled when handling merge block fetch failure` ? To make it clear what we are expecting the condition to be for.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org