You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by lianhuiwang <gi...@git.apache.org> on 2015/01/09 03:28:33 UTC
[GitHub] spark pull request: [SPARK-4955]With executor dynamic scaling enab...
GitHub user lianhuiwang opened a pull request:
https://github.com/apache/spark/pull/3962
[SPARK-4955]With executor dynamic scaling enabled,executor shoude be added or killed in yarn-cluster mode.
With executor dynamic scaling enabled, executor number shoude be added or killed in yarn-cluster mode.so in yarn-cluster mode, ApplicationMaster start a AMActor that add or kill a executor. then YarnSchedulerActor in YarnSchedulerBackend send message to am's AMActor.
@andrewor14 @ChengXiangLi
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lianhuiwang/spark SPARK-4955
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3962.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 #3962
----
commit 6dfeeecd4a206b9a82952e3b9f78128a0013d3c9
Author: lianhuiwang <li...@gmail.com>
Date: 2015-01-09T02:17:38Z
in yarn-cluster mode,executor number can be added or killed.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23511414
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala ---
@@ -94,12 +91,14 @@ private[spark] abstract class YarnSchedulerBackend(
/**
* An actor that communicates with the ApplicationMaster.
*/
- private class YarnSchedulerActor extends Actor {
+ protected class YarnSchedulerActor(isClusterMode: Boolean) extends Actor {
--- End diff --
this seems like an inappropriate use of class hierarchy. Instead I would make this an abstract class, and declare a `YarnClientSchedulerActor` in `YarnClientSchedulerBackend` that overrides `preStart()`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71425412
[Test build #26084 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26084/consoleFull) for PR 3962 at commit [`12426af`](https://github.com/apache/spark/commit/12426afb86bf626d9f5000614132e5abb9d5bb22).
* 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22910255
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -474,13 +482,17 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
// we can monitor Lifecycle Events.
driver ! "Hello"
driver ! RegisterClusterManager
- context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
+ if (isDriver) {
+ context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
+ }
}
override def receive = {
case x: DisassociatedEvent =>
logInfo(s"Driver terminated or disconnected! Shutting down. $x")
- finish(FinalApplicationStatus.SUCCEEDED, ApplicationMaster.EXIT_SUCCESS)
+ if (!isDriver) {
+ finish(FinalApplicationStatus.SUCCEEDED, ApplicationMaster.EXIT_SUCCESS)
+ }
--- End diff --
yes, when in cluster mode, driver is user class's main method. when driver terminated or disconnected, AM can monitor and can determine whether main class is success or not. so there is not 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69882212
[Test build #25524 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25524/consoleFull) for PR 3962 at commit [`f43bda8`](https://github.com/apache/spark/commit/f43bda817a539bd7b61469ed393ed637e082d59c).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69888848
[Test build #25524 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25524/consoleFull) for PR 3962 at commit [`f43bda8`](https://github.com/apache/spark/commit/f43bda817a539bd7b61469ed393ed637e082d59c).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor(isClusterMode: Boolean) extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69297562
[Test build #25299 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25299/consoleFull) for PR 3962 at commit [`7d33791`](https://github.com/apache/spark/commit/7d33791c16fa0853568f3d72c22265b153484db1).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71425735
[Test build #26082 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26082/consoleFull) for PR 3962 at commit [`9318fc1`](https://github.com/apache/spark/commit/9318fc1c9c707701864f87264e68c8d7541dd8ed).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71953371
@andrewor14 thanks for you help.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71421649
[Test build #26083 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26083/consoleFull) for PR 3962 at commit [`45da3b0`](https://github.com/apache/spark/commit/45da3b0bf03c22d747706489a47c7b0140e0d59a).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23719605
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -231,6 +231,25 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
reporterThread = launchReporterThread()
}
+ /**
+ * Creates an actor that ApplicationMaster communicates with YarnScheduler of driver
+ * in Yarn deploy mode.
+ *
+ * If isDriver is set to true, AMActor and driver belong to same process.
+ * so AMActor don't monitor lifecycle of driver.
+ */
+ private def runAMActor(
+ host: String,
+ port: String,
+ isDriver: Boolean): Unit = {
--- End diff --
that's fine. I actually proposed the renaming in other patches too. Let's just do it separately then.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22910550
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -464,7 +472,7 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
/**
* Actor that communicates with the driver in client deploy mode.
--- End diff --
i donot think it, when yarn client mode, it need to transfer false to AMActor. because AMActor and Driver do not belong to same process. so AMActor need to keep a long connect with Driver. when Driver exit, AMActor need to detect it. but in yarn cluster mode, because AMActor and driver belong to same process, they do not need to keep a long connect.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69289908
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25291/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-70623765
[Test build #25809 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25809/consoleFull) for PR 3962 at commit [`265c36d`](https://github.com/apache/spark/commit/265c36d3c6c0feccf6db6298e99021f2d14a4060).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor(isClusterMode: Boolean) extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23510879
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -231,6 +231,18 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
reporterThread = launchReporterThread()
}
+ private def runAMActor(securityMgr: SecurityManager, host: String, port: String,
+ isDriver: Boolean): Unit = {
--- End diff --
also can you add a quick java doc on what this does
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69855510
@andrewor14 in comment's reply, I think I should answer your questions in my reply. if you have any question, please tell me and i will update for your comments. thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69888859
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25524/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22882168
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -245,6 +245,14 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
ApplicationMaster.EXIT_SC_NOT_INITED,
"Timed out waiting for SparkContext.")
} else {
+ actorSystem = AkkaUtils.createActorSystem("sparkYarnAM", Utils.localHostName, 0,
+ conf = sparkConf, securityManager = securityMgr)._1
+ val driverUrl = "akka.tcp://%s@%s:%s/user/%s".format(
+ SparkEnv.driverActorSystemName,
+ sc.getConf.get("spark.driver.host"),
+ sc.getConf.get("spark.driver.port"),
+ YarnSchedulerBackend.ACTOR_NAME)
+ actor = actorSystem.actorOf(Props(new AMActor(driverUrl, true)), name = "YarnAM")
--- End diff --
This is duplicated from `runExecutorLauncher`. Can you abstract these to a common place?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69878264
[Test build #25520 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25520/consoleFull) for PR 3962 at commit [`bbc4d5a`](https://github.com/apache/spark/commit/bbc4d5a05ebd08484bd85970ef1f3c2d40fab104).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71510021
@lianhuiwang Great that the latest changes are now much simpler and minimal. However, I still don't fully agree with one point:
> so if AMActor subscribe to disassociated event and finish with FinalApplicationStatus.SUCCEEDED, that's incorrect to do so
Why is it incorrect? Although the AM and the driver belong to the same process, the driver still runs in its own thread. If the driver exits cleanly then the AM will exit anyway since it joins on the driver thread, in which case the exit code will still be 0. On the other hand, if the driver failed with an exception, then we already handle that by finishing with non-zero exit code in `startUserThread`. Listening on the disassociated event is just one way we detect whether the driver has exited, and the behavior should be the same across deploy modes.
By the way, I'm not saying that this patch is incorrect in its current state. I just don't find the `isDriver` check in `AMActor` necessary because the ultimate behavior should be the same without it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69301620
[Test build #25303 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25303/consoleFull) for PR 3962 at commit [`1b029a4`](https://github.com/apache/spark/commit/1b029a4563ae768839e6ec64bf4e8c0f829385bb).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor(isDriver: Boolean) extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23550612
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -462,9 +478,9 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
}
/**
- * Actor that communicates with the driver in client deploy mode.
+ * Actor that communicates with the driver in Yarn deploy mode.
--- End diff --
to be fair, YARN is not a deploy mode. I would just update this to say
```
An actor that communicates with the driver's YarnSchedulerBackend.
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69301622
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25303/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71421335
[Test build #26082 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26082/consoleFull) for PR 3962 at commit [`9318fc1`](https://github.com/apache/spark/commit/9318fc1c9c707701864f87264e68c8d7541dd8ed).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69878693
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25520/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69792806
Hi @lianhuiwang thanks for fixing this. It's pretty clear why it's not working in cluster mode; the actor that acts as a server for allocation requests is simply not started in this mode... this seems like a critical omission on my part.
The approach is pretty straightforward in this PR. However, is there a reason why we need to subscribe to disassociated events in one deploy mode but not the other?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22910282
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -245,6 +245,14 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
ApplicationMaster.EXIT_SC_NOT_INITED,
"Timed out waiting for SparkContext.")
} else {
+ actorSystem = AkkaUtils.createActorSystem("sparkYarnAM", Utils.localHostName, 0,
+ conf = sparkConf, securityManager = securityMgr)._1
+ val driverUrl = "akka.tcp://%s@%s:%s/user/%s".format(
+ SparkEnv.driverActorSystemName,
+ sc.getConf.get("spark.driver.host"),
+ sc.getConf.get("spark.driver.port"),
+ YarnSchedulerBackend.ACTOR_NAME)
+ actor = actorSystem.actorOf(Props(new AMActor(driverUrl, true)), name = "YarnAM")
--- End diff --
yes, good idea, i will do it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69285227
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25290/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71575632
[Test build #26134 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26134/consoleFull) for PR 3962 at commit [`48d9ebb`](https://github.com/apache/spark/commit/48d9ebb6a9e8c82c5bbd9221dec5d8baca987a82).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22881772
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala ---
@@ -94,12 +91,14 @@ private[spark] abstract class YarnSchedulerBackend(
/**
* An actor that communicates with the ApplicationMaster.
*/
- private class YarnSchedulerActor extends Actor {
+ protected class YarnSchedulerActor(isDriver: Boolean) extends Actor {
private var amActor: Option[ActorRef] = None
override def preStart(): Unit = {
// Listen for disassociation events
- context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
+ if (!isDriver) {
+ context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
+ }
--- End diff --
why not listen to these in cluster mode?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71411015
[Test build #26071 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26071/consoleFull) for PR 3962 at commit [`08ba473`](https://github.com/apache/spark/commit/08ba4734496a9562d0e32d8c799742f97a1094dd).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69879021
[Test build #25522 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25522/consoleFull) for PR 3962 at commit [`7a7767a`](https://github.com/apache/spark/commit/7a7767a47d0e3bfd7c2f176ba93af09061cfdb6a).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor(isClusterMode: Boolean) extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71409528
@lianhuiwang I still don't understand why we don't want to subscribe to disassociated event in cluster mode. What would happen if we subscribed to disassociated events in both modes? Is it not needed, or is it incorrect to do so in cluster mode? Right now there's some complexity introduced here that I don't find necessary, but maybe there's something I'm missing. My understanding is that all that is required here is to also start the same actors (`YarnSchedulerActor` and `AMActor`) in cluster mode as we do in client mode.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71411565
in cluster mode, AMActor donot need to subscribe to disassociated event. because sometime driver has some errors, Now AMActor donot understand what happened in driver. so if AMActor subscribe to disassociated event and finish with FinalApplicationStatus.SUCCEEDED, that's incorrect to do so. @andrewor14
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71581696
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26134/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71574630
@andrewor14 if we don't finish with SUCCEEDED on driver disassociation, AM should finish with non-zero. example: if driver's main class throw some exception and exit, I think now AMActor will listen on the disassociated event and finish with SUCCEEDED.if AMActor donot listen on the disassociated event, userThread in AM can catch these exception from driver's main class and finish with FAILED, not SUCCEEDED.
so i think AMActor is unnecessary to listening to driver's disassociated event in yarn cluster mode because userThread in AM can monitor any exceptions of driver's main class. but AMActor donot know any exceptions of driver. Do you understan what i said? or Do you have different opinions?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23511995
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala ---
@@ -94,12 +91,14 @@ private[spark] abstract class YarnSchedulerBackend(
/**
* An actor that communicates with the ApplicationMaster.
*/
- private class YarnSchedulerActor extends Actor {
+ protected class YarnSchedulerActor(isClusterMode: Boolean) extends Actor {
--- End diff --
yes, I can try to do it as you said.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71413356
[Test build #26071 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26071/consoleFull) for PR 3962 at commit [`08ba473`](https://github.com/apache/spark/commit/08ba4734496a9562d0e32d8c799742f97a1094dd).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor(isClusterMode: Boolean) extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23510871
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -231,6 +231,18 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
reporterThread = launchReporterThread()
}
+ private def runAMActor(securityMgr: SecurityManager, host: String, port: String,
+ isDriver: Boolean): Unit = {
--- End diff --
Style should be
```
private def runAMActor(
securityMgr: ...
isDriver: Boolean): Unit = {
...
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22882004
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -464,7 +472,7 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
/**
* Actor that communicates with the driver in client deploy mode.
--- End diff --
This is no longer true
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69286459
[Test build #25291 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25291/consoleFull) for PR 3962 at commit [`2164ea8`](https://github.com/apache/spark/commit/2164ea88edd33c833fbbd0c7baa86426ef3534c0).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3962
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71910271
> so i think AMActor is unnecessary to listening to driver's disassociated event in yarn cluster mode because userThread in AM can monitor any exceptions of driver's main class. but AMActor donot know any exceptions of driver. Do you understand what i said?
Yes I understand. Note however we already catch exceptions in the user thread and `finish` with failed exit code [here](https://github.com/apache/spark/blob/c8e934ef3cd06f02f9a2946e96a1a52293c22490/yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala#L450). Since `finish` can only be called once, when an exception occurs we already exit with a non-zero code so the disassociation event shouldn't do anything. However, this assumes that the disassociation event comes *after* the exception is caught, which may not necessarily be true. For this reason I think it's safer to leave the check in there as you suggest. We definitely need some comment to explain why the behavior is different in cluster mode though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22881812
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala ---
@@ -94,12 +91,14 @@ private[spark] abstract class YarnSchedulerBackend(
/**
* An actor that communicates with the ApplicationMaster.
*/
- private class YarnSchedulerActor extends Actor {
+ protected class YarnSchedulerActor(isDriver: Boolean) extends Actor {
--- End diff --
this should be `isClusterMode` instead. I don't understand what `isDriver` means
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71422216
@andrewor14 I have looked at it in depth. YarnSchedulerActor can work very well in both yarn cluster and yarn client mode and i have tested in these two mode. Now we just change small code of AM. Can you review this PR again? thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71581689
[Test build #26134 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26134/consoleFull) for PR 3962 at commit [`48d9ebb`](https://github.com/apache/spark/commit/48d9ebb6a9e8c82c5bbd9221dec5d8baca987a82).
* 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69285123
[Test build #25290 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25290/consoleFull) for PR 3962 at commit [`6dfeeec`](https://github.com/apache/spark/commit/6dfeeecd4a206b9a82952e3b9f78128a0013d3c9).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-70616605
[Test build #25809 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25809/consoleFull) for PR 3962 at commit [`265c36d`](https://github.com/apache/spark/commit/265c36d3c6c0feccf6db6298e99021f2d14a4060).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22881954
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -474,13 +482,17 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
// we can monitor Lifecycle Events.
driver ! "Hello"
driver ! RegisterClusterManager
- context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
+ if (isDriver) {
+ context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
+ }
}
override def receive = {
case x: DisassociatedEvent =>
logInfo(s"Driver terminated or disconnected! Shutting down. $x")
- finish(FinalApplicationStatus.SUCCEEDED, ApplicationMaster.EXIT_SUCCESS)
+ if (!isDriver) {
+ finish(FinalApplicationStatus.SUCCEEDED, ApplicationMaster.EXIT_SUCCESS)
+ }
--- End diff --
IIUC you'll never go into this case right, because you subscribe to the disassociated events only in cluster mode?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23550729
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -231,6 +231,25 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
reporterThread = launchReporterThread()
}
+ /**
+ * Creates an actor that ApplicationMaster communicates with YarnScheduler of driver
+ * in Yarn deploy mode.
+ *
+ * If isDriver is set to true, AMActor and driver belong to same process.
+ * so AMActor don't monitor lifecycle of driver.
+ */
+ private def runAMActor(
+ host: String,
+ port: String,
+ isDriver: Boolean): Unit = {
--- End diff --
can you rename `isDriver` to `isClusterMode`? It might be apparent to us that these two mean the same thing but for newcomers it's not obvious at all.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23740893
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -231,6 +231,25 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
reporterThread = launchReporterThread()
}
+ /**
+ * Creates an actor that ApplicationMaster communicates with YarnScheduler of driver
+ * in Yarn deploy mode.
+ *
+ * If isDriver is set to true, AMActor and driver belong to same process.
+ * so AMActor don't monitor lifecycle of driver.
+ */
+ private def runAMActor(
+ host: String,
+ port: String,
+ isDriver: Boolean): Unit = {
--- End diff --
can you tell me JIR id or PR id? if you do not work it ,i think i can do it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22910606
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala ---
@@ -94,12 +91,14 @@ private[spark] abstract class YarnSchedulerBackend(
/**
* An actor that communicates with the ApplicationMaster.
*/
- private class YarnSchedulerActor extends Actor {
+ protected class YarnSchedulerActor(isDriver: Boolean) extends Actor {
--- End diff --
ok, i agree with you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-70705088
Hey @lianhuiwang I will look at this within today or tomorrow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71421976
[Test build #26084 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26084/consoleFull) for PR 3962 at commit [`12426af`](https://github.com/apache/spark/commit/12426afb86bf626d9f5000614132e5abb9d5bb22).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69285225
[Test build #25290 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25290/consoleFull) for PR 3962 at commit [`6dfeeec`](https://github.com/apache/spark/commit/6dfeeecd4a206b9a82952e3b9f78128a0013d3c9).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor(isDriver: Boolean) extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69879022
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25522/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71425417
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26084/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69878631
[Test build #25522 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25522/consoleFull) for PR 3962 at commit [`7a7767a`](https://github.com/apache/spark/commit/7a7767a47d0e3bfd7c2f176ba93af09061cfdb6a).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69289902
[Test build #25291 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25291/consoleFull) for PR 3962 at commit [`2164ea8`](https://github.com/apache/spark/commit/2164ea88edd33c833fbbd0c7baa86426ef3534c0).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor(isDriver: Boolean) extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69302144
[Test build #25299 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25299/consoleFull) for PR 3962 at commit [`7d33791`](https://github.com/apache/spark/commit/7d33791c16fa0853568f3d72c22265b153484db1).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor(isDriver: Boolean) extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71413359
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26071/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69878690
[Test build #25520 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25520/consoleFull) for PR 3962 at commit [`bbc4d5a`](https://github.com/apache/spark/commit/bbc4d5a05ebd08484bd85970ef1f3c2d40fab104).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` protected class YarnSchedulerActor(isClusterMode: Boolean) extends Actor `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71425209
[Test build #26083 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26083/consoleFull) for PR 3962 at commit [`45da3b0`](https://github.com/apache/spark/commit/45da3b0bf03c22d747706489a47c7b0140e0d59a).
* 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71404828
@andrewor14 have you some feedback about this PR? thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-70623771
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25809/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71425742
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26082/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69894322
@andrewor14 can you take at look at it? thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69298114
[Test build #25303 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25303/consoleFull) for PR 3962 at commit [`1b029a4`](https://github.com/apache/spark/commit/1b029a4563ae768839e6ec64bf4e8c0f829385bb).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23550858
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -231,6 +231,25 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
reporterThread = launchReporterThread()
}
+ /**
+ * Creates an actor that ApplicationMaster communicates with YarnScheduler of driver
+ * in Yarn deploy mode.
--- End diff --
This can just be
```
Create an actor that communicates with the driver.
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23511386
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala ---
@@ -41,10 +41,7 @@ private[spark] abstract class YarnSchedulerBackend(
protected var totalExpectedExecutors = 0
- private val yarnSchedulerActor: ActorRef =
- actorSystem.actorOf(
- Props(new YarnSchedulerActor),
- name = YarnSchedulerBackend.ACTOR_NAME)
+ protected var yarnSchedulerActor: ActorRef = _
--- End diff --
I would just omit the `= _` and force the subclasses to override it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23511367
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -464,7 +472,7 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
/**
* Actor that communicates with the driver in client deploy mode.
--- End diff --
I don't understand. In both client and cluster modes, this actor is used to communicate with the driver. It's just that in cluster mode since the driver and the AM are run in the same JVM, the communication is local.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-69302147
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25299/
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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r23581653
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
@@ -231,6 +231,25 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
reporterThread = launchReporterThread()
}
+ /**
+ * Creates an actor that ApplicationMaster communicates with YarnScheduler of driver
+ * in Yarn deploy mode.
+ *
+ * If isDriver is set to true, AMActor and driver belong to same process.
+ * so AMActor don't monitor lifecycle of driver.
+ */
+ private def runAMActor(
+ host: String,
+ port: String,
+ isDriver: Boolean): Unit = {
--- End diff --
we need to consider again. because isDriver in here consistent with ApplicationMaster's variable "isDriver". if we rename isDriver to isClusterMode in here, maybe we need to rename all isDriver in ApplicationMaster. so i think we should keep isDriver.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71910884
LGTM actually. I'm going to merge this into master after adding the comment I talked about earlier. Thanks for explaining your reasoning @lianhuiwang.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by lianhuiwang <gi...@git.apache.org>.
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/3962#discussion_r22910916
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala ---
@@ -94,12 +91,14 @@ private[spark] abstract class YarnSchedulerBackend(
/**
* An actor that communicates with the ApplicationMaster.
*/
- private class YarnSchedulerActor extends Actor {
+ protected class YarnSchedulerActor(isDriver: Boolean) extends Actor {
private var amActor: Option[ActorRef] = None
override def preStart(): Unit = {
// Listen for disassociation events
- context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
+ if (!isDriver) {
+ context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
+ }
--- End diff --
because in cluster mode, YarnSchedulerActor and driver use same actorSystem, if YarnSchedulerActor subscribe and listen driver's actor, messages from driver's actor that should to send to executor will send to YarnSchedulerActor .so there is a big wrong and we cannot listen for cluster mode.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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: [SPARK-4955]With executor dynamic scaling enab...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3962#issuecomment-71425214
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26083/
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