Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
250cb95
Do not ignore spark.driver.extra* for client mode
andrewor14 Aug 4, 2014
a2ab1b0
Parse spark.driver.extra* in bash
andrewor14 Aug 6, 2014
0025474
Revert SparkSubmit handling of --driver-* options for only cluster mode
andrewor14 Aug 6, 2014
63ed2e9
Merge branch 'master' of github.com:apache/spark into submit-driver-e…
andrewor14 Aug 6, 2014
75ee6b4
Remove accidentally added file
andrewor14 Aug 6, 2014
8843562
Fix compilation issues...
andrewor14 Aug 6, 2014
98dd8e3
Add warning if properties file does not exist
andrewor14 Aug 6, 2014
130f295
Handle spark.driver.memory too
andrewor14 Aug 6, 2014
4edcaa8
Redirect stdout to stderr for python
andrewor14 Aug 6, 2014
e5cfb46
Collapse duplicate code + fix potential whitespace issues
andrewor14 Aug 6, 2014
4ec22a1
Merge branch 'master' of github.com:apache/spark into submit-driver-e…
andrewor14 Aug 6, 2014
ef12f74
Minor formatting
andrewor14 Aug 6, 2014
fa2136e
Escape Java options + parse java properties files properly
andrewor14 Aug 7, 2014
dec2343
Only export variables if they exist
andrewor14 Aug 7, 2014
a4df3c4
Move parsing and escaping logic to utils.sh
andrewor14 Aug 7, 2014
de765c9
Print spark-class command properly
andrewor14 Aug 7, 2014
8e552b7
Include an example of spark.*.extraJavaOptions
andrewor14 Aug 7, 2014
c13a2cb
Merge branch 'master' of github.com:apache/spark into submit-driver-e…
andrewor14 Aug 7, 2014
c854859
Add small comment
andrewor14 Aug 7, 2014
1cdc6b1
Fix bug: escape escaped double quotes properly
andrewor14 Aug 7, 2014
45a1eb9
Fix bug: escape escaped backslashes and quotes properly...
andrewor14 Aug 7, 2014
aabfc7e
escape -> split (minor)
andrewor14 Aug 7, 2014
a992ae2
Escape spark.*.extraJavaOptions correctly
andrewor14 Aug 7, 2014
c7b9926
Minor changes to spark-defaults.conf.template
andrewor14 Aug 7, 2014
5d8f8c4
Merge branch 'master' of github.com:apache/spark into submit-driver-e…
andrewor14 Aug 7, 2014
e793e5f
Handle multi-line arguments
andrewor14 Aug 8, 2014
c2273fc
Fix typo (minor)
andrewor14 Aug 8, 2014
b3c4cd5
Fix bug: count the number of quotes instead of detecting presence
andrewor14 Aug 8, 2014
4ae24c3
Fix bug: escape properly in quote_java_property
andrewor14 Aug 8, 2014
8d26a5c
Add tests for bash/utils.sh
andrewor14 Aug 8, 2014
2732ac0
Integrate BASH tests into dev/run-tests + log error properly
andrewor14 Aug 9, 2014
aeb79c7
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 Aug 9, 2014
8d4614c
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 Aug 16, 2014
56ac247
Use eval and set to simplify splitting
andrewor14 Aug 16, 2014
bd0d468
Simplify parsing config file by ignoring multi-line arguments
andrewor14 Aug 16, 2014
be99eb3
Fix tests to not include multi-line configs
andrewor14 Aug 16, 2014
371cac4
Add function prefix (minor)
andrewor14 Aug 16, 2014
fa11ef8
Parse the properties file only if the special configs exist
andrewor14 Aug 16, 2014
7396be2
Explicitly comment that multi-line properties are not supported
andrewor14 Aug 16, 2014
7a4190a
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 Aug 16, 2014
c886568
Fix lines too long + a few comments / style (minor)
andrewor14 Aug 16, 2014
0effa1e
Add code in Scala that handles special configs
andrewor14 Aug 19, 2014
a396eda
Nullify my own hard work to simplify bash
andrewor14 Aug 19, 2014
c37e08d
Revert a few more changes
andrewor14 Aug 19, 2014
3a8235d
Only parse the properties file if special configs exist
andrewor14 Aug 19, 2014
7d94a8d
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 Aug 19, 2014
b71f52b
Revert a few more changes (minor)
andrewor14 Aug 19, 2014
c84f5c8
Remove debug print statement (minor)
andrewor14 Aug 19, 2014
158f813
Remove "client mode" boolean argument
andrewor14 Aug 19, 2014
a91ea19
Fix precedence of library paths, classpath, java opts and memory
andrewor14 Aug 19, 2014
1ea6bbe
SparkClassLauncher -> SparkSubmitDriverBootstrapper
andrewor14 Aug 19, 2014
d6488f9
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 Aug 19, 2014
19464ad
SPARK_SUBMIT_JAVA_OPTS -> SPARK_SUBMIT_OPTS
andrewor14 Aug 19, 2014
8867a09
A few more naming things (minor)
andrewor14 Aug 19, 2014
9ba37e2
Don't barf when the properties file does not exist
andrewor14 Aug 19, 2014
a78cb26
Revert a few changes in utils.sh (minor)
andrewor14 Aug 19, 2014
d0f20db
Don't pass empty library paths, classpath, java opts etc.
andrewor14 Aug 19, 2014
9a778f6
Fix PySpark: actually kill driver on termination
andrewor14 Aug 20, 2014
51aeb01
Filter out JVM memory in Scala rather than Bash (minor)
andrewor14 Aug 20, 2014
ff34728
Minor comments
andrewor14 Aug 20, 2014
08fd788
Warn against external usages of SparkSubmitDriverBootstrapper
andrewor14 Aug 20, 2014
24dba60
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 Aug 20, 2014
bed4bdf
Change a few comments / messages (minor)
andrewor14 Aug 20, 2014
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
escape -> split (minor)
  • Loading branch information
andrewor14 committed Aug 7, 2014
commit aabfc7e1da8897b266020da6c480cbe7d774bc99
10 changes: 5 additions & 5 deletions bin/spark-class
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ if [ -e "$FWDIR/conf/java-opts" ] ; then
JAVA_OPTS="$JAVA_OPTS `cat $FWDIR/conf/java-opts`"
fi

# Escape JAVA_OPTS properly to handle whitespace, double quotes and backslashes
# This exports the escaped java options into ESCAPED_JAVA_OPTS
escape_java_options "$JAVA_OPTS"
# Split JAVA_OPTS properly to handle whitespace, double quotes and backslashes
# This exports the split java options into SPLIT_JAVA_OPTS
split_java_options "$JAVA_OPTS"
Copy link
Contributor

Choose a reason for hiding this comment

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

This actually solves a more general problem than those reported in SPARK-2849 and SPARK-2914... the problem/feature is that in general we don't support quotes in any of the java option strings we have. I tested this in master and confirmed it doesn't work:

$ export SPARK_JAVA_OPTS="-Dfoo=\"bar baz\""
$ ./bin/spark-shell
Spark assembly has been built with Hive, including Datanucleus jars on classpath
Error: Could not find or load main class baz"

So it might be good to create another JIRA as well for this PR. One that just explains that none of the JAVA_OPTS variants we have correctly support quoted strings.


# Attention: when changing the way the JAVA_OPTS are assembled, the change must be reflected in CommandUtils.scala!

Expand Down Expand Up @@ -159,10 +159,10 @@ export CLASSPATH
if [ "$SPARK_PRINT_LAUNCH_COMMAND" == "1" ]; then
# Put quotes around system properties in case they contain spaces
# This exports the resulting list of java opts into QUOTED_JAVA_OPTS
quote_java_property "${ESCAPED_JAVA_OPTS[@]}"
quote_java_property "${SPLIT_JAVA_OPTS[@]}"
echo -n "Spark Command: " 1>&2
echo "$RUNNER" -cp "$CLASSPATH" "${QUOTED_JAVA_OPTS[@]}" "$@" 1>&2
echo -e "========================================\n" 1>&2
fi

exec "$RUNNER" -cp "$CLASSPATH" "${ESCAPED_JAVA_OPTS[@]}" "$@"
exec "$RUNNER" -cp "$CLASSPATH" "${SPLIT_JAVA_OPTS[@]}" "$@"
16 changes: 8 additions & 8 deletions bin/utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ parse_java_property() {
export JAVA_PROPERTY_VALUE
}

# Properly escape java options, dealing with whitespace, double quotes and backslashes.
# This accepts a string and returns the escaped list through ESCAPED_JAVA_OPTS.
escape_java_options() {
ESCAPED_JAVA_OPTS=() # return value
option_buffer="" # buffer for collecting parts of an option
opened_quotes=0 # whether we are expecting a closing double quotes
# Properly split java options, dealing with whitespace, double quotes and backslashes.
# This accepts a string and returns the resulting list through SPLIT_JAVA_OPTS.
split_java_options() {
SPLIT_JAVA_OPTS=() # return value
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could be solved using an eval statement much more easily. At first I thought there are some security concerns, but I don't think this allows people to do anything they couldn't already do:

test="One \"This \\\"is two\" Three"
eval "set -- $test"
for i in "$@"
do
  echo $i
done

When executed this runs:

One
This "is two
Three

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that does exactly what we want.

option_buffer="" # buffer for collecting parts of an option
opened_quotes=0 # whether we are expecting a closing double quotes
for word in $1; do
contains_quote=$(echo "$word" | sed "s/\\\\\"//g" | grep "\"")
if [[ -n "$contains_quote" ]]; then
Expand All @@ -49,7 +49,7 @@ escape_java_options() {
fi
if [[ $opened_quotes == 0 ]]; then
# Remove all non-escaped quotes around the value
ESCAPED_JAVA_OPTS+=("$(
SPLIT_JAVA_OPTS+=("$(
echo "$option_buffer $word" | \
sed "s/^[[:space:]]*//" | \
sed "s/\([^\\]\)\"/\1/g" | \
Expand All @@ -66,7 +66,7 @@ escape_java_options() {
echo "Java options parse error! Expecting closing double quotes." 1>&2
exit 1
fi
export ESCAPED_JAVA_OPTS
export SPLIT_JAVA_OPTS
}

# Put double quotes around each of the given java options that is a system property.
Expand Down