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

[SPARK-21798]: No config to replace deprecated SPARK_CLASSPATH config for launching daemons like History Server #19047

Closed
wants to merge 12 commits into from

Conversation

pgandhi999
Copy link

@pgandhi999 pgandhi999 commented Aug 24, 2017

History Server Launch uses SparkClassCommandBuilder for launching the server. It is observed that SPARK_CLASSPATH has been removed and deprecated. For spark-submit this takes a different route and spark.driver.extraClasspath takes care of specifying additional jars in the classpath that were previously specified in the SPARK_CLASSPATH. Right now the only way specify the additional jars for launching daemons such as history server is using SPARK_DIST_CLASSPATH (https://spark.apache.org/docs/latest/hadoop-provided.html) but this I presume is a distribution classpath. It would be nice to have a similar config like spark.driver.extraClasspath for launching daemons similar to history server.

Added new environment variable SPARK_DAEMON_CLASSPATH to set classpath for launching daemons. Tested and verified for History Server and Standalone Mode.

How was this patch tested?

Initially, history server start script would fail for the reason being that it could not find the required jars for launching the server in the java classpath. Same was true for running Master and Worker in standalone mode. By adding the environment variable SPARK_DAEMON_CLASSPATH to the java classpath, both the daemons(History Server, Standalone daemons) are starting up and running.

pgandhi and others added 11 commits July 21, 2017 16:00
Added the case ExecutorLostFailure which was previously not there, thus, the default case would be executed in which case, task would be marked as completed.
Apache Spark Pull Request - July 26, 2017
…es not create SparkContext

Added a flag to check whether user has initialized Spark Context. If it is true, then we let Application Master unregister with Resource Manager else we do not.
… that does not create SparkContext"

This reverts commit f454c89.

"Merged another issue to this one by mistake"
[SPARK-21503]- Making Changes as per comments: Removed match case statement and replaced it with an if clause.
Spark - August 1, 2017
[SPARK-21503]: Reverting Unit Test Code - Not needed.
SPARK - August 24, 2017
… for launching daemons like History Server

Adding new env variable SPARK_DAEMON_CLASSPATH to set classpath for launching daemons. Tested and verified for History Server and Standalone Mode.
@@ -136,7 +136,8 @@ void addOptionString(List<String> cmd, String options) {

Set<String> cp = new LinkedHashSet<>();
addToClassPath(cp, appClassPath);

addToClassPath(cp, getenv("SPARK_DAEMON_CLASSPATH"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to only support this env variable for daemon process like HistoryServer, ExternalShuffleService and others, like what we did for SPARK_DAEMON_JAVA_OPTS. Currently with your fix normal Spark application will also honor this env variable.

Copy link
Author

Choose a reason for hiding this comment

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

I agree with you on this. I have made the necessary changes.

… for launching daemons like History Server

Reverted the previous code change and added the environment variable SPARK_DAEMON_CLASSPATH only for launching daemon processes.
@pgandhi999 pgandhi999 changed the title [SPARK-21798]: No config to replace deprecated SPARK_CLASSPATH config for launching daemons like History Server [SPARK-21798][Launcher]: No config to replace deprecated SPARK_CLASSPATH config for launching daemons like History Server Aug 25, 2017
@pgandhi999 pgandhi999 changed the title [SPARK-21798][Launcher]: No config to replace deprecated SPARK_CLASSPATH config for launching daemons like History Server [SPARK-21798]: No config to replace deprecated SPARK_CLASSPATH config for launching daemons like History Server Aug 25, 2017
@tgravescs
Copy link
Contributor

ok to test

@SparkQA
Copy link

SparkQA commented Aug 25, 2017

Test build #81133 has finished for PR 19047 at commit e421a03.

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

@tgravescs
Copy link
Contributor

test this please

@vanzin
Copy link
Contributor

vanzin commented Aug 25, 2017

LGTM. I'd like to see these daemons start using normal Spark configs like the applications do, but that's a separate, larger change...

@SparkQA
Copy link

SparkQA commented Aug 25, 2017

Test build #81141 has finished for PR 19047 at commit e421a03.

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

@tgravescs
Copy link
Contributor

+1

asfgit pushed a commit that referenced this pull request Aug 28, 2017
…for launching daemons like History Server

History Server Launch uses SparkClassCommandBuilder for launching the server. It is observed that SPARK_CLASSPATH has been removed and deprecated. For spark-submit this takes a different route and spark.driver.extraClasspath takes care of specifying additional jars in the classpath that were previously specified in the SPARK_CLASSPATH. Right now the only way specify the additional jars for launching daemons such as history server is using SPARK_DIST_CLASSPATH (https://spark.apache.org/docs/latest/hadoop-provided.html) but this I presume is a distribution classpath. It would be nice to have a similar config like spark.driver.extraClasspath for launching daemons similar to history server.

Added new environment variable SPARK_DAEMON_CLASSPATH to set classpath for launching daemons. Tested and verified for History Server and Standalone Mode.

## How was this patch tested?
Initially, history server start script would fail for the reason being that it could not find the required jars for launching the server in the java classpath. Same was true for running Master and Worker in standalone mode. By adding the environment variable SPARK_DAEMON_CLASSPATH to the java classpath, both the daemons(History Server, Standalone daemons) are starting up and running.

Author: pgandhi <pgandhi@yahoo-inc.com>
Author: pgandhi999 <parthkgandhi9@gmail.com>

Closes #19047 from pgandhi999/master.

(cherry picked from commit 24e6c18)
Signed-off-by: Tom Graves <tgraves@yahoo-inc.com>
@asfgit asfgit closed this in 24e6c18 Aug 28, 2017
MatthewRBruce pushed a commit to Shopify/spark that referenced this pull request Jul 31, 2018
…for launching daemons like History Server

History Server Launch uses SparkClassCommandBuilder for launching the server. It is observed that SPARK_CLASSPATH has been removed and deprecated. For spark-submit this takes a different route and spark.driver.extraClasspath takes care of specifying additional jars in the classpath that were previously specified in the SPARK_CLASSPATH. Right now the only way specify the additional jars for launching daemons such as history server is using SPARK_DIST_CLASSPATH (https://spark.apache.org/docs/latest/hadoop-provided.html) but this I presume is a distribution classpath. It would be nice to have a similar config like spark.driver.extraClasspath for launching daemons similar to history server.

Added new environment variable SPARK_DAEMON_CLASSPATH to set classpath for launching daemons. Tested and verified for History Server and Standalone Mode.

## How was this patch tested?
Initially, history server start script would fail for the reason being that it could not find the required jars for launching the server in the java classpath. Same was true for running Master and Worker in standalone mode. By adding the environment variable SPARK_DAEMON_CLASSPATH to the java classpath, both the daemons(History Server, Standalone daemons) are starting up and running.

Author: pgandhi <pgandhi@yahoo-inc.com>
Author: pgandhi999 <parthkgandhi9@gmail.com>

Closes apache#19047 from pgandhi999/master.

(cherry picked from commit 24e6c18)
Signed-off-by: Tom Graves <tgraves@yahoo-inc.com>
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.

5 participants