[SPARK-2242] HOTFIX: pyspark shell hangs on simple job #1178

andrewor14 · 2014-06-23T04:19:51Z

This reverts a change introduced in 3870248, which redirected all stderr to the OS pipe instead of directly to the bin/pyspark shell output. This causes a simple job to hang in two ways:

If the cluster is not configured correctly or does not have enough resources, the job hangs without producing any output, because the relevant warning messages are masked.
If the stderr volume is large, this could lead to a deadlock if we redirect everything to the OS pipe. From the python docs:

Note Do not use stdout=PIPE or stderr=PIPE with this function as that can deadlock
based on the child process output volume. Use Popen with the communicate() method
when you need pipes.

Note that we cannot remove stdout=PIPE in a similar way, because we currently use it to communicate the py4j port. However, it should be fine (as it has been for a long time) because we do not produce a ton of traffic through stdout.

That commit was not merged in branch-1.0, so this fix is for master only.

AmplabJenkins · 2014-06-23T04:20:09Z

Merged build triggered.

AmplabJenkins · 2014-06-23T04:20:15Z

Merged build started.

@shivaram

If the gateway process fails to start correctly (e.g., because JAVA_HOME isn't set correctly, there's no Spark jar, etc.), right now pyspark fails because of a very difficult-to-understand error, where we try to parse stdout to get the port where Spark started and there's nothing there. This commit properly catches the error and throws an exception that includes the stderr output for much easier debugging. Thanks to @shivaram and @stogers for helping to fix this issue! Author: Kay Ousterhout <[email protected]> Closes #383 from kayousterhout/pyspark and squashes the following commits: 36dd54b [Kay Ousterhout] [SPARK-1466] Raise exception if Gateway process doesn't start.

AmplabJenkins · 2014-06-23T04:50:09Z

Merged build triggered.

AmplabJenkins · 2014-06-23T04:50:15Z

Merged build started.

AmplabJenkins · 2014-06-23T05:03:00Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-06-23T05:03:00Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16015/

AmplabJenkins · 2014-06-23T05:30:05Z

Merged build finished.

AmplabJenkins · 2014-06-23T05:30:05Z

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16016/

andrewor14 · 2014-06-23T05:38:14Z

Jenkins test this please

AmplabJenkins · 2014-06-23T05:40:09Z

Merged build triggered.

AmplabJenkins · 2014-06-23T05:40:15Z

Merged build started.

AmplabJenkins · 2014-06-23T06:24:11Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-06-23T06:24:11Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16017/

kayousterhout · 2014-06-23T19:00:31Z

Sorry about this and thanks for fixing it @andrewor14 ! Just one change -- I think you should also delete proc.stderr.readLines() on line 57, since that won't return anything now.

kayousterhout · 2014-06-23T19:58:39Z

BTW did you look into using communicate() at all? If not I'll look into that to do a long-term fix later today.

andrewor14 · 2014-06-23T20:05:07Z

Hm no I have not, though I'm not sure if we can use communicate() because I think that waits till the end of the process before returning any stdout or stderr, so that's not quite applicable to the shell (I could be totally making this up, but that was my impression when I ran into something similar before)

andrewor14 · 2014-06-23T20:08:29Z

Unless we do a communicate() only for the except case. It would still be good to print out why "Launching GatewayServer failed with exit code"

kayousterhout · 2014-06-23T20:10:02Z

Yeah the latter thing is what I was thinking

Before: ValueError: invalid literal for int() with base 10 After: Launching GatewayServer failed because of stdout interference. Silence the following and try again.

AmplabJenkins · 2014-06-23T21:05:12Z

Merged build triggered.

AmplabJenkins · 2014-06-23T21:05:21Z

Merged build started.

andrewor14 · 2014-06-23T21:09:51Z

Looks like if an exception is thrown because of casting the output to int (rather than reading the output of the process itself), then the process exit code returned is None because the process hasn't terminated yet. We should add a separate except for bad casting, which happens if there is other output other than the py4j port, and this is the most likely cause of an exception in the first place. I added this in the latest commit.

I haven't invested a ton of time on figuring out how to use communicate() correctly, though I'm not entirely sure if we need it, actually. Because our error messages actually surface on stdout, and our output messages on stderr, I think what we have now is sufficient as a fix. In the long term we might want to avoid using stdout to communicate the py4j port entirely, but maybe instead write it to a temporary file so pyspark doesn't die on us if we echo a seemingly benign "hello" somewhere :)

AmplabJenkins · 2014-06-23T21:15:12Z

Merged build triggered.

AmplabJenkins · 2014-06-23T21:15:21Z

Merged build started.

AmplabJenkins · 2014-06-23T21:51:19Z

Merged build finished.

AmplabJenkins · 2014-06-23T21:51:19Z

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16040/

AmplabJenkins · 2014-06-24T19:35:16Z

Merged build triggered.

AmplabJenkins · 2014-06-24T19:35:24Z

Merged build started.

andrewor14 · 2014-06-24T19:39:56Z

@mattf The second issue that your changes don't address is that the existing code also masks stderr output, which contains important Spark logging information. Also, I tried your patch out, and bin/pyspark still hangs when an exception is thrown.

andrewor14 · 2014-06-24T19:43:53Z

Here's the output of the latest commit in the event of stdout interference:

//... stack trace here
Exception: Launching GatewayServer failed! (Warning: unexpected output detected.)

Dearly beloved, did you forget to silence this message?
Here's how you do it. First you climb the ladder of--zzzz
52254

kayousterhout · 2014-06-24T19:45:53Z

This looks perfect!

On Tue, Jun 24, 2014 at 12:44 PM, andrewor14 [email protected]
wrote:

Here's the output of the latest commit in the event of stdout interference:

//... stack trace here
Exception: Launching GatewayServer failed! (Warning: unexpected output detected.)

Dearly beloved, did you forget to silence this message?
Here's how you do it. First you climb the ladder of--zzzz
52254

—
Reply to this email directly or view it on GitHub
#1178 (comment).

AmplabJenkins · 2014-06-24T20:14:02Z

Merged build finished.

AmplabJenkins · 2014-06-24T20:14:02Z

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16064/

andrewor14 · 2014-06-24T20:14:21Z

Jenkins, test this please

AmplabJenkins · 2014-06-24T20:15:17Z

Merged build triggered.

AmplabJenkins · 2014-06-24T20:15:24Z

Merged build started.

AmplabJenkins · 2014-06-24T20:25:29Z

Merged build finished.

AmplabJenkins · 2014-06-24T20:25:29Z

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16065/

andrewor14 · 2014-06-24T20:44:42Z

Jenkins, test this please

AmplabJenkins · 2014-06-24T20:45:17Z

Merged build triggered.

AmplabJenkins · 2014-06-24T20:45:25Z

Merged build started.

AmplabJenkins · 2014-06-24T22:11:46Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-06-24T22:11:47Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16069/

mattf · 2014-06-25T17:06:56Z

@andrewor14 what's the reproducer for the "hangs when an exception is thrown" case?

andrewor14 · 2014-06-25T17:28:20Z

@mattf try adding the following lines to bin/spark-class (anywhere near the lines with SPARK_MEM is fine):

echo "Hello. This goes to stdout..."
echo "and interferes with pyspark reading the py4j port as an int"

What pyspark tries to do is to read the string "Hello. This goes to stdout..." as an int and throws an exception. I think whether it hangs depends on the environment, but on mine I ran into the deadlock the python docs warned against.

mattf · 2014-06-25T17:39:14Z

@andrewor14 thanks, i've been able to reproduce a hang when spark-class outputs something other than the port #

mengxr · 2014-06-25T17:46:28Z

This looks good to me. I'm going merge it since pyspark is broken without this patch.

This reverts a change introduced in 3870248, which redirected all stderr to the OS pipe instead of directly to the `bin/pyspark` shell output. This causes a simple job to hang in two ways: 1. If the cluster is not configured correctly or does not have enough resources, the job hangs without producing any output, because the relevant warning messages are masked. 2. If the stderr volume is large, this could lead to a deadlock if we redirect everything to the OS pipe. From the [python docs](https://docs.python.org/2/library/subprocess.html): ``` Note Do not use stdout=PIPE or stderr=PIPE with this function as that can deadlock based on the child process output volume. Use Popen with the communicate() method when you need pipes. ``` Note that we cannot remove `stdout=PIPE` in a similar way, because we currently use it to communicate the py4j port. However, it should be fine (as it has been for a long time) because we do not produce a ton of traffic through `stdout`. That commit was not merged in branch-1.0, so this fix is for master only. Author: Andrew Or <[email protected]> Closes apache#1178 from andrewor14/fix-python and squashes the following commits: e68e870 [Andrew Or] Merge branch 'master' of github.com:apache/spark into fix-python 20849a8 [Andrew Or] Tone down stdout interference message a09805b [Andrew Or] Return more than 1 line of error message to user 6dfbd1e [Andrew Or] Don't swallow original exception 0d1861f [Andrew Or] Provide more helpful output if stdout is garbled 21c9d7c [Andrew Or] Do not mask stderr from output

* [CARMEL-6426] Adjust scan parallelism dynamically improvement * Use ExplodeBase instead of Explode

…-36944 (…" (apache#1178) This reverts commit 4272502245483e4f7680c1b3faa8b02bf0e466c6.

Do not mask stderr from output

21c9d7c

andrewor14 changed the title ~~[SPARK-2242] HOTFIX: Do not mask pyspark stderr from output~~ [SPARK-2242] HOTFIX: pyspark shell hangs on simple job Jun 23, 2014

Provide more helpful output if stdout is garbled

0d1861f

Before: ValueError: invalid literal for int() with base 10 After: Launching GatewayServer failed because of stdout interference. Silence the following and try again.

Don't swallow original exception

6dfbd1e

rxin mentioned this pull request Jun 25, 2014

[SPARK-2244] Fix hang introduced by SPARK-1466 #1197

Closed

asfgit closed this in 5603e4c Jun 25, 2014

andrewor14 deleted the fix-python branch June 25, 2014 20:54

wangyum pushed a commit that referenced this pull request May 26, 2023

[CARMEL-6426] Adjust scan parallelism dynamically improvement (#1178)

b178cb7

* [CARMEL-6426] Adjust scan parallelism dynamically improvement * Use ExplodeBase instead of Explode

mapr-devops pushed a commit to mapr/spark that referenced this pull request May 8, 2025

Revert "[EZAF-4618] Build Spark-3.5.0 with Scala-2.13 to fix CVE-2022…

98e6048

…-36944 (…" (apache#1178) This reverts commit 4272502245483e4f7680c1b3faa8b02bf0e466c6.

[SPARK-2242] HOTFIX: pyspark shell hangs on simple job #1178

[SPARK-2242] HOTFIX: pyspark shell hangs on simple job #1178

Uh oh!

Conversation

andrewor14 commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

andrewor14 commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

kayousterhout commented Jun 23, 2014

Uh oh!

kayousterhout commented Jun 23, 2014

Uh oh!

andrewor14 commented Jun 23, 2014

Uh oh!

andrewor14 commented Jun 23, 2014

Uh oh!

kayousterhout commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

andrewor14 commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 23, 2014

Uh oh!

AmplabJenkins commented Jun 24, 2014

Uh oh!

AmplabJenkins commented Jun 24, 2014

Uh oh!

andrewor14 commented Jun 24, 2014

Uh oh!

andrewor14 commented Jun 24, 2014

Uh oh!

kayousterhout commented Jun 24, 2014

Uh oh!

AmplabJenkins commented Jun 24, 2014

Uh oh!

AmplabJenkins commented Jun 24, 2014

Uh oh!

andrewor14 commented Jun 24, 2014

Uh oh!

AmplabJenkins commented Jun 24, 2014

Uh oh!

AmplabJenkins commented Jun 24, 2014

Uh oh!

AmplabJenkins commented Jun 24, 2014

Uh oh!

AmplabJenkins commented Jun 24, 2014

Uh oh!