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-17742][core] Handle child process exit in SparkLauncher. #18877

Closed
wants to merge 2 commits into from

Conversation

vanzin
Copy link
Contributor

@vanzin vanzin commented Aug 7, 2017

Currently the launcher handle does not monitor the child spark-submit
process it launches; this means that if the child exits with an error,
the handle's state will never change, and an application will not know
that the application has failed.

This change adds code to monitor the child process, and changes the
handle state appropriately when the child process exits.

Tested with added unit tests.

Currently the launcher handle does not monitor the child spark-submit
process it launches; this means that if the child exits with an error,
the handle's state will never change, and an application will not know
that the application has failed.

This change adds code to monitor the child process, and changes the
handle state appropriately when the child process exits.

Tested with added unit tests.
@SparkQA
Copy link

SparkQA commented Aug 8, 2017

Test build #80368 has finished for PR 18877 at commit 15e1a73.

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

@vanzin
Copy link
Contributor Author

vanzin commented Aug 9, 2017

trying to get some reviews: @srowen @zsxwing

int ec;
try {
ec = childProc.exitValue();
} catch (Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

might want to log the exception here

try {
childProc.waitFor();
} catch (Exception e) {
// Try again.
Copy link
Contributor

Choose a reason for hiding this comment

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

log exception here?

And log exceptions. And add a transition to KILLED state that
was missing.
@SparkQA
Copy link

SparkQA commented Aug 11, 2017

Test build #80511 has finished for PR 18877 at commit def2329.

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

@vanzin
Copy link
Contributor Author

vanzin commented Aug 13, 2017

If there's no more feedback, I plan to push this soon to unblock other work on this module.

@vanzin
Copy link
Contributor Author

vanzin commented Aug 15, 2017

Alright, merging this to master.

@asfgit asfgit closed this in cba826d Aug 15, 2017
@vanzin vanzin deleted the SPARK-17742 branch August 15, 2017 21:07
@danelkotev
Copy link

@vanzin @SparkQA
Hi, Is this fix was merged into master?

@felixcheung
Copy link
Member

yes @danelkotev asfgit closed this in cba826d on Aug 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants