-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-2849] Handle driver configs separately in client mode #1845
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
Closed
Closed
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 a2ab1b0
Parse spark.driver.extra* in bash
andrewor14 0025474
Revert SparkSubmit handling of --driver-* options for only cluster mode
andrewor14 63ed2e9
Merge branch 'master' of github.com:apache/spark into submit-driver-e…
andrewor14 75ee6b4
Remove accidentally added file
andrewor14 8843562
Fix compilation issues...
andrewor14 98dd8e3
Add warning if properties file does not exist
andrewor14 130f295
Handle spark.driver.memory too
andrewor14 4edcaa8
Redirect stdout to stderr for python
andrewor14 e5cfb46
Collapse duplicate code + fix potential whitespace issues
andrewor14 4ec22a1
Merge branch 'master' of github.com:apache/spark into submit-driver-e…
andrewor14 ef12f74
Minor formatting
andrewor14 fa2136e
Escape Java options + parse java properties files properly
andrewor14 dec2343
Only export variables if they exist
andrewor14 a4df3c4
Move parsing and escaping logic to utils.sh
andrewor14 de765c9
Print spark-class command properly
andrewor14 8e552b7
Include an example of spark.*.extraJavaOptions
andrewor14 c13a2cb
Merge branch 'master' of github.com:apache/spark into submit-driver-e…
andrewor14 c854859
Add small comment
andrewor14 1cdc6b1
Fix bug: escape escaped double quotes properly
andrewor14 45a1eb9
Fix bug: escape escaped backslashes and quotes properly...
andrewor14 aabfc7e
escape -> split (minor)
andrewor14 a992ae2
Escape spark.*.extraJavaOptions correctly
andrewor14 c7b9926
Minor changes to spark-defaults.conf.template
andrewor14 5d8f8c4
Merge branch 'master' of github.com:apache/spark into submit-driver-e…
andrewor14 e793e5f
Handle multi-line arguments
andrewor14 c2273fc
Fix typo (minor)
andrewor14 b3c4cd5
Fix bug: count the number of quotes instead of detecting presence
andrewor14 4ae24c3
Fix bug: escape properly in quote_java_property
andrewor14 8d26a5c
Add tests for bash/utils.sh
andrewor14 2732ac0
Integrate BASH tests into dev/run-tests + log error properly
andrewor14 aeb79c7
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 8d4614c
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 56ac247
Use eval and set to simplify splitting
andrewor14 bd0d468
Simplify parsing config file by ignoring multi-line arguments
andrewor14 be99eb3
Fix tests to not include multi-line configs
andrewor14 371cac4
Add function prefix (minor)
andrewor14 fa11ef8
Parse the properties file only if the special configs exist
andrewor14 7396be2
Explicitly comment that multi-line properties are not supported
andrewor14 7a4190a
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 c886568
Fix lines too long + a few comments / style (minor)
andrewor14 0effa1e
Add code in Scala that handles special configs
andrewor14 a396eda
Nullify my own hard work to simplify bash
andrewor14 c37e08d
Revert a few more changes
andrewor14 3a8235d
Only parse the properties file if special configs exist
andrewor14 7d94a8d
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 b71f52b
Revert a few more changes (minor)
andrewor14 c84f5c8
Remove debug print statement (minor)
andrewor14 158f813
Remove "client mode" boolean argument
andrewor14 a91ea19
Fix precedence of library paths, classpath, java opts and memory
andrewor14 1ea6bbe
SparkClassLauncher -> SparkSubmitDriverBootstrapper
andrewor14 d6488f9
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 19464ad
SPARK_SUBMIT_JAVA_OPTS -> SPARK_SUBMIT_OPTS
andrewor14 8867a09
A few more naming things (minor)
andrewor14 9ba37e2
Don't barf when the properties file does not exist
andrewor14 a78cb26
Revert a few changes in utils.sh (minor)
andrewor14 d0f20db
Don't pass empty library paths, classpath, java opts etc.
andrewor14 9a778f6
Fix PySpark: actually kill driver on termination
andrewor14 51aeb01
Filter out JVM memory in Scala rather than Bash (minor)
andrewor14 ff34728
Minor comments
andrewor14 08fd788
Warn against external usages of SparkSubmitDriverBootstrapper
andrewor14 24dba60
Merge branch 'master' of github.com:apache/spark into handle-configs-…
andrewor14 bed4bdf
Change a few comments / messages (minor)
andrewor14 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Remove "client mode" boolean argument
... and then make SparkClassLauncher private[spark].
- Loading branch information
commit 158f813e176b6d5b05bac35bfe7e76095ab693b2
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,18 +25,18 @@ import org.apache.spark.util.{RedirectThread, Utils} | |
|
|
||
| /** | ||
| * Wrapper of `bin/spark-class` that prepares the launch environment of the child JVM properly. | ||
| * This is currently only used for running Spark submit in client mode. The goal moving forward | ||
| * is to use this class for all use cases of `bin/spark-class`. | ||
| */ | ||
| object SparkClassLauncher { | ||
| private[spark] object SparkClassLauncher { | ||
|
|
||
| // TODO: This is currently only used for running Spark submit in client mode. | ||
| // The goal moving forward is to use this class for all use cases of `bin/spark-class`. | ||
|
|
||
| /** | ||
| * Launch a Spark class with the given class paths, library paths, java options and memory. | ||
| * If we are launching an application through Spark submit in client mode, we must also | ||
| * take into account special `spark.driver.*` properties needed to start the driver JVM. | ||
| * Launch a Spark class with the given class paths, library paths, java options and memory, | ||
| * taking into account special `spark.driver.*` properties needed to start the driver JVM. | ||
| */ | ||
| def main(args: Array[String]): Unit = { | ||
| if (args.size < 8) { | ||
| if (args.size < 7) { | ||
| System.err.println( | ||
| """ | ||
| |Usage: org.apache.spark.deploy.SparkClassLauncher | ||
|
|
@@ -47,14 +47,13 @@ object SparkClassLauncher { | |
| | [java library paths] - library paths to pass to the child JVM | ||
| | [java opts] - java options to pass to the child JVM | ||
| | [java memory] - memory used to launch the child JVM | ||
| | [client mode] - whether the child JVM will run the Spark driver | ||
| | [main class] - main class to run in the child JVM | ||
| | <main args> - arguments passed to this main class | ||
| | | ||
| |Example: | ||
| | org.apache.spark.deploy.SparkClassLauncher.SparkClassLauncher | ||
| | conf/spark-defaults.conf java /classpath1:/classpath2 /librarypath1:/librarypath2 | ||
| | "-XX:-UseParallelGC -Dsome=property" 5g true org.apache.spark.deploy.SparkSubmit | ||
| | "-XX:-UseParallelGC -Dsome=property" 5g org.apache.spark.deploy.SparkSubmit | ||
| | --master local --class org.apache.spark.examples.SparkPi 10 | ||
| """.stripMargin) | ||
| System.exit(1) | ||
|
|
@@ -65,29 +64,18 @@ object SparkClassLauncher { | |
| val clLibraryPaths = args(3) | ||
| val clJavaOpts = Utils.splitCommandString(args(4)) | ||
| val clJavaMemory = args(5) | ||
| val clientMode = args(6) == "true" | ||
| val mainClass = args(7) | ||
| val mainClass = args(6) | ||
|
|
||
| // In client deploy mode, parse the properties file for certain `spark.driver.*` configs. | ||
| // These configs encode java options, class paths, and library paths needed to launch the JVM. | ||
| val properties = | ||
| if (clientMode) { | ||
| SparkSubmitArguments.getPropertiesFromFile(new File(propertiesFile)).toMap | ||
| } else { | ||
| Map[String, String]() | ||
| } | ||
| val properties = SparkSubmitArguments.getPropertiesFromFile(new File(propertiesFile)).toMap | ||
| val confDriverMemory = properties.get("spark.driver.memory") | ||
| val confClassPaths = properties.get("spark.driver.extraClassPath") | ||
| val confLibraryPaths = properties.get("spark.driver.extraLibraryPath") | ||
| val confJavaOpts = properties.get("spark.driver.extraJavaOptions") | ||
|
|
||
| // Merge relevant command line values with the config equivalents, if any | ||
| val javaMemory = | ||
| if (clientMode) { | ||
| confDriverMemory.getOrElse(clJavaMemory) | ||
| } else { | ||
| clJavaMemory | ||
| } | ||
| val javaMemory = confDriverMemory.getOrElse(clJavaMemory) | ||
| val pathSeparator = sys.props("path.separator") | ||
| val classPaths = clClassPaths + confClassPaths.map(pathSeparator + _).getOrElse("") | ||
| val libraryPaths = clLibraryPaths + confLibraryPaths.map(pathSeparator + _).getOrElse("") | ||
|
||
|
|
@@ -104,7 +92,7 @@ object SparkClassLauncher { | |
| filteredJavaOpts ++ | ||
| Seq(s"-Xms$javaMemory", s"-Xmx$javaMemory") ++ | ||
| Seq(mainClass) ++ | ||
| args.slice(8, args.size) | ||
| args.slice(7, args.size) | ||
| val builder = new ProcessBuilder(command) | ||
| val process = builder.start() | ||
| new RedirectThread(System.in, process.getOutputStream, "redirect stdin").start() | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
To keep it simpler for now, rather than having a bunch of command line arguments, why not just directly read the environment variables set in
spark-class? Then you could just have this script take the main class and the arguments and you could just directly readRUNNER,PROPERTEIS_FILE,CLASSPATH,SPARK_SUBMIT_LIBRARY_PATH,JAVA_OPTS, andOUR_JAVA_MEM. This is one fewer levels of interpretation/parsing to worry about for now and would overall make this patch smaller.