Skip to content
Closed
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,21 @@ trait ClientBase extends Logging {
}
}

def getArg(arg: String, envVar: String, sysProp: String): String = {
if (arg != null && !arg.isEmpty) {
arg
} else if (System.getenv(envVar) != null && !System.getenv(envVar).isEmpty) {
System.getenv(envVar)
} else {
sparkConf.getOption(sysProp).orNull
}
}
var cachedSecondaryJarLinks = ListBuffer.empty[String]
val fileLists = List( (args.addJars, LocalResourceType.FILE, true),
(args.files, LocalResourceType.FILE, false),
(args.archives, LocalResourceType.ARCHIVE, false) )
val fileLists = List((args.addJars, LocalResourceType.FILE, true),
(getArg(args.files, "SPARK_YARN_DIST_FILES", "spark.yarn.dist.files"),
LocalResourceType.FILE, false),
(getArg(args.archives, "SPARK_YARN_DIST_ARCHIVES", "spark.yarn.dist.archives"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think env variables and conf entries should be handled here like this.

YarnClientSchedulerBackend already deals with the env variable and command line option for client mode. It seems that SparkSubmit might be missing code to handle the env variable for cluster mode, though. Probably better to fix it there, and leave this code to deal only with the command line args (which are already correctly parsed).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vanzin This is a yarn cluster problems. Don't use YarnClientSchedulerBackend.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you see the other PR I mentioned (see #969 (comment))?

There's a bug in the Yarn code where configuration is not propagated correctly in cluster mode, which I fixed there. So you don't need this here because with my fix, these options should be correctly propagated without any extra code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, Weekend, I will test your branch, Please merge the master changes.

LocalResourceType.ARCHIVE, false))
fileLists.foreach { case (flist, resType, addToClasspath) =>
if (flist != null && !flist.isEmpty()) {
flist.split(',').foreach { case file: String =>
Expand Down