Skip to content

[SPARK-21490][core] Make sure SparkLauncher redirects needed streams.#18696

Closed
vanzin wants to merge 3 commits intoapache:masterfrom
vanzin:SPARK-21490
Closed

[SPARK-21490][core] Make sure SparkLauncher redirects needed streams.#18696
vanzin wants to merge 3 commits intoapache:masterfrom
vanzin:SPARK-21490

Conversation

@vanzin
Copy link
Contributor

@vanzin vanzin commented Jul 20, 2017

The code was failing to account for some cases when setting up log
redirection. For example, if a user redirected only stdout to a file,
the launcher code would leave stderr without redirection, which could
lead to child processes getting stuck because stderr wasn't being
read.

So detect cases where only one of the streams is redirected, and
redirect the other stream to the log as appropriate.

For the old "launch()" API, redirection of the unconfigured stream
only happens if the user has explicitly requested for log redirection.
Log redirection is on by default with "startApplication()".

Most of the change is actually adding new unit tests to make sure the
different cases work as expected. As part of that, I moved some tests
that were in the core/ module to the launcher/ module instead, since
they don't depend on spark-submit.

The code was failing to account for some cases when setting up log
redirection. For example, if a user redirected only stdout to a file,
the launcher code would leave stderr without redirection, which could
lead to child processes getting stuck because stderr wasn't being
read.

So detect cases where only one of the streams is redirected, and
redirect the other stream to the log as appropriate.

For the old "launch()" API, redirection of the unconfigured stream
only happens if the user has explicitly requested for log redirection.
Log redirection is on by default with "startApplication()".

Most of the change is actually adding new unit tests to make sure the
different cases work as expected. As part of that, I moved some test
that were in the core/ module to the launcher/ module instead, since
they don't depend on spark-submit.
@SparkQA
Copy link

SparkQA commented Jul 20, 2017

Test build #79808 has started for PR 18696 at commit 7d3db5f.

@vanzin
Copy link
Contributor Author

vanzin commented Jul 21, 2017

retest this please

@SparkQA
Copy link

SparkQA commented Jul 21, 2017

Test build #79848 has finished for PR 18696 at commit 7d3db5f.

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

@SparkQA
Copy link

SparkQA commented Jul 23, 2017

Test build #79872 has finished for PR 18696 at commit ad55dd1.

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

@SparkQA
Copy link

SparkQA commented Jul 24, 2017

Test build #79913 has finished for PR 18696 at commit 39ca992.

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

@vanzin
Copy link
Contributor Author

vanzin commented Jul 25, 2017

@srowen want to take a look?

@vanzin
Copy link
Contributor Author

vanzin commented Jul 31, 2017

retest this please

@SparkQA
Copy link

SparkQA commented Jul 31, 2017

Test build #80084 has finished for PR 18696 at commit 39ca992.

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

@vanzin
Copy link
Contributor Author

vanzin commented Aug 1, 2017

I'll wait until tomorrow for a review, otherwise I'll push this to unblock further work on this code.

@vanzin
Copy link
Contributor Author

vanzin commented Aug 2, 2017

Merging to master.

@vanzin vanzin closed this Aug 2, 2017
@vanzin vanzin deleted the SPARK-21490 branch August 2, 2017 19:07
asfgit pushed a commit that referenced this pull request Aug 2, 2017
The code was failing to account for some cases when setting up log
redirection. For example, if a user redirected only stdout to a file,
the launcher code would leave stderr without redirection, which could
lead to child processes getting stuck because stderr wasn't being
read.

So detect cases where only one of the streams is redirected, and
redirect the other stream to the log as appropriate.

For the old "launch()" API, redirection of the unconfigured stream
only happens if the user has explicitly requested for log redirection.
Log redirection is on by default with "startApplication()".

Most of the change is actually adding new unit tests to make sure the
different cases work as expected. As part of that, I moved some tests
that were in the core/ module to the launcher/ module instead, since
they don't depend on spark-submit.

Author: Marcelo Vanzin <vanzin@cloudera.com>

Closes #18696 from vanzin/SPARK-21490.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants