You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by bersprockets <gi...@git.apache.org> on 2018/01/29 18:36:55 UTC
[GitHub] spark pull request #20424: [Spark 23240][python] Better error message when e...
GitHub user bersprockets opened a pull request:
https://github.com/apache/spark/pull/20424
[Spark 23240][python] Better error message when extraneous data in pyspark.daemon's stdout
## What changes were proposed in this pull request?
Print more helpful message when daemon module's stdout is empty or contains a bad port number.
## How was this patch tested?
Manually recreated the environmental issues that caused the mysterious exceptions at one site. Tested that the expected messages are logged.
Also, ran all scala unit tests.
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bersprockets/spark SPARK-23240_prop2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20424.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20424
----
commit b938ca4b910ee66575aea7544ffb7b1083102324
Author: Bruce Robbins <be...@...>
Date: 2018-01-28T18:59:58Z
Initial commit of fix of SPARK-23240
commit a1cb1a89d679840845142facf58f15e870f7c81d
Author: Bruce Robbins <be...@...>
Date: 2018-01-29T15:30:43Z
Fix IDE's messing with imports
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87461 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87461/testReport)** for PR 20424 at commit [`eceb24e`](https://github.com/apache/spark/commit/eceb24e61798f9e5da0ed3c4dfb94d677d08b10e).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87362 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87362/testReport)** for PR 20424 at commit [`b63abee`](https://github.com/apache/spark/commit/b63abee881f2b4379f375500d51fdef706d6d512).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
Thank you @squito .. LGTM otherwise.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87362/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87461 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87461/testReport)** for PR 20424 at commit [`eceb24e`](https://github.com/apache/spark/commit/eceb24e61798f9e5da0ed3c4dfb94d677d08b10e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164617043
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ throw new IOException(s"Bad port number in $daemonModule's stdout: " +
+ f"0x$daemonPort%08x")
--- End diff --
`SparkException` too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87461/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87377 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87377/testReport)** for PR 20424 at commit [`b63abee`](https://github.com/apache/spark/commit/b63abee881f2b4379f375500d51fdef706d6d512).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86787/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20424
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark 23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/20424
lgtm
@bersprockets you mentioned wanting to try the other route as well -- whats the status on that? shoudl we still wait on this one?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/20424
@HyukjinKwon @squito
Before merging this, allow me make an alternative PR that actually bypasses the problem (rather than simply reporting on the problem). If it seems like the solution adds to much complexity, we can reject the alternate PR and we can go with this one instead. Does that make sense?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/20424
@squito We made a few adjustments since your "lgtm". Do you want to take a quick look? @HyukjinKwon also gave his "lgtm" after the adjustments.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
LGTM!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164637553
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ throw new IOException(s"Bad port number in $daemonModule's stdout: " +
+ f"0x$daemonPort%08x")
--- End diff --
just a thought:
this error message won't mean much to the typical user. Would it be sensible to tell the user exactly what python command to run themselves to figure out the problem? Eg. "unexpected stdout from /foo/bar/some/path/to/python -m /path/to/daemon.py". That's what would help with that sitecustomization.py case. Or not useful in general?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168353782
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,29 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new SparkException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output. Invalid port number:
--- End diff --
>Oh, I believe double spaces were actually a style nit just to be consistent.
Oops, did I miss something?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164619346
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
Why don't we just copy the condition in `java.net.InetSocketAddress.checkPort`: `if (port < 0 || port > 0xFFFF)`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/20424
ah got it. sounds good to me, I will defer to @HyukjinKwon 's judgement. I think this change looks fine
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168352697
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,29 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new SparkException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output. Invalid port number:
--- End diff --
Oh, I believe double spaces were actually a style nit just to be consistent.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/20424
@squito
>you mentioned wanting to try the other route as well
That was this PR: https://github.com/apache/spark/pull/20519
@HyukjinKwon would prefer to go with the error message for now and wait and see if the issue comes up again.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #86782 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86782/testReport)** for PR 20424 at commit [`a1cb1a8`](https://github.com/apache/spark/commit/a1cb1a89d679840845142facf58f15e870f7c81d).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168075988
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,26 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
--- End diff --
Here too, `SparkException`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/20424
Hi @HyukjinKwon
>Do you guys have some more information to add in the error message?
If we go with this solution, I would like to improve the message before the PR is merged.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168075909
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,26 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output.
+ |Expected valid port number, got 0x$daemonPort%08x.
+ |PYTHONPATH set to '$pythonPath'
+ |Python command is '${command.asScala.mkString(" ")}'
+ |One possibility is a sitecustomize.py module in your python installation
+ |that is printing to stdout"""
--- End diff --
Shall we keep the format same as:
https://github.com/apache/spark/blob/b63abee881f2b4379f375500d51fdef706d6d512/core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala#L232-L235
?
the current message looks:
```
...
Caused by: java.io.IOException:
Bad data in pyspark.daemon's standard output.
Expected valid port number, got 0x4920616d.
PYTHONPATH set to '/.../spark/python/lib/pyspark.zip:/.../spark/python/lib/py4j-0.10.6-src.zip:/.../spark/assembly/target/scala-2.11/jars/spark-core_2.11-2.4.0-SNAPSHOT.jar:/.../spark/python/lib/py4j-0.10.6-src.zip:/.../spark/python/:'
Python command is 'python -m pyspark.daemon'
Check if you have a sitecustomize.py module in your python installation.
...
```
I made a suggestion while verifying this PR:
```
...
Error from bad data in pyspark.daemon's standard output. Invalid port number:
1633771786 (0x6161610a)
Python command to execute the daemon was:
python -m pyspark.daemon
One possibility is a sitecustomize module printing some data to the standard output.
This module 'sitecustomize.py' can be located in your Python path:
/.../spark/python/lib/pyspark.zip:/.../spark/python/lib/py4j-0.10.6-src.zip:/.../spark/assembly/target/scala-2.11/jars/spark-core_2.11-2.4.0-SNAPSHOT.jar:/.../spark/python/lib/py4j-0.10.6-src.zip:/.../spark/python/:
...
```
Here is the diff I used. I also did some insane nitpicks here as well.
```diff
diff --git a/core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala b/core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala
index 5790c050a7f..b44aa6064bb 100644
--- a/core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala
+++ b/core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala
@@ -196,20 +196,24 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemonPort = in.readInt()
} catch {
case _: EOFException =>
- throw new IOException(s"No port number in $daemonModule's stdout")
+ throw new SparkException(s"No port number in $daemonModule's standard output.")
}
- // test that the returned port number is within a valid range.
- // note: this does not cover the case where the port number
- // is arbitrary data but is also coincidentally within range
+ // Check if the returned port number is within a valid range.
+ // Note: this does not cover the case where the port number is arbitrary data but is
+ // also coincidentally within range.
if (daemonPort < 1 || daemonPort > 0xffff) {
val exceptionMessage = f"""
- |Bad data in $daemonModule's standard output.
- |Expected valid port number, got 0x$daemonPort%08x.
- |PYTHONPATH set to '$pythonPath'
- |Python command is '${command.asScala.mkString(" ")}'
- |Check if you have a sitecustomize.py module in your python installation."""
- throw new IOException(exceptionMessage.stripMargin)
+ |Error from bad data in $daemonModule's standard output. Invalid port number:
+ | $daemonPort (0x$daemonPort%08x)
+ |Python command to execute the daemon was:
+ | ${command.asScala.mkString(" ")}
+ |
+ |One possibility is a sitecustomize module printing some data to the standard output.
+ |This module 'sitecustomize.py' can be located in your Python path:
+ | $pythonPath
+ |"""
+ throw new SparkException(exceptionMessage.stripMargin)
}
// Redirect daemon stdout and stderr
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87179 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87179/testReport)** for PR 20424 at commit [`9286118`](https://github.com/apache/spark/commit/928611825d3065abe59ea3fb7aa5615b0307e441).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168075968
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,26 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output.
+ |Expected valid port number, got 0x$daemonPort%08x.
+ |PYTHONPATH set to '$pythonPath'
+ |Python command is '${command.asScala.mkString(" ")}'
+ |One possibility is a sitecustomize.py module in your python installation
+ |that is printing to stdout"""
+ throw new IOException(exceptionMessage.stripMargin)
--- End diff --
I think this rather should be `SparkException`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87377/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87389/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #86782 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86782/testReport)** for PR 20424 at commit [`a1cb1a8`](https://github.com/apache/spark/commit/a1cb1a89d679840845142facf58f15e870f7c81d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r167424753
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,25 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output.
+ |Expected valid port number, got 0x$daemonPort%08x.
+ |PYTHONPATH set to '$pythonPath'
+ |Python command is '${command.asScala.mkString(" ")}'
+ |Check if you have a sitecustomize.py module in your python installation."""
--- End diff --
This sounds like there's only one case - `sitecustomize`. I believe there are many possibility, for example, custom python executable, `usercustomize` and etc. Can we just say few words to implies `sitecustomize` is one potential case?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #86787 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86787/testReport)** for PR 20424 at commit [`a1cb1a8`](https://github.com/apache/spark/commit/a1cb1a89d679840845142facf58f15e870f7c81d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87179 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87179/testReport)** for PR 20424 at commit [`9286118`](https://github.com/apache/spark/commit/928611825d3065abe59ea3fb7aa5615b0307e441).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets closed the pull request at:
https://github.com/apache/spark/pull/20424
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #86787 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86787/testReport)** for PR 20424 at commit [`a1cb1a8`](https://github.com/apache/spark/commit/a1cb1a89d679840845142facf58f15e870f7c81d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87389 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87389/testReport)** for PR 20424 at commit [`b63abee`](https://github.com/apache/spark/commit/b63abee881f2b4379f375500d51fdef706d6d512).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
Will take a final look for this if we are all fine with this for now.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168350555
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -17,7 +17,7 @@
package org.apache.spark.api.python
-import java.io.{DataInputStream, DataOutputStream, InputStream, OutputStreamWriter}
+import java.io._
--- End diff --
👍
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
Thanks @squito. Will merge this one in few days.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
So, I think I am fine. I believe it's already a corner case and I think we don't have to put a lot of efforts on this ... to be honest. I only wanted to double check if there is a slight step back in the case of `0`.
@vanzin, @squito and @bersprockets, so, are we fine with that error message for now here?
I was thinking we could just go ahead but I believe it's nicer if I can double check / make sure before going and merging this one.
Do you guys have some more information to add in the error message?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on the issue:
https://github.com/apache/spark/pull/20424
The test that finished last succeeded, but the one that started last had a spurious error. Can I get a retest?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164616947
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
--- End diff --
Shall we throw `SparkException(s"No port number in $daemonModule's stdout", eof)`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164649608
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
I mean, I left my sign-off because what we do is basically move the _same_ check (`java.net.InetSocketAddress.checkPort`) ahead and another one is simply to wraps an exception, `EOFException`. I think we are here safe in theory.
I got your point of reserved ports and now the condition became narrower. I should check other things like which error it produces before in this case and if the current error message is nicer. Also, this seems not completely addressing the concerns about it.
I was wondering if this is worth doing these stuff. If you strongly prefer this, I won't stay against but may request few more investigations.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164643325
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
I am saying we can just safely move the same check and fail fast, which is simple and theoretically safe.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
LGTM except two nits and one question.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168080320
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,26 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output.
+ |Expected valid port number, got 0x$daemonPort%08x.
+ |PYTHONPATH set to '$pythonPath'
+ |Python command is '${command.asScala.mkString(" ")}'
+ |One possibility is a sitecustomize.py module in your python installation
+ |that is printing to stdout"""
--- End diff --
Nice, except this one thing:
>This module 'sitecustomize.py' can be located in your Python path:
/.../spark/python/lib/pyspark.zip:/.../spark/python/lib/py4j-0.10.6-src.zip:/.../spark/assembly/target/scala-2.11/jars/spark-core_2.11-2.4.0-SNAPSHOT.jar:/.../spark/python/lib/py4j-0.10.6-src.zip:/.../spark/python/:
I display the path because maybe you accidentally configured the path to have some old .zip or other incompatible versions of expected python modules on your path.
sitecustomize.py might not be in your path, although it could be. I found a machine that had it here: /usr/lib/python2.7/sitecustomize.py. Another, I found it here: /usr/lib64/python2.7/sitecustomize.py
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87459/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
Yup, makes sense.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168354622
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,29 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new SparkException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output. Invalid port number:
--- End diff --
OK, I will change that.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168354159
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,29 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new SparkException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output. Invalid port number:
--- End diff --
I meant ...
```scala
val exceptionMessage = f"""
|...
|...
...
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164646512
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
Sorry @HyukjinKwon , I didn't quite get the last point. Could you rephrase?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87389 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87389/testReport)** for PR 20424 at commit [`b63abee`](https://github.com/apache/spark/commit/b63abee881f2b4379f375500d51fdef706d6d512).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r165255299
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
ah I see, I think you are worried about something other than what bruce and I thought. Your concern is that we might throw an exception for some values that are actually perfectly legitimate. Port 0 being special is a pretty standard thing -- its mentioned in the constructor for ServerSocket: https://docs.oracle.com/javase/7/docs/api/java/net/ServerSocket.html#ServerSocket%28int%29
which implies that you shouldn't ever open a Socket on port 0, though I don't see that officially documented. At least on my laptop, I get different errors if I try to connect to port 0, vs. just connecting to a bogus port:
```scala
scala> val s2 = new Socket("localhost", 1234)
java.net.ConnectException: Connection refused
at java.net.PlainSocketImpl.socketConnect(Native Method)
at java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:350)
at java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:206)
at java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:188)
at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
at java.net.Socket.connect(Socket.java:589)
at java.net.Socket.connect(Socket.java:538)
at java.net.Socket.<init>(Socket.java:434)
at java.net.Socket.<init>(Socket.java:211)
... 29 elided
scala> val s3 = new Socket("localhost", 0)
java.net.NoRouteToHostException: Can't assign requested address
at java.net.PlainSocketImpl.socketConnect(Native Method)
at java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:350)
at java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:206)
at java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:188)
at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
at java.net.Socket.connect(Socket.java:589)
at java.net.Socket.connect(Socket.java:538)
at java.net.Socket.<init>(Socket.java:434)
at java.net.Socket.<init>(Socket.java:211)
... 29 elided
```
so I think its pretty safe to say that daemon.py (or whatever) shouldn't be passing back `0` as the port to bind to.
Still -- it is *clearly* safer to instead have the port written to some other file, or (another) socket, so that you we wouldn't have to worry about the details of this error handling.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86782/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87459 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87459/testReport)** for PR 20424 at commit [`f7a9b29`](https://github.com/apache/spark/commit/f7a9b2933c04c9d03ef010a8a76b33a8c299611a).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r165261830
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
Ah, OK. Thanks for clarification. Maybe, I was caring too much about it. Thanks all for bearing with me. I am fine as is.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164640285
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ throw new IOException(s"Bad port number in $daemonModule's stdout: " +
+ f"0x$daemonPort%08x")
--- End diff --
Yes, that makes sense. I might not be able to get it as clear as the exact path to the file, since the PythonWorkerFactory sets a PYTHONPATH environmental variable and then lets python itself figure out where on those paths the module actually lives. But I could tell the user how the PYTHONPATH was set up (in a generic sense, without using any shell's syntax) and then how the python command was subsequently run.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87479 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87479/testReport)** for PR 20424 at commit [`eceb24e`](https://github.com/apache/spark/commit/eceb24e61798f9e5da0ed3c4dfb94d677d08b10e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168081472
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,26 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output.
+ |Expected valid port number, got 0x$daemonPort%08x.
+ |PYTHONPATH set to '$pythonPath'
+ |Python command is '${command.asScala.mkString(" ")}'
+ |One possibility is a sitecustomize.py module in your python installation
+ |that is printing to stdout"""
--- End diff --
Oh, I see. I meant it might be located. please fix few my bad words :). One thing I was worried of is, it might print the Python path twice if it contained stderr (it can be wrapped in L232 -L235).
I think in most cases the Python paths will be printed out actually because it should usually throw an exception.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164642503
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
--- End diff --
I see, exc not used.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87459 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87459/testReport)** for PR 20424 at commit [`f7a9b29`](https://github.com/apache/spark/commit/f7a9b2933c04c9d03ef010a8a76b33a8c299611a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164765247
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
yeah this is kind of what I was getting at below -- what value are we adding with this extra handling, over the original exception?
Another possibility is to change this to not use stdout, but that adds more complexity. You could use sockets, or right the port to some dedicated temporary file.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87377 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87377/testReport)** for PR 20424 at commit [`b63abee`](https://github.com/apache/spark/commit/b63abee881f2b4379f375500d51fdef706d6d512).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168345838
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -17,7 +17,7 @@
package org.apache.spark.api.python
-import java.io.{DataInputStream, DataOutputStream, InputStream, OutputStreamWriter}
+import java.io._
--- End diff --
To save on jenkins resources :), I want to verify that the latest version of the message is OK:
<pre>
org.apache.spark.SparkException:
Bad data in pyspark.daemon's standard output. Invalid port number:
1315905645 (0x4e6f206d)
Python command to execute the daemon was:
python -m pyspark.daemon
Check that you don't have any unexpected modules or libraries in
your PYTHONPATH:
/.../github/spark_fork/python/lib/pyspark.zip:/.../github/spark_fork/python/lib/py4j-0.10.6-src.zip:/.../github/spark_fork/assembly/target/scala-2.11/jars/spark-core_2.11-2.4.0-SNAPSHOT.jar:/.../github/spark_fork/python/lib/py4j-0.10.6-src.zip:/.../github/spark_fork/python/:
Also, check if you have a sitecustomize.py module in your python path,
or in your python installation, that is printing to standard output
</pre>
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168076901
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -17,7 +17,7 @@
package org.apache.spark.api.python
-import java.io.{DataInputStream, DataOutputStream, InputStream, OutputStreamWriter}
+import java.io._
--- End diff --
BTW, don't forget to get rid of this import change too. This import looks needed due to `IOException`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
Sure, you can. retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
GitHub user bersprockets reopened a pull request:
https://github.com/apache/spark/pull/20424
[Spark-23240][python] Better error message when extraneous data in pyspark.daemon's stdout
## What changes were proposed in this pull request?
Print more helpful message when daemon module's stdout is empty or contains a bad port number.
## How was this patch tested?
Manually recreated the environmental issues that caused the mysterious exceptions at one site. Tested that the expected messages are logged.
Also, ran all scala unit tests.
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bersprockets/spark SPARK-23240_prop2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20424.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20424
----
commit b938ca4b910ee66575aea7544ffb7b1083102324
Author: Bruce Robbins <be...@...>
Date: 2018-01-28T18:59:58Z
Initial commit of fix of SPARK-23240
commit a1cb1a89d679840845142facf58f15e870f7c81d
Author: Bruce Robbins <be...@...>
Date: 2018-01-29T15:30:43Z
Fix IDE's messing with imports
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87179/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r165116866
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
yeah I could go either way on this. Personally i think just the enhanced error message you have above would be useful, without going through the added trouble of using another approach. I'll defer to your opinion @HyukjinKwon
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87362 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87362/testReport)** for PR 20424 at commit [`b63abee`](https://github.com/apache/spark/commit/b63abee881f2b4379f375500d51fdef706d6d512).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
Merged to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20424
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20424
**[Test build #87479 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87479/testReport)** for PR 20424 at commit [`eceb24e`](https://github.com/apache/spark/commit/eceb24e61798f9e5da0ed3c4dfb94d677d08b10e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r168086187
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +192,26 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case _: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ val exceptionMessage = f"""
+ |Bad data in $daemonModule's standard output.
+ |Expected valid port number, got 0x$daemonPort%08x.
+ |PYTHONPATH set to '$pythonPath'
+ |Python command is '${command.asScala.mkString(" ")}'
+ |One possibility is a sitecustomize.py module in your python installation
+ |that is printing to stdout"""
--- End diff --
But I think there's no need to put some additional logics here to deal with it tho. Let's just fix the error message to be in the similar format.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20424
Ah, OK. fixing the error message is fine as a separate improvement.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164855511
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
Oh, I see. Let me address the two parts of the comment separately:
First part: What's the point of throwing an exception for a bad port number when the original handling did that already?
The original handling was:
<pre>
java.lang.IllegalArgumentException: port out of range:1315905645
at java.net.InetSocketAddress.checkPort(InetSocketAddress.java:143)
</pre>
This error occurred in a different function than the one that obtained the port number. So you had to track down the source of the port number. This actually added an extra step to the original debugging for the sitecustomize.py issue.
The proposed handling is:
<pre>
java.io.IOException: Bad port number in pyspark.daemon's stdout: 0x4e6f206d
at org.apache.spark.api.python.PythonWorkerFactory.startDaemon(PythonWorkerFactory.scala:205)
</pre>
Here we're not saying "somehow we got a bad port number", but that a particular python module (the name of which is displayed, since the name of that module is configurable) has returned bad data.
Also, the check is at the point at which the port number is obtained, not waiting until sometime later in another function where the port number is used (and where possibly something else has changed the port number, which is not likely, but you would need to check that).
Perhaps the message could be better, e.g.,:
<pre>
Bad data in pyspark.daemon's output.
Expected valid port number, got 0x4e6f206d.
PYTHONPATH set to '/Users/brobbins/github/spark_fork/python/lib/pyspark.zip:/Users/brobbins/github/spark_fork/python/lib/py4j-0.10.6-src.zip:/Users/brobbins/github/spark_fork/assembly/target/scala-2.11/jars/spark-core_2.11-2.4.0-SNAPSHOT.jar:/Users/brobbins/github/spark_fork/python/lib/py4j-0.10.6-src.zip:/Users/brobbins/github/spark_fork/python/:'
Command to run python module was 'python -m pyspark.daemon'
Check whether you have a sitecustomize.py module that may be printing output to stdout.
</pre>
Second part: Why don't we fix this?
That's reasonable. A couple of points:
- The name of the python daemon module is now configurable so that the module can be wrapped with customizations. It appears that this is only in the main branch and not even released on Spark 2.3, so it might be safe to change daemon.py (and potentially its existing wrappers) to return the port number in a different way.
- I don't have a good feel for how often sitecustomize.py is used, so not sure of the relative value of some mild hacking up of this code vs. just letting the user know what happened.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164617708
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
--- End diff --
nit: `exc` -> `_`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20424
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87479/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20424: [Spark-23240][python] Better error message when extraneo...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/20424
still lgtm, thanks
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164641641
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
Port 0 has special meaning. A program passes port 0 when it wants the system to choose an unused port on the program's behalf. So, the daemon should not return 0.
It's valid to pass port 0 to InetSocketAddress, since you might be asking for the system to assign a port for you.
However, following my own logic, the code in my pull request really should be checking for the range 49152-65535 (ephemeral range) instead of 1-65535, but I didn't have the nerve to make it that restrictive.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r165236983
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
Thanks, @squito. My only worry is, what is the error message like befer / after this change when `daemonPort` happens to be 0 because it was filled in pyspark.daemon's stdout unexpectedly by the sitecustomize or usercustomize whatever?
I assume the error message becomes different as you described above because `java.net.InetSocketAddress.checkPort` passes `0` case but your proposed change also catches `0` case too for the reason you described above.
This case `0` is different because the previous codes probably try to attempt a connection whether it's a valid port or not, after passing `java.net.InetSocketAddress.checkPort`, if I didn't misunderstand.
I want to be very sure on this. Is it guranteed to be always failed without any hole and the error message is always better in this case? How about other ports reserved or "well-known" ? Should we really take this into account here?
Please let me know if I missed something.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r164619864
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
+ throw new IOException(s"Bad port number in $daemonModule's stdout: " +
+ f"0x$daemonPort%08x")
--- End diff --
Shall we also show the actual number?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20424: [Spark-23240][python] Better error message when e...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20424#discussion_r165528385
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala ---
@@ -191,7 +191,20 @@ private[spark] class PythonWorkerFactory(pythonExec: String, envVars: Map[String
daemon = pb.start()
val in = new DataInputStream(daemon.getInputStream)
- daemonPort = in.readInt()
+ try {
+ daemonPort = in.readInt()
+ } catch {
+ case exc: EOFException =>
+ throw new IOException(s"No port number in $daemonModule's stdout")
+ }
+
+ // test that the returned port number is within a valid range.
+ // note: this does not cover the case where the port number
+ // is arbitrary data but is also coincidentally within range
+ if (daemonPort < 1 || daemonPort > 0xffff) {
--- End diff --
> Still -- it is clearly safer to instead have the port written to some other file, or (another) socket, so that you we wouldn't have to worry about the details of this error handling.
Let's go that route. I will have a PR in the coming days.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org