[GitHub] [spark] tgravescs commented on a change in pull request #30164: [SPARK-32919][SHUFFLE] Driver side changes for coordinating push based shuffle by selecting external shuffle services for merging partitions

[GitBox <gi...@apache.org>]

tgravescs commented on a change in pull request #30164:
URL: https://github.com/apache/spark/pull/30164#discussion_r517674691



##########
File path: core/src/main/scala/org/apache/spark/storage/BlockManagerMasterEndpoint.scala
##########
@@ -657,6 +679,14 @@ class BlockManagerMasterEndpoint(
     }
   }
 
+  private def getMergerLocations(
+      numMergersNeeded: Int,
+      hostsToFilter: Set[String]): Seq[BlockManagerId] = {
+    // Copying the merger locations to a list so that the original mergerLocations won't be shuffled
+    val mergers = mergerLocations.values.filterNot(x => hostsToFilter.contains(x.host)).toSeq
+    Utils.randomize(mergers).take(numMergersNeeded)

Review comment:
       yeah the main thing I was thinking was you already have these executors so you don't have to worry about requesting them from YARN and them potentially being full or not available at that point.    If you have the same number of reducers as mappers then you don't even need to ask YARN for different executors as you potentially already have them.  Now you do obviously have the potential that you send the data to an executor and then it idle times out anyway because another map is the long tail and takes a long time.
   I get that the shuffle is trying to get away from hot spots and I obviously have no data on this. It would definitely be interesting to look at more though. Perhaps shuffle active ones, grab the number you need, if that isn't enough grab from  nonactive. That makes your tracking more complex though.  
   
   I think in your experiments you were getting pretty good locality in production right?  Its going to depend on how busy your clusters are though too. Perhaps this is just a file a separate issue to investigate.




----------------------------------------------------------------
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:
users@infra.apache.org



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