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.

Sign up for GitHub

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

Jump to bottom

[SPARK-3560] Fixed setting spark.jars system property in yarn-cluster mode #2449

Closed
wants to merge 2 commits into from
Closed

[SPARK-3560] Fixed setting spark.jars system property in yarn-cluster mode #2449

wants to merge 2 commits into from

Conversation

Victsm
Copy link
Contributor

@Victsm Victsm commented Sep 18, 2014

No description provided.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@vanzin
Copy link
Contributor

vanzin commented Sep 18, 2014

Hi, mostly so I understand what's going on (I've traced the code but it's kinda hard to know if I covered everything), how are jars distributed in yarn cluster mode after your change?

It seems to me that your change just makes the --jars command line option be ignored for yarn cluster mode. If that's the case, I'm not sure I like it so much...

If that's not the case, could you clarify in the PR description how --jars works in yarn cluster mode? That would help others who might want to understand why this change works in spite of looking counter-intuitive.

@andrewor14
Copy link
Contributor

ok to test

@andrewor14
Copy link
Contributor

@vanzin --jars for yarn-cluster mode becomes --addJars in Client. Previously it was dealt with twice and there was interference in the two ways we distribute the jars, and this fixes it by making sure we don't also set spark.jars for yarn-cluster in addition to turning it into --addJars.

@Victsm
Copy link
Contributor Author

Victsm commented Sep 18, 2014

@vanzin The --jars will still work in YARN cluster mode through the distributed cache. Removing the spark.jars system property only prevents the HttpFileServer mechanism to also take effect. Having both in YARN cluster mode will cause problems because of how YARN set the permissions of the files in distributed cache.

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have started for PR 2449 at commit 4502a2a.

  • This patch merges cleanly.

andrewor14
andrewor14 reviewed Sep 18, 2014
View reviewed changes
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala
@@ -205,6 +205,7 @@ object SparkSubmit {
OptionAssigner(args.jars, YARN, CLUSTER, clOption = "--addJars"),

// Other options
OptionAssigner(args.jars, STANDALONE, CLUSTER, sysProp = "spark.jars"),
Copy link
Contributor

Choose a reason for hiding this comment

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

1 too many space here

@andrewor14
Copy link
Contributor

LGTM

@vanzin
Copy link
Contributor

vanzin commented Sep 18, 2014

Ah, I see. Missed this in SparkSubmit.scala:

OptionAssigner(args.jars, YARN, CLUSTER, clOption = "--addJars"),

LGTM.

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have started for PR 2449 at commit 918405a.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have finished for PR 2449 at commit 4502a2a.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@andrewor14
Copy link
Contributor

Thanks I have merged this into master and 1.1.

@Victsm Do you have a JIRA account? I want to assign you to https://issues.apache.org/jira/browse/SPARK-3560 but I can't find your username.

@asfgit asfgit closed this in b3ed37e Sep 18, 2014
asfgit pushed a commit that referenced this pull request Sep 18, 2014
@Victsm @andrewor14
[SPARK-3560] Fixed setting spark.jars system property in yarn-cluster…
832dff6 
… mode

Author: Victsm <[email protected]>
Author: Min Shen <[email protected]>

Closes #2449 from Victsm/SPARK-3560 and squashes the following commits:

918405a [Victsm] Removed the additional space
4502a2a [Min Shen] [SPARK-3560] Fixed setting spark.jars system property in yarn-cluster mode.
@Victsm
Copy link
Contributor Author

Victsm commented Sep 18, 2014

@andrewor14 My JIRA account is mshen. That issues has just been closed.

@andrewor14
Copy link
Contributor

Yup, I just assigned it to you. Thanks.

sryza
sryza reviewed Sep 18, 2014
View reviewed changes
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala
@@ -205,6 +205,7 @@ object SparkSubmit {
OptionAssigner(args.jars, YARN, CLUSTER, clOption = "--addJars"),

// Other options
OptionAssigner(args.jars, STANDALONE, CLUSTER, sysProp = "spark.jars"),
Copy link
Contributor

Choose a reason for hiding this comment

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

This should go under the "standalone" section.

@sryza
Copy link
Contributor

sryza commented Sep 18, 2014

Saw this already went in, but had one stylistic/organization nit that might be worth fixing

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have finished for PR 2449 at commit 918405a.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@andrewor14
Copy link
Contributor

Oh yeah, good catch. I have opened #2452 to fix this.

@andrewor14 andrewor14 mentioned this pull request Sep 18, 2014
Closed
asfgit pushed a commit that referenced this pull request Sep 19, 2014
@andrewor14
[Minor Hot Fix] Move a line in SparkSubmit to the right place
cf15b22 
This was introduced in #2449

Author: Andrew Or <[email protected]>

Closes #2452 from andrewor14/standalone-hot-fix and squashes the following commits:

d5190ca [Andrew Or] Put that line in the right place

(cherry picked from commit 9306297)
Signed-off-by: Andrew Or <[email protected]>
asfgit pushed a commit that referenced this pull request Sep 19, 2014
@andrewor14
[Minor Hot Fix] Move a line in SparkSubmit to the right place
9306297 
This was introduced in #2449

Author: Andrew Or <[email protected]>

Closes #2452 from andrewor14/standalone-hot-fix and squashes the following commits:

d5190ca [Andrew Or] Put that line in the right place
markhamstra pushed a commit to markhamstra/spark that referenced this pull request Sep 19, 2014
@Victsm @markhamstra
[SPARK-3560] Fixed setting spark.jars system property in yarn-cluster…
4bb1030 
… mode

Author: Victsm <[email protected]>
Author: Min Shen <[email protected]>

Closes apache#2449 from Victsm/SPARK-3560 and squashes the following commits:

918405a [Victsm] Removed the additional space
4502a2a [Min Shen] [SPARK-3560] Fixed setting spark.jars system property in yarn-cluster mode.
markhamstra pushed a commit to markhamstra/spark that referenced this pull request Sep 19, 2014
@andrewor14 @markhamstra
[Minor Hot Fix] Move a line in SparkSubmit to the right place
019fb10 
This was introduced in apache#2449

Author: Andrew Or <[email protected]>

Closes apache#2452 from andrewor14/standalone-hot-fix and squashes the following commits:

d5190ca [Andrew Or] Put that line in the right place

(cherry picked from commit 9306297)
Signed-off-by: Andrew Or <[email protected]>
jdauphant added a commit to jdauphant/spark that referenced this pull request Oct 2, 2014
@jdauphant
Apply patch apache#2449
576191d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Victsm @AmplabJenkins @vanzin @andrewor14 @SparkQA @sryza