mridulm commented on code in PR #37922:
URL: https://github.com/apache/spark/pull/37922#discussion_r1067741652


##########
core/src/main/scala/org/apache/spark/storage/BlockManagerMasterEndpoint.scala:
##########
@@ -321,6 +321,12 @@ class BlockManagerMasterEndpoint(
   }
 
   private def removeShuffle(shuffleId: Int): Future[Seq[Boolean]] = {
+    val mergerLocations =
+      if (Utils.isPushBasedShuffleEnabled(conf, isDriver)) {
+        mapOutputTracker.getShufflePushMergerLocations(shuffleId)
+      } else {
+        Seq.empty[BlockManagerId]
+      }

Review Comment:
   To give context, `mapOutputTrackerMaster.unregisterShuffle` happens after 
`shuffleDriverComponents.removeShuffle` - and BlockManagerMaster.removeShuffle 
does an inf wait for `RemoveShuffle` to complete.
   So we should have non-empty mergers, since `shuffleStatuses` has not yet 
been cleaned up for the shuffle id.
   
   The `RemoveShuffle` from within `master.removeShuffle` sends `RemoveShuffle` 
to workers - where it is simply cleanup the client side maps.
   
   Let me know if I am missing something !



-- 
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-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to