[SPARK-1395] Fix "local:" URI support in Yarn mode (again).

[vanzin]

Recent changes ignored the fact that path may be defined with "local:"
URIs, which means they need to be explicitly added to the classpath
everywhere a remote process is started. This change fixes that by:

• Using the correct methods to add paths to the classpath
• Creating SparkConf settings for the Spark jar itself and for the
  user's jar
• Propagating those two settings to the remote processes where needed

This ensures that both in client and in cluster mode, the driver has
the necessary info to build the executor's classpath and have things
still work when they contain "local:" references.

The change also fixes some confusion in ClientBase about whether
to use SparkConf or system properties to propagate config options to
the driver and executors, by standardizing on using data held by
SparkConf.

On the cleanup front, I removed the hacky way that log4j configuration
was being propagated to handle the "local:" case. It's much more cleanly
(and generically) handled by using spark-submit arguments (--files to
upload a config file, or setting spark.executor.extraJavaOptions to pass
JVM arguments and use a local file).

[AmplabJenkins]

Can one of the admins verify this patch?

[vanzin]

Just rebased on top of master. No changes.

[vanzin]

I rebased and cleaned up the code some more. I think it's in pretty good shape now, and the tests are much better.

Tested on yarn client / cluster, with and without extra jar dependencies.

[cmccabe]

I was just talking about the log message in yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/ExecutorRunnable.scala. The rest is shared, if I understand correctly.

[vanzin]

Ah, that guy. Sorry, my bad. :-) It made my life easier debugging things, but not sure how many people actively develop Spark on top of yarn-alpha. Wouldn't hurt to add it, though.

[tgravescs]

can you document the config

[tgravescs]

I haven't gotten all the way through this patch but please explain what you have done with the log4j stuff. at quick glance it looks like you just removed it all? Perhaps I'm missing something though.
You expect people to pass it with the --files option?

What about in the case where I don't want to specify one and want the default one (log4j-spark-container.properties) to be used? (added in https://issues.apache.org/jira/browse/SPARK-1252)

Also note we can't just remove the way it is done now as it breaks backwards compatibility.

[vanzin]

Hi Tom,

I just removed all the log4j-related code. You can achieve both previously available use cases with spark-submit arguments:

• To use a custom config file, just say "--files /path/to/log4j.properties", and that gets distributed to the worker nodes and is available at the root of the classpath, so it overrides the default log4j-spark-container.properties.
• To use a local config file, just say "spark.executor.extraJavaOpts=-Dlog4j.configuration=file:/path/to/log4j.properties"

Note that since local: is broken in 1.0.0, the second one above cannot be achieved by using SPARK_LOG4J_CONF in that release.

As for backwards compatibility, SPARK_LOG4J_CONF doesn't seem to be documented anywhere, and the tricks above work fine with 1.0.0. I guess I could add the content of SPARK_LOG4J_CONF to the list of copied files if you think that's important.

[tgravescs]

Unfortunately I guess our documentation has very little on logging, but that doesn't mean that isn't how it worked. That is the way it worked in 0.9 and still worked in 1.0 so I don't want to break that method so would like it added back. Hopefully its not to painful to just add to the list of files to copy as you mention.

I would also like to see the documentation updated to make it clear to users how they do it. If you would like to do it here that would be fine or we can file a separate jira also.

[vanzin]

@pwendell the last commit I pushed to the PR changes some code you added in fb98488. Can you take a look in case I misunderstood what you meant originally?

[witgo]

@vanzin Can you merge the latest code?

[vanzin]

@witgo: on it, will retest and update soon. I'll also take a closer look at your related change to make sure I got everything right.

[vanzin]

Hi, does anybody have any more feedback about this?

[tgravescs]

was this change necessary? If not I would prefer to split this off into a separate jira. If it was please explain why.

[tgravescs]

Note that this change breaks backwards compatibility with being able to set SPARK_JAVA_OPTS to include configs. I would rather not change it in this pr as it will take more investigation and testing to make sure it doesn't break anything.

[vanzin]

Can you elaborate? SparkConf will include all the system properties that start with "spark", which is exactly what the previous code was doing when reading system properties directly.

I'll re-test without this fix (my tests were failing without it but after reading the rest of the code I'm not so sure it was because of this), but in general I think this is a cleaner way of doing it (since it treats both cases as the same).

[tgravescs]

export SPARK_JAVA_OPTS="-Dspark...=foo" . This needs to work for backwards compatibility but it doesn't with this code change because SparkConf throws an error saying you aren't supposed to do this.

I agree it is cleaner the way you have it but its going to require more investigation and a bunch of testing to make sure it works properly and I would rather have it split off into another jira/pr. There are already alot of things in this pr.

[vanzin]

So here's what's happening.

prepareLocalResources() propagates some settings (previously CONF_SPARK_YARN_SECONDARY_JARS, and now the two new settings I'm adding) using SparkConf.set. In cluster mode, createContainerLaunchContext() propagates only things that are set as system properties, so those config options are not provided to the executors. So if your executor depends on something that is in those options, it will fail.

I'll double-check whether SPARK_JAVA_OPTS works as before and make adjustments if necessary, but the change is needed for things to work correctly.

[tgravescs]

@vanzin I'm not sure how your change fixes still being able to use SPARK_JAVA_OPTS to set configs.
SPARK_JAVA_OPTS=-Dspark.authentication=true. When I just tried I still get the error (with both spark-submit and spark-class)

Caused by: java.lang.Exception: spark.executor.extraJavaOptions is not allowed to set Spark options (was '-Dspark.authentication=true)'. Set them directly on a SparkConf or in a properties file when using ./bin/spark-submit.

[vanzin]

HI @tgravescs,

When I tested I used a non spark. property and it worked. But I just want to make sure we're on the same page as what do you think SPARK_JAVA_OPTS should do. Here's some code I have which is called both by the driver and executors:

  val silly1 = System.getProperty("silly.property")
  val silly2 = System.getProperty("spark.silly.property")
  logInfo(s"silly.property = $silly1, spark.silly.property = $silly2")
  if (silly1 == null && silly2 == null) {
    throw new IllegalArgumentException("silly.property nor spark.silly.property is set!")
  }

I then run this (against the master branch) in yarn-cluster mode:

SPARK_JAVA_OPTS='-Dspark.silly.property=bar -Dsilly.property=foo' ./bin/spark-submit ...

The driver launches but executors die with:

14/06/19 10:31:19 INFO SillyDepSleeper: silly.property = null, spark.silly.property = null
14/06/19 10:31:19 ERROR Executor: Exception in task ID 10
java.lang.IllegalArgumentException: silly.property nor spark.silly.property is set!
        at com.cloudera.ss.SillyDepSleeper$.checkProps(SillyDepSleeper.scala:32)

So I'm not sure SPARK_JAVA_OPTS is really working on master at all. Another thing I tried is to use just SPARK_JAVA_OPTS='-Dsilly.property=foo', and in that case the driver doesn't start either (meaning non-"spark." properties are not propagated).

I can look at fixing the problem you mention, but it looks like SPARK_JAVA_OPTS has some questionable behaviour even without my changes.

[tgravescs]

so the idea is that you can set spark configs via SPARK_JAVA_OPTS in order to be backwards compatible with 0.9. Before the SparkConf (conf/spark-defaults.conf) and spark-submit the only way to specify the spark configs was either through SPARK_JAVA_OPTS on command line or spark-env.sh. So that is what I've been looking at.

My examples which is pretty easy to see if works is set:
SPARK_JAVA_OPTS="-Dspark.authenticate=true -Dspark.ui.acls.enable=true"

Run one of the Spark examples (SparkPi):
./bin/spark-submit examples/target/scala-2.10/spark-examples-1.1.0-SNAPSHOT-hadoop0.23.9.jar --master yarn --deploy-mode cluster --class org.apache.spark.examples.SparkPi

Hopefully people using spark-submit will have converted to use configs so testing with spark-class is more important for those that haven't converted:
./bin/spark-class org.apache.spark.deploy.yarn.Client --jar examples/target/scala-2.10/spark-examples-1.1.0-SNAPSHOT-hadoop0.23.9.jar --class org.apache.spark.examples.SparkPi

With this change it fails with the error I listed, without this change it works and you can view the logs and see that the configs take affect:
14/06/19 17:54:34 INFO SecurityManager: SecurityManager: authentication enabled; ui acls enabled; users with view permissions: Set(yarn, tgraves)

[vanzin]

Your example works but only if you just want to propagate the property to the driver, and not the executors. Without being familiar with how things worked in 0.9. I'm a little sceptical that it's the full correct behaviour (since I'd expect SPARK_JAVA_OPTS to show up on both).

Anyway, I can make changes so that the use case you mention is covered.

[tgravescs]

what config is not showing up for you on the executor? the above example works fine for both driver and executor for me.

[vanzin]

On the master branch, when set using SPARK_JAVA_OPTS: anything that does not start with "spark." does not show up in the driver. Nothing at all shows up in executors.

[tgravescs]

so the example I gave above is on the master branch and the configs I set show up on both driver and executors. I'm not concerned with configs that don't start with "spark." as those aren't spark configs. When you say it doesn't show up on executors how are you checking? You don't see the logs in the executors that I listed? I ran on both 0.23 and 2.4 clusters. So I'm wondering why its not working for you.

Note that most configs get converted into SparkConf and sent to the executor via akka when it registers so they won't show up in the processes line with -D's or via getProperty. The security settings are special as they are needed before the registration happens.

Again I'm only concerned with actual spark configs (spark.*) and I'm only concerned about the spark framework properly reading them. I'm not concerned with application code reading them.

Another example you can use is :
export SPARK_JAVA_OPTS="-Dspark.authenticate=true -Dspark.ui.acls.enable=true -Dspark.akka.threads=10 -Dspark.akka.logAkkaConfig=true"

make sure that the akka settings get logged at the beginning of the executor processes.

[vanzin]

@tgravescs I posted my test code a few comments above. I'd expect "SPARK_JAVA_OPTS" to be just that - JVM options, not SparkConf options. So the properties I'm setting via JVM options are not showing up in the executors.

[vanzin]

Rebased and fixed the SPARK_JAVA_OPTS issues. @tgravescs, let me know if that addresses your concerns. Thanks!

[vanzin]

BTW I also tested "-Dspark.akka.logAkkaConfig=true" aside from my previous test case, and saw the akka logs in both driver and executor.

[tgravescs]

+1 looks good, thanks @vanzin.