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

    https://github.com/apache/spark/pull/18950#discussion_r133482787
  
    --- Diff: 
core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -314,7 +316,7 @@ private[spark] class ExecutorAllocationManager(
           // Do not change our target while we are still initializing,
           // Otherwise the first job may have to ramp up unnecessarily
           0
    -    } else if (maxNeeded < numExecutorsTarget) {
    +    } else if (maxNeeded <= numExecutorsTarget) {
           // The target number exceeds the number we actually need, so stop 
adding new
    --- End diff --
    
    I'm curious if we needed the = here?  It doesn't appear that it should be 
needed but perhaps there is corner case.  I don't think it hurts anything 
either so more curious if there is an existing bug.
    
    If we do keep the = we should update the comment here for is the number 
exceeds or is equal


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to