Victsm commented on a change in pull request #30164:
URL: https://github.com/apache/spark/pull/30164#discussion_r523710562
##########
File path:
core/src/main/scala/org/apache/spark/storage/BlockManagerMasterEndpoint.scala
##########
@@ -657,6 +681,13 @@ class BlockManagerMasterEndpoint(
}
}
+ private def getShufflePushMergerLocations(
+ numMergersNeeded: Int,
+ hostsToFilter: Set[String]): Seq[BlockManagerId] = {
+ val mergers = shuffleMergerLocations.values.filterNot(x =>
hostsToFilter.contains(x.host)).toSeq
+ mergers.take(numMergersNeeded)
Review comment:
I thought the reason we didn't do that internally is because the merger
locations is picked at the beginning of the map stage and is only relevant with
executor placement of the following reduce stage.
Since there is no guarantee on whether the active executors at the beginning
of the map stage would still be active at the beginning of the reduce stage, we
didn't do that.
Is that reasoning not applicable any more?
----------------------------------------------------------------
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.
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
