ulysses-you commented on a change in pull request #35719:
URL: https://github.com/apache/spark/pull/35719#discussion_r819326181
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/ShuffledRowRDD.scala
##########
@@ -177,19 +177,36 @@ class ShuffledRowRDD(
val tracker =
SparkEnv.get.mapOutputTracker.asInstanceOf[MapOutputTrackerMaster]
partition.asInstanceOf[ShuffledRowRDDPartition].spec match {
case CoalescedPartitionSpec(startReducerIndex, endReducerIndex, _) =>
- // TODO order by partition size.
- startReducerIndex.until(endReducerIndex).flatMap { reducerIndex =>
- tracker.getPreferredLocationsForShuffle(dependency, reducerIndex)
- }
+ tracker.getPreferredLocationsForShuffle(
+ dependency,
+ 0,
+ dependency.rdd.getNumPartitions,
+ startReducerIndex,
+ endReducerIndex)
- case PartialReducerPartitionSpec(_, startMapIndex, endMapIndex, _) =>
- tracker.getMapLocation(dependency, startMapIndex, endMapIndex)
+ case PartialReducerPartitionSpec(reducerIndex, startMapIndex,
endMapIndex, _) =>
+ tracker.getPreferredLocationsForShuffle(
Review comment:
According to the original idea of preferred locations. It's more
effective to schedule task to a bigger data size node. And same explanation
with the comment of the threshold `REDUCER_PREF_LOCS_FRACTION`.
```scala
// Fraction of total map output that must be at a location for it to
considered as a preferred
// location for a reduce task. Making this larger will focus on fewer
locations where most data
// can be read locally, but may lead to more delay in scheduling if those
locations are busy.
```
If return locations of all blocks, the preferred locations will be less
meaning since the location can acutally contain less data and we can only
reduce a little data size with network traffic. One more probleam, store all
locations at driver side will cause OOM, and it get worser if we allocate more
executors.
So this pr just correct the behavior of preferred locations. But if you
think it's worth to return more locations, we can expose the
`REDUCER_PREF_LOCS_FRACTION` as a config (I think it should be another thread
?).
Hope it makes sense for you.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
