(spark) branch branch-4.x updated: [SPARK-59714][CORE] Align `TaskSchedulerImpl.executorDecommission` locking with `executorLost`

[do...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dongjoon-hyun pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-4.x by this push:
     new 5b218b5ec2dd [SPARK-59714][CORE] Align `TaskSchedulerImpl.executorDecommission` locking with `executorLost`
5b218b5ec2dd is described below

commit 5b218b5ec2dd0e98fe689a5738a8c2e3b96fdf34
Author: Dongjoon Hyun <do...@apache.org>
AuthorDate: Tue Sep 22 12:16:18 2026 -0700

    [SPARK-59714][CORE] Align `TaskSchedulerImpl.executorDecommission` locking with `executorLost`
    
    ### What changes were proposed in this pull request?
    
    This PR moves `rootPool.executorDecommission(executorId)` inside the `synchronized` block that
    `TaskSchedulerImpl.executorDecommission()` already has, keeping `backend.reviveOffers()` outside it.
    
    ### Why are the changes needed?
    
    `rootPool.executorDecommission()` reaches `TaskSetManager.recomputeLocality()`, and
    `TaskSchedulerImpl` documents that "TaskSetManagers are not thread safe, so any access to one should
    be synchronized on this class". The sibling `executorLost()` path already follows this: it calls
    `rootPool.executorLost()` inside the lock and keeps only the `dagScheduler` and `backend` calls
    outside. This makes the two paths consistent.
    
    There is no behavior change. The only production caller,
    `CoarseGrainedSchedulerBackend.decommissionExecutors()`, is `withLock`, which already holds this
    same monitor, so the call was effectively synchronized in practice. This PR makes the method
    self-contained instead of relying on its caller.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    Pass the CIs.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Opus 5
    
    Closes #58970 from dongjoon-hyun/SPARK-59714.
    
    Authored-by: Dongjoon Hyun <do...@apache.org>
    Signed-off-by: Dongjoon Hyun <do...@apache.org>
    (cherry picked from commit 582e28bf7074598ac85a0b7de9c5916c44cc5661)
    Signed-off-by: Dongjoon Hyun <do...@apache.org>
---
 .../main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala    | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala b/core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala
index 4600993d2664..d1eb646fc316 100644
--- a/core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala
+++ b/core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala
@@ -1059,8 +1059,11 @@ private[spark] class TaskSchedulerImpl(
         executorsPendingDecommission(executorId) =
           ExecutorDecommissionState(clock.getTimeMillis(), decommissionInfo.workerHost)
       }
+      // This reaches TaskSetManager.recomputeLocality(), and TaskSetManagers are not thread
+      // safe, so it must be called while holding the lock, like rootPool.executorLost() is.
+      rootPool.executorDecommission(executorId)
     }
-    rootPool.executorDecommission(executorId)
+    // Call backend.reviveOffers() without holding the lock on this to prevent deadlock
     backend.reviveOffers()
   }
 


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