Skip to content

[SPARK-20814][mesos] Restore support for spark.executor.extraClassPath. #18037

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
Closed
wants to merge 2 commits into from
Closed

[SPARK-20814][mesos] Restore support for spark.executor.extraClassPath. #18037

wants to merge 2 commits into from

Conversation

@vanzin
Copy link
Contributor

@vanzin vanzin commented May 19, 2017

Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.

[SPARK-20814][mesos] Restore support for spark.executor.extraClassPath.
d129327 
Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.
@vanzin
Copy link
Contributor Author

vanzin commented May 19, 2017

@mgummelt

@SparkQA
Copy link

SparkQA commented May 19, 2017

Test build #77106 has finished for PR 18037 at commit d129327.

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

@SparkQA
Copy link

SparkQA commented May 20, 2017

Test build #77129 has finished for PR 18037 at commit a861819.

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

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

Ping? Let me invoke @srowen or @squito since we should have this in 2.2.0.

@mgummelt
Copy link

mgummelt commented May 22, 2017

looking...

@mgummelt
Copy link

mgummelt commented May 22, 2017

Was spark.executor.extraClassPath ever supported in Mesos? I see no code for it.

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

Yes, it just set SPARK_CLASSPATH and relied on the launcher code to respect that env variable, but that code was recently removed.

srowen
srowen approved these changes May 22, 2017
View reviewed changes
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

I trust your judgment on this one

@mgummelt
Copy link

mgummelt commented May 22, 2017

It looks like Standalone gets around this by constructing a raw java command instead of a spark-class command. Will YARN be using this new SPARK_EXECUTOR_CLASSPATH variable as well?

I don't really see why we would have deprecated SPARK_CLASSPATH only to introduce SPARK_EXECUTOR_CLASSPATH, but I'm fine with this change.

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

Neither standalone nor YARN use env variables to create the executor's classpath, so I guess that's why Mesos was missed. I'd rather Mesos stop using env variables too, but that's a bigger change, and I want to get this in 2.2.0 to avoid a pretty user-visible regression.

@mgummelt
Copy link

mgummelt commented May 22, 2017 •
edited
Loading

It looks like YARN just sets the generic java CLASSPATH env var. Maybe Mesos should do that too, in the future.

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

YARN does not run spark-class, Mesos does. And spark-class does not honor CLASSPATH, IIRC.

@vanzin
Copy link
Contributor Author

vanzin commented May 22, 2017

I'm gonna merge this to unblock 2.2; if there's a desire to clean up this code later, we can do it separately.

asfgit pushed a commit that referenced this pull request May 22, 2017
[SPARK-20814][MESOS] Restore support for spark.executor.extraClassPath.
d8328d8 
Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.

Author: Marcelo Vanzin <[email protected]>

Closes #18037 from vanzin/SPARK-20814.

(cherry picked from commit df64fa7)
Signed-off-by: Marcelo Vanzin <[email protected]>
@vanzin vanzin closed this May 22, 2017
@vanzin vanzin deleted the SPARK-20814 branch May 22, 2017 19:36
asfgit pushed a commit that referenced this pull request May 22, 2017
[SPARK-20814][MESOS] Restore support for spark.executor.extraClassPath.
df64fa7 
Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.

Author: Marcelo Vanzin <[email protected]>

Closes #18037 from vanzin/SPARK-20814.
liyichao pushed a commit to liyichao/spark that referenced this pull request May 24, 2017
@liyichao
[SPARK-20814][MESOS] Restore support for spark.executor.extraClassPath.
7b5a638 
Restore code that was removed as part of SPARK-17979, but instead of
using the deprecated env variable name to propagate the class path, use
a new one.

Verified by running "./bin/spark-class o.a.s.executor.CoarseGrainedExecutorBackend"
manually.

Author: Marcelo Vanzin <[email protected]>

Closes apache#18037 from vanzin/SPARK-20814.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srowen srowen srowen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vanzin @SparkQA @mgummelt @srowen