[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-148866912
  
Forgot to add: merged into master 1.5.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[asfgit]

Github user asfgit closed the pull request at:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-147625660
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-147625645
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-147626144
  
  [Test build #43628 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43628/consoleFull)
 for   PR 8945 at commit 
[`da13040`](https://github.com/apache/spark/commit/da13040aca20f0c739be8958675f5b39dac4d82c).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-147661007
  
Merged build finished. Test PASSed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-147660586
  
  [Test build #43628 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43628/console)
 for   PR 8945 at commit 
[`da13040`](https://github.com/apache/spark/commit/da13040aca20f0c739be8958675f5b39dac4d82c).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

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

https://github.com/apache/spark/pull/8945#discussion_r41947268
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,38 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+  test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+eventually(timeout(10.seconds), interval(10.millis)) {
+  val apps = getApplications()
+  assert(apps.size === 1)
+  assert(apps.head.id === appId)
+  assert(apps.head.executors.size === 2)
+  assert(apps.head.getExecutorLimit === Int.MaxValue)
+}
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor 1, and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
+var apps = getApplications()
+assert(apps.head.executors.size === 2)
--- End diff --

@vanzin I have changed. Please review. Thanks.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-147900244
  
LGTM, will let Andrew have a final look.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

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

https://github.com/apache/spark/pull/8945#discussion_r41775279
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,38 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+  test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+eventually(timeout(10.seconds), interval(10.millis)) {
+  val apps = getApplications()
+  assert(apps.size === 1)
+  assert(apps.head.id === appId)
+  assert(apps.head.executors.size === 2)
+  assert(apps.head.getExecutorLimit === Int.MaxValue)
+}
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor 1, and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
+var apps = getApplications()
+assert(apps.head.executors.size === 2)
--- End diff --

I understand that. But what I'm saying is, in the master's view of the 
world, what happens is that one executor goes away and a new executor comes up. 
So there may be a window in which the executor count, as seen from the master, 
is "1", temporarily, before the new executor starts. And that would cause your 
test to fail sporadically.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

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

https://github.com/apache/spark/pull/8945#discussion_r41752408
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,38 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+  test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+eventually(timeout(10.seconds), interval(10.millis)) {
+  val apps = getApplications()
+  assert(apps.size === 1)
+  assert(apps.head.id === appId)
+  assert(apps.head.executors.size === 2)
+  assert(apps.head.getExecutorLimit === Int.MaxValue)
+}
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor 1, and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
+var apps = getApplications()
+assert(apps.head.executors.size === 2)
--- End diff --

@vanzin 
```
   assert(sc.killAndReplaceExecutor(executors.head))
var apps = getApplications()
assert(apps.head.executors.size === 2)
```
I want  to say, kill a executor and a new executor should replaces it. The 
total number of executor should be not changed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

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

https://github.com/apache/spark/pull/8945#discussion_r41820348
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,38 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+  test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+eventually(timeout(10.seconds), interval(10.millis)) {
+  val apps = getApplications()
+  assert(apps.size === 1)
+  assert(apps.head.id === appId)
+  assert(apps.head.executors.size === 2)
+  assert(apps.head.getExecutorLimit === Int.MaxValue)
+}
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor 1, and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
+var apps = getApplications()
+assert(apps.head.executors.size === 2)
--- End diff --

@vanzin I understand. Can you tell me how to change? Thanks.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

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

https://github.com/apache/spark/pull/8945#discussion_r41820574
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,38 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+  test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+eventually(timeout(10.seconds), interval(10.millis)) {
+  val apps = getApplications()
+  assert(apps.size === 1)
+  assert(apps.head.id === appId)
+  assert(apps.head.executors.size === 2)
+  assert(apps.head.getExecutorLimit === Int.MaxValue)
+}
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor 1, and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
+var apps = getApplications()
+assert(apps.head.executors.size === 2)
--- End diff --

Just follow examples in the same file. e.g.:

eventually(timeout(10.seconds), interval(10.millis)) {
  val apps = getApplications()
  assert(apps.size === 1)
  assert(apps.head.id === appId)
  assert(apps.head.executors.size === 2)
  assert(apps.head.getExecutorLimit === Int.MaxValue)
}



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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146807928
  
  [Test build #43464 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43464/consoleFull)
 for   PR 8945 at commit 
[`cb69dc5`](https://github.com/apache/spark/commit/cb69dc5b1795e49a1f10db2db0e7fe8a5ba6e5cc).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146806550
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146806538
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

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

https://github.com/apache/spark/pull/8945#discussion_r41674241
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,38 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+  test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+eventually(timeout(10.seconds), interval(10.millis)) {
+  val apps = getApplications()
+  assert(apps.size === 1)
+  assert(apps.head.id === appId)
+  assert(apps.head.executors.size === 2)
+  assert(apps.head.getExecutorLimit === Int.MaxValue)
+}
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor 1, and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
+var apps = getApplications()
+assert(apps.head.executors.size === 2)
--- End diff --

I think this code is racy; the executors count goes down to 1 before coming 
back to 2, and depending on the order of the events happening in the 
background, this test might fail. Using `eventually` here might be safer, since 
you do want the count to eventually come back to 2.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

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

https://github.com/apache/spark/pull/8945#discussion_r41692752
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,38 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+  test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+eventually(timeout(10.seconds), interval(10.millis)) {
+  val apps = getApplications()
+  assert(apps.size === 1)
+  assert(apps.head.id === appId)
+  assert(apps.head.executors.size === 2)
+  assert(apps.head.getExecutorLimit === Int.MaxValue)
+}
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor 1, and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
+var apps = getApplications()
+assert(apps.head.executors.size === 2)
+// kill executor 1
+assert(sc.killExecutor(executors.head))
+apps = getApplications()
+assert(apps.head.executors.size === 2)
--- End diff --

@andrewor14 Here i use sc.killExecutor(executors.head), I want to say a 
executor lost and a new executor should start to replace. Before the new 
executor registers, the executor is idle timeout. Then the total number of 
executors should not change. So  "apps.head.executors.size === 2"


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146834200
  
Merged build finished. Test PASSed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146834103
  
  [Test build #43464 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43464/console)
 for   PR 8945 at commit 
[`cb69dc5`](https://github.com/apache/spark/commit/cb69dc5b1795e49a1f10db2db0e7fe8a5ba6e5cc).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

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

https://github.com/apache/spark/pull/8945#discussion_r41660042
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,38 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+  test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+eventually(timeout(10.seconds), interval(10.millis)) {
+  val apps = getApplications()
+  assert(apps.size === 1)
+  assert(apps.head.id === appId)
+  assert(apps.head.executors.size === 2)
+  assert(apps.head.getExecutorLimit === Int.MaxValue)
+}
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor 1, and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
+var apps = getApplications()
+assert(apps.head.executors.size === 2)
+// kill executor 1
+assert(sc.killExecutor(executors.head))
+apps = getApplications()
+assert(apps.head.executors.size === 2)
--- End diff --

hm, not sure if I understand this. Why does the number of executors stay at 
2 after you call `sc.killExecutor`, which does not replace it? Shouldn't it go 
down to 1?


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146436500
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146436545
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146440195
  
  [Test build #43389 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43389/console)
 for   PR 8945 at commit 
[`e382315`](https://github.com/apache/spark/commit/e382315e9905b735837bc37a63acd16b72242ada).
 * This patch **fails MiMa tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146440226
  
Merged build finished. Test FAILed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146437914
  
  [Test build #43389 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43389/consoleFull)
 for   PR 8945 at commit 
[`e382315`](https://github.com/apache/spark/commit/e382315e9905b735837bc37a63acd16b72242ada).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

Github user KaiXinXiaoLei commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146446834
  
jenkins test please.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146730978
  
retest this please


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146732399
  
  [Test build #43453 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43453/consoleFull)
 for   PR 8945 at commit 
[`e382315`](https://github.com/apache/spark/commit/e382315e9905b735837bc37a63acd16b72242ada).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146737544
  
Thanks LGTM. I'll merge this once tests pass.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146747357
  
Merged build finished. Test PASSed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146747096
  
  [Test build #43453 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43453/console)
 for   PR 8945 at commit 
[`e382315`](https://github.com/apache/spark/commit/e382315e9905b735837bc37a63acd16b72242ada).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

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

https://github.com/apache/spark/pull/8945#discussion_r41591815
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,35 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+  test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+eventually(timeout(10.seconds), interval(10.millis)) {
+  val apps = getApplications()
+  assert(apps.size === 1)
+  assert(apps.head.id === appId)
+  assert(apps.head.executors.size === 2)
+  assert(apps.head.getExecutorLimit === Int.MaxValue)
+}
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor,and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
+assert(master.apps.head.executors.size === 2)
+
+assert(sc.killExecutor(executors.head))
+assert(master.apps.head.executors.size === 2)
--- End diff --

This has the same problem I mentioned before in the other PR. 
`sc.killExecutor` does not immediately update the state of the `master.apps` 
list. So these tests are bound to fail in weird ways.

You need to use `killNExecutors` instead of `sc.killExecutor` and 
`getApplications` instead of `master.apps`. See other tests in this same file.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146731465
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146731448
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146754992
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146754984
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146755685
  
Merged build finished. Test FAILed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146755683
  
  [Test build #43458 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43458/console)
 for   PR 8945 at commit 
[`b7b42cc`](https://github.com/apache/spark/commit/b7b42ccbd8364fedbd652e6763fd3c566539bf93).
 * This patch **fails Scala style tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-146755528
  
  [Test build #43458 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43458/consoleFull)
 for   PR 8945 at commit 
[`b7b42cc`](https://github.com/apache/spark/commit/b7b42ccbd8364fedbd652e6763fd3c566539bf93).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

Github user KaiXinXiaoLei commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-146389770
  
see https://github.com/apache/spark/pull/8945


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

Github user KaiXinXiaoLei closed the pull request at:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-144707186
  
@KaiXinXiaoLei do you mind closing this PR?


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

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

https://github.com/apache/spark/pull/8945#discussion_r40827345
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,35 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+test("the pending replacement executors should not be lost (SPARK-10515)") 
{
--- End diff --

indentation is off


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-144490428
  
@KaiXinXiaoLei in the future please just have one PR open for the same 
issue. You can do this by pushing to the same branch instead of creating a 
whole new one (i.e. `deleteRequest`). It's confusing to reviewers to have many 
very similar patches open. Please close this PR if your other patch supersedes 
this.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

GitHub user KaiXinXiaoLei opened a pull request:

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

[SPARK-10515] When killing executor, the pending replacement executors 
should not be lost

If the heartbeat receiver kills executors (and new ones are not registered 
to replace them), the idle timeout for the old executors will be lost (and then 
change a total number of executors requested by Driver), So new ones will be 
not to asked to replace them.
For example, executorsPendingToRemove=Set(1), and executor 2 is idle 
timeout before a new executor is asked to replace executor 1. Then driver kill 
executor 2, and sending RequestExecutors to AM. But 
executorsPendingToRemove=Set(1,2), So AM doesn't allocate a executor to replace 
1.

see: https://github.com/apache/spark/pull/8668

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

$ git pull https://github.com/KaiXinXiaoLei/spark pendingexecutor

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

https://github.com/apache/spark/pull/8945.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 #8945


commit e559482ae4404320a1a242d0aa082f00611e02bb
Author: huleilei 
Date:   2015-09-30T05:16:27Z

add unit test

commit 1bdde8e88ed36194e020e773db415e65181357a9
Author: huleilei 
Date:   2015-09-30T06:35:51Z

add numPendingExecutors




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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-144311524
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-144311537
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-144311863
  
  [Test build #43124 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43124/consoleFull)
 for   PR 8945 at commit 
[`1bdde8e`](https://github.com/apache/spark/commit/1bdde8e88ed36194e020e773db415e65181357a9).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-144337201
  
  [Test build #43124 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43124/console)
 for   PR 8945 at commit 
[`1bdde8e`](https://github.com/apache/spark/commit/1bdde8e88ed36194e020e773db415e65181357a9).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-144337257
  
Merged build finished. Test FAILed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

Github user KaiXinXiaoLei commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-144338392
  
@andrewor14 I change code according to your suggest. see: 
https://github.com/apache/spark/pull/8945


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/8945#issuecomment-144389195
  
Hmm, test failure looks like it might be related.

Also, does this replace #8668? If so, could you close one of them?


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

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

https://github.com/apache/spark/pull/8945#discussion_r40827411
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -369,6 +369,35 @@ class StandaloneDynamicAllocationSuite
 assert(apps.head.getExecutorLimit === 1)
   }
 
+test("the pending replacement executors should not be lost (SPARK-10515)") 
{
+  sc = new SparkContext(appConf)
+  val appId = sc.applicationId
+  eventually(timeout(10.seconds), interval(10.millis)) {
+val apps = getApplications()
+assert(apps.size === 1)
+assert(apps.head.id === appId)
+assert(apps.head.executors.size === 2)
+assert(apps.head.getExecutorLimit === Int.MaxValue)
+  }
+  // sync executors between the Master and the driver, needed because
+  // the driver refuses to kill executors it does not know about
+  syncExecutors(sc)
--- End diff --

indentation is off


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

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

https://github.com/apache/spark/pull/8668#discussion_r40715442
  
--- Diff: 
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
 ---
@@ -431,7 +439,10 @@ class CoarseGrainedSchedulerBackend(scheduler: 
TaskSchedulerImpl, val rpcEnv: Rp
 // take into account executors that are pending to be added or removed.
 if (!replace) {
   doRequestTotalExecutors(
-numExistingExecutors + numPendingExecutors - 
executorsPendingToRemove.size)
+numExistingExecutors + numPendingExecutors - 
executorsPendingToRemove.size
++ numReplacingExecutors)
+} else {
+  numReplacingExecutors += knownExecutors.size
--- End diff --

do we need another variable? Can't we just do
```
numPendingExecutors += knownExecutors.size
```
This makes sense on a high level too; if we replace an executor we expect 
to get one back, so it should be pending in the mean time.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

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

https://github.com/apache/spark/pull/8668#discussion_r40713413
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -303,6 +303,32 @@ class StandaloneDynamicAllocationSuite
 assert(master.apps.head.getExecutorLimit === 1)
   }
 
+   test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+assert(master.apps.size === 1)
--- End diff --

please update this to reflect the changes made in #8914


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

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

https://github.com/apache/spark/pull/8668#discussion_r40715552
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -303,6 +303,32 @@ class StandaloneDynamicAllocationSuite
 assert(master.apps.head.getExecutorLimit === 1)
   }
 
+   test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+assert(master.apps.size === 1)
+assert(master.apps.head.id === appId)
+assert(master.apps.head.executors.size === 2)
+assert(master.apps.head.getExecutorLimit === Int.MaxValue)
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor,and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
--- End diff --

yes, this might be flaky.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-144160428
  
@KaiXinXiaoLei The problem makes sense now and I think this fix is correct, 
though I suggested a way to make it simpler. Once you address the comments 
would you mind also updating the JIRA? Right now the description is empty and 
there are no details on the bug: 
https://issues.apache.org/jira/browse/SPARK-10515


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-143307328
  
FYI: #8914 makes some changes to these tests to avoid the races I alluded 
to in my last comment.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

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

https://github.com/apache/spark/pull/8668#discussion_r40228114
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala
 ---
@@ -303,6 +303,32 @@ class StandaloneDynamicAllocationSuite
 assert(master.apps.head.getExecutorLimit === 1)
   }
 
+   test("the pending replacement executors should not be lost 
(SPARK-10515)") {
+sc = new SparkContext(appConf)
+val appId = sc.applicationId
+assert(master.apps.size === 1)
+assert(master.apps.head.id === appId)
+assert(master.apps.head.executors.size === 2)
+assert(master.apps.head.getExecutorLimit === Int.MaxValue)
+// sync executors between the Master and the driver, needed because
+// the driver refuses to kill executors it does not know about
+syncExecutors(sc)
+val executors = getExecutorIds(sc)
+assert(executors.size === 2)
+
+// kill executor,and replace it
+assert(sc.killAndReplaceExecutor(executors.head))
--- End diff --

Hmm... this doesn't look right. It should probably be using a new version 
of `killNExecutors` that calls `killAndReplaceExecutor` and syncs things.

@andrewor14 might be a better person to comment on this, since he wrote the 
original tests. I'm not sure about how much we can trust the counts to update 
atomically when `killNExecutors` and friends are called, but the other tests 
seem to be passing reliably...


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142504403
  
Merged build finished. Test PASSed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142504363
  
  [Test build #42881 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42881/console)
 for   PR 8668 at commit 
[`d738641`](https://github.com/apache/spark/commit/d738641590f62adacd198cf5f8b61150372a).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

Github user KaiXinXiaoLei commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142507056
  
@vanzin I  add a unit test for this problem. Thanks.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

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

https://github.com/apache/spark/pull/8668#discussion_r40121163
  
--- Diff: 
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
 ---
@@ -147,6 +150,10 @@ class CoarseGrainedSchedulerBackend(scheduler: 
TaskSchedulerImpl, val rpcEnv: Rp
   numPendingExecutors -= 1
   logDebug(s"Decremented number of pending executors 
($numPendingExecutors left)")
 }
+if (numReplacingExecutors > 0) {
+  numReplacingExecutors -= 1
+  logDebug(s"Decremented number of replaceing executors 
($numReplacingExecutors left)")
--- End diff --

nit: "number of executors being replaced"


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

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

https://github.com/apache/spark/pull/8668#discussion_r40121097
  
--- Diff: 
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
 ---
@@ -66,6 +66,9 @@ class CoarseGrainedSchedulerBackend(scheduler: 
TaskSchedulerImpl, val rpcEnv: Rp
   // Executors we have requested the cluster manager to kill that have not 
died yet
   private val executorsPendingToRemove = new HashSet[String]
 
+  // Number of executors requested from the cluster manager that have not 
replaced yet
--- End diff --

nit: "have not been replaced"


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142363549
  
  [Test build #42839 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42839/console)
 for   PR 8668 at commit 
[`7e0c199`](https://github.com/apache/spark/commit/7e0c199e5c0f78273d39e333e407a7baef41a5fc).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142363712
  
Merged build finished. Test PASSed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142369819
  
The change LGTM; I'd be more comfortable if there was a unit test for this 
code, but I tried to craft one and `CoarseGrainedSchedulerBackend` is not very 
unit-testable at all. Perhaps that's something to look at, given how hard it is 
to grok this code.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142482194
  
Merged build finished. Test FAILed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142485675
  
  [Test build #42883 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42883/consoleFull)
 for   PR 8668 at commit 
[`0041fde`](https://github.com/apache/spark/commit/0041fde08d903102f27dff8adca72997736c7387).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142490404
  
  [Test build #42883 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42883/console)
 for   PR 8668 at commit 
[`0041fde`](https://github.com/apache/spark/commit/0041fde08d903102f27dff8adca72997736c7387).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142481582
  
  [Test build #42881 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42881/consoleFull)
 for   PR 8668 at commit 
[`d738641`](https://github.com/apache/spark/commit/d738641590f62adacd198cf5f8b61150372a).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142481452
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142482191
  
  [Test build #42882 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42882/console)
 for   PR 8668 at commit 
[`71a59a3`](https://github.com/apache/spark/commit/71a59a3c2c256f5e448e3678c9128c28dff9f317).
 * This patch **fails Scala style tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142481446
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142481985
  
  [Test build #42882 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42882/consoleFull)
 for   PR 8668 at commit 
[`71a59a3`](https://github.com/apache/spark/commit/71a59a3c2c256f5e448e3678c9128c28dff9f317).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142481939
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142481925
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142485209
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142485169
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142490419
  
Merged build finished. Test FAILed.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

Github user KaiXinXiaoLei commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142201407
  
@vanzin Yes, ```addExecutors``` will often send message to AM, but the 
total number of executors in ```spark-dynamic-executor-allocation``` will  be 
the same with AM. I think, the total number of executors is get in  
```spark-dynamic-executor-allocation```, and send this value to AM,  instead of 
calculated in ```CoarseGrainedSchedulerBackend```. 


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

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

https://github.com/apache/spark/pull/8668#discussion_r40096873
  
--- Diff: 
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
 ---
@@ -236,6 +246,12 @@ class CoarseGrainedSchedulerBackend(scheduler: 
TaskSchedulerImpl, val rpcEnv: Rp
 addressToExecutorId -= executorInfo.executorAddress
 executorDataMap -= executorId
 executorsPendingToRemove -= executorId
+if (executorsToReplace.contains(executorId)) {
--- End diff --

Yes, this code is not need, thanks. 


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142317605
  
Merged build started.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142317529
  
 Merged build triggered.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142320061
  
  [Test build #42839 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42839/consoleFull)
 for   PR 8668 at commit 
[`7e0c199`](https://github.com/apache/spark/commit/7e0c199e5c0f78273d39e333e407a7baef41a5fc).


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[KaiXinXiaoLei]

Github user KaiXinXiaoLei commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142322746
  
@vanzin  I think the code I changed can resolve the problem about 
[SPARK-10515]. Thanks


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142073376
  
There's one possible race in this current code; let's say executor `X` 
fails a heartbeat and the driver kills it expecting a replacement. Slightly 
later, the allocation manager triggers an idle timeout for `X`, and tries to 
kill it again, with not replacement. The code should remove that executor from 
the list of executors expecting a replacement, and inform the backend that its 
accounting should be updated.

Also, let me go through your last comment and try to understand it.


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

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

https://github.com/apache/spark/pull/8668#discussion_r40009333
  
--- Diff: 
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
 ---
@@ -236,6 +246,12 @@ class CoarseGrainedSchedulerBackend(scheduler: 
TaskSchedulerImpl, val rpcEnv: Rp
 addressToExecutorId -= executorInfo.executorAddress
 executorDataMap -= executorId
 executorsPendingToRemove -= executorId
+if (executorsToReplace.contains(executorId)) {
--- End diff --

minor: `if (executorsToReplace.remove(executorId)) { ... }`


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

[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/8668#issuecomment-142106259
  
@KaiXinXiaoLei I think it understood what the change you propose in your 
last comment would do. I think  it would work, except for a very minor issue 
where it might cause unnecessary messages to be sent to allocator when the 
number of executors hasn't changed.

e.g.

* at t=1, there are `x` executors running
* at t=2, `addExecutors` is called and `x + 1` executors are requested
* at t=3, no executor has been added yet; `addExecutors` is called with `x 
+ 1` again, and instead of not sending the message to the allocator, a message 
is sent.

That is fine since these messages are not sent that often (at worst, once a 
second), and the allocator should handle this situation fine.

It would be nice to confirm that theory with tests, though; are the current 
unit tests enough to make sure that change would do the right thing?


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