Review feedback

Author: pwendell

core/src/main/scala/org/apache/spark/SparkConf.scala

@@ -225,7 +225,8 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging {
     // Validate spark.executor.extraJavaOptions
     settings.get(executorOptsKey).map { javaOpts =>
       if (javaOpts.contains("-Dspark")) {
-        val msg = s"$executorOptsKey is not allowed to set Spark options. Was '$javaOpts'"
+        val msg = s"$executorOptsKey is not allowed to set Spark options (was '$javaOpts)'. " +
+          "Set them directly on a SparkConf or in a properties file when using ./bin/spark-submit."
         throw new Exception(msg)
       }
       if (javaOpts.contains("-Xmx") || javaOpts.contains("-Xms")) {
@@ -245,7 +246,7 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging {
           |Please instead use:
           | - ./spark-submit with conf/spark-defaults.conf to set properties for an application
           | - ./spark-submit with --driver-java-options to set -X options for a driver
-          | - spark.executor.executor.extraJavaOptions to set -X options for executors
+          | - spark.executor.extraJavaOptions to set -X options for executors
           | - SPARK_DAEMON_OPTS to set java options for standalone daemons (i.e. master, worker)
         """.stripMargin
       logError(error)
@@ -268,7 +269,7 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging {
           |
           |Please instead use:
           | - ./spark-submit with --driver-class-path to augment the driver classpath
-          | - spark.executor.executor.extraClassPath to augment the executor classpath
+          | - spark.executor.extraClassPath to augment the executor classpath
         """.stripMargin
       logError(error)
 

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

@@ -17,17 +17,15 @@
 
 package org.apache.spark.deploy
 
-import java.io.{IOException, FileInputStream, PrintStream, File}
+import java.io.{File, FileInputStream, IOException, PrintStream}
 import java.net.URL
 import java.util.Properties
 
-import org.apache.spark.executor.ExecutorURLClassLoader
-
 import scala.collection.JavaConversions._
-import scala.collection.mutable.ArrayBuffer
-import scala.collection.mutable.HashMap
-import scala.collection.mutable.Map
+import scala.collection.mutable.{ArrayBuffer, HashMap, Map}
+
 import org.apache.spark.SparkException
+import org.apache.spark.executor.ExecutorURLClassLoader
 
 /**
  * Scala code behind the spark-submit script.  The script handles setting up the classpath with
@@ -114,7 +112,7 @@ object SparkSubmit {
 
     // Load system properties by default from the file, if present
     if (appArgs.verbose) printStream.println(s"Using properties file: ${appArgs.propertiesFile}")
-    Option(appArgs.propertiesFile).map { filename =>
+    Option(appArgs.propertiesFile).foreach { filename =>
       val file = new File(filename)
       getDefaultProperties(file).foreach { case (k, v) =>
         if (k.startsWith("spark")) {
@@ -142,14 +140,12 @@ object SparkSubmit {
 
     val options = List[OptionAssigner](
       new OptionAssigner(appArgs.master, ALL_CLUSTER_MGRS, false, sysProp = "spark.master"),
-
       new OptionAssigner(appArgs.driverExtraClassPath, STANDALONE | YARN, true,
         sysProp = "spark.driver.extraClassPath"),
       new OptionAssigner(appArgs.driverExtraJavaOptions, STANDALONE | YARN, true,
-        sysProp = "spark.driver.extraJavaOpts"),
+        sysProp = "spark.driver.extraJavaOptions"),
       new OptionAssigner(appArgs.driverExtraLibraryPath, STANDALONE | YARN, true,
         sysProp = "spark.driver.extraLibraryPath"),
-
       new OptionAssigner(appArgs.driverMemory, YARN, true, clOption = "--driver-memory"),
       new OptionAssigner(appArgs.name, YARN, true, clOption = "--name"),
       new OptionAssigner(appArgs.queue, YARN, true, clOption = "--queue"),

core/src/main/scala/org/apache/spark/scheduler/cluster/SparkDeploySchedulerBackend.scala

@@ -44,8 +44,7 @@ private[spark] class SparkDeploySchedulerBackend(
     val driverUrl = "akka.tcp://spark@%s:%s/user/%s".format(
       conf.get("spark.driver.host"), conf.get("spark.driver.port"),
       CoarseGrainedSchedulerBackend.ACTOR_NAME)
-    val args = Seq(driverUrl, "{{EXECUTOR_ID}}", "{{HOSTNAME}}",
-      "{{CORES}}", "{{WORKER_URL}}")
+    val args = Seq(driverUrl, "{{EXECUTOR_ID}}", "{{HOSTNAME}}", "{{CORES}}", "{{WORKER_URL}}")
     val extraJavaOpts = sc.conf.getOption("spark.executor.extraJavaOptions")
     val classPathEntries = sys.props.get("spark.executor.extraClassPath").toSeq.flatMap { cp =>
       cp.split(java.io.File.pathSeparator)

docs/configuration.md

@@ -662,7 +662,7 @@ Apart from these, the following properties are also available, and may be useful
   <td>(none)</td>
   <td>
     Extra classpath entries to append to the classpath of executors. This exists primarily
-    for backwards-compatiblity with older versions of Spark. Users typically should not need
+    for backwards-compatibility with older versions of Spark. Users typically should not need
     to set this option.
   </td>
 </tr>

yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala

@@ -305,7 +305,7 @@ trait ClientBase extends Logging {
 
     val amMemory = calculateAMMemory(newApp)
 
-    var JAVA_OPTS = ListBuffer[String]()
+    val JAVA_OPTS = ListBuffer[String]()
 
     // Add Xmx for AM memory
     JAVA_OPTS += "-Xmx" + amMemory + "m"

yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala

@@ -43,7 +43,7 @@ import org.apache.spark.util.Utils
 
 
 /**
- * An application master that runs the users driver program and allocates executors.
+ * An application master that runs the user's driver program and allocates executors.
  */
 class ApplicationMaster(args: ApplicationMasterArguments, conf: Configuration,
                         sparkConf: SparkConf) extends Logging {