Skip to content
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

Fix SPARK-1609: Executor fails to start when Command.extraJavaOptions contains multiple Java options #547

Closed
wants to merge 9 commits into from

Conversation

witgo
Copy link
Contributor

@witgo witgo commented Apr 25, 2014

No description provided.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

val extraOpts = command.extraJavaOptions.toSeq
val extraOpts = command.extraJavaOptions match {
case Some(opts) =>
opts.split("\\s+").toSeq
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Utils.splitCommandString instead of this, in case people pass in stuff with quotes.

@@ -48,7 +48,13 @@ object CommandUtils extends Logging {
def buildJavaOpts(command: Command, memory: Int, sparkHome: String): Seq[String] = {
val memoryOpts = Seq(s"-Xms${memory}M", s"-Xmx${memory}M")
// Note, this will coalesce multiple options into a single command component
val extraOpts = command.extraJavaOptions.toSeq
val extraOpts = command.extraJavaOptions match {
Copy link
Contributor

Choose a reason for hiding this comment

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

Up to you, but could be written a bit more consicely

val extraOpts = command.extraJavaOptions.map(Utils.splitCommandString).getOrElse(Seq())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, val extraOpts = command.extraJavaOptions.map(Utils.splitCommandString).getOrElse(Seq()) is better

@pwendell
Copy link
Contributor

Jenkins, test this please.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14521/

@pwendell
Copy link
Contributor

Thanks for the fix! I've merged this

asfgit pushed a commit that referenced this pull request Apr 28, 2014
… contains multiple Java options

Author: witgo <witgo@qq.com>

Closes #547 from witgo/SPARK-1609 and squashes the following commits:

deb6a4c [witgo] review commit
91da0bb [witgo] Merge branch 'master' of https://github.com/apache/spark into SPARK-1609
0640852 [witgo] review commit
8f90b22 [witgo] Merge branch 'master' of https://github.com/apache/spark into SPARK-1609
bcf36cb [witgo] Merge branch 'master' of https://github.com/apache/spark into SPARK-1609
1185605 [witgo] fix extraJavaOptions split
f7c0ab7 [witgo] bugfix
86fc4bb [witgo] bugfix
8a265b7 [witgo] Fix SPARK-1609: Executor fails to start when use spark-submit
@asfgit asfgit closed this in 71f4d26 Apr 28, 2014
@witgo witgo deleted the SPARK-1609 branch April 28, 2014 03:45
pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
… contains multiple Java options

Author: witgo <witgo@qq.com>

Closes apache#547 from witgo/SPARK-1609 and squashes the following commits:

deb6a4c [witgo] review commit
91da0bb [witgo] Merge branch 'master' of https://github.com/apache/spark into SPARK-1609
0640852 [witgo] review commit
8f90b22 [witgo] Merge branch 'master' of https://github.com/apache/spark into SPARK-1609
bcf36cb [witgo] Merge branch 'master' of https://github.com/apache/spark into SPARK-1609
1185605 [witgo] fix extraJavaOptions split
f7c0ab7 [witgo] bugfix
86fc4bb [witgo] bugfix
8a265b7 [witgo] Fix SPARK-1609: Executor fails to start when use spark-submit
helenyugithub pushed a commit to helenyugithub/spark that referenced this pull request Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants