[SPARK-2894] spark-shell doesn't accept flags by sarutak · Pull Request #1825 · apache/spark

sarutak · 2014-08-07T05:58:36Z

As @sryza reported, spark-shell doesn't accept any flags.
The root cause is wrong usage of spark-submit in spark-shell and it come to the surface by #1801

rxin · 2014-08-07T06:10:13Z

Jenkins, add to whitelist.

SparkQA · 2014-08-07T06:14:28Z

QA tests have started for PR 1825. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18100/consoleFull

sryza · 2014-08-07T06:20:27Z

This will allow spark-shell to take spark-submit options, but will remove its ability to take spark-shell-specific options (currently there's only one, "file"). I'm unclear on the best way to support both of these.

pwendell · 2014-08-07T06:28:08Z

does the shell actually take flags? I didn't realize this when I OK'd #1801. If there are specific flags, we should trap them and pass them after spark-shell. For an example see the code in the SQL shell:
https://github.com/apache/spark/blob/master/bin/spark-sql#L66

andrewor14 · 2014-08-07T06:33:33Z

AFAIK I don't believe spark-shell takes in any application-specific arguments, at least not documented ones. @sryza What file are you referring to?

sryza · 2014-08-07T06:34:41Z

org.apache.spark.repl.SparkRunnerSettings

andrewor14 · 2014-08-07T06:40:06Z

ah... looks like we need some special logic to filter that one out here

SparkQA · 2014-08-07T06:58:03Z

QA results for PR 1825:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18100/consoleFull

SparkQA · 2014-08-07T10:19:22Z

QA tests have started for PR 1825. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18118/consoleFull

SparkQA · 2014-08-07T11:10:15Z

QA results for PR 1825:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds the following public classes (experimental):
$FWDIR/bin/spark-submit --class org.apache.spark.repl.Main ${SPARK_SUBMIT_ARGS[@]} spark-shell ${SPARK_SHELL_ARGS[@]}
$FWDIR/bin/spark-submit --class org.apache.spark.repl.Main ${SPARK_SUBMIT_ARGS[@]} spark-shell ${SPARK_SHELL_ARGS[@]}

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18118/consoleFull

andrewor14 · 2014-08-07T21:54:45Z

It would be good to verify whether pyspark is affected too (see java_gateway.py)

JoshRosen · 2014-08-07T22:01:33Z

There seems to be a similar PySpark issue, too (I just ran into it during testing):

https://issues.apache.org/jira/browse/SPARK-2110

pwendell · 2014-08-07T23:23:43Z

For consistency, could you use the same approach that we use in spark-sql script?

O.K. I'll check it.

liancheng · 2014-08-07T23:52:46Z

Maybe instead of filtering out all spark-shell (and pyspark) options, we can do the opposite as what I did in bin/spark-sql: filter out all spark-submit options, and pass all others to the main class. At least we have control on the spark-submit option list.

I'll try to write a utility script to filter out spark-submit options today.

andrewor14 · 2014-08-08T00:24:44Z

@liancheng However that means every time we want to change a spark-submit config, we need to change it in multiple places, which might make things harder to maintain. For spark-shell, aren't we only expecting one type of argument? I'm not super familiar with this functionality of passing a settings file to spark-shell, but I don't think this file path matches --*, so maybe we can filter this argument out this way. This might be simpler than enumerating all of spark-submit's flags.

liancheng · 2014-08-08T02:25:09Z

@andrewor14 Ah OK, I only had a glance at repl.Main and SparkILoop before writing my last comment and thought spark-shell might accept all standard Scala REPL options by mistake.

@sarutak Thanks for help fixing this. I'd also suggest what @pwendell suggested, i.e. using a similar while/case structure in bin/spark-sql and sbin/start-thriftserver.sh to filter out the -i <init-file> option, can be easier to read. And we need to do the same thing for pyspark.

liancheng · 2014-08-08T02:26:29Z

We should move this call before the if statement to cover the Cygwin branch. And please use 4-space indentation here :)

liancheng · 2014-08-08T02:48:20Z

@JoshRosen I'm checking pyspark, but I'm not very familiar with this. Would you mind to help confirming are there any other Python/IPython/PySpark specific command line options that need to be handled?

JoshRosen · 2014-08-08T03:01:43Z

@lianchengipython has a ton of command line options that someone might hypothetically want to use (just take a look at ipython --help). Maybe try things out with a few of these options? I'm not sure which pyspark options users rely on.

SparkQA · 2014-08-08T05:44:33Z

QA tests have started for PR 1825. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18190/consoleFull

SparkQA · 2014-08-08T06:35:01Z

QA results for PR 1825:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds the following public classes (experimental):
$FWDIR/bin/spark-submit --class org.apache.spark.repl.Main ${SUBMISSION_ARGS[]} spark-shell ${CLIARGS[]}
$FWDIR/bin/spark-submit --class org.apache.spark.repl.Main ${SUBMISSION_ARGS[]} spark-shell ${CLI_ARGS[]}

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18190/consoleFull

andrewor14 · 2014-08-08T06:56:32Z

indentation is off

Sorry, included tabs.

SparkQA · 2014-08-08T07:09:39Z

QA tests have started for PR 1825. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18197/consoleFull

SparkQA · 2014-08-08T07:53:27Z

QA results for PR 1825:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds the following public classes (experimental):
$FWDIR/bin/spark-submit --class org.apache.spark.repl.Main ${SUBMISSION_ARGS[]} spark-shell ${CLIARGS[]}
$FWDIR/bin/spark-submit --class org.apache.spark.repl.Main ${SUBMISSION_ARGS[]} spark-shell ${CLI_ARGS[]}

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18197/consoleFull

andrewor14 · 2014-08-08T17:55:40Z

This failed a flaky test. Retest this please

SparkQA · 2014-08-08T17:59:31Z

QA tests have started for PR 1825. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18211/consoleFull

liancheng · 2014-08-08T18:00:51Z

@JoshRosen Thanks, then I think gathering all spark-submit options rather than options of spark-shell/pyspark or any other potential applications can be more scalable, since we have control over these options. I opened a WIP branch for this, and it seems that we can fix both pyspark (Python, IPython and the deprecated pyspark app.py style) and spark-shell easily.

SparkQA · 2014-08-09T16:09:31Z

QA tests have started for PR 1825. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18254/consoleFull

liancheng · 2014-08-09T16:18:19Z

The following case fails because this line doesn't handle quoted string with spaces properly:
./bin/pyspark app.py --master spark://lian-laptop.local:7077 --name "awesome name"
A possible fix is to replace this line with the trick bin/pyspark uses:
whitespace="[[:space:]]" i=$1if [[ $1=~\" ]];then i=$(echo $1| sed 's/\"/\\\"/g');fiif [[ $1=~$whitespace ]];then i=\"$1\";fi SUBMISSION_OPTS+=($i);shift

Thanks for pointing that. Newer PR is modified that.
I think the main reason is $1 is not double-quoted in utils.sh.

Hmm, I'm afraid only adding double quotes doesn't help, the case I mentioned above still fails, and the pyspark trick can be helpful. And we only need to deal with the argument part of those options that need an argument.

Actually double quotes should be sufficient here, since it preserves whitespaces by default. The only reason why we needed the special handling for pyspark is that we need to pass the string literal in as an environment variable, otherwise python won't know how to split our arguments. We don't need to do the same here because this is already split by bash.

liancheng · 2014-08-09T16:20:48Z

Hey @sarutak, thanks for fixing this :) Did some tests locally and the only issue I found is the quoted string case I just commented. Otherwise LGTM.

andrewor14 · 2014-08-09T18:23:34Z

We should add a note here that says something like All changes here must be reflected in SparkSubmitArguments.scala

and add a similar comment in SparkSubmitArguments.scala too. It would be bad if these two go out of sync

andrewor14 · 2014-08-09T18:51:50Z

I have tested this locally and I was able to get both spark shell and pyspark working as before. I also tried it with quoted arguments and backslashes and the arguments are still propagated properly. I haven't tried the settings file for spark-shell, however. Pending a few comments this LGTM, and we should get this in quickly if possible.

Added comments in utils.py and SparkSubmitArguments.scala to notice someone that both of the files should be modified simultaneously

SparkQA · 2014-08-09T19:04:32Z

QA tests have started for PR 1825. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18257/consoleFull

Added comments in utils.py and SparkSubmitArguments.scala to notice someone that both of the files should be modified simultaneously

SparkQA · 2014-08-09T21:39:34Z

QA tests have started for PR 1825. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18261/consoleFull

andrewor14 · 2014-08-09T21:48:13Z

LGTM, thanks @sarutak and @liancheng.

pwendell · 2014-08-09T21:48:34Z

Okay cool - we can merge this once the tests pass.

pwendell · 2014-08-10T04:10:05Z

Tests passed so I'm going to merge it

As sryza reported, spark-shell doesn't accept any flags. The root cause is wrong usage of spark-submit in spark-shell and it come to the surface by #1801 Author: Kousuke Saruta <sarutak@oss.nttdata.co.jp> Author: Cheng Lian <lian.cs.zju@gmail.com> Closes#1715, Closes#1864, and Closes#1861Closes#1825 from sarutak/SPARK-2894 and squashes the following commits: 47f3510 [Kousuke Saruta] Merge branch 'master' of git://git.apache.org/spark into SPARK-2894 2c899ed [Kousuke Saruta] Removed useless code from java_gateway.py 98287ed [Kousuke Saruta] Removed useless code from java_gateway.py 513ad2e [Kousuke Saruta] Modified util.sh to enable to use option including white spaces 28a374e [Kousuke Saruta] Modified java_gateway.py to recognize arguments 5afc584 [Cheng Lian] Filter out spark-submit options when starting Python gateway e630d19 [Cheng Lian] Fixing pyspark and spark-shell CLI options

JoshRosen · 2014-08-15T19:07:46Z

Does this work with Python? I ask because

IPYTHON=1 ./bin/pyspark --no-banner --no-confirm-exit

doesn't seem to respect my ipython flags. Am I using this right?

andrewor14 · 2014-08-15T19:13:57Z

You need to set them through IPYTHON_OPTS

JoshRosen · 2014-08-15T19:19:03Z

Doh, I forgot about that. I tried a bunch of other Python configurations and in every case the behavior seems to match 1.0.2, which is great. Thanks!

As sryza reported, spark-shell doesn't accept any flags. The root cause is wrong usage of spark-submit in spark-shell and it come to the surface by apache#1801 Author: Kousuke Saruta <sarutak@oss.nttdata.co.jp> Author: Cheng Lian <lian.cs.zju@gmail.com> Closesapache#1715, Closesapache#1864, and Closesapache#1861Closesapache#1825 from sarutak/SPARK-2894 and squashes the following commits: 47f3510 [Kousuke Saruta] Merge branch 'master' of git://git.apache.org/spark into SPARK-2894 2c899ed [Kousuke Saruta] Removed useless code from java_gateway.py 98287ed [Kousuke Saruta] Removed useless code from java_gateway.py 513ad2e [Kousuke Saruta] Modified util.sh to enable to use option including white spaces 28a374e [Kousuke Saruta] Modified java_gateway.py to recognize arguments 5afc584 [Cheng Lian] Filter out spark-submit options when starting Python gateway e630d19 [Cheng Lian] Fixing pyspark and spark-shell CLI options

concretevitamin mentioned this pull request Aug 7, 2014
[SPARK-2678][Core][SQL] A workaround for SPARK-2678 #1801
Closed

pwendell reviewed Aug 7, 2014
View reviewed changes

liancheng reviewed Aug 8, 2014
View reviewed changes

andrewor14 reviewed Aug 8, 2014
View reviewed changes

Fixing pyspark and spark-shell CLI options
e630d19

Modified util.sh to enable to use option including white spaces
513ad2e

liancheng reviewed Aug 9, 2014
View reviewed changes

andrewor14 reviewed Aug 9, 2014
View reviewed changes

Removed useless code from java_gateway.py
98287ed
Added comments in utils.py and SparkSubmitArguments.scala to notice someone that both of the files should be modified simultaneously

sarutak added 2 commits August 10, 2014 06:35

Removed useless code from java_gateway.py
2c899ed
Added comments in utils.py and SparkSubmitArguments.scala to notice someone that both of the files should be modified simultaneously

Merge branch 'master' of git://git.apache.org/spark into SPARK-2894
47f3510

asfgit closed this in 4f4a988Aug 10, 2014

andrewor14 mentioned this pull request Aug 13, 2014
[SPARK-3006] Failed to execute spark-shell in Windows OS #1918
Closed

andrewor14 mentioned this pull request Aug 25, 2014
[SPARK-3216] Spark-shell is broken in branch-1.0 #2122
Closed

sarutak deleted the SPARK-2894 branch April 11, 2015 05:22

Uh oh!

Conversation

sarutak commented Aug 7, 2014

Uh oh!

rxin commented Aug 7, 2014

Uh oh!

SparkQA commented Aug 7, 2014

Uh oh!

sryza commented Aug 7, 2014

Uh oh!

pwendell commented Aug 7, 2014

Uh oh!

andrewor14 commented Aug 7, 2014

Uh oh!

sryza commented Aug 7, 2014

Uh oh!

andrewor14 commented Aug 7, 2014

Uh oh!

SparkQA commented Aug 7, 2014

Uh oh!

SparkQA commented Aug 7, 2014

Uh oh!

SparkQA commented Aug 7, 2014

Uh oh!

andrewor14 commented Aug 7, 2014

Uh oh!

JoshRosen commented Aug 7, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liancheng commented Aug 7, 2014

Uh oh!

andrewor14 commented Aug 8, 2014

Uh oh!

liancheng commented Aug 8, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liancheng commented Aug 8, 2014

Uh oh!

JoshRosen commented Aug 8, 2014

Uh oh!

SparkQA commented Aug 8, 2014

Uh oh!

SparkQA commented Aug 8, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 8, 2014

Uh oh!

SparkQA commented Aug 8, 2014

Uh oh!

andrewor14 commented Aug 8, 2014

Uh oh!

SparkQA commented Aug 8, 2014

Uh oh!

liancheng commented Aug 8, 2014

Uh oh!

SparkQA commented Aug 9, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liancheng commented Aug 9, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented Aug 9, 2014

Uh oh!