-
Notifications
You must be signed in to change notification settings - Fork 28.9k
SPARK-2310. Support arbitrary Spark properties on the command line with ... #1253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Verified this on a pseudo-distributed cluster |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
Hey @sryza - I did a straw poll offline discussing this with a few other contributors. The consensus was that it might be better to have a I.e. On admittedly bad thing about this approach is that if users have arguments with spaces in them, they will have to quote the entire thing: Which might not be intuitive, so it would be good to document that (provided you are generally okay with this proposed syntax). |
|
how about -Dspark.app.name=blah? because in jvm or Hadoop, they use -D flag to represent conf properties. |
|
-D feels more natural indeed; I would expect those args to be passed through to the JVM as-is. Because that's a way to set these env properties too right? In fact, wouldn't it be nicer to just let any -D arg through? |
|
IMO |
|
I agree, -D is for JVM options, but these are not arbitrary JVM options. |
|
Good points. I meant triaging all -D options but yes those then have very 'local' semantics. |
|
Updated patch to use --conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So here, it might be a bit nicer to do something like:
value match {
case k :: "=" :: v =>
sparkProperties(k) = v
case _ =>
// throw exception saying there is a bad format
}
Would this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That didn't compile for me. As far as I can tell you can't do pattern matching on strings in that way. Switching it to something that looks a little prettier. Let me know if I'm missing anything.
|
Updated patch addresses Patrick's feedback |
|
QA tests have started for PR 1253. This patch merges cleanly. |
|
QA results for PR 1253: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you want a val here?
|
Can you support Also, the spark-submit doc (http://spark.apache.org/docs/latest/submitting-applications.html) should be updated to list this option. |
|
QA tests have started for PR 1253. This patch merges cleanly. |
|
QA results for PR 1253: |
|
The failure appears to be unrelated (MIMA compatibility issue in MLLib). |
|
Jenkins, retest this please. |
|
QA tests have started for PR 1253. This patch merges cleanly. |
|
QA results for PR 1253: |
|
@sryza I merged this just now because another patch was going to change this code and I wanted to avoid you having to rebase again. That said, I found an issue with this after merging. Would you be able to fix this? |
|
Thanks, I'm on it. |
…th ... ...spark-submit The PR allows invocations like spark-submit --class org.MyClass --spark.shuffle.spill false myjar.jar Author: Sandy Ryza <[email protected]> Closes apache#1253 from sryza/sandy-spark-2310 and squashes the following commits: 1dc9855 [Sandy Ryza] More doc and cleanup 00edfb9 [Sandy Ryza] Review comments 91b244a [Sandy Ryza] Change format to --conf PROP=VALUE 8fabe77 [Sandy Ryza] SPARK-2310. Support arbitrary Spark properties on the command line with spark-submit
### What changes were proposed in this pull request? This PR adds logic for rewriting row-level commands in 3.2. ### Why are the changes needed? These changes are needed to support DELETE, UPDATE, MERGE commands in Iceberg. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Locally and tests in Iceberg.
… blacklisted files (apache#1253) * [classpathfilter] Do not expand classpath entries which don't contain blacklisted files * [classpathfilter] Remove unneeded checks * [classpathfilter] Don't check for duplicates to keep everything as simple as possible
...spark-submit
The PR allows invocations like
spark-submit --class org.MyClass --spark.shuffle.spill false myjar.jar