You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sitalkedia <gi...@git.apache.org> on 2017/10/26 17:33:38 UTC

[GitHub] spark pull request #19580: [SPARK-22312][CORE] Fix bug in Executor allocatio...

GitHub user sitalkedia opened a pull request:

    https://github.com/apache/spark/pull/19580

    [SPARK-22312][CORE] Fix bug in Executor allocation manager in running tasks calculation

    ## What changes were proposed in this pull request?
    
    We often see the issue of Spark jobs stuck because the Executor Allocation Manager does not ask for any executor even if there are pending tasks in case dynamic allocation is turned on. Looking at the logic in Executor Allocation Manager, which calculates the running tasks, it can happen that the calculation will be wrong and the number of running tasks can become negative.
    
    ## How was this patch tested?
    
    Added unit test


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/sitalkedia/spark skedia/fix_stuck_job

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/19580.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #19580
    
----
commit f8fcc3560e087440c7618b33cc892f3feafd4a3a
Author: Sital Kedia <sk...@fb.com>
Date:   2017-10-19T05:24:38Z

    [SPARK-22312][CORE] Fix bug in Executor allocation manager in running tasks calculation

----


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83099/
    Test PASSed.


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147288373
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -787,7 +791,9 @@ private[spark] class ExecutorAllocationManager(
         /**
          * The number of tasks currently running across all stages.
          */
    -    def totalRunningTasks(): Int = numRunningTasks
    +    def totalRunningTasks(): Int = {
    +      stageIdToNumRunningTask.values.sum
    --- End diff --
    
    This needs to be inside `allocationManager.synchronized`, no?


---

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


[GitHub] spark issue #19580: [SPARK-22312][CORE] Fix bug in Executor allocation manag...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Could you fix the bug number (SPARK-11334)?


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83099 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83099/testReport)** for PR 19580 at commit [`e884a96`](https://github.com/apache/spark/commit/e884a96a877e5c3d74df6b073fade3cf9f9d9e0e).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147303973
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -678,7 +679,9 @@ private[spark] class ExecutorAllocationManager(
           val executorId = taskStart.taskInfo.executorId
     
           allocationManager.synchronized {
    -        numRunningTasks += 1
    +        if (stageIdToNumRunningTask.contains(stageId)) {
    +          stageIdToNumRunningTask(stageId) = stageIdToNumRunningTask(stageId) + 1
    --- End diff --
    
    nit: this can be changed to `stageIdToNumRunningTask(stageId) += 1`


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147325260
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -267,6 +267,10 @@ private[spark] class ExecutorAllocationManager(
         (numRunningOrPendingTasks + tasksPerExecutor - 1) / tasksPerExecutor
       }
     
    +  private def totalRunningTasks(): Int = synchronized {
    --- End diff --
    
    I'm not sure why do we need to add a method which only used for unit test. If want to verify the behavior of `totalRunningTasks`, I think `maxNumExecutorsNeeded` can also be used indirectly for verification.


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147288434
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -491,7 +495,6 @@ private[spark] class ExecutorAllocationManager(
             s"when it is already pending to be removed!")
           return false
         }
    -
    --- End diff --
    
    nit: no need for this change.


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147289166
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -787,7 +791,9 @@ private[spark] class ExecutorAllocationManager(
         /**
          * The number of tasks currently running across all stages.
          */
    -    def totalRunningTasks(): Int = numRunningTasks
    +    def totalRunningTasks(): Int = {
    +      stageIdToNumRunningTask.values.sum
    --- End diff --
    
    Nevermind, this is called from a synchronized context. Except in your unit tests, that is (which call the private`totalRunningTasks` you added to the manager).


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by sitalkedia <gi...@git.apache.org>.
Github user sitalkedia commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147328834
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -267,6 +267,10 @@ private[spark] class ExecutorAllocationManager(
         (numRunningOrPendingTasks + tasksPerExecutor - 1) / tasksPerExecutor
       }
     
    +  private def totalRunningTasks(): Int = synchronized {
    --- End diff --
    
    Its okay to add a method which is used for unit testing purpose only. I am not inclined towards the idea of using `maxNumExecutorsNeeded` to indirectly verify `totalRunningTasks` for the following reason - 
    
    Currently, the test case is testing what it is supposed to. If you check for `maxNumExecutorsNeeded` instead, it might not be clear what we are testing. 
    
    



---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83099 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83099/testReport)** for PR 19580 at commit [`e884a96`](https://github.com/apache/spark/commit/e884a96a877e5c3d74df6b073fade3cf9f9d9e0e).


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147291760
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -787,7 +791,9 @@ private[spark] class ExecutorAllocationManager(
         /**
          * The number of tasks currently running across all stages.
          */
    -    def totalRunningTasks(): Int = numRunningTasks
    +    def totalRunningTasks(): Int = {
    +      stageIdToNumRunningTask.values.sum
    --- End diff --
    
    It'd be nice to make the other method calling this synchronized, just to be paranoid.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/19580


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Merging to master / 2.2 / 2.1.


---

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


[GitHub] spark issue #19580: [SPARK-22312][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83089 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83089/testReport)** for PR 19580 at commit [`f8fcc35`](https://github.com/apache/spark/commit/f8fcc3560e087440c7618b33cc892f3feafd4a3a).


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83101 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83101/testReport)** for PR 19580 at commit [`6f22f93`](https://github.com/apache/spark/commit/6f22f93688c2f13dfd8c403af206ffc066aec2e1).


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Jenkins, retest this please.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    failed to merge to 2.2...


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83107 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83107/testReport)** for PR 19580 at commit [`8abaa2c`](https://github.com/apache/spark/commit/8abaa2cbc4c54dd728b7493ddfedabddd8c8ba7b).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147304306
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -709,7 +712,9 @@ private[spark] class ExecutorAllocationManager(
           val taskIndex = taskEnd.taskInfo.index
           val stageId = taskEnd.stageId
           allocationManager.synchronized {
    -        numRunningTasks -= 1
    +        if (stageIdToNumRunningTask.contains(stageId)) {
    +          stageIdToNumRunningTask(stageId) = stageIdToNumRunningTask(stageId) - 1
    --- End diff --
    
    ditto.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83101/
    Test PASSed.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83101 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83101/testReport)** for PR 19580 at commit [`6f22f93`](https://github.com/apache/spark/commit/6f22f93688c2f13dfd8c403af206ffc066aec2e1).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83257/
    Test PASSed.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83257 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83257/testReport)** for PR 19580 at commit [`8abaa2c`](https://github.com/apache/spark/commit/8abaa2cbc4c54dd728b7493ddfedabddd8c8ba7b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83089/
    Test PASSed.


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147304200
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -267,6 +267,10 @@ private[spark] class ExecutorAllocationManager(
         (numRunningOrPendingTasks + tasksPerExecutor - 1) / tasksPerExecutor
       }
     
    +  private def totalRunningTasks(): Int = synchronized {
    --- End diff --
    
    Looks like no one invoke this method?


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19580: [SPARK-22312][CORE] Fix bug in Executor allocation manag...

Posted by sitalkedia <gi...@git.apache.org>.
Github user sitalkedia commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    duplicate of https://github.com/apache/spark/pull/19534
    
    cc - @vanzin, 


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83107/
    Test FAILed.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83256/
    Test FAILed.


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by sitalkedia <gi...@git.apache.org>.
Github user sitalkedia commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147320096
  
    --- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
    @@ -267,6 +267,10 @@ private[spark] class ExecutorAllocationManager(
         (numRunningOrPendingTasks + tasksPerExecutor - 1) / tasksPerExecutor
       }
     
    +  private def totalRunningTasks(): Int = synchronized {
    --- End diff --
    
    This is being called from the test.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    jenkins, retest this please.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83089 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83089/testReport)** for PR 19580 at commit [`f8fcc35`](https://github.com/apache/spark/commit/f8fcc3560e087440c7618b33cc892f3feafd4a3a).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83257 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83257/testReport)** for PR 19580 at commit [`8abaa2c`](https://github.com/apache/spark/commit/8abaa2cbc4c54dd728b7493ddfedabddd8c8ba7b).


---

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


[GitHub] spark pull request #19580: [SPARK-11334][CORE] Fix bug in Executor allocatio...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19580#discussion_r147288495
  
    --- Diff: core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala ---
    @@ -227,6 +227,23 @@ class ExecutorAllocationManagerSuite
         assert(numExecutorsToAdd(manager) === 1)
       }
     
    +  test("Ignore task end events from completed stages") {
    --- End diff --
    
    nit: lower case "ignore" to match other tests.


---

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


[GitHub] spark issue #19580: [SPARK-11334][CORE] Fix bug in Executor allocation manag...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19580
  
    **[Test build #83107 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83107/testReport)** for PR 19580 at commit [`8abaa2c`](https://github.com/apache/spark/commit/8abaa2cbc4c54dd728b7493ddfedabddd8c8ba7b).


---

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