From a8d57ba0ee3e4e3b46484bf8a8582a02910cd4f6 Mon Sep 17 00:00:00 2001 From: Nicolas Fraison Date: Fri, 30 Jun 2023 09:01:47 +0200 Subject: [PATCH 1/3] [SPARK-44242] Improve Max Heap not set check Currently if any Xmx string is available in the driver java options even if not related to Max Heap setting the job sumission failed --- .../launcher/SparkSubmitCommandBuilder.java | 14 ++++++---- .../SparkSubmitCommandBuilderSuite.java | 28 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java b/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java index 9618673b4ce0a..e83fc32ae8029 100644 --- a/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java +++ b/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java @@ -311,11 +311,15 @@ private List buildSparkSubmitCommand(Map env) } private void checkJavaOptions(String javaOptions) { - if (!isEmpty(javaOptions) && javaOptions.contains("Xmx")) { - String msg = String.format("Not allowed to specify max heap(Xmx) memory settings through " + - "java options (was %s). Use the corresponding --driver-memory or " + - "spark.driver.memory configuration instead.", javaOptions); - throw new IllegalArgumentException(msg); + if (!isEmpty(javaOptions)) { + for (String javaOption: CommandBuilderUtils.parseOptionString(javaOptions)) { + if (javaOption.startsWith("-Xmx")) { + String msg = String.format("Not allowed to specify max heap(Xmx) memory settings through " + + "java options (was %s). Use the corresponding --driver-memory or " + + "spark.driver.memory configuration instead.", javaOptions); + throw new IllegalArgumentException(msg); + } + } } } 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 967df3a563c8e..459069dbd6fba 100644 --- a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java +++ b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java @@ -72,6 +72,34 @@ public void testCliHelpAndNoArg() throws Exception { cmd.contains("org.apache.spark.deploy.SparkSubmit")); } + @Test(expected = IllegalArgumentException.class) + public void testCheckJavaOptionsThrowException() throws Exception { + Map env = new HashMap<>(); + List sparkSubmitArgs = Arrays.asList( + parser.MASTER, + "local", + parser.DRIVER_CLASS_PATH, + "/driverCp", + parser.DRIVER_JAVA_OPTIONS, + "-Xmx64g -Dprop=Other -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" -Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", + SparkLauncher.NO_RESOURCE); + buildCommand(sparkSubmitArgs, env); + } + + @Test + public void testCheckJavaOptions() throws Exception { + Map env = new HashMap<>(); + List sparkSubmitArgs = Arrays.asList( + parser.MASTER, + "local", + parser.DRIVER_CLASS_PATH, + "/driverCp", + parser.DRIVER_JAVA_OPTIONS, + "-Dprop=-Xmx -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" -Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", + SparkLauncher.NO_RESOURCE); + buildCommand(sparkSubmitArgs, env); + } + @Test public void testCliKillAndStatus() throws Exception { List params = Arrays.asList("driver-20160531171222-0000"); From b262ec308104df499121617eee081db46608cbc0 Mon Sep 17 00:00:00 2001 From: Nicolas Fraison Date: Mon, 3 Jul 2023 08:37:27 +0200 Subject: [PATCH 2/3] [SPARK-44242] Fix lint --- .../apache/spark/launcher/SparkSubmitCommandBuilder.java | 6 +++--- .../spark/launcher/SparkSubmitCommandBuilderSuite.java | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java b/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java index e83fc32ae8029..6c103d3cdf933 100644 --- a/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java +++ b/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java @@ -314,9 +314,9 @@ private void checkJavaOptions(String javaOptions) { if (!isEmpty(javaOptions)) { for (String javaOption: CommandBuilderUtils.parseOptionString(javaOptions)) { if (javaOption.startsWith("-Xmx")) { - String msg = String.format("Not allowed to specify max heap(Xmx) memory settings through " + - "java options (was %s). Use the corresponding --driver-memory or " + - "spark.driver.memory configuration instead.", javaOptions); + String msg = String.format("Not allowed to specify max heap(Xmx) memory settings " + + "through java options (was %s). Use the corresponding --driver-memory or " + + "spark.driver.memory configuration instead.", javaOptions); throw new IllegalArgumentException(msg); } } 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 459069dbd6fba..8c06bc32a5cc1 100644 --- a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java +++ b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java @@ -81,7 +81,8 @@ public void testCheckJavaOptionsThrowException() throws Exception { parser.DRIVER_CLASS_PATH, "/driverCp", parser.DRIVER_JAVA_OPTIONS, - "-Xmx64g -Dprop=Other -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" -Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", + "-Xmx64g -Dprop=Other -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" " + + "-Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", SparkLauncher.NO_RESOURCE); buildCommand(sparkSubmitArgs, env); } @@ -95,7 +96,8 @@ public void testCheckJavaOptions() throws Exception { parser.DRIVER_CLASS_PATH, "/driverCp", parser.DRIVER_JAVA_OPTIONS, - "-Dprop=-Xmx -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" -Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", + "-Dprop=-Xmx -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" " + + "-Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", SparkLauncher.NO_RESOURCE); buildCommand(sparkSubmitArgs, env); } From 644a92898d3544d696141389ec81a59d8e09ad47 Mon Sep 17 00:00:00 2001 From: Nicolas Fraison Date: Mon, 3 Jul 2023 09:01:20 +0200 Subject: [PATCH 3/3] [SPARK-44242] Fix indentation --- .../SparkSubmitCommandBuilderSuite.java | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 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 8c06bc32a5cc1..7a623bb76f3ad 100644 --- a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java +++ b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java @@ -76,14 +76,14 @@ public void testCliHelpAndNoArg() throws Exception { public void testCheckJavaOptionsThrowException() throws Exception { Map env = new HashMap<>(); List sparkSubmitArgs = Arrays.asList( - parser.MASTER, - "local", - parser.DRIVER_CLASS_PATH, - "/driverCp", - parser.DRIVER_JAVA_OPTIONS, - "-Xmx64g -Dprop=Other -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" " + - "-Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", - SparkLauncher.NO_RESOURCE); + parser.MASTER, + "local", + parser.DRIVER_CLASS_PATH, + "/driverCp", + parser.DRIVER_JAVA_OPTIONS, + "-Xmx64g -Dprop=Other -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" " + + "-Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", + SparkLauncher.NO_RESOURCE); buildCommand(sparkSubmitArgs, env); } @@ -91,14 +91,14 @@ public void testCheckJavaOptionsThrowException() throws Exception { public void testCheckJavaOptions() throws Exception { Map env = new HashMap<>(); List sparkSubmitArgs = Arrays.asList( - parser.MASTER, - "local", - parser.DRIVER_CLASS_PATH, - "/driverCp", - parser.DRIVER_JAVA_OPTIONS, - "-Dprop=-Xmx -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" " + - "-Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", - SparkLauncher.NO_RESOURCE); + parser.MASTER, + "local", + parser.DRIVER_CLASS_PATH, + "/driverCp", + parser.DRIVER_JAVA_OPTIONS, + "-Dprop=-Xmx -Dprop1=\"-Xmx -Xmx\" -Dprop2=\"-Xmx '-Xmx\" " + + "-Dprop3='-Xmx -Xmx' -Dprop4='-Xmx \"-Xmx'", + SparkLauncher.NO_RESOURCE); buildCommand(sparkSubmitArgs, env); }