[GitHub] [spark] vanzin commented on a change in pull request #23842: [SPARK-26927]Fix race condition may cause dynamic allocation not working

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

vanzin commented on a change in pull request #23842: [SPARK-26927]Fix race condition may cause dynamic allocation not working
URL: https://github.com/apache/spark/pull/23842#discussion_r262660766
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
 ##########
 @@ -725,10 +740,15 @@ private[spark] class ExecutorAllocationManager(
         if (stageIdToNumRunningTask.contains(stageId)) {
           stageIdToNumRunningTask(stageId) += 1
         }
-        // This guards against the race condition in which the `SparkListenerTaskStart`
-        // event is posted before the `SparkListenerBlockManagerAdded` event, which is
-        // possible because these events are posted in different threads. (see SPARK-4951)
-        if (!allocationManager.executorIds.contains(executorId)) {
+        // This guards against the following race condition:
+        // 1. The `SparkListenerTaskStart` event is posted before the
+        // `SparkListenerExecutorAdded` event
+        // 2. The `SparkListenerExecutorRemoved` event is posted before the
+        // `SparkListenerTaskStart` event
+        // Above cases are possible because these events are posted in different threads.
+        // (see SPARK-4951 SPARK-26927)
+        if (!allocationManager.executorIds.contains(executorId) &&
+          !allocationManager.removedExecutorIds.contains(executorId)) {
 
 Review comment:
   I wonder if you couldn't achieve the same here by asking the scheduler backend whether the executor is known? `SparkListenerExecutorRemoved` is posted after the executor is removed from the internal state, so as far as I can see that would be the same check without needing to keep the extra state here.

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


With regards,
Apache Git Services

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