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

    https://github.com/apache/spark/pull/17480#discussion_r111299488
  
    --- Diff: 
core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -249,7 +249,14 @@ private[spark] class ExecutorAllocationManager(
        * yarn-client mode when AM re-registers after a failure.
        */
       def reset(): Unit = synchronized {
    -    initializing = true
    +    /**
    +     * When some tasks need to be scheduled and initial executor = 0, 
resetting the initializing
    +     * field may cause it to not be set to false in yarn.
    +     * SPARK-20079: https://issues.apache.org/jira/browse/SPARK-20079
    +     */
    +    if (maxNumExecutorsNeeded() == 0) {
    +      initializing = true
    --- End diff --
    
    One downside could be:
    
    > During running tasks, when the total number of executors is the value of 
spark.dynamicAllocation.maxExecutors and the AM is failed. Then a new AM 
restarts. Because in ExecutorAllocationManager, the total number of executors 
does not changed, driver does not send RequestExecutors to AM to ask executors. 
Then the total number of executors is the value of 
spark.dynamicAllocation.initialExecutors . So the total number of executors in 
driver and AM is different.
    
    Because when AM is restarted, it will change it's state to the initial 
state, whereas if `ExecutorAllocationManager`'s state is still the current 
state, then the state maintained in two sides will be out of sync, and required 
executor number calculated in AM side will be wrong.


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