From be67f954270a28f2046b33d9422e983b3b9230a6 Mon Sep 17 00:00:00 2001 From: Jeff Zhang Date: Wed, 14 Oct 2015 16:09:59 +0800 Subject: [PATCH 1/4] SPARK-11099 [SPARK SHELL] [SPARK SUBMIT] Default conf property file is not loaded --- .../launcher/AbstractCommandBuilder.java | 14 ++++------- .../SparkSubmitCommandBuilderSuite.java | 23 ++++++++++++------- .../src/test/resources/spark-default.conf | 5 ++++ 3 files changed, 25 insertions(+), 17 deletions(-) create mode 100644 launcher/src/test/resources/spark-default.conf diff --git a/launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java b/launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java index cf3729b7febc3..3ee6bd92e47fc 100644 --- a/launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java +++ b/launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java @@ -272,15 +272,11 @@ void setPropertiesFile(String path) { Map getEffectiveConfig() throws IOException { if (effectiveConfig == null) { - if (propertiesFile == null) { - effectiveConfig = conf; - } else { - effectiveConfig = new HashMap<>(conf); - Properties p = loadPropertiesFile(); - for (String key : p.stringPropertyNames()) { - if (!effectiveConfig.containsKey(key)) { - effectiveConfig.put(key, p.getProperty(key)); - } + effectiveConfig = new HashMap<>(conf); + Properties p = loadPropertiesFile(); + for (String key : p.stringPropertyNames()) { + if (!effectiveConfig.containsKey(key)) { + effectiveConfig.put(key, p.getProperty(key)); } } } diff --git a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java index d5397b0685046..e449a5e2c11a4 100644 --- a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java +++ b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java @@ -48,12 +48,14 @@ public static void cleanUp() throws Exception { @Test public void testDriverCmdBuilder() throws Exception { - testCmdBuilder(true); + testCmdBuilder(true, true); + testCmdBuilder(true, false); } @Test public void testClusterCmdBuilder() throws Exception { - testCmdBuilder(false); + testCmdBuilder(false, true); + testCmdBuilder(false, false); } @Test @@ -80,7 +82,7 @@ public void testCliParser() throws Exception { assertTrue("Driver -Xms should be configured.", cmd.contains("-Xms42g")); assertTrue("Driver -Xmx should be configured.", cmd.contains("-Xmx42g")); assertTrue("Command should contain user-defined conf.", - Collections.indexOfSubList(cmd, Arrays.asList(parser.CONF, "spark.randomOption=foo")) > 0); + Collections.indexOfSubList(cmd, Arrays.asList(parser.CONF, "spark.randomOption=foo")) > 0); } @Test @@ -149,7 +151,7 @@ public void testPySparkFallback() throws Exception { assertEquals("arg1", cmd.get(cmd.size() - 1)); } - private void testCmdBuilder(boolean isDriver) throws Exception { + private void testCmdBuilder(boolean isDriver, boolean usePropertyFile) throws Exception { String deployMode = isDriver ? "client" : "cluster"; SparkSubmitCommandBuilder launcher = @@ -164,11 +166,16 @@ private void testCmdBuilder(boolean isDriver) throws Exception { launcher.setPropertiesFile(dummyPropsFile.getAbsolutePath()); launcher.appArgs.add("foo"); launcher.appArgs.add("bar"); - launcher.conf.put(SparkLauncher.DRIVER_MEMORY, "1g"); - launcher.conf.put(SparkLauncher.DRIVER_EXTRA_CLASSPATH, "/driver"); - launcher.conf.put(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS, "-Ddriver -XX:MaxPermSize=256m"); - launcher.conf.put(SparkLauncher.DRIVER_EXTRA_LIBRARY_PATH, "/native"); launcher.conf.put("spark.foo", "foo"); + // either set the property through "--conf" or through property file + if (!usePropertyFile) { + launcher.conf.put(SparkLauncher.DRIVER_MEMORY, "1g"); + launcher.conf.put(SparkLauncher.DRIVER_EXTRA_CLASSPATH, "/driver"); + launcher.conf.put(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS, "-Ddriver -XX:MaxPermSize=256m"); + launcher.conf.put(SparkLauncher.DRIVER_EXTRA_LIBRARY_PATH, "/native"); + } else { + launcher.childEnv.put("SPARK_CONF_DIR", "test/resources"); + } Map env = new HashMap(); List cmd = launcher.buildCommand(env); diff --git a/launcher/src/test/resources/spark-default.conf b/launcher/src/test/resources/spark-default.conf new file mode 100644 index 0000000000000..479681d6832f9 --- /dev/null +++ b/launcher/src/test/resources/spark-default.conf @@ -0,0 +1,5 @@ +spark.driver.memory=1g +spark.driver.extraClassPath=/driver +spark.driver.extraJavaOptions=-Ddriver -XX:MaxPermSize=256m +spark.driver.extraLibraryPath=/native +spark.foo=foo \ No newline at end of file From c4277c154dff7b9d525dba1e95b18afc57b9cd24 Mon Sep 17 00:00:00 2001 From: Jeff Zhang Date: Wed, 14 Oct 2015 16:32:39 +0800 Subject: [PATCH 2/4] SPARK-11099 add apache license header --- launcher/src/test/resources/spark-default.conf | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/launcher/src/test/resources/spark-default.conf b/launcher/src/test/resources/spark-default.conf index 479681d6832f9..d7f65004532cd 100644 --- a/launcher/src/test/resources/spark-default.conf +++ b/launcher/src/test/resources/spark-default.conf @@ -1,3 +1,20 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + spark.driver.memory=1g spark.driver.extraClassPath=/driver spark.driver.extraJavaOptions=-Ddriver -XX:MaxPermSize=256m From ee79b20bb460e42e4a05fc43c1aa7a6375a2ad18 Mon Sep 17 00:00:00 2001 From: Jeff Zhang Date: Thu, 15 Oct 2015 07:54:30 +0800 Subject: [PATCH 3/4] SPARK-11099 Fix unit test failure --- .../apache/spark/launcher/SparkSubmitCommandBuilderSuite.java | 3 ++- launcher/src/test/resources/spark-default.conf | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java index e449a5e2c11a4..75295da4f81c4 100644 --- a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java +++ b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java @@ -174,7 +174,8 @@ private void testCmdBuilder(boolean isDriver, boolean usePropertyFile) throws Ex launcher.conf.put(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS, "-Ddriver -XX:MaxPermSize=256m"); launcher.conf.put(SparkLauncher.DRIVER_EXTRA_LIBRARY_PATH, "/native"); } else { - launcher.childEnv.put("SPARK_CONF_DIR", "test/resources"); + launcher.childEnv.put("SPARK_CONF_DIR", System.getProperty("spark.test.home") + + "/test/resources"); } Map env = new HashMap(); diff --git a/launcher/src/test/resources/spark-default.conf b/launcher/src/test/resources/spark-default.conf index d7f65004532cd..239fc57883e98 100644 --- a/launcher/src/test/resources/spark-default.conf +++ b/launcher/src/test/resources/spark-default.conf @@ -18,5 +18,4 @@ spark.driver.memory=1g spark.driver.extraClassPath=/driver spark.driver.extraJavaOptions=-Ddriver -XX:MaxPermSize=256m -spark.driver.extraLibraryPath=/native -spark.foo=foo \ No newline at end of file +spark.driver.extraLibraryPath=/native \ No newline at end of file From 115a8f1938aa41d9f57cb364d3d75f159bcc15df Mon Sep 17 00:00:00 2001 From: Jeff Zhang Date: Thu, 15 Oct 2015 11:22:17 +0800 Subject: [PATCH 4/4] SPARK-11099 Fix unit test failure --- .../launcher/SparkSubmitCommandBuilderSuite.java | 14 ++++++++------ .../{spark-default.conf => spark-defaults.conf} | 0 2 files changed, 8 insertions(+), 6 deletions(-) rename launcher/src/test/resources/{spark-default.conf => spark-defaults.conf} (100%) diff --git a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java index 75295da4f81c4..60594c3e1fd41 100644 --- a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java +++ b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java @@ -151,7 +151,7 @@ public void testPySparkFallback() throws Exception { assertEquals("arg1", cmd.get(cmd.size() - 1)); } - private void testCmdBuilder(boolean isDriver, boolean usePropertyFile) throws Exception { + private void testCmdBuilder(boolean isDriver, boolean useDefaultPropertyFile) throws Exception { String deployMode = isDriver ? "client" : "cluster"; SparkSubmitCommandBuilder launcher = @@ -163,19 +163,19 @@ private void testCmdBuilder(boolean isDriver, boolean usePropertyFile) throws Ex launcher.appResource = "/foo"; launcher.appName = "MyApp"; launcher.mainClass = "my.Class"; - launcher.setPropertiesFile(dummyPropsFile.getAbsolutePath()); launcher.appArgs.add("foo"); launcher.appArgs.add("bar"); launcher.conf.put("spark.foo", "foo"); - // either set the property through "--conf" or through property file - if (!usePropertyFile) { + // either set the property through "--conf" or through default property file + if (!useDefaultPropertyFile) { + launcher.setPropertiesFile(dummyPropsFile.getAbsolutePath()); launcher.conf.put(SparkLauncher.DRIVER_MEMORY, "1g"); launcher.conf.put(SparkLauncher.DRIVER_EXTRA_CLASSPATH, "/driver"); launcher.conf.put(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS, "-Ddriver -XX:MaxPermSize=256m"); launcher.conf.put(SparkLauncher.DRIVER_EXTRA_LIBRARY_PATH, "/native"); } else { launcher.childEnv.put("SPARK_CONF_DIR", System.getProperty("spark.test.home") - + "/test/resources"); + + "/launcher/src/test/resources"); } Map env = new HashMap(); @@ -224,7 +224,9 @@ private void testCmdBuilder(boolean isDriver, boolean usePropertyFile) throws Ex } // Checks below are the same for both driver and non-driver mode. - assertEquals(dummyPropsFile.getAbsolutePath(), findArgValue(cmd, parser.PROPERTIES_FILE)); + if (!useDefaultPropertyFile) { + assertEquals(dummyPropsFile.getAbsolutePath(), findArgValue(cmd, parser.PROPERTIES_FILE)); + } assertEquals("yarn", findArgValue(cmd, parser.MASTER)); assertEquals(deployMode, findArgValue(cmd, parser.DEPLOY_MODE)); assertEquals("my.Class", findArgValue(cmd, parser.CLASS)); diff --git a/launcher/src/test/resources/spark-default.conf b/launcher/src/test/resources/spark-defaults.conf similarity index 100% rename from launcher/src/test/resources/spark-default.conf rename to launcher/src/test/resources/spark-defaults.conf