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