Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/303#discussion_r11539966
--- Diff:
yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -424,29 +408,110 @@ object ClientBase {
}
}
- def populateClasspath(conf: Configuration, sparkConf: SparkConf,
addLog4j: Boolean, env: HashMap[String, String]) {
+ /**
+ * Returns the java command line argument for setting up log4j. If there
is a log4j.properties
+ * in the given local resources, it is used, otherwise the
SPARK_LOG4J_CONF environment variable
+ * is checked.
+ */
+ def getLog4jConfiguration(localResources: HashMap[String,
LocalResource]): String = {
+ var log4jConf = LOG4J_PROP
+ if (!localResources.contains(log4jConf)) {
+ log4jConf = System.getenv(LOG4J_CONF_ENV_KEY) match {
+ case conf: String =>
+ val confUri = new URI(conf)
+ if (ClientBase.LOCAL_SCHEME.equals(confUri.getScheme())) {
+ "file://" + confUri.getPath()
+ } else {
+ ClientBase.LOG4J_PROP
+ }
+ case null => "log4j-spark-container.properties"
+ }
+ }
+ " -Dlog4j.configuration=" + log4jConf
+ }
+
+ def populateClasspath(args: ClientArguments, conf: Configuration,
sparkConf: SparkConf,
+ log4jConf: String, env: HashMap[String, String]) {
Apps.addToEnvironment(env, Environment.CLASSPATH.name,
Environment.PWD.$())
- // If log4j present, ensure ours overrides all others
- if (addLog4j) {
- Apps.addToEnvironment(env, Environment.CLASSPATH.name,
Environment.PWD.$() +
- Path.SEPARATOR + LOG4J_PROP)
+ if (log4jConf != null) {
+ // If a custom log4j config file is provided as a local: URI, add
its parent directory to the
+ // classpath. Note that this only works if the custom config's file
name is
+ // "log4j.properties".
+ val localPath = getLocalPath(log4jConf)
+ if (localPath != null) {
+ val parentPath = new File(localPath).getParent()
+ Apps.addToEnvironment(env, Environment.CLASSPATH.name, parentPath)
+ }
}
// Normally the users app.jar is last in case conflicts with spark jars
val userClasspathFirst =
sparkConf.get("spark.yarn.user.classpath.first", "false")
.toBoolean
if (userClasspathFirst) {
- Apps.addToEnvironment(env, Environment.CLASSPATH.name,
Environment.PWD.$() +
- Path.SEPARATOR + APP_JAR)
+ addUserClasspath(args, env)
}
- Apps.addToEnvironment(env, Environment.CLASSPATH.name,
Environment.PWD.$() +
- Path.SEPARATOR + SPARK_JAR)
+ addClasspathEntry(System.getenv("SPARK_JAR"), SPARK_JAR, env);
ClientBase.populateHadoopClasspath(conf, env)
-
if (!userClasspathFirst) {
- Apps.addToEnvironment(env, Environment.CLASSPATH.name,
Environment.PWD.$() +
- Path.SEPARATOR + APP_JAR)
+ addUserClasspath(args, env)
}
- Apps.addToEnvironment(env, Environment.CLASSPATH.name,
Environment.PWD.$() +
- Path.SEPARATOR + "*")
+ Apps.addToEnvironment(env, Environment.CLASSPATH.name,
+ Environment.PWD.$() + Path.SEPARATOR + "*")
}
+
+ /**
+ * Adds the user jars which have local: URIs (or alternate names, such
as APP_JAR) explicitly
+ * to the classpath.
+ */
+ private def addUserClasspath(args: ClientArguments, env: Map[String,
String]) = {
+ if (args != null) {
+ addClasspathEntry(args.userJar, APP_JAR, env)
+ }
+
+ if (args != null && args.addJars != null) {
+ args.addJars.split(",").foreach { case file: String =>
+ addClasspathEntry(file, null, env)
+ }
+ }
+ }
+
+ /**
+ * Adds the given path to the classpath, handling "local:" URIs
correctly.
+ * <p/>
--- End diff --
That's javadoc (which is supposed to be well-formatted HTML). I saw
somewhere that comments should use javadoc-style instead of scaladoc style, but
that might have been just for the first line. I'll take a look at what scaladoc
expects.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
