[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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
[GitHub] spark pull request: [SPARK-10515] When killing executor, the pendi...
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...
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