Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-13711][Core]Don't call SparkUncaughtExceptionHandler in AppClient as it's in driver #11566

Closed
wants to merge 1 commit into from

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Mar 7, 2016

What changes were proposed in this pull request?

AppClient runs in the driver side. It should not call Utils.tryOrExit as it will send exception to SparkUncaughtExceptionHandler and call System.exit. This PR just removed Utils.tryOrExit.

How was this patch tested?

manual tests.

@zsxwing
Copy link
Member Author

zsxwing commented Mar 7, 2016

cc @vanzin as you added Utils.tryOrExit here

@vanzin
Copy link
Contributor

vanzin commented Mar 8, 2016

Actually it seems it was @markhamstra and not me (in 17f3075).

@zsxwing
Copy link
Member Author

zsxwing commented Mar 8, 2016

@vanzin Sorry. I was looking a wrong place...

@markhamstra
Copy link
Contributor

So what happens now if the scheduled Runnable throws an exception?

@zsxwing
Copy link
Member Author

zsxwing commented Mar 8, 2016

So what happens now if the scheduled Runnable throws an exception?

Just go to Thread.getDefaultUncaughtExceptionHandler().

registerMasterFutures.get.foreach(_.cancel(true))
registerMasterThreadPool.shutdownNow()
} else if (nthRetry >= REGISTRATION_RETRIES) {
markDead("All masters are unresponsive! Giving up.")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markhamstra
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Mar 8, 2016

Test build #52609 has finished for PR 11566 at commit 398859c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@zsxwing
Copy link
Member Author

zsxwing commented Mar 8, 2016

Thanks. Merging to master and 1.6

@asfgit asfgit closed this in 017cdf2 Mar 8, 2016
@zsxwing zsxwing deleted the SPARK-13711 branch March 8, 2016 04:58
asfgit pushed a commit that referenced this pull request Mar 8, 2016
…ient as it's in driver

## What changes were proposed in this pull request?

AppClient runs in the driver side. It should not call `Utils.tryOrExit` as it will send exception to SparkUncaughtExceptionHandler and call `System.exit`. This PR just removed `Utils.tryOrExit`.

## How was this patch tested?

manual tests.

Author: Shixiong Zhu <shixiong@databricks.com>

Closes #11566 from zsxwing/SPARK-13711.
roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
…ient as it's in driver

## What changes were proposed in this pull request?

AppClient runs in the driver side. It should not call `Utils.tryOrExit` as it will send exception to SparkUncaughtExceptionHandler and call `System.exit`. This PR just removed `Utils.tryOrExit`.

## How was this patch tested?

manual tests.

Author: Shixiong Zhu <shixiong@databricks.com>

Closes apache#11566 from zsxwing/SPARK-13711.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants