venkata91 commented on a change in pull request #34122:
URL: https://github.com/apache/spark/pull/34122#discussion_r785243416



##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -2487,6 +2501,21 @@ private[spark] class DAGScheduler(
       executorFailureEpoch -= execId
     }
     shuffleFileLostEpoch -= execId
+
+    if (pushBasedShuffleEnabled) {

Review comment:
       Briefly looked at this one, currently there isn't a way to communicate 
from `BlockManagerMasterEndpoint` to `DAGScheduler`. Also the state info is in 
3 different places, `DAGScheduler` querying merger locations, 
`YarnSchedulerBackend` having the logic of desired num mergers and passing the 
`excludeNodes` info to `BlockManagerMasterEndpoint`. 
`BlockManagerMasterEndpoint` itself preferring active executors hosts first and 
the remaining merger locations from the dead executors host.
   
   Apart from this the number of `ExecutorAdded` events should be much smaller 
than the `TaskStart and TaskEnd` events, so this shouldn't be any worse than 
what we have already. Let me know your thoughts.




-- 
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]

Reply via email to