You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/06/13 02:17:02 UTC
[GitHub] [spark] holdenk opened a new pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
holdenk opened a new pull request #28818:
URL: https://github.com/apache/spark/pull/28818
This is WIP since it is on top of SPARK-31197 (which itself is WIP on top off SPARK-20629 ) and should probably have more testing.
### What changes were proposed in this pull request?
If graceful decommissioning is enabled, Spark's dynamic scaling uses this instead of directly killing executors.
### Why are the changes needed?
When scaling down Spark we should avoid triggering recomputes as much as possible.
### Does this PR introduce _any_ user-facing change?
Hopefully their jobs run faster. It also enables experimental shuffle service free decommissioning when graceful decommissioning is enabled.
### How was this patch tested?
For now I've extended the ExecutorAllocationManagerSuite to cover this. I'll also add a more integration style test on K8s.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645699935
**[Test build #124181 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124181/testReport)** for PR 28818 at commit [`2674055`](https://github.com/apache/spark/commit/2674055de049c606ca32ffeb34bcc66235e14d1c).
* This patch **fails to generate documentation**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662239504
**[Test build #126292 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126292/testReport)** for PR 28818 at commit [`daf96dd`](https://github.com/apache/spark/commit/daf96ddae8d1cc2440b8b5452a1cd7c3e499f278).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662145183
**[Test build #126273 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126273/testReport)** for PR 28818 at commit [`9565c40`](https://github.com/apache/spark/commit/9565c4051a20f26c409f038543742161507ca593).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r441178546
##########
File path: core/src/main/scala/org/apache/spark/scheduler/dynalloc/ExecutorMonitor.scala
##########
@@ -333,11 +335,26 @@ private[spark] class ExecutorMonitor(
}
override def onBlockUpdated(event: SparkListenerBlockUpdated): Unit = {
- if (!event.blockUpdatedInfo.blockId.isInstanceOf[RDDBlockId]) {
- return
- }
val exec = ensureExecutorIsTracked(event.blockUpdatedInfo.blockManagerId.executorId,
UNKNOWN_RESOURCE_PROFILE_ID)
+
+ // Check if it is a shuffle file, or RDD to pick the correct codepath for update
+ if (event.blockUpdatedInfo.blockId.isInstanceOf[ShuffleDataBlockId] && shuffleTrackingEnabled) {
+ /**
+ * The executor monitor keeps track of locations of cache and shuffle blocks and this can be
+ * used to decide which executor(s) Spark should shutdown first. Since we move shuffle blocks
+ * around now this wires it up so that it keeps track of it. We only do this for data blocks
+ * as index and other blocks blocks do not necessarily mean the entire block has been
+ * committed.
+ */
+ event.blockUpdatedInfo.blockId match {
+ case ShuffleDataBlockId(shuffleId, _, _) => exec.addShuffle(shuffleId)
Review comment:
Since we are touching `ExecutorMonitor`, when do we have a counter operation, `exec.removeShuffle`? In this PR, it seems that `executorsKilled` is used. Is it enough?
cc @dbtsai
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-672831710
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643714577
**[Test build #123995 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123995/testReport)** for PR 28818 at commit [`ef3f523`](https://github.com/apache/spark/commit/ef3f52364afffb9943d0e0a9e36e2b043c4a3284).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-663245768
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662067572
**[Test build #126273 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126273/testReport)** for PR 28818 at commit [`9565c40`](https://github.com/apache/spark/commit/9565c4051a20f26c409f038543742161507ca593).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645036743
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-644982203
**[Test build #124139 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124139/testReport)** for PR 28818 at commit [`7691d2d`](https://github.com/apache/spark/commit/7691d2d31bc3677902c7d2b9b74ec976dd3342e2).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645621489
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643571011
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/123956/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-644982203
**[Test build #124139 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124139/testReport)** for PR 28818 at commit [`7691d2d`](https://github.com/apache/spark/commit/7691d2d31bc3677902c7d2b9b74ec976dd3342e2).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662067572
**[Test build #126273 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126273/testReport)** for PR 28818 at commit [`9565c40`](https://github.com/apache/spark/commit/9565c4051a20f26c409f038543742161507ca593).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r458391164
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala
##########
@@ -81,6 +81,35 @@ private[spark] trait ExecutorAllocationClient {
countFailures: Boolean,
force: Boolean = false): Seq[String]
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @param adjustTargetNumExecutors whether the target number of executors will be adjusted down
+ * after these executors have been decommissioned.
+ * @return the ids of the executors acknowledged by the cluster manager to be removed.
+ */
+ def decommissionExecutors(
+ executorIds: Seq[String],
+ adjustTargetNumExecutors: Boolean): Seq[String]
+
+
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def decommissionExecutor(executorId: String): Boolean = {
+ val decommissionedExecutors = decommissionExecutors(Seq(executorId),
+ adjustTargetNumExecutors = true)
Review comment:
It's a new function, what are we changing?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662203969
**[Test build #126292 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126292/testReport)** for PR 28818 at commit [`daf96dd`](https://github.com/apache/spark/commit/daf96ddae8d1cc2440b8b5452a1cd7c3e499f278).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gatorsmile commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645771384
cc @Ngone51 @jiangxb1987
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662239897
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643727333
**[Test build #123995 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123995/testReport)** for PR 28818 at commit [`ef3f523`](https://github.com/apache/spark/commit/ef3f52364afffb9943d0e0a9e36e2b043c4a3284).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643571010
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r439785650
##########
File path: core/src/main/scala/org/apache/spark/scheduler/dynalloc/ExecutorMonitor.scala
##########
@@ -333,11 +335,19 @@ private[spark] class ExecutorMonitor(
}
override def onBlockUpdated(event: SparkListenerBlockUpdated): Unit = {
- if (!event.blockUpdatedInfo.blockId.isInstanceOf[RDDBlockId]) {
- return
- }
val exec = ensureExecutorIsTracked(event.blockUpdatedInfo.blockManagerId.executorId,
UNKNOWN_RESOURCE_PROFILE_ID)
+
+ // Check if it is a shuffle file, or RDD to pick the correct codepath for update
+ if (event.blockUpdatedInfo.blockId.isInstanceOf[ShuffleDataBlockId] && shuffleTrackingEnabled) {
+ event.blockUpdatedInfo.blockId match {
+ case ShuffleDataBlockId(shuffleId, _, _) => exec.addShuffle(shuffleId)
+ case _ => // For now we only update on data blocks
+ }
Review comment:
So it's not (I still want to get to SPARK-31974). The executor monitor keeps track of locations of cache and shuffle blocks and this can be used to decide which executor(s) Spark should shutdown first. Since we move shuffle blocks around now this wires it up so that it keeps track of it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-663245051
**[Test build #126435 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126435/testReport)** for PR 28818 at commit [`f921ddd`](https://github.com/apache/spark/commit/f921ddd3913d8b749abd2d8de645e5d098d73823).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645621489
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662128707
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643571010
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] agrawaldevesh commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
agrawaldevesh commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r452561283
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala
##########
@@ -81,6 +81,35 @@ private[spark] trait ExecutorAllocationClient {
countFailures: Boolean,
force: Boolean = false): Seq[String]
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @param adjustTargetNumExecutors whether the target number of executors will be adjusted down
+ * after these executors have been decommissioned.
+ * @return the ids of the executors acknowledged by the cluster manager to be removed.
+ */
+ def decommissionExecutors(
+ executorIds: Seq[String],
+ adjustTargetNumExecutors: Boolean): Seq[String]
+
+
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def decommissionExecutor(executorId: String): Boolean = {
+ val decommissionedExecutors = decommissionExecutors(Seq(executorId),
+ adjustTargetNumExecutors = true)
Review comment:
Why is `adjustTargetNumExecutors` defaulting to true here ? This would mean that all schedulers would try to replinish the executor when asked to `DecommissionExecutor(...)` -- for example by the Master or when an executor gets a SIGPWR.
I think it shouldn't be the default -- it should atleast be configurable. It only makes sense to have `adjustTargetNumExecutors=true` when called from `org.apache.spark.streaming.scheduler.ExecutorAllocationManager#killExecutor` (ie when it is truly called from dynamic allocation codepath and we have decided that we want to replinish the executor).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662204302
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] agrawaldevesh commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
agrawaldevesh commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r458381415
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala
##########
@@ -81,6 +81,35 @@ private[spark] trait ExecutorAllocationClient {
countFailures: Boolean,
force: Boolean = false): Seq[String]
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @param adjustTargetNumExecutors whether the target number of executors will be adjusted down
+ * after these executors have been decommissioned.
+ * @return the ids of the executors acknowledged by the cluster manager to be removed.
+ */
+ def decommissionExecutors(
+ executorIds: Seq[String],
+ adjustTargetNumExecutors: Boolean): Seq[String]
+
+
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def decommissionExecutor(executorId: String): Boolean = {
+ val decommissionedExecutors = decommissionExecutors(Seq(executorId),
+ adjustTargetNumExecutors = true)
Review comment:
Hmm, Should we rename decommissionExecutor (singular) to decommissionAndKillExecutor to reflect its purpose better ? It would be too easy to confuse it with decommissionExecutors (on line 95 of this file which allows to not replenish the target number of executors).
Do you want to make the change to the callers of decommissionExecutor in this PR and switch them to `decommissioExecutors(Seq(executorId), false)` instead. The ones I am most concerned about are:
- The handling of message DecommissionExecutor (both sync and async variants) in CoarseGrainedSchedulerBackend
- StandaloneSchedulerBackend.executorDecommissioned
In both the above cases, I think we may not _always_ want replenishing. For example, in the standalone case, when the Worker gets a SIGPWR -- do we want to replenish the executors on the remaining workers (ie oversubscribe the remaining workers) ? Similarly when an executor gets a SIGPWR, do we want to put that load on the remaining executors ? I think the answer to both should be NO unless we are doing a dynamic allocation.
Personally I am fine with any choice of naming here as long as the semantics are not silently changed under the cover, as is the case presently.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662068161
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r439788769
##########
File path: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
##########
@@ -497,6 +453,70 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
protected def minRegisteredRatio: Double = _minRegisteredRatio
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
Review comment:
So that's copied from the trait it's implementing (it is the part doing the override), I'll delete it here though since it's confusing in this context, good catch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-648519084
Just as a follow up since @HyukjinKwon requested an SPIP for this work I won't be moving this PR out of WIP this week as originally planned.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662145746
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645624300
**[Test build #124179 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124179/testReport)** for PR 28818 at commit [`81f9dbb`](https://github.com/apache/spark/commit/81f9dbb17160e64ac7d77e3e8de2e3f0f51fabe9).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645696269
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r444585499
##########
File path: core/src/main/scala/org/apache/spark/scheduler/dynalloc/ExecutorMonitor.scala
##########
@@ -333,11 +335,26 @@ private[spark] class ExecutorMonitor(
}
override def onBlockUpdated(event: SparkListenerBlockUpdated): Unit = {
- if (!event.blockUpdatedInfo.blockId.isInstanceOf[RDDBlockId]) {
- return
- }
val exec = ensureExecutorIsTracked(event.blockUpdatedInfo.blockManagerId.executorId,
UNKNOWN_RESOURCE_PROFILE_ID)
+
+ // Check if it is a shuffle file, or RDD to pick the correct codepath for update
+ if (event.blockUpdatedInfo.blockId.isInstanceOf[ShuffleDataBlockId] && shuffleTrackingEnabled) {
+ /**
+ * The executor monitor keeps track of locations of cache and shuffle blocks and this can be
+ * used to decide which executor(s) Spark should shutdown first. Since we move shuffle blocks
+ * around now this wires it up so that it keeps track of it. We only do this for data blocks
+ * as index and other blocks blocks do not necessarily mean the entire block has been
+ * committed.
+ */
+ event.blockUpdatedInfo.blockId match {
+ case ShuffleDataBlockId(shuffleId, _, _) => exec.addShuffle(shuffleId)
Review comment:
Yeah so since we're only doing migrations during decommissioning, whatever shuffle files remain on the host when it dies will do the cleanup. I can't think of why we would need to do a delete operation here as well, but if it would be useful for your follow on work I can add it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643714697
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643714697
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-644982702
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645156370
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645036362
**[Test build #124139 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124139/testReport)** for PR 28818 at commit [`7691d2d`](https://github.com/apache/spark/commit/7691d2d31bc3677902c7d2b9b74ec976dd3342e2).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645699963
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124181/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643555190
**[Test build #123956 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123956/testReport)** for PR 28818 at commit [`2ff94ec`](https://github.com/apache/spark/commit/2ff94ece8eb9b33524fd6705bfcaca6427855450).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662181907
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643714577
**[Test build #123995 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123995/testReport)** for PR 28818 at commit [`ef3f523`](https://github.com/apache/spark/commit/ef3f52364afffb9943d0e0a9e36e2b043c4a3284).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645156028
**[Test build #124154 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124154/testReport)** for PR 28818 at commit [`9bb0293`](https://github.com/apache/spark/commit/9bb0293125af3dcde87b52be5bf0ba4dcc5f4a0f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662068161
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643555190
**[Test build #123956 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123956/testReport)** for PR 28818 at commit [`2ff94ec`](https://github.com/apache/spark/commit/2ff94ece8eb9b33524fd6705bfcaca6427855450).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-644982702
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645156028
**[Test build #124154 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124154/testReport)** for PR 28818 at commit [`9bb0293`](https://github.com/apache/spark/commit/9bb0293125af3dcde87b52be5bf0ba4dcc5f4a0f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645036743
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk closed pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk closed pull request #28818:
URL: https://github.com/apache/spark/pull/28818
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643555300
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645157060
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662181907
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-663245496
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] agrawaldevesh commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
agrawaldevesh commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r439788125
##########
File path: core/src/main/scala/org/apache/spark/scheduler/dynalloc/ExecutorMonitor.scala
##########
@@ -333,11 +335,19 @@ private[spark] class ExecutorMonitor(
}
override def onBlockUpdated(event: SparkListenerBlockUpdated): Unit = {
- if (!event.blockUpdatedInfo.blockId.isInstanceOf[RDDBlockId]) {
- return
- }
val exec = ensureExecutorIsTracked(event.blockUpdatedInfo.blockManagerId.executorId,
UNKNOWN_RESOURCE_PROFILE_ID)
+
+ // Check if it is a shuffle file, or RDD to pick the correct codepath for update
+ if (event.blockUpdatedInfo.blockId.isInstanceOf[ShuffleDataBlockId] && shuffleTrackingEnabled) {
+ event.blockUpdatedInfo.blockId match {
+ case ShuffleDataBlockId(shuffleId, _, _) => exec.addShuffle(shuffleId)
+ case _ => // For now we only update on data blocks
+ }
Review comment:
Makes perfect sense ! A comment in the code would be helpful ;-).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-663245768
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-663245774
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126435/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-663245051
**[Test build #126435 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126435/testReport)** for PR 28818 at commit [`f921ddd`](https://github.com/apache/spark/commit/f921ddd3913d8b749abd2d8de645e5d098d73823).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] agrawaldevesh commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
agrawaldevesh commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-674318466
@holdenk can this PR be abandoned/closed now since this is finally in ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645157066
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124154/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] agrawaldevesh commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
agrawaldevesh commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r458397240
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala
##########
@@ -81,6 +81,35 @@ private[spark] trait ExecutorAllocationClient {
countFailures: Boolean,
force: Boolean = false): Seq[String]
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @param adjustTargetNumExecutors whether the target number of executors will be adjusted down
+ * after these executors have been decommissioned.
+ * @return the ids of the executors acknowledged by the cluster manager to be removed.
+ */
+ def decommissionExecutors(
+ executorIds: Seq[String],
+ adjustTargetNumExecutors: Boolean): Seq[String]
+
+
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def decommissionExecutor(executorId: String): Boolean = {
+ val decommissionedExecutors = decommissionExecutors(Seq(executorId),
+ adjustTargetNumExecutors = true)
Review comment:
ExecutorAllocationClient is a base class of CoarseGrainedSchedulerBackend. We moved decommissionExecutor from the latter class to the former and as such it is not a new function. Since CoarseGrainedSchedulerBackend no longer overrides decommissionExecutor, ExecutorAllocationClient.decommissionExecutor will be called when CoarseGrainedSchedulerBackend gets a DecommissionExecutor message -- and the semantics of that codepath have been changed to unconditionally impose adjustTargetNumExecutors=true.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662203969
**[Test build #126292 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126292/testReport)** for PR 28818 at commit [`daf96dd`](https://github.com/apache/spark/commit/daf96ddae8d1cc2440b8b5452a1cd7c3e499f278).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645631754
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r439788707
##########
File path: core/src/main/scala/org/apache/spark/scheduler/dynalloc/ExecutorMonitor.scala
##########
@@ -333,11 +335,19 @@ private[spark] class ExecutorMonitor(
}
override def onBlockUpdated(event: SparkListenerBlockUpdated): Unit = {
- if (!event.blockUpdatedInfo.blockId.isInstanceOf[RDDBlockId]) {
- return
- }
val exec = ensureExecutorIsTracked(event.blockUpdatedInfo.blockManagerId.executorId,
UNKNOWN_RESOURCE_PROFILE_ID)
+
+ // Check if it is a shuffle file, or RDD to pick the correct codepath for update
+ if (event.blockUpdatedInfo.blockId.isInstanceOf[ShuffleDataBlockId] && shuffleTrackingEnabled) {
+ event.blockUpdatedInfo.blockId match {
+ case ShuffleDataBlockId(shuffleId, _, _) => exec.addShuffle(shuffleId)
+ case _ => // For now we only update on data blocks
+ }
Review comment:
Sure, I'll add one :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-663245496
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643727410
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643727410
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662145746
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-646310567
To be clear this a WIP PR and not yet ready for review. I created it to give additional context after talking with @agrawaldevesh about our respective goals. I'll try and get a "read for review" PR out next week.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645156370
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] agrawaldevesh commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
agrawaldevesh commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r458373023
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala
##########
@@ -81,6 +81,35 @@ private[spark] trait ExecutorAllocationClient {
countFailures: Boolean,
force: Boolean = false): Seq[String]
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @param adjustTargetNumExecutors whether the target number of executors will be adjusted down
+ * after these executors have been decommissioned.
+ * @return the ids of the executors acknowledged by the cluster manager to be removed.
+ */
+ def decommissionExecutors(
+ executorIds: Seq[String],
+ adjustTargetNumExecutors: Boolean): Seq[String]
+
+
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def decommissionExecutor(executorId: String): Boolean = {
+ val decommissionedExecutors = decommissionExecutors(Seq(executorId),
+ adjustTargetNumExecutors = true)
Review comment:
Can you please point me to where is the configurable call ? I don't see a config check in the code paths that call this method.
It's fine for killExecutor to unconditionally adjust the target number of executors because it is only called in the dynamic allocation codepath, but decommissionExecutor would be called from many other codepaths as well (for example when the driver gets a DecommissionExecutor message) -- and thus I think it should just assume that it should replenish the executor.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645036750
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124139/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645695757
**[Test build #124181 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124181/testReport)** for PR 28818 at commit [`2674055`](https://github.com/apache/spark/commit/2674055de049c606ca32ffeb34bcc66235e14d1c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662128123
**[Test build #126281 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126281/testReport)** for PR 28818 at commit [`3db60f2`](https://github.com/apache/spark/commit/3db60f24b04a278ca4e900d48ff8454c10ef0659).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643555300
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645699958
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662065707
@agrawaldevesh So this PR depends on the behaviour of the VM eventually exiting (https://issues.apache.org/jira/browse/SPARK-31197 ) since it's replacing the usage of killExecutor during dynamic allocation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662239900
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126292/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] agrawaldevesh commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
agrawaldevesh commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r439775571
##########
File path: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
##########
@@ -497,6 +453,70 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
protected def minRegisteredRatio: Double = _minRegisteredRatio
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
Review comment:
Is this comment valid ? I don't see this method being overridden.
##########
File path: core/src/main/scala/org/apache/spark/scheduler/dynalloc/ExecutorMonitor.scala
##########
@@ -333,11 +335,19 @@ private[spark] class ExecutorMonitor(
}
override def onBlockUpdated(event: SparkListenerBlockUpdated): Unit = {
- if (!event.blockUpdatedInfo.blockId.isInstanceOf[RDDBlockId]) {
- return
- }
val exec = ensureExecutorIsTracked(event.blockUpdatedInfo.blockManagerId.executorId,
UNKNOWN_RESOURCE_PROFILE_ID)
+
+ // Check if it is a shuffle file, or RDD to pick the correct codepath for update
+ if (event.blockUpdatedInfo.blockId.isInstanceOf[ShuffleDataBlockId] && shuffleTrackingEnabled) {
+ event.blockUpdatedInfo.blockId match {
+ case ShuffleDataBlockId(shuffleId, _, _) => exec.addShuffle(shuffleId)
+ case _ => // For now we only update on data blocks
+ }
Review comment:
Is this change related to the purpose of 'Using decommissioning for dynamic allocation' ? I didn't fully understand why this is done wrt to just the decommissioning logic. Perhaps this is about https://issues.apache.org/jira/browse/SPARK-31974 ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662239897
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645631754
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645624300
**[Test build #124179 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124179/testReport)** for PR 28818 at commit [`81f9dbb`](https://github.com/apache/spark/commit/81f9dbb17160e64ac7d77e3e8de2e3f0f51fabe9).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] agrawaldevesh commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
agrawaldevesh commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r458395427
##########
File path: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
##########
@@ -419,48 +419,6 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
scheduler.workerRemoved(workerId, host, message)
}
- /**
- * Mark a given executor as decommissioned and stop making resource offers for it.
- */
- private def decommissionExecutor(executorId: String): Boolean = {
Review comment:
@holdenk ... to answer your question: This block of code was moved to the base class `ExecutorAllocationClient`. So the code in `ExecutorAllocationClient` is not "New". Furthermore, the semantics of this code were changed as it was moved to now unconditionally replenish the executors.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645695757
**[Test build #124181 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124181/testReport)** for PR 28818 at commit [`2674055`](https://github.com/apache/spark/commit/2674055de049c606ca32ffeb34bcc66235e14d1c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645796051
cc @tgravescs too
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643570926
**[Test build #123956 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123956/testReport)** for PR 28818 at commit [`2ff94ec`](https://github.com/apache/spark/commit/2ff94ece8eb9b33524fd6705bfcaca6427855450).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662181447
**[Test build #126281 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126281/testReport)** for PR 28818 at commit [`3db60f2`](https://github.com/apache/spark/commit/3db60f24b04a278ca4e900d48ff8454c10ef0659).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r459640803
##########
File path: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
##########
@@ -419,48 +419,6 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
scheduler.workerRemoved(workerId, host, message)
}
- /**
- * Mark a given executor as decommissioned and stop making resource offers for it.
- */
- private def decommissionExecutor(executorId: String): Boolean = {
Review comment:
Makes sense.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662128707
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645631760
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124179/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645631715
**[Test build #124179 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124179/testReport)** for PR 28818 at commit [`81f9dbb`](https://github.com/apache/spark/commit/81f9dbb17160e64ac7d77e3e8de2e3f0f51fabe9).
* This patch **fails to generate documentation**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r458416294
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala
##########
@@ -81,6 +81,35 @@ private[spark] trait ExecutorAllocationClient {
countFailures: Boolean,
force: Boolean = false): Seq[String]
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @param adjustTargetNumExecutors whether the target number of executors will be adjusted down
+ * after these executors have been decommissioned.
+ * @return the ids of the executors acknowledged by the cluster manager to be removed.
+ */
+ def decommissionExecutors(
+ executorIds: Seq[String],
+ adjustTargetNumExecutors: Boolean): Seq[String]
+
+
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def decommissionExecutor(executorId: String): Boolean = {
+ val decommissionedExecutors = decommissionExecutors(Seq(executorId),
+ adjustTargetNumExecutors = true)
Review comment:
cool, I'll update the previous calls to `decommissioExecutor`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-672831710
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-663245753
**[Test build #126435 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126435/testReport)** for PR 28818 at commit [`f921ddd`](https://github.com/apache/spark/commit/f921ddd3913d8b749abd2d8de645e5d098d73823).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662128123
**[Test build #126281 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126281/testReport)** for PR 28818 at commit [`3db60f2`](https://github.com/apache/spark/commit/3db60f24b04a278ca4e900d48ff8454c10ef0659).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r458341897
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala
##########
@@ -81,6 +81,35 @@ private[spark] trait ExecutorAllocationClient {
countFailures: Boolean,
force: Boolean = false): Seq[String]
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @param adjustTargetNumExecutors whether the target number of executors will be adjusted down
+ * after these executors have been decommissioned.
+ * @return the ids of the executors acknowledged by the cluster manager to be removed.
+ */
+ def decommissionExecutors(
+ executorIds: Seq[String],
+ adjustTargetNumExecutors: Boolean): Seq[String]
+
+
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def decommissionExecutor(executorId: String): Boolean = {
+ val decommissionedExecutors = decommissionExecutors(Seq(executorId),
+ adjustTargetNumExecutors = true)
Review comment:
If you look above there is a configurable call. This matches how `killExecutor` is implemented down on line 124.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #28818:
URL: https://github.com/apache/spark/pull/28818#discussion_r458373949
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala
##########
@@ -81,6 +81,35 @@ private[spark] trait ExecutorAllocationClient {
countFailures: Boolean,
force: Boolean = false): Seq[String]
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @param adjustTargetNumExecutors whether the target number of executors will be adjusted down
+ * after these executors have been decommissioned.
+ * @return the ids of the executors acknowledged by the cluster manager to be removed.
+ */
+ def decommissionExecutors(
+ executorIds: Seq[String],
+ adjustTargetNumExecutors: Boolean): Seq[String]
+
+
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorIds identifiers of executors to decommission
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def decommissionExecutor(executorId: String): Boolean = {
+ val decommissionedExecutors = decommissionExecutors(Seq(executorId),
+ adjustTargetNumExecutors = true)
Review comment:
Look on line 95 of this file. I think we should match the semantics of `killExecutor` as much as possible. If there's a place where we don't want it we can use `decommissionExecutors`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645696269
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-662204302
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-643727411
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/123995/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645699958
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645157060
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #28818: [WIP][SPARK-31198][CORE] Use graceful decommissioning as part of dynamic scaling
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28818:
URL: https://github.com/apache/spark/pull/28818#issuecomment-645157055
**[Test build #124154 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124154/testReport)** for PR 28818 at commit [`9bb0293`](https://github.com/apache/spark/commit/9bb0293125af3dcde87b52be5bf0ba4dcc5f4a0f).
* This patch **fails build dependency tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org