This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 2420fe91ce98 [SPARK-53167][DEPLOY] Spark launcher isRemote also
respects properties file
2420fe91ce98 is described below
commit 2420fe91ce980a444ab51cfdaa9af3d540c72652
Author: Cheng Pan <cheng...@apache.org>
AuthorDate: Thu Aug 7 07:51:36 2025 -0700
[SPARK-53167][DEPLOY] Spark launcher isRemote also respects properties file
### What changes were proposed in this pull request?
This PR modifies the `SparkSubmitCommandBuilder` to use "effective config"
to evaluate `isRemote` instead of the passed args only. This makes
`spark.remote` configured in the properties file(`spark-defaults.conf` or other
files specified by `--properties-file`) effective.
### Why are the changes needed?
```
$ sbin/start-connect-server.sh
$ cat > conf/spark-connect.conf <<EOF
spark.reomte=sc://localhost:15002
EOF
$ bin/spark-shell --properties-file conf/spark-connect.conf
```
```
25/08/07 13:35:56 ERROR Main: Failed to initialize Spark session.
org.apache.spark.SparkException: A master URL must be set in your
configuration
at org.apache.spark.SparkContext.<init>(SparkContext.scala:421)
at
org.apache.spark.SparkContext$.getOrCreate(SparkContext.scala:3055)
at
org.apache.spark.sql.classic.SparkSession$Builder.$anonfun$build$2(SparkSession.scala:839)
at scala.Option.getOrElse(Option.scala:201)
at
org.apache.spark.sql.classic.SparkSession$Builder.build(SparkSession.scala:830)
at
org.apache.spark.sql.classic.SparkSession$Builder.getOrCreate(SparkSession.scala:859)
at
org.apache.spark.sql.classic.SparkSession$Builder.getOrCreate(SparkSession.scala:732)
at
org.apache.spark.sql.SparkSession$Builder.getOrCreate(SparkSession.scala:923)
at org.apache.spark.repl.Main$.createSparkSession(Main.scala:116)
```
### Does this PR introduce _any_ user-facing change?
Yes, a kind of bug fix.
### How was this patch tested?
UT is added, plus manually test:
```
$ sbin/start-connect-server.sh
$ cat > conf/spark-connect.conf <<EOF
spark.reomte=sc://localhost:15002
EOF
$ bin/spark-shell --properties-file conf/spark-connect.conf
```
The previously failed case now works.
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #51896 from pan3793/SPARK-53167.
Authored-by: Cheng Pan <cheng...@apache.org>
Signed-off-by: Dongjoon Hyun <dongj...@apache.org>
---
.../spark/launcher/SparkSubmitCommandBuilder.java | 18 ++++----
.../launcher/SparkSubmitCommandBuilderSuite.java | 49 +++++++++++++---------
2 files changed, 40 insertions(+), 27 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 5efa3bef78bc..bdbb954dbe08 100644
---
a/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java
+++
b/launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java
@@ -172,6 +172,16 @@ class SparkSubmitCommandBuilder extends
AbstractCommandBuilder {
@Override
public List<String> buildCommand(Map<String, String> env)
throws IOException, IllegalArgumentException {
+ for (Map.Entry<String, String> entry : getEffectiveConfig().entrySet()) {
+ // If both spark.remote and spark.master are set, the error will be
thrown later
+ // when the application is started.
+ if (entry.getKey().equals("spark.remote")) {
+ isRemote = true;
+ } else if (entry.getKey().equals(SparkLauncher.SPARK_API_MODE)) {
+ // Respects if the API mode is explicitly set.
+ isRemote = entry.getValue().equalsIgnoreCase("connect");
+ }
+ }
if (PYSPARK_SHELL.equals(appResource) && !isSpecialCommand) {
return buildPySparkShellCommand(env);
} else if (SPARKR_SHELL.equals(appResource) && !isSpecialCommand) {
@@ -550,14 +560,6 @@ class SparkSubmitCommandBuilder extends
AbstractCommandBuilder {
checkArgument(value != null, "Missing argument to %s", CONF);
String[] setConf = value.split("=", 2);
checkArgument(setConf.length == 2, "Invalid argument to %s: %s",
CONF, value);
- // If both spark.remote and spark.mater are set, the error will be
thrown later when
- // the application is started.
- if (setConf[0].equals("spark.remote")) {
- isRemote = true;
- } else if (setConf[0].equals(SparkLauncher.SPARK_API_MODE)) {
- // Respects if the API mode is explicitly set.
- isRemote = setConf[1].equalsIgnoreCase("connect");
- }
conf.put(setConf[0], setConf[1]);
}
case CLASS -> {
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 31318f1318ca..f0a295fb4228 100644
---
a/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java
+++
b/launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java
@@ -18,12 +18,16 @@
package org.apache.spark.launcher;
import java.io.File;
+import java.io.FileReader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Properties;
import java.util.regex.Pattern;
import org.junit.jupiter.api.*;
@@ -33,29 +37,35 @@ import static org.junit.jupiter.api.Assertions.*;
public class SparkSubmitCommandBuilderSuite extends BaseSuite {
private static File dummyPropsFile;
+ private static File connectPropsFile;
private static SparkSubmitOptionParser parser;
@BeforeAll
public static void setUp() throws Exception {
dummyPropsFile = File.createTempFile("spark", "properties");
+ connectPropsFile = File.createTempFile("spark", "properties");
+ Files.writeString(connectPropsFile.toPath(),
"spark.remote=sc://connect-server:15002");
parser = new SparkSubmitOptionParser();
}
@AfterAll
public static void cleanUp() throws Exception {
dummyPropsFile.delete();
+ connectPropsFile.delete();
}
@Test
public void testDriverCmdBuilder() throws Exception {
- testCmdBuilder(true, true);
- testCmdBuilder(true, false);
+ testCmdBuilder(true, null);
+ testCmdBuilder(true, dummyPropsFile);
+ testCmdBuilder(true, connectPropsFile);
}
@Test
public void testClusterCmdBuilder() throws Exception {
- testCmdBuilder(false, true);
- testCmdBuilder(false, false);
+ testCmdBuilder(false, null);
+ testCmdBuilder(false, dummyPropsFile);
+ testCmdBuilder(false, connectPropsFile);
}
@Test
@@ -307,7 +317,7 @@ public class SparkSubmitCommandBuilderSuite extends
BaseSuite {
assertTrue(builder.isClientMode(userProps));
}
- private void testCmdBuilder(boolean isDriver, boolean
useDefaultPropertyFile) throws Exception {
+ private void testCmdBuilder(boolean isDriver, File propertiesFile) throws
Exception {
final String DRIVER_DEFAULT_PARAM = "-Ddriver-default";
final String DRIVER_EXTRA_PARAM = "-Ddriver-extra";
String deployMode = isDriver ? "client" : "cluster";
@@ -325,16 +335,16 @@ public class SparkSubmitCommandBuilderSuite extends
BaseSuite {
launcher.appArgs.add("bar");
launcher.conf.put("spark.foo", "foo");
// either set the property through "--conf" or through default property
file
- if (!useDefaultPropertyFile) {
- launcher.setPropertiesFile(dummyPropsFile.getAbsolutePath());
+ if (propertiesFile == null) {
+ launcher.childEnv.put("SPARK_CONF_DIR",
System.getProperty("spark.test.home")
+ + "/launcher/src/test/resources");
+ } else {
+ launcher.setPropertiesFile(propertiesFile.getAbsolutePath());
launcher.conf.put(SparkLauncher.DRIVER_MEMORY, "1g");
launcher.conf.put(SparkLauncher.DRIVER_EXTRA_CLASSPATH, "/driver");
launcher.conf.put(SparkLauncher.DRIVER_DEFAULT_JAVA_OPTIONS,
DRIVER_DEFAULT_PARAM);
launcher.conf.put(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS,
DRIVER_EXTRA_PARAM);
launcher.conf.put(SparkLauncher.DRIVER_EXTRA_LIBRARY_PATH, "/native");
- } else {
- launcher.childEnv.put("SPARK_CONF_DIR",
System.getProperty("spark.test.home")
- + "/launcher/src/test/resources");
}
Map<String, String> env = new HashMap<>();
@@ -348,13 +358,7 @@ public class SparkSubmitCommandBuilderSuite extends
BaseSuite {
"Driver default options should be configured.");
assertTrue(cmd.contains(DRIVER_EXTRA_PARAM), "Driver extra options
should be configured.");
} else {
- boolean found = false;
- for (String arg : cmd) {
- if (arg.startsWith("-Xmx")) {
- found = true;
- break;
- }
- }
+ boolean found = cmd.stream().anyMatch(arg -> arg.startsWith("-Xmx"));
assertFalse(found, "Memory arguments should not be set.");
assertFalse(cmd.contains(DRIVER_DEFAULT_PARAM),
"Driver default options should not be configured.");
@@ -379,8 +383,15 @@ public class SparkSubmitCommandBuilderSuite extends
BaseSuite {
}
// Checks below are the same for both driver and non-driver mode.
- if (!useDefaultPropertyFile) {
- assertEquals(dummyPropsFile.getAbsolutePath(), findArgValue(cmd,
parser.PROPERTIES_FILE));
+ if (propertiesFile != null) {
+ assertEquals(propertiesFile.getAbsolutePath(), findArgValue(cmd,
parser.PROPERTIES_FILE));
+ try (FileReader reader = new FileReader(propertiesFile,
StandardCharsets.UTF_8)) {
+ Properties props = new Properties();
+ props.load(reader);
+ if (props.containsKey("spark.remote")) {
+ assertTrue(launcher.isRemote);
+ }
+ }
}
assertEquals("yarn", findArgValue(cmd, parser.MASTER));
assertEquals(deployMode, findArgValue(cmd, parser.DEPLOY_MODE));
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
