Github user squito commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14079#discussion_r69910685
  
    --- Diff: 
yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala
 ---
    @@ -125,8 +125,11 @@ private[spark] abstract class YarnSchedulerBackend(
        * This includes executors already pending or running.
        */
       override def doRequestTotalExecutors(requestedTotal: Int): Boolean = {
    +
    +    val nodeBlacklist: Set[String] = 
scheduler.blacklistTracker.nodeBlacklist()
    --- End diff --
    
    this is safe to call without a lock on the task scheduler because the 
nodeBlacklist is stored in an AtomicReference, which is set whenever the node 
blacklist changes.  We could just get a lock on the task scheduler here -- but 
I felt that it would just be harder to ensure correctness (not just in this 
change, but making sure the right lock was always held through future changes), 
and the only real cost is that we duplicate the set of nodes in the blacklist, 
which is hopefully very small.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Reply via email to