[SPARK-3293] yarn's web show "SUCCEEDED" when the driver throw a exception in yarn-client #2311

witgo · 2014-09-07T12:58:53Z

No description provided.

SparkQA · 2014-09-07T13:45:53Z

QA tests have started for PR 2311 at commit 3828707.

This patch merges cleanly.

SparkQA · 2014-09-07T14:47:06Z

QA tests have finished for PR 2311 at commit 3828707.

This patch fails unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class NoExitsException(exitCode: Int) extends SecurityException

ScrapCodes · 2014-09-08T05:10:39Z

You will have to rebase your patch with master for tests to pass.

SparkQA · 2014-09-08T10:47:36Z

QA tests have started for PR 2311 at commit 2ebe62e.

This patch merges cleanly.

SparkQA · 2014-09-08T11:36:46Z

QA tests have finished for PR 2311 at commit 2ebe62e.

This patch fails unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class NoExitsException(exitCode: Int) extends SecurityException

witgo · 2014-09-08T12:55:25Z

Jenkins, retest this please.

SparkQA · 2014-09-08T13:47:37Z

QA tests have started for PR 2311 at commit 2ebe62e.

This patch merges cleanly.

SparkQA · 2014-09-08T14:48:39Z

QA tests have finished for PR 2311 at commit 2ebe62e.

This patch passes unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class NoExitsException(exitCode: Int) extends SecurityException

vanzin · 2014-09-08T16:59:09Z

yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala

+        System.setSecurityManager(new java.lang.SecurityManager() {
+          override def checkExit(paramInt: Int) {
+            if (!stopped) {
+              throw new NoExitsException(paramInt)


Instead of throwing the exception, why not just set exitStatus directly and avoid having a custom exception? You can also use that as a trigger to call "finish()" if it hasn't been called yet, with a nice error message about the app having called System.exit.

That avoids messing with the app's exception handling, since very few people realize System.exit can throw and thus don't really plan for it.

SparkQA · 2014-09-09T14:50:18Z

QA tests have started for PR 2311 at commit 7efcca0.

This patch merges cleanly.

SparkQA · 2014-09-09T15:47:44Z

QA tests have finished for PR 2311 at commit 7efcca0.

This patch fails unit tests.
This patch merges cleanly.
This patch adds no public classes.

tgravescs · 2014-09-21T18:56:18Z

Jenkins, retest this please.

SparkQA · 2014-09-21T18:59:30Z

QA tests have started for PR 2311 at commit 7efcca0.

This patch does not merge cleanly!

SparkQA · 2014-09-21T20:35:28Z

QA tests have finished for PR 2311 at commit 7efcca0.

This patch fails unit tests.
This patch does not merge cleanly!

SparkQA · 2014-09-22T10:04:21Z

QA tests have started for PR 2311 at commit 5f0883d.

This patch merges cleanly.

SparkQA · 2014-09-22T11:11:33Z

QA tests have finished for PR 2311 at commit 5f0883d.

This patch passes unit tests.
This patch merges cleanly.
This patch adds no public classes.

vanzin · 2014-09-22T17:20:22Z

yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala

@@ -119,13 +126,7 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
    } else {
      runExecutorLauncher(securityMgr)
    }
-
-    if (finalStatus != FinalApplicationStatus.UNDEFINED) {


Since you're changing this logic, you need to modify runExecutorLauncher so that it correctly reports an error on failure. Right now it will always exit with 0 and will not report the final status to the RM.

tgravescs · 2014-09-22T17:27:05Z

I've been having a multiple different issues with the success/failure reporting (SPARK-3627). Does this handle all of those?

vanzin · 2014-09-22T17:40:26Z

@tgravescs this change should handle uncaught exceptions and explicit System.exit in the driver code. Not sure if that covers all the issues you're seeing.

tgravescs · 2014-09-22T18:31:30Z

yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala

@@ -29,6 +29,7 @@ import org.apache.hadoop.yarn.api._
 import org.apache.hadoop.yarn.api.records._
 import org.apache.hadoop.yarn.conf.YarnConfiguration

+import org.apache.spark.executor.{ExecutorUncaughtExceptionHandler, ExecutorExitCode}


I don't see these used anywhere?

SparkQA · 2014-09-23T14:07:17Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20706/

witgo · 2014-09-23T14:11:08Z

Jenkins, retest this please.

SparkQA · 2014-09-23T14:14:22Z

QA tests have started for PR 2311 at commit 617efef.

This patch merges cleanly.

SparkQA · 2014-09-23T15:22:27Z

QA tests have finished for PR 2311 at commit 617efef.

This patch passes unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2014-09-23T15:22:31Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20707/

SparkQA · 2014-09-25T13:54:27Z

QA tests have started for PR 2311 at commit 9655aa8.

This patch merges cleanly.

SparkQA · 2014-09-25T14:45:53Z

QA tests have finished for PR 2311 at commit 9655aa8.

This patch fails unit tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-09-25T14:45:56Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20808/

tgravescs · 2014-09-26T02:04:45Z

@witgo There are cases not covered by this pr that I would like to fix up dealing with exit codes and final status. Would you mind if I just pull these changes into mine? I want to try to fix it up across the board. I hope to have something up tomorrow if all the testing goes well.

witgo · 2014-09-26T02:21:50Z

Ok, no problem.

tgravescs · 2014-09-26T02:24:16Z

also maybe I'm missing something but I don't see how the changes help in yarn-client mode? The changes you made were on the application master running on a separate host then the driver itself, startUserClass is only used in cluster mode, and when the disconnect happens in client mode we always report success.

witgo · 2014-09-26T03:02:52Z

Yes, this does not involve org.apache.spark.deploy.yarn.Client class, which run outside the cluster.
We should call YarnClient.killApplication when an uncaught exception is thrown.

witgo force-pushed the SPARK-3293 branch from 3828707 to 2ebe62e Compare September 8, 2014 10:42

vanzin reviewed Sep 8, 2014
View reviewed changes

witgo force-pushed the SPARK-3293 branch from 2ebe62e to 7efcca0 Compare September 9, 2014 14:06

witgo force-pushed the SPARK-3293 branch from 7efcca0 to 5f0883d Compare September 22, 2014 09:58

vanzin reviewed Sep 22, 2014
View reviewed changes

tgravescs reviewed Sep 22, 2014
View reviewed changes

witgo force-pushed the SPARK-3293 branch from 5f0883d to 617efef Compare September 23, 2014 13:54

witgo added 2 commits September 25, 2014 21:43

yarn check exit code

5300aac

review commit

9655aa8

witgo force-pushed the SPARK-3293 branch from 617efef to 9655aa8 Compare September 25, 2014 13:48

tgravescs mentioned this pull request Sep 29, 2014

[SPARK-3627] - [yarn] - fix exit code and final status reporting to RM #2577

Closed

witgo closed this Sep 29, 2014

witgo deleted the SPARK-3293 branch September 29, 2014 17:53

SaintBacchus mentioned this pull request Nov 28, 2014

[YARN][SPARK-3293]Fix yarn's web show "SUCCEEDED" when the driver throw a exception in yarn-client #3508

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-3293] yarn's web show "SUCCEEDED" when the driver throw a exception in yarn-client #2311

[SPARK-3293] yarn's web show "SUCCEEDED" when the driver throw a exception in yarn-client #2311

witgo commented Sep 7, 2014

SparkQA commented Sep 7, 2014

SparkQA commented Sep 7, 2014

ScrapCodes commented Sep 8, 2014

SparkQA commented Sep 8, 2014

SparkQA commented Sep 8, 2014

witgo commented Sep 8, 2014

SparkQA commented Sep 8, 2014

SparkQA commented Sep 8, 2014

vanzin Sep 8, 2014

SparkQA commented Sep 9, 2014

SparkQA commented Sep 9, 2014

tgravescs commented Sep 21, 2014

SparkQA commented Sep 21, 2014

SparkQA commented Sep 21, 2014

SparkQA commented Sep 22, 2014

SparkQA commented Sep 22, 2014

vanzin Sep 22, 2014

tgravescs commented Sep 22, 2014

vanzin commented Sep 22, 2014

tgravescs Sep 22, 2014

SparkQA commented Sep 23, 2014

witgo commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 25, 2014

SparkQA commented Sep 25, 2014

AmplabJenkins commented Sep 25, 2014

tgravescs commented Sep 26, 2014

witgo commented Sep 26, 2014

tgravescs commented Sep 26, 2014

witgo commented Sep 26, 2014

[SPARK-3293] yarn's web show "SUCCEEDED" when the driver throw a exception in yarn-client #2311

[SPARK-3293] yarn's web show "SUCCEEDED" when the driver throw a exception in yarn-client #2311

Conversation

witgo commented Sep 7, 2014

SparkQA commented Sep 7, 2014

SparkQA commented Sep 7, 2014

ScrapCodes commented Sep 8, 2014

SparkQA commented Sep 8, 2014

SparkQA commented Sep 8, 2014

witgo commented Sep 8, 2014

SparkQA commented Sep 8, 2014

SparkQA commented Sep 8, 2014

vanzin Sep 8, 2014

Choose a reason for hiding this comment

SparkQA commented Sep 9, 2014

SparkQA commented Sep 9, 2014

tgravescs commented Sep 21, 2014

SparkQA commented Sep 21, 2014

SparkQA commented Sep 21, 2014

SparkQA commented Sep 22, 2014

SparkQA commented Sep 22, 2014

vanzin Sep 22, 2014

Choose a reason for hiding this comment

tgravescs commented Sep 22, 2014

vanzin commented Sep 22, 2014

tgravescs Sep 22, 2014

Choose a reason for hiding this comment

SparkQA commented Sep 23, 2014

witgo commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 23, 2014

SparkQA commented Sep 25, 2014

SparkQA commented Sep 25, 2014

AmplabJenkins commented Sep 25, 2014

tgravescs commented Sep 26, 2014

witgo commented Sep 26, 2014

tgravescs commented Sep 26, 2014

witgo commented Sep 26, 2014