You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jsoltren <gi...@git.apache.org> on 2017/01/20 01:46:17 UTC
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
GitHub user jsoltren opened a pull request:
https://github.com/apache/spark/pull/16650
[SPARK-16554][CORE] Automatically Kill Executors and Nodes when they are Blacklisted
## What changes were proposed in this pull request?
In SPARK-8425, we introduced a mechanism for blacklisting executors and nodes (hosts). After a certain number of failures, these resources would be "blacklisted" and no further work would be assigned to them for some period of time.
In some scenarios, it is better to fail fast, and to simply kill these unreliable resources. This changes proposes to do so by having the BlacklistTracker kill unreliable resources when they would otherwise be "blacklisted".
In order to be thread safe, this code depends on the CoarseGrainedSchedulerBackend sending a message to the driver backend in order to do the actual killing. This also helps to prevent a race which would permit work to begin on a resource (executor or node), between the time the resource is marked for killing and the time at which it is finally killed.
## How was this patch tested?
./dev/run-tests
Ran https://github.com/jsoltren/jose-utils/blob/master/blacklist/test-blacklist.sh, and checked logs to see executors and nodes being killed.
Testing can likely be improved here; suggestions welcome.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jsoltren/spark SPARK-16554-submit
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16650.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #16650
----
commit 81af45fbcbe9609cd3edaed692cb92520ea3f6e6
Author: Jos� Hiram Soltren <jo...@cloudera.com>
Date: 2016-12-01T04:01:42Z
Add test case for killExecutorsOnHost
commit 33ac3643a799eaf3a4b8a48db001eb4aa05c39ef
Author: Jos� Hiram Soltren <jo...@cloudera.com>
Date: 2016-12-01T04:36:26Z
BlacklistTracker can ask the SparkContext to kill executors on a host. Still need to wire in configuration.
commit da1d91df24310bdc0a748466f1bde746c080ea6f
Author: Jos� Hiram Soltren <jo...@cloudera.com>
Date: 2016-12-02T17:14:12Z
Respond to review feedback: basic changes
commit 87bb328f13c9c49c9c0d210394236015aa068690
Author: Jos� Hiram Soltren <jo...@cloudera.com>
Date: 2016-12-02T21:13:07Z
Add documentation for configuration.md
commit 974999c314be2b7b96a0643cb1f20de42210d29a
Author: Jos� Hiram Soltren <jo...@cloudera.com>
Date: 2016-12-02T22:33:13Z
First implementation of actual executor killing in BlacklistTracker
commit ebe35f6fc356acc15edb2a0fa1284ed3976481da
Author: Jos� Hiram Soltren <jo...@cloudera.com>
Date: 2016-12-02T23:14:35Z
Additional updates. Not sure if this killing is thread or race safe.
commit 56b5b96fc65220604495d5c4e817cfc5071efe22
Author: Jos� Hiram Soltren <jo...@cloudera.com>
Date: 2016-12-02T23:25:21Z
Add some implementation thoughts in comments to BlacklistTracker
commit c4556bd6680b393ffb949bc5b321e38209d91d37
Author: Jos� Hiram Soltren <jo...@cloudera.com>
Date: 2016-12-13T03:30:11Z
Update killing of nodes to use an RPC method for synchronization
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72038 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72038/testReport)** for PR 16650 at commit [`708a57b`](https://github.com/apache/spark/commit/708a57bf9de7285fd53d77644a3ada1ef924edf3).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72105 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72105/testReport)** for PR 16650 at commit [`fe062e5`](https://github.com/apache/spark/commit/fe062e5296842dabd62e86d6cc4dbad328a24f16).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on the issue:
https://github.com/apache/spark/pull/16650
The error was:
`ERROR: Error fetching remote repo 'origin'
hudson.plugins.git.GitException: Failed to fetch from https://github.com/apache/spark.git`
I don't think that my code did this - I'd be really impressed if it did. AMPLab issue?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72053 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72053/testReport)** for PR 16650 at commit [`580999e`](https://github.com/apache/spark/commit/580999e51692824af778fc3e191d735b2badb724).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72038/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97877918
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala ---
@@ -148,6 +153,12 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
if (executorDataMap.contains(executorId)) {
executorRef.send(RegisterExecutorFailed("Duplicate executor ID: " + executorId))
context.reply(true)
+ } else if (scheduler.nodeBlacklist != null &&
+ scheduler.nodeBlacklist.contains(executorId)) {
+ // Handle a race where the cluster manager finishes creating an executor, just after
+ // the executor is blacklisted and just before it is possibly killed.
+ executorRef.send(RegisterExecutorFailed("Executor is blacklisted: " + executorId))
+ context.reply(true)
--- End diff --
can you change the comment to
If the cluster manager gives us an executor on a blacklisted node (because it already started allocating those resources before we informed it of our blacklist, or if it ignored our blacklist), then we reject that executor immediately.
I'd also add a `logInfo` here about rejecting the executor b/c its on a blacklisted node.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72466 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72466/testReport)** for PR 16650 at commit [`37248a2`](https://github.com/apache/spark/commit/37248a202c15807fffe9e25e5b630a27dda38204).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97881374
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,24 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ // Add executor to blacklist before attempting to kill it. This allows a scheduler backend
+ // to immediately fail to allocate resources on this executor, since killing could be
+ // asynchronous.
+ conf.get(config.BLACKLIST_KILL_ENABLED) match {
+ case Some(enabled) =>
+ if (enabled) {
+ allocationClient match {
+ case Some(allocationClient) =>
+ logInfo(s"Killing blacklisted executor id $exec" +
+ s"since spark.blacklist.kill is set.")
+ allocationClient.killExecutors(Seq(exec), true, true)
+ case None =>
+ logWarning(s"Not attempting to kill blacklisted executor id $exec" +
+ s"since allocation client is not defined.")
--- End diff --
indentation
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on the issue:
https://github.com/apache/spark/pull/16650
Working through some test failures in org.apache.spark.deploy.StandaloneDynamicAllocationSuite and org.apache.spark.HeartbeatReceiverSuite...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98281415
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -187,6 +199,17 @@ private[scheduler] class BlacklistTracker (
nodeIdToBlacklistExpiryTime.put(node, expiryTimeForNewBlacklists)
listenerBus.post(SparkListenerNodeBlacklisted(now, node, blacklistedExecsOnNode.size))
_nodeBlacklist.set(nodeIdToBlacklistExpiryTime.keySet.toSet)
+ if (conf.get(config.BLACKLIST_KILL_ENABLED)) {
+ allocationClient match {
+ case Some(allocationClient) =>
+ logInfo(s"Killing all executors on blacklisted host $node" +
+ s"since spark.blacklist.kill is set.")
--- End diff --
space
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #71951 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71951/testReport)** for PR 16650 at commit [`0a3e3f1`](https://github.com/apache/spark/commit/0a3e3f126e2bb0e0c2d84f3750da16de4117dcf5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` case class KillExecutorsOnHost(host: String, replace: Boolean)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97887173
--- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -139,6 +139,11 @@ package object config {
.timeConf(TimeUnit.MILLISECONDS)
.createOptional
+ private[spark] val BLACKLIST_KILL_ENABLED =
+ ConfigBuilder("spark.blacklist.kill")
--- End diff --
I'd suggest a more verbose config name ("killBadExecutors"?) to avoid confusion ("what's being killed? the backlist?"). Or at least add ".enabled" to the name, to follow what other configs of the kind look like.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98781173
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,16 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ if (conf.get(config.BLACKLIST_KILL_ENABLED)) {
+ allocationClient match {
+ case Some(allocationClient) =>
+ logInfo(s"Killing blacklisted executor id $exec since spark.blacklist.kill is set.")
--- End diff --
the config name is wrong here now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72087 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72087/testReport)** for PR 16650 at commit [`eed4112`](https://github.com/apache/spark/commit/eed4112c092d49b4eafab363f3b0a16d83ec7c9d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #71947 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71947/testReport)** for PR 16650 at commit [`b2bf1f7`](https://github.com/apache/spark/commit/b2bf1f78a3287e79b86074653b45710ddb127d98).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72368 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72368/testReport)** for PR 16650 at commit [`3daafcc`](https://github.com/apache/spark/commit/3daafccf808e998bb8943e12be7675440b1027ea).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/16650
I think this is basically ready, perhaps one more test case. @tgravescs any other comments?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71947/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99597739
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -467,6 +469,51 @@ class StandaloneDynamicAllocationSuite
}
}
+ test("kill all executors on localhost") {
+ sc = new SparkContext(appConf)
+ val appId = sc.applicationId
+ eventually(timeout(10.seconds), interval(10.millis)) {
+ val apps = getApplications()
+ assert(apps.size === 1)
+ assert(apps.head.id === appId)
+ assert(apps.head.executors.size === 2)
+ assert(apps.head.getExecutorLimit === Int.MaxValue)
+ }
+ val beforeList = getApplications().head.executors.keys.toSet
+ // kill all executors without replacement
+ assert(killExecutorsOnHost(sc, "localhost").equals(true))
+
+ syncExecutors(sc)
+ val afterList = getApplications().head.executors.keys.toSet
+
+ eventually(timeout(10.seconds), interval(100.millis)) {
+ assert(beforeList.intersect(afterList).size == 0)
+ }
+ }
+
+ test("executor registration on a blacklisted host must fail") {
+ sc = new SparkContext(appConf.set(config.BLACKLIST_ENABLED.key, "true"))
+ val endpointRef = mock(classOf[RpcEndpointRef])
+ val mockAddress = mock(classOf[RpcAddress])
+ when(endpointRef.address).thenReturn(mockAddress)
+ val message = RegisterExecutor("one", endpointRef, "localhost", 10, Map.empty)
+
+ // Get "localhost" on a blacklist.
+ val taskScheduler = mock(classOf[TaskSchedulerImpl])
+ when(taskScheduler.nodeBlacklist()).thenReturn(Set("localhost"))
--- End diff --
though this doesn't really matter, its a little strange to have this use "localhost", it seems like its a special case. I'd change to "host-1" or something
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99595765
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala ---
@@ -600,6 +603,16 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
*/
protected def doKillExecutors(executorIds: Seq[String]): Future[Boolean] =
Future.successful(false)
+
+ /**
+ * Request that the cluster manager kill all executors on a given host.
+ * @return whether the kill request is acknowledged.
+ */
+ final override def killExecutorsOnHost(host: String): Boolean = {
+ logInfo(s"Requesting to kill any and all executors on host ${host}")
+ driverEndpoint.send(KillExecutorsOnHost(host))
--- End diff --
I think it would be helpful to add a comment here explaining why do this in the event loop, rather than killing immediatley, eg.
We might have an executor register on the bad host, at the same time that we try to kill all executors on that host. By killing all executors within the event loop (rather than in the calling thread), we ensure that the registering and killing of executors is serialized.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97889814
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -528,6 +548,16 @@ class StandaloneDynamicAllocationSuite
}
}
+ /** Kill the executors on a given host. */
+ private def killExecutorsOnHost(sc: SparkContext, host: String): Boolean = {
+ syncExecutors(sc)
+ sc.schedulerBackend match {
+ case b: CoarseGrainedSchedulerBackend =>
+ b.killExecutorsOnHost(host, false)
--- End diff --
I see that he's calling the new method with "replace = true" when doing the killing, so it seems like a legitimate change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98126123
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala ---
@@ -148,6 +153,14 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
if (executorDataMap.contains(executorId)) {
executorRef.send(RegisterExecutorFailed("Duplicate executor ID: " + executorId))
context.reply(true)
+ } else if (scheduler.nodeBlacklist != null &&
+ scheduler.nodeBlacklist.contains(hostname)) {
+ // If the cluster manager gives us an executor on a blacklisted node (because it
+ // already started allocating those resources before we informed it of our blacklist,
+ // or if it ignored our blacklist), then we reject that executor immediately.
+ logInfo(s"Rejecting $executorId as it has been blacklisted.")
+ executorRef.send(RegisterExecutorFailed("Executor is blacklisted: " + executorId))
--- End diff --
nit: use interpolation for consistency.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98071059
--- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -139,6 +139,11 @@ package object config {
.timeConf(TimeUnit.MILLISECONDS)
.createOptional
+ private[spark] val BLACKLIST_KILL_ENABLED =
+ ConfigBuilder("spark.blacklist.kill")
+ .booleanConf
+ .createOptional
--- End diff --
Updated to `createWithDefault(false)`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72468 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72468/testReport)** for PR 16650 at commit [`cb24167`](https://github.com/apache/spark/commit/cb241672692db3e604c18bcd56f441f6863a09e4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #71947 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71947/testReport)** for PR 16650 at commit [`b2bf1f7`](https://github.com/apache/spark/commit/b2bf1f78a3287e79b86074653b45710ddb127d98).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97622210
--- Diff: core/src/test/scala/org/apache/spark/scheduler/TaskSetManagerSuite.scala ---
@@ -429,7 +429,7 @@ class TaskSetManagerSuite extends SparkFunSuite with LocalSparkContext with Logg
// We don't directly use the application blacklist, but its presence triggers blacklisting
// within the taskset.
val mockListenerBus = mock(classOf[LiveListenerBus])
- val blacklistTrackerOpt = Some(new BlacklistTracker(mockListenerBus, conf, clock))
+ val blacklistTrackerOpt = Some(new BlacklistTracker(null, conf, None, clock))
--- End diff --
Yes, indeed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #71743 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71743/testReport)** for PR 16650 at commit [`c4556bd`](https://github.com/apache/spark/commit/c4556bd6680b393ffb949bc5b321e38209d91d37).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` case class KillExecutorsOnHost(host: String) extends CoarseGrainedClusterMessage`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #71743 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71743/testReport)** for PR 16650 at commit [`c4556bd`](https://github.com/apache/spark/commit/c4556bd6680b393ffb949bc5b321e38209d91d37).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72053/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72105 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72105/testReport)** for PR 16650 at commit [`fe062e5`](https://github.com/apache/spark/commit/fe062e5296842dabd62e86d6cc4dbad328a24f16).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72087/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97878659
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -528,6 +548,16 @@ class StandaloneDynamicAllocationSuite
}
}
+ /** Kill the executors on a given host. */
+ private def killExecutorsOnHost(sc: SparkContext, host: String): Boolean = {
+ syncExecutors(sc)
+ sc.schedulerBackend match {
+ case b: CoarseGrainedSchedulerBackend =>
+ b.killExecutorsOnHost(host, false)
--- End diff --
is this test the only reason to expose the `replace` argument? if so, I think you could change the test to just check for new executors. `apps.head.executors.keys.toSet` to get the executors. (you may need to use `keys.toSet`, instead of `.keySet`, because `.keySet` will just be a view which is not static.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97342524
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -50,10 +50,11 @@ import org.apache.spark.util.{Clock, SystemClock, Utils}
private[scheduler] class BlacklistTracker (
private val listenerBus: LiveListenerBus,
conf: SparkConf,
+ scheduler: Option[ExecutorAllocationClient],
clock: Clock = new SystemClock()) extends Logging {
- def this(sc: SparkContext) = {
- this(sc.listenerBus, sc.conf)
+ def this(sc: SparkContext, scheduler: Option[ExecutorAllocationClient]) = {
--- End diff --
perhaps the variable here should be allocationClient or similar instead of scheduler since it wouldn't have to be a scheduler backend.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97887675
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,24 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ // Add executor to blacklist before attempting to kill it. This allows a scheduler backend
+ // to immediately fail to allocate resources on this executor, since killing could be
+ // asynchronous.
+ conf.get(config.BLACKLIST_KILL_ENABLED) match {
+ case Some(enabled) =>
+ if (enabled) {
--- End diff --
Or:
```
case Some(true) =>
case _ =>
```
But having a default in the config is way better.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71743/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #71944 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71944/testReport)** for PR 16650 at commit [`bc3d969`](https://github.com/apache/spark/commit/bc3d969a7fe72b6ea54fd187b996f11965048367).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97888370
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala ---
@@ -140,6 +140,11 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
// Ignoring the task kill since the executor is not registered.
logWarning(s"Attempted to kill task $taskId for unknown executor $executorId.")
}
+
+ case KillExecutorsOnHost(host, replace) =>
+ scheduler.getExecutorsAliveOnHost(host).foreach(exec =>
--- End diff --
`.foreach { exec =>`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98086588
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala ---
@@ -59,6 +59,19 @@ private[spark] trait ExecutorAllocationClient {
def killExecutors(executorIds: Seq[String]): Seq[String]
/**
+ * Request that the cluster manager try harder to kill the specified executors,
+ * and maybe replace them.
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def killExecutors(executorIds: Seq[String], replace: Boolean, force: Boolean): Seq[String]
--- End diff --
Let's go with default values for now. I agree that it is good to collapse these into one function.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72368 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72368/testReport)** for PR 16650 at commit [`3daafcc`](https://github.com/apache/spark/commit/3daafccf808e998bb8943e12be7675440b1027ea).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72087 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72087/testReport)** for PR 16650 at commit [`eed4112`](https://github.com/apache/spark/commit/eed4112c092d49b4eafab363f3b0a16d83ec7c9d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99661191
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -467,6 +469,51 @@ class StandaloneDynamicAllocationSuite
}
}
+ test("kill all executors on localhost") {
+ sc = new SparkContext(appConf)
+ val appId = sc.applicationId
+ eventually(timeout(10.seconds), interval(10.millis)) {
+ val apps = getApplications()
+ assert(apps.size === 1)
+ assert(apps.head.id === appId)
+ assert(apps.head.executors.size === 2)
+ assert(apps.head.getExecutorLimit === Int.MaxValue)
+ }
+ val beforeList = getApplications().head.executors.keys.toSet
+ // kill all executors without replacement
+ assert(killExecutorsOnHost(sc, "localhost").equals(true))
+
+ syncExecutors(sc)
+ val afterList = getApplications().head.executors.keys.toSet
+
+ eventually(timeout(10.seconds), interval(100.millis)) {
+ assert(beforeList.intersect(afterList).size == 0)
+ }
+ }
+
+ test("executor registration on a blacklisted host must fail") {
+ sc = new SparkContext(appConf.set(config.BLACKLIST_ENABLED.key, "true"))
+ val endpointRef = mock(classOf[RpcEndpointRef])
+ val mockAddress = mock(classOf[RpcAddress])
+ when(endpointRef.address).thenReturn(mockAddress)
+ val message = RegisterExecutor("one", endpointRef, "localhost", 10, Map.empty)
+
+ // Get "localhost" on a blacklist.
+ val taskScheduler = mock(classOf[TaskSchedulerImpl])
+ when(taskScheduler.nodeBlacklist()).thenReturn(Set("localhost"))
--- End diff --
Let's call it "blacklisted-host".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98078786
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,24 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ // Add executor to blacklist before attempting to kill it. This allows a scheduler backend
+ // to immediately fail to allocate resources on this executor, since killing could be
+ // asynchronous.
--- End diff --
I put this comment here to avoid a race between an individual executor being blacklisted, and being scheduled for killing. The comment makes more sense in the context of an individual executor, and not an entire node.
What you're talking about is something different, and more complex: the need to - explicitly - blacklist all the executors on a node before marking that node for killing.
For example, consider hostA with executors 1, 2, and 3. We can blacklist 1, and then 2, and then hostA, but 3 is never explicitly blacklisted. The `nodeBlacklist` case needs to blacklist 3, which means we need some explicit mapping between hosts and the executors on that host in BlacklistTracker. To date we've been trying to avoid exactly this to "keep the BlacklistTracker simple".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97890113
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala ---
@@ -59,6 +59,19 @@ private[spark] trait ExecutorAllocationClient {
def killExecutors(executorIds: Seq[String]): Seq[String]
/**
+ * Request that the cluster manager try harder to kill the specified executors,
+ * and maybe replace them.
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def killExecutors(executorIds: Seq[String], replace: Boolean, force: Boolean): Seq[String]
--- End diff --
I wonder if it's not better to just change the existing method to have the extra arguments with default values. I see only a handful of calls to the 1-arg version, so maybe even forcing all of them to pass all arguments would be ok.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98283223
--- Diff: core/src/test/scala/org/apache/spark/scheduler/BlacklistTrackerSuite.scala ---
@@ -17,7 +17,8 @@
package org.apache.spark.scheduler
-import org.mockito.Mockito.{verify, when}
+import org.mockito.Matchers._
--- End diff --
I think you only need to import `Matchers.any`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72368/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97875223
--- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -139,6 +139,11 @@ package object config {
.timeConf(TimeUnit.MILLISECONDS)
.createOptional
+ private[spark] val BLACKLIST_KILL_ENABLED =
+ ConfigBuilder("spark.blacklist.kill")
+ .booleanConf
+ .createOptional
--- End diff --
I think it makes more sense to use `createWithDefault(false)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97888750
--- Diff: core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala ---
@@ -1154,4 +1154,13 @@ private class DummyLocalSchedulerBackend (sc: SparkContext, sb: SchedulerBackend
override def reviveOffers(): Unit = sb.reviveOffers()
override def defaultParallelism(): Int = sb.defaultParallelism()
+
+ // Unused.
+ override def killExecutorsOnHost(host: String, replace: Boolean): Boolean = {
+ false
+ }
+
+ // Unused.
+ override def killExecutors(executorIds: Seq[String], replace: Boolean, force: Boolean)
--- End diff --
style: if you need a line break, then every argument goes on a separate line.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97341837
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -168,6 +169,21 @@ private[scheduler] class BlacklistTracker (
if (newTotal >= MAX_FAILURES_PER_EXEC && !executorIdToBlacklistStatus.contains(exec)) {
logInfo(s"Blacklisting executor id: $exec because it has $newTotal" +
s" task failures in successful task sets")
+ conf.get(config.BLACKLIST_ENABLED) match {
--- End diff --
shouldn't this just be using isBlacklistEnabled? and actually I don't think you will even get here if its disabled because it won't create the BlacklistTracker at all so this shouldn't be needed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99664910
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -467,6 +469,52 @@ class StandaloneDynamicAllocationSuite
}
}
+ test("kill all executors on localhost") {
+ sc = new SparkContext(appConf)
+ val appId = sc.applicationId
+ eventually(timeout(10.seconds), interval(10.millis)) {
+ val apps = getApplications()
+ assert(apps.size === 1)
+ assert(apps.head.id === appId)
+ assert(apps.head.executors.size === 2)
+ assert(apps.head.getExecutorLimit === Int.MaxValue)
+ }
+ val beforeList = getApplications().head.executors.keys.toSet
+ assert(killExecutorsOnHost(sc, "localhost").equals(true))
+
+ syncExecutors(sc)
+ val afterList = getApplications().head.executors.keys.toSet
+
+ eventually(timeout(10.seconds), interval(100.millis)) {
+ assert(beforeList.intersect(afterList).size == 0)
+ }
+ }
+
+ test("executor registration on a blacklisted host must fail") {
+ sc = new SparkContext(appConf.set(config.BLACKLIST_ENABLED.key, "true"))
+ val endpointRef = mock(classOf[RpcEndpointRef])
+ val mockAddress = mock(classOf[RpcAddress])
+ when(endpointRef.address).thenReturn(mockAddress)
+ val message = RegisterExecutor("one", endpointRef, "blacklisted-host", 10, Map.empty)
+
+ // Get "localhost" on a blacklist.
+ val taskScheduler = mock(classOf[TaskSchedulerImpl])
+ when(taskScheduler.nodeBlacklist()).thenReturn(Set("blacklisted-host"))
+ when(taskScheduler.sc).thenReturn(sc)
+ sc.taskScheduler = taskScheduler
+
+ // Create a fresh scheduler backend to blacklist "localhost".
+ sc.schedulerBackend.stop()
+ val backend =
+ new StandaloneSchedulerBackend(taskScheduler, sc, Array(masterRpcEnv.address.toSparkURL))
--- End diff --
Would be really nice to have automated style checks...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72466 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72466/testReport)** for PR 16650 at commit [`37248a2`](https://github.com/apache/spark/commit/37248a202c15807fffe9e25e5b630a27dda38204).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99661763
--- Diff: core/src/test/scala/org/apache/spark/scheduler/BlacklistTrackerSuite.scala ---
@@ -456,4 +461,69 @@ class BlacklistTrackerSuite extends SparkFunSuite with BeforeAndAfterEach with M
conf.remove(config)
}
}
+
+ test("blacklisting kills executors, configured by BLACKLIST_KILL_ENABLED") {
+ val allocationClientMock = mock[ExecutorAllocationClient]
+ when(allocationClientMock.killExecutors(any(), any(), any())).thenReturn(Seq("called"))
+ when(allocationClientMock.killExecutorsOnHost("hostA")).thenAnswer(new Answer[Boolean] {
+ override def answer(invocation: InvocationOnMock): Boolean = {
+ if (blacklist.nodeBlacklist.contains("hostA") == false) {
+ throw new IllegalStateException("hostA should be on the blacklist")
--- End diff --
Sure. I've used your text with very minor modification.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/16650
one more thought on tests -- your mock `ExecutorAllocationClient` could make sure that when `killExecutorsOnHost` is called, the host is already in `nodeBlacklist`, to avoid that last race you found.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99660908
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -467,6 +469,51 @@ class StandaloneDynamicAllocationSuite
}
}
+ test("kill all executors on localhost") {
+ sc = new SparkContext(appConf)
+ val appId = sc.applicationId
+ eventually(timeout(10.seconds), interval(10.millis)) {
+ val apps = getApplications()
+ assert(apps.size === 1)
+ assert(apps.head.id === appId)
+ assert(apps.head.executors.size === 2)
+ assert(apps.head.getExecutorLimit === Int.MaxValue)
+ }
+ val beforeList = getApplications().head.executors.keys.toSet
+ // kill all executors without replacement
--- End diff --
Best to just delete the comment, then. Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97886943
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala ---
@@ -59,6 +59,19 @@ private[spark] trait ExecutorAllocationClient {
def killExecutors(executorIds: Seq[String]): Seq[String]
/**
+ * Request that the cluster manager try harder to kill the specified executors,
+ * and maybe replace them.
+ * @return whether the request is acknowledged by the cluster manager.
--- End diff --
The doc is also not accurate (return value description is not right).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on the issue:
https://github.com/apache/spark/pull/16650
@squito and I chatted about this some offline. Not only does receiveAndReply in CoarseGrainedSchedulerBackend need to handle the situation where it is trying to do a RegisterExecutor on an already blacklisted executor, but also, BlacklistTracker needs to perform blacklisting before requesting killing (and not the other way around) to avoid introducing a potential race. I'll send an amended patch with these modifications shortly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98281403
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,17 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ if (conf.get(config.BLACKLIST_KILL_ENABLED)) {
+ allocationClient match {
+ case Some(allocationClient) =>
+ logInfo(s"Killing blacklisted executor id $exec" +
+ s"since spark.blacklist.kill is set.")
+ allocationClient.killExecutors(Seq(exec), true, true)
+ case None =>
+ logWarning(s"Not attempting to kill blacklisted executor id $exec" +
+ s"since allocation client is not defined.")
--- End diff --
space
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97874677
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala ---
@@ -59,6 +59,19 @@ private[spark] trait ExecutorAllocationClient {
def killExecutors(executorIds: Seq[String]): Seq[String]
/**
+ * Request that the cluster manager try harder to kill the specified executors,
+ * and maybe replace them.
+ * @return whether the request is acknowledged by the cluster manager.
--- End diff --
I'd just copy the doc on this method from CoarseGrainedSchedulerBackend -- its at least worth explaining `force`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on the issue:
https://github.com/apache/spark/pull/16650
My solution for the `org.apache.spark.deploy.StandaloneDynamicAllocationSuite.kill all executors on localhost` failure was to add a Boolean argument to killExecutorsOnHost specifying if killed executors should be recreated, and not re-creating them in the test. Also, I threw the check for killing in an `eventually` block since it is an asynchronous action.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97888681
--- Diff: core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala ---
@@ -1154,4 +1154,13 @@ private class DummyLocalSchedulerBackend (sc: SparkContext, sb: SchedulerBackend
override def reviveOffers(): Unit = sb.reviveOffers()
override def defaultParallelism(): Int = sb.defaultParallelism()
+
+ // Unused.
--- End diff --
unnecessary comment (also below)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98084608
--- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -139,6 +139,11 @@ package object config {
.timeConf(TimeUnit.MILLISECONDS)
.createOptional
+ private[spark] val BLACKLIST_KILL_ENABLED =
+ ConfigBuilder("spark.blacklist.kill")
--- End diff --
Sure. Let's call it `.killBlacklistedExecutors`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99664044
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -467,6 +469,52 @@ class StandaloneDynamicAllocationSuite
}
}
+ test("kill all executors on localhost") {
+ sc = new SparkContext(appConf)
+ val appId = sc.applicationId
+ eventually(timeout(10.seconds), interval(10.millis)) {
+ val apps = getApplications()
+ assert(apps.size === 1)
+ assert(apps.head.id === appId)
+ assert(apps.head.executors.size === 2)
+ assert(apps.head.getExecutorLimit === Int.MaxValue)
+ }
+ val beforeList = getApplications().head.executors.keys.toSet
+ assert(killExecutorsOnHost(sc, "localhost").equals(true))
+
+ syncExecutors(sc)
+ val afterList = getApplications().head.executors.keys.toSet
+
+ eventually(timeout(10.seconds), interval(100.millis)) {
+ assert(beforeList.intersect(afterList).size == 0)
+ }
+ }
+
+ test("executor registration on a blacklisted host must fail") {
+ sc = new SparkContext(appConf.set(config.BLACKLIST_ENABLED.key, "true"))
+ val endpointRef = mock(classOf[RpcEndpointRef])
+ val mockAddress = mock(classOf[RpcAddress])
+ when(endpointRef.address).thenReturn(mockAddress)
+ val message = RegisterExecutor("one", endpointRef, "blacklisted-host", 10, Map.empty)
+
+ // Get "localhost" on a blacklist.
+ val taskScheduler = mock(classOf[TaskSchedulerImpl])
+ when(taskScheduler.nodeBlacklist()).thenReturn(Set("blacklisted-host"))
+ when(taskScheduler.sc).thenReturn(sc)
+ sc.taskScheduler = taskScheduler
+
+ // Create a fresh scheduler backend to blacklist "localhost".
+ sc.schedulerBackend.stop()
+ val backend =
+ new StandaloneSchedulerBackend(taskScheduler, sc, Array(masterRpcEnv.address.toSparkURL))
--- End diff --
super nit: looks like this is only indented one space, not two
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #71951 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71951/testReport)** for PR 16650 at commit [`0a3e3f1`](https://github.com/apache/spark/commit/0a3e3f126e2bb0e0c2d84f3750da16de4117dcf5).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98281375
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,17 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ if (conf.get(config.BLACKLIST_KILL_ENABLED)) {
+ allocationClient match {
+ case Some(allocationClient) =>
+ logInfo(s"Killing blacklisted executor id $exec" +
+ s"since spark.blacklist.kill is set.")
--- End diff --
add a space either after $exec or before "since" (inside the string)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98078837
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,24 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ // Add executor to blacklist before attempting to kill it. This allows a scheduler backend
+ // to immediately fail to allocate resources on this executor, since killing could be
+ // asynchronous.
+ conf.get(config.BLACKLIST_KILL_ENABLED) match {
+ case Some(enabled) =>
+ if (enabled) {
--- End diff --
Yes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72468 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72468/testReport)** for PR 16650 at commit [`cb24167`](https://github.com/apache/spark/commit/cb241672692db3e604c18bcd56f441f6863a09e4).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #71944 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71944/testReport)** for PR 16650 at commit [`bc3d969`](https://github.com/apache/spark/commit/bc3d969a7fe72b6ea54fd187b996f11965048367).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99474371
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -489,6 +491,29 @@ class StandaloneDynamicAllocationSuite
}
}
+ test("executor registration on a blacklisted host must fail") {
+ sc = new SparkContext(appConf.set(config.BLACKLIST_ENABLED.key, "true"))
+ val endpointRef = mock(classOf[RpcEndpointRef])
+ val mockAddress = mock(classOf[RpcAddress])
+ when(endpointRef.address).thenReturn(mockAddress)
+ val message = RegisterExecutor("one", endpointRef, "localhost", 10, Map.empty)
+
+ // Get "localhost" on a blacklist.
+ val taskScheduler = mock(classOf[TaskSchedulerImpl])
+ when(taskScheduler.nodeBlacklist()).thenReturn(Set("localhost"))
+ when(taskScheduler.sc).thenReturn(sc)
+ sc.taskScheduler = taskScheduler
+
+ // Create a fresh scheduler backend to blacklist "localhost".
+ sc.schedulerBackend.stop()
+ val backend =
+ new StandaloneSchedulerBackend(taskScheduler, sc, Array(masterRpcEnv.address.toSparkURL))
+ backend.start()
+
+ backend.driverEndpoint.ask[Boolean](message)
+ verify(endpointRef).send(RegisterExecutorFailed(any()))
--- End diff --
You need to wrap this in an `eventually` since its async.
btw, a simple way to see if your test is flaky locally is to run it in in a loop:
```scala
(0 until 100).foreach { idx =>
test(s"executor registration on a blacklisted host must fail $idx") {
```
and then in sbt
```
test-only *StandaloneDynamicAllocationSuite -- -z "must fail"
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97622417
--- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
@@ -719,7 +719,11 @@ private[spark] object TaskSchedulerImpl {
private def maybeCreateBlacklistTracker(sc: SparkContext): Option[BlacklistTracker] = {
if (BlacklistTracker.isBlacklistEnabled(sc.conf)) {
- Some(new BlacklistTracker(sc))
+ val executorAllocClient: Option[ExecutorAllocationClient] = sc.schedulerBackend match {
+ case b: ExecutorAllocationClient => Some(b.asInstanceOf[ExecutorAllocationClient])
--- End diff --
...I didn't realize we got a free type cast with the case match. Neat. Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97622188
--- Diff: core/src/test/scala/org/apache/spark/scheduler/BlacklistTrackerSuite.scala ---
@@ -43,7 +43,7 @@ class BlacklistTrackerSuite extends SparkFunSuite with BeforeAndAfterEach with M
clock.setTime(0)
listenerBusMock = mock[LiveListenerBus]
- blacklist = new BlacklistTracker(listenerBusMock, conf, clock)
+ blacklist = new BlacklistTracker(null, conf, None, clock)
--- End diff --
Yes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97625393
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -168,6 +169,21 @@ private[scheduler] class BlacklistTracker (
if (newTotal >= MAX_FAILURES_PER_EXEC && !executorIdToBlacklistStatus.contains(exec)) {
logInfo(s"Blacklisting executor id: $exec because it has $newTotal" +
s" task failures in successful task sets")
+ conf.get(config.BLACKLIST_ENABLED) match {
--- End diff --
Yes indeed. The only non-test usage of BlacklistTracker.isBlacklistEnabled is in the TaskSchedulerImpl's maybeCreateBlacklistTracker, which uses it as a condition for creating the BlacklistTracker at all. So I agree, we don't need a check here to see if the blacklist enabled, and if we did, isBlacklistEnabled would be a better choice.
However, it is still meaningful to see if spark.blacklist.kill is set. I believe this was just a typo: s/BLACKLIST_ENABLED/BLACKLIST_KILL_ENABLED/. This is fixed in my latest commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72105/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97881356
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,24 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ // Add executor to blacklist before attempting to kill it. This allows a scheduler backend
+ // to immediately fail to allocate resources on this executor, since killing could be
+ // asynchronous.
+ conf.get(config.BLACKLIST_KILL_ENABLED) match {
+ case Some(enabled) =>
+ if (enabled) {
+ allocationClient match {
+ case Some(allocationClient) =>
+ logInfo(s"Killing blacklisted executor id $exec" +
+ s"since spark.blacklist.kill is set.")
--- End diff --
nit: the following line should just be indented (2 spaces), not aligned. a couple other spots too
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71944/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98090656
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,24 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ // Add executor to blacklist before attempting to kill it. This allows a scheduler backend
+ // to immediately fail to allocate resources on this executor, since killing could be
+ // asynchronous.
--- End diff --
We chatted offline and there were two misunderstandings (on my end) here. One is that the order of killing and adding to the blacklist in `BlacklistTracker` doesn't matter since that code is holding the taskScheduler lock. The other is that `scheduler.nodeBlacklist.contains(executorId))` in CoarseGrainedSchedulerBackend was wrong - it should be `scheduler.nodeBlacklist.contains(hostname))`.
The next patch set that I upload will address both these issues.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72053 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72053/testReport)** for PR 16650 at commit [`580999e`](https://github.com/apache/spark/commit/580999e51692824af778fc3e191d735b2badb724).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98125826
--- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -139,6 +139,11 @@ package object config {
.timeConf(TimeUnit.MILLISECONDS)
.createOptional
+ private[spark] val BLACKLIST_KILL_ENABLED =
+ ConfigBuilder("spark.blacklist.killBlacklistedExecutors")
+ .booleanConf
--- End diff --
nit: indent extra level
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/16650
Jenkins, ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99596040
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -467,6 +469,51 @@ class StandaloneDynamicAllocationSuite
}
}
+ test("kill all executors on localhost") {
+ sc = new SparkContext(appConf)
+ val appId = sc.applicationId
+ eventually(timeout(10.seconds), interval(10.millis)) {
+ val apps = getApplications()
+ assert(apps.size === 1)
+ assert(apps.head.id === appId)
+ assert(apps.head.executors.size === 2)
+ assert(apps.head.getExecutorLimit === Int.MaxValue)
+ }
+ val beforeList = getApplications().head.executors.keys.toSet
+ // kill all executors without replacement
--- End diff --
"without replacement" is now wrong
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99660755
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala ---
@@ -600,6 +603,16 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
*/
protected def doKillExecutors(executorIds: Seq[String]): Future[Boolean] =
Future.successful(false)
+
+ /**
+ * Request that the cluster manager kill all executors on a given host.
+ * @return whether the kill request is acknowledged.
+ */
+ final override def killExecutorsOnHost(host: String): Boolean = {
+ logInfo(s"Requesting to kill any and all executors on host ${host}")
+ driverEndpoint.send(KillExecutorsOnHost(host))
--- End diff --
Sure. I've paraphrased this a bit but it's a helpful comment to add.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98781265
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -187,6 +198,19 @@ private[scheduler] class BlacklistTracker (
nodeIdToBlacklistExpiryTime.put(node, expiryTimeForNewBlacklists)
listenerBus.post(SparkListenerNodeBlacklisted(now, node, blacklistedExecsOnNode.size))
_nodeBlacklist.set(nodeIdToBlacklistExpiryTime.keySet.toSet)
+ if (conf.get(config.BLACKLIST_KILL_ENABLED)) {
+ allocationClient match {
+ case Some(allocationClient) =>
+ logInfo(s"Killing all executors on blacklisted host $node " +
+ s"since spark.blacklist.kill is set.")
--- End diff --
config name wrong
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98079880
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala ---
@@ -148,6 +153,12 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
if (executorDataMap.contains(executorId)) {
executorRef.send(RegisterExecutorFailed("Duplicate executor ID: " + executorId))
context.reply(true)
+ } else if (scheduler.nodeBlacklist != null &&
+ scheduler.nodeBlacklist.contains(executorId)) {
+ // Handle a race where the cluster manager finishes creating an executor, just after
+ // the executor is blacklisted and just before it is possibly killed.
+ executorRef.send(RegisterExecutorFailed("Executor is blacklisted: " + executorId))
+ context.reply(true)
--- End diff --
Sure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97875600
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,24 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ // Add executor to blacklist before attempting to kill it. This allows a scheduler backend
+ // to immediately fail to allocate resources on this executor, since killing could be
+ // asynchronous.
+ conf.get(config.BLACKLIST_KILL_ENABLED) match {
+ case Some(enabled) =>
+ if (enabled) {
--- End diff --
after changing the option to use `createWithDefault(false)`, you only need
```
if (conf.get(config.BLACKLIST_KILL_ENABLED))
```
(even without the default, you could still use `conf.get(config.BLACKLIST_KILL_ENABLED)).getOrElse(false)`)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97162706
--- Diff: core/src/test/scala/org/apache/spark/scheduler/BlacklistTrackerSuite.scala ---
@@ -43,7 +43,7 @@ class BlacklistTrackerSuite extends SparkFunSuite with BeforeAndAfterEach with M
clock.setTime(0)
listenerBusMock = mock[LiveListenerBus]
- blacklist = new BlacklistTracker(listenerBusMock, conf, clock)
+ blacklist = new BlacklistTracker(null, conf, None, clock)
--- End diff --
looks like a bad merge here -- you still want the `listenerBusMock`, right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98281426
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -187,6 +199,17 @@ private[scheduler] class BlacklistTracker (
nodeIdToBlacklistExpiryTime.put(node, expiryTimeForNewBlacklists)
listenerBus.post(SparkListenerNodeBlacklisted(now, node, blacklistedExecsOnNode.size))
_nodeBlacklist.set(nodeIdToBlacklistExpiryTime.keySet.toSet)
+ if (conf.get(config.BLACKLIST_KILL_ENABLED)) {
+ allocationClient match {
+ case Some(allocationClient) =>
+ logInfo(s"Killing all executors on blacklisted host $node" +
+ s"since spark.blacklist.kill is set.")
+ allocationClient.killExecutorsOnHost(node)
+ case None =>
+ logWarning(s"Not attempting to kill executors on blacklisted host $node" +
+ s"since allocation client is not defined.")
--- End diff --
space
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/16650
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72466/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16650
**[Test build #72038 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72038/testReport)** for PR 16650 at commit [`708a57b`](https://github.com/apache/spark/commit/708a57bf9de7285fd53d77644a3ada1ef924edf3).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98126533
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala ---
@@ -54,11 +54,28 @@ private[spark] trait ExecutorAllocationClient {
/**
* Request that the cluster manager kill the specified executors.
+ *
+ * When asking the executor to be replaced, the executor loss is considered a failure, and
+ * killed tasks that are running on the executor will count towards the failure limits. If no
+ * replacement is being requested, then the tasks will not count towards the limit.
+ *
+ * @param executorIds identifiers of executors to kill
+ * @param replace whether to replace the killed executors with new ones, default false
+ * @param force whether to force kill busy executors, default false
* @return the ids of the executors acknowledged by the cluster manager to be removed.
*/
- def killExecutors(executorIds: Seq[String]): Seq[String]
+ def killExecutors(
+ executorIds: Seq[String],
+ replace: Boolean = false,
+ force: Boolean = false): Seq[String]
/**
+ * Request that the cluster manager kill every executor on the specified host.
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def killExecutorsOnHost(host: String): Boolean
--- End diff --
Probably good to specify here what's the behavior regarding "force" and "replace", since they're not arguments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99596922
--- Diff: core/src/test/scala/org/apache/spark/scheduler/BlacklistTrackerSuite.scala ---
@@ -456,4 +461,69 @@ class BlacklistTrackerSuite extends SparkFunSuite with BeforeAndAfterEach with M
conf.remove(config)
}
}
+
+ test("blacklisting kills executors, configured by BLACKLIST_KILL_ENABLED") {
+ val allocationClientMock = mock[ExecutorAllocationClient]
+ when(allocationClientMock.killExecutors(any(), any(), any())).thenReturn(Seq("called"))
+ when(allocationClientMock.killExecutorsOnHost("hostA")).thenAnswer(new Answer[Boolean] {
+ override def answer(invocation: InvocationOnMock): Boolean = {
+ if (blacklist.nodeBlacklist.contains("hostA") == false) {
+ throw new IllegalStateException("hostA should be on the blacklist")
--- End diff --
Worth adding a comment here explaining what the point of this check is
To avoid a race between blacklisting and killing, its important that the nodeBlacklist is updated *before* we ask the executor allocation client to kill executors on the host.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97162721
--- Diff: core/src/test/scala/org/apache/spark/scheduler/TaskSetManagerSuite.scala ---
@@ -429,7 +429,7 @@ class TaskSetManagerSuite extends SparkFunSuite with LocalSparkContext with Logg
// We don't directly use the application blacklist, but its presence triggers blacklisting
// within the taskset.
val mockListenerBus = mock(classOf[LiveListenerBus])
- val blacklistTrackerOpt = Some(new BlacklistTracker(mockListenerBus, conf, clock))
+ val blacklistTrackerOpt = Some(new BlacklistTracker(null, conf, None, clock))
--- End diff --
same here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97881401
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala ---
@@ -148,6 +153,12 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
if (executorDataMap.contains(executorId)) {
executorRef.send(RegisterExecutorFailed("Duplicate executor ID: " + executorId))
context.reply(true)
+ } else if (scheduler.nodeBlacklist != null &&
+ scheduler.nodeBlacklist.contains(executorId)) {
--- End diff --
indentation
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97162877
--- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
@@ -719,7 +719,11 @@ private[spark] object TaskSchedulerImpl {
private def maybeCreateBlacklistTracker(sc: SparkContext): Option[BlacklistTracker] = {
if (BlacklistTracker.isBlacklistEnabled(sc.conf)) {
- Some(new BlacklistTracker(sc))
+ val executorAllocClient: Option[ExecutorAllocationClient] = sc.schedulerBackend match {
+ case b: ExecutorAllocationClient => Some(b.asInstanceOf[ExecutorAllocationClient])
--- End diff --
you shouldn't need the `asInstanceOf` -- its automatic from your `case`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/16650
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r99660675
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -489,6 +491,29 @@ class StandaloneDynamicAllocationSuite
}
}
+ test("executor registration on a blacklisted host must fail") {
+ sc = new SparkContext(appConf.set(config.BLACKLIST_ENABLED.key, "true"))
+ val endpointRef = mock(classOf[RpcEndpointRef])
+ val mockAddress = mock(classOf[RpcAddress])
+ when(endpointRef.address).thenReturn(mockAddress)
+ val message = RegisterExecutor("one", endpointRef, "localhost", 10, Map.empty)
+
+ // Get "localhost" on a blacklist.
+ val taskScheduler = mock(classOf[TaskSchedulerImpl])
+ when(taskScheduler.nodeBlacklist()).thenReturn(Set("localhost"))
+ when(taskScheduler.sc).thenReturn(sc)
+ sc.taskScheduler = taskScheduler
+
+ // Create a fresh scheduler backend to blacklist "localhost".
+ sc.schedulerBackend.stop()
+ val backend =
+ new StandaloneSchedulerBackend(taskScheduler, sc, Array(masterRpcEnv.address.toSparkURL))
+ backend.start()
+
+ backend.driverEndpoint.ask[Boolean](message)
+ verify(endpointRef).send(RegisterExecutorFailed(any()))
--- End diff --
Thanks for the tip. Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98070834
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala ---
@@ -59,6 +59,19 @@ private[spark] trait ExecutorAllocationClient {
def killExecutors(executorIds: Seq[String]): Seq[String]
/**
+ * Request that the cluster manager try harder to kill the specified executors,
+ * and maybe replace them.
+ * @return whether the request is acknowledged by the cluster manager.
--- End diff --
I've copied the doc from CoarseGrainedSchedulerBackend here and changed the incorrect `return` doc.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71951/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98283147
--- Diff: core/src/test/scala/org/apache/spark/scheduler/BlacklistTrackerSuite.scala ---
@@ -456,4 +459,61 @@ class BlacklistTrackerSuite extends SparkFunSuite with BeforeAndAfterEach with M
conf.remove(config)
}
}
+
+ test("blacklisting kills executors, configured by BLACKLIST_KILL_ENABLED") {
+ val allocationClientMock = mock[ExecutorAllocationClient]
+ when(allocationClientMock.killExecutors(any(), any(), any())).thenReturn(Seq("called"))
+ when(allocationClientMock.killExecutorsOnHost(any())).thenReturn(true)
+ blacklist = new BlacklistTracker(listenerBusMock, conf, Some(allocationClientMock), clock)
+
+ // Disable auto-kill. Blacklist an executor and make sure killExecutors is not called.
+ conf.set(config.BLACKLIST_KILL_ENABLED, false)
+
+ val taskSetBlacklist0 = createTaskSetBlacklist(stageId = 0)
+ // Fail 4 tasks in one task set on executor 1, so that executor gets blacklisted for the whole
+ // application.
+ (0 until 4).foreach { partition =>
+ taskSetBlacklist0.updateBlacklistForFailedTask("hostA", exec = "1", index = partition)
+ }
+ blacklist.updateBlacklistForSuccessfulTaskSet(0, 0, taskSetBlacklist0.execToFailures)
+
+ verify(allocationClientMock, times(0)).killExecutor(any())
--- End diff --
you could also use `Mockito.never` instead of `times(0)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97887397
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,24 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ // Add executor to blacklist before attempting to kill it. This allows a scheduler backend
--- End diff --
"the scheduler backend..."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72468/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97877539
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -173,6 +174,24 @@ private[scheduler] class BlacklistTracker (
listenerBus.post(SparkListenerExecutorBlacklisted(now, exec, newTotal))
executorIdToFailureList.remove(exec)
updateNextExpiryTime()
+ // Add executor to blacklist before attempting to kill it. This allows a scheduler backend
+ // to immediately fail to allocate resources on this executor, since killing could be
+ // asynchronous.
--- End diff --
This comment is only needed above the nodeBlacklist case. Each executor ID is always unique, so we dont' need to worry about the cluster manager registering the same executor again.
Also, I think "allows ... to fail" is a little confusing. How about "This is so the scheduler backend can reject any additional resources the cluster manager gives on the blacklisted node."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98782219
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -187,6 +198,19 @@ private[scheduler] class BlacklistTracker (
nodeIdToBlacklistExpiryTime.put(node, expiryTimeForNewBlacklists)
listenerBus.post(SparkListenerNodeBlacklisted(now, node, blacklistedExecsOnNode.size))
_nodeBlacklist.set(nodeIdToBlacklistExpiryTime.keySet.toSet)
+ if (conf.get(config.BLACKLIST_KILL_ENABLED)) {
+ allocationClient match {
+ case Some(allocationClient) =>
+ logInfo(s"Killing all executors on blacklisted host $node " +
+ s"since spark.blacklist.kill is set.")
+ if(allocationClient.killExecutorsOnHost(node) == false) {
--- End diff --
nit space after if before (
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16650
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71949/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97875716
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -187,6 +206,22 @@ private[scheduler] class BlacklistTracker (
nodeIdToBlacklistExpiryTime.put(node, expiryTimeForNewBlacklists)
listenerBus.post(SparkListenerNodeBlacklisted(now, node, blacklistedExecsOnNode.size))
_nodeBlacklist.set(nodeIdToBlacklistExpiryTime.keySet.toSet)
+ // As before, blacklist this node before killing all its executors.
+ conf.get(config.BLACKLIST_KILL_ENABLED) match {
+ case Some(enabled) =>
+ if (enabled) {
--- End diff --
same here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by jsoltren <gi...@git.apache.org>.
Github user jsoltren commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r98082949
--- Diff: core/src/test/scala/org/apache/spark/deploy/StandaloneDynamicAllocationSuite.scala ---
@@ -528,6 +548,16 @@ class StandaloneDynamicAllocationSuite
}
}
+ /** Kill the executors on a given host. */
+ private def killExecutorsOnHost(sc: SparkContext, host: String): Boolean = {
+ syncExecutors(sc)
+ sc.schedulerBackend match {
+ case b: CoarseGrainedSchedulerBackend =>
+ b.killExecutorsOnHost(host, false)
--- End diff --
Yes, the test was the sole reason for exposing replace, so it can be removed. I was having some issues with .keySet earlier, and, we do need to `syncExecutors(sc)` after killing to pass the test. I'm keeping the `eventually` block since killing happens asynchronously.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16650: [SPARK-16554][CORE] Automatically Kill Executors and Nod...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/16650
merged to master, thanks @jsoltren
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16650: [SPARK-16554][CORE] Automatically Kill Executors ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/16650#discussion_r97887303
--- Diff: core/src/main/scala/org/apache/spark/scheduler/BlacklistTracker.scala ---
@@ -50,10 +50,11 @@ import org.apache.spark.util.{Clock, SystemClock, Utils}
private[scheduler] class BlacklistTracker (
private val listenerBus: LiveListenerBus,
conf: SparkConf,
+ allocationClient: Option[ExecutorAllocationClient],
clock: Clock = new SystemClock()) extends Logging {
- def this(sc: SparkContext) = {
- this(sc.listenerBus, sc.conf)
+ def this(sc: SparkContext, allocationClient: Option[ExecutorAllocationClient]) = {
+ this(sc.listenerBus, sc.getConf, allocationClient)
--- End diff --
`sc.conf`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org