Skip to content

[SPARK-4697][YARN]System properties should override environment variables #3557

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

Closed
wants to merge 7 commits into from
Closed

[SPARK-4697][YARN]System properties should override environment variables #3557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5f43f45
System property can override environment variable
WangTaoTheTonic Dec 2, 2014
40934b4
just switch blocks
WangTaoTheTonic Dec 3, 2014
81833bb
rebase
WangTaoTheTonic Jan 8, 2015
bee9447
wrong brace
WangTaoTheTonic Jan 8, 2015
1262d57
handle spark.app.name and SPARK_YARN_APP_NAME in SparkSubmitArguments
WangTaoTheTonic Jan 12, 2015
e3e486a
remove the comma
WangTaoTheTonic Jan 12, 2015
836b9ef
fix type mismatch
WangTaoTheTonic Jan 12, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ private[spark] class SparkSubmitArguments(args: Seq[String], env: Map[String, St
// Global defaults. These should be keep to minimum to avoid confusing behavior.
master = Option(master).getOrElse("local[*]")

// In yarn mode, app name can be set via SPARK_YARN_APP_NAME
if (master.contains("yarn")) {
Copy link
Contributor

@andrewor14 andrewor14 Jan 13, 2015

Choose a reason for hiding this comment

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

should be startsWith, also in the comments we generally capitalize YARN

Copy link
Contributor

@sryza sryza Jan 13, 2015

Choose a reason for hiding this comment

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

Just to be clear, you mean capitalize YARN in the comment, right?

Copy link
Contributor

@andrewor14 andrewor14 Jan 13, 2015

Choose a reason for hiding this comment

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

yeah

Copy link
Contributor

@sryza sryza Jan 13, 2015

Choose a reason for hiding this comment

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

Woops didn't see your wrote comments, sorry for the noise

name = Option(name).orElse(env.get("SPARK_YARN_APP_NAME")).orNull
}
// Set name from main class if not given
name = Option(name).orElse(Option(mainClass)).orNull
if (name == null && primaryResource != null) {
Expand Down
4 changes: 2 additions & 2 deletions yarn/src/main/scala/org/apache/spark/deploy/yarn/ClientArguments.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ private[spark] class ClientArguments(args: Array[String], sparkConf: SparkConf)
// For backward compatibility, SPARK_YARN_DIST_{ARCHIVES/FILES} should be resolved to hdfs://,
// while spark.yarn.dist.{archives/files} should be resolved to file:// (SPARK-2051).
files = Option(files)
.orElse(sys.env.get("SPARK_YARN_DIST_FILES"))
.orElse(sparkConf.getOption("spark.yarn.dist.files").map(p => Utils.resolveURIs(p)))
.orElse(sys.env.get("SPARK_YARN_DIST_FILES"))
.orNull
archives = Option(archives)
.orElse(sys.env.get("SPARK_YARN_DIST_ARCHIVES"))
.orElse(sparkConf.getOption("spark.yarn.dist.archives").map(p => Utils.resolveURIs(p)))
.orElse(sys.env.get("SPARK_YARN_DIST_ARCHIVES"))
.orNull
// If dynamic allocation is enabled, start at the max number of executors
if (isDynamicAllocationEnabled) {
Expand Down
16 changes: 8 additions & 8 deletions yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnClientSchedulerBackend.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ private[spark] class YarnClientSchedulerBackend(
("--executor-memory", "SPARK_EXECUTOR_MEMORY", "spark.executor.memory"),
("--executor-cores", "SPARK_WORKER_CORES", "spark.executor.cores"),
("--executor-cores", "SPARK_EXECUTOR_CORES", "spark.executor.cores"),
("--queue", "SPARK_YARN_QUEUE", "spark.yarn.queue"),
("--name", "SPARK_YARN_APP_NAME", "spark.app.name")
("--queue", "SPARK_YARN_QUEUE", "spark.yarn.queue")
)
// Warn against the following deprecated environment variables: env var -> suggestion
val deprecatedEnvVars = Map(
Expand All @@ -88,18 +87,19 @@ private[spark] class YarnClientSchedulerBackend(
// Do the same for deprecated properties: property -> suggestion
val deprecatedProps = Map("spark.master.memory" -> "--driver-memory through spark-submit")
optionTuples.foreach { case (optionName, envVar, sparkProp) =>
if (System.getenv(envVar) != null) {
extraArgs += (optionName, System.getenv(envVar))
if (deprecatedEnvVars.contains(envVar)) {
logWarning(s"NOTE: $envVar is deprecated. Use ${deprecatedEnvVars(envVar)} instead.")
}
} else if (sc.getConf.contains(sparkProp)) {
if (sc.getConf.contains(sparkProp)) {
extraArgs += (optionName, sc.getConf.get(sparkProp))
if (deprecatedProps.contains(sparkProp)) {
logWarning(s"NOTE: $sparkProp is deprecated. Use ${deprecatedProps(sparkProp)} instead.")
}
} else if (System.getenv(envVar) != null) {
extraArgs += (optionName, System.getenv(envVar))
if (deprecatedEnvVars.contains(envVar)) {
logWarning(s"NOTE: $envVar is deprecated. Use ${deprecatedEnvVars(envVar)} instead.")
}
}
}
sc.getConf.getOption("spark.app.name").foreach(v => extraArgs += ("--name", v))
Copy link
Contributor

@andrewor14 andrewor14 Jan 13, 2015

Choose a reason for hiding this comment

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

We need to add a comment here to explain why it's a special case

extraArgs
}

Expand Down