Skip to content

Pass Spark job server JVM args #39242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Abacn merged 1 commit into from
Jul 8, 2026
Merged

Pass Spark job server JVM args #39242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/trigger_files/beam_PostCommit_XVR_Spark3.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"trigger-2026-04-04": "portable_runner expand_sdf opt-in"
"trigger-2026-07-08": "portable_runner expand_sdf opt-in 2"
}
17 changes: 16 additions & 1 deletion runners/spark/job-server/spark_job_server.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,28 @@ tasks.register("validatesRunnerSickbay", Test) {
def jobPort = BeamModulePlugin.getRandomPort()
def artifactPort = BeamModulePlugin.getRandomPort()

def sparkJobServerJvmArgs() {
def testJavaVer = project.findProperty('testJavaVersion') ? (project.property('testJavaVersion') as int) : JavaVersion.current().majorVersion.toInteger()

@gemini-code-assist gemini-code-assist Bot Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Groovy/Gradle, using as int on a property that might be a String can throw a NumberFormatException if the property is not a valid integer. Additionally, JavaVersion.current().majorVersion is already a String, so calling .toInteger() on it is safe, but we should make sure testJavaVersion is safely parsed.

Consider using Integer.parseInt() or a safe cast with a fallback to avoid potential build failures if testJavaVersion is configured incorrectly.

  def testJavaVer = project.findProperty('testJavaVersion') ? (project.property('testJavaVersion').toString().toInteger()) : JavaVersion.current().majorVersion.toInteger()

if (testJavaVer >= 17) {
return [
"--add-opens=java.base/sun.nio.ch=ALL-UNNAMED",
"--add-opens=java.base/java.nio=ALL-UNNAMED",
"--add-opens=java.base/java.util=ALL-UNNAMED",
"--add-opens=java.base/java.lang.invoke=ALL-UNNAMED"
]
}
return []
}

def setupTask = project.tasks.register("sparkJobServerSetup", Exec) {
dependsOn shadowJar
def pythonDir = project.project(":sdks:python").projectDir
def sparkJobServerJar = shadowJar.archivePath
def jvmArgs = sparkJobServerJvmArgs().join(' ')
def jvmArgsOpt = jvmArgs ? "--jvm_args \"${jvmArgs}\"" : ""

executable 'sh'
args '-c', "$pythonDir/scripts/run_job_server.sh stop --group_id ${project.name} && $pythonDir/scripts/run_job_server.sh start --group_id ${project.name} --job_port ${jobPort} --artifact_port ${artifactPort} --job_server_jar ${sparkJobServerJar}"
args '-c', "$pythonDir/scripts/run_job_server.sh stop --group_id ${project.name} && $pythonDir/scripts/run_job_server.sh start --group_id ${project.name} --job_port ${jobPort} --artifact_port ${artifactPort} --job_server_jar ${sparkJobServerJar} ${jvmArgsOpt}"
}

def cleanupTask = project.tasks.register("sparkJobServerCleanup", Exec) {
Expand Down
8 changes: 7 additions & 1 deletion sdks/python/scripts/run_job_server.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Options:
--job_port [port for job endpoint, default 8099]
--artifact_port [port for artifact service, default 8098]
--job_server_jar [path to job server jar]
--jvm_args [additional JVM arguments, e.g. --add-opens flags]
END

JOB_PORT=8099
Expand Down Expand Up @@ -61,6 +62,11 @@ while [[ $# -gt 0 ]]; do
shift
shift
;;
--jvm_args)
JVM_ARGS="$2"
shift
shift
;;
start)
STARTSTOP="$1"
shift
Expand Down Expand Up @@ -107,7 +113,7 @@ case $STARTSTOP in
fi

echo "Launching job server @ $JOB_PORT ..."
"$JAVA_CMD" -jar $JOB_SERVER_JAR --job-port=$JOB_PORT --artifact-port=$ARTIFACT_PORT --expansion-port=0 $ADDITIONAL_ARGS >$TEMP_DIR/$FILE_BASE.log 2>&1 </dev/null &
"$JAVA_CMD" $JVM_ARGS -jar $JOB_SERVER_JAR --job-port=$JOB_PORT --artifact-port=$ARTIFACT_PORT --expansion-port=0 $ADDITIONAL_ARGS >$TEMP_DIR/$FILE_BASE.log 2>&1 </dev/null &

@gemini-code-assist gemini-code-assist Bot Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When $JVM_ARGS is unquoted, it undergoes word splitting, which is necessary here to pass multiple JVM arguments (like --add-opens) as separate parameters to the java command. However, if $JVM_ARGS is empty or unset, this can sometimes lead to unexpected behavior depending on shell settings, or if any of the arguments contain spaces.

To ensure robustness and avoid issues with empty values, it is safer to use an array for arguments in bash, or explicitly handle the expansion. Since this script is executed via sh (which might be dash or bash depending on the system), we should be careful. If we are sure it's run with bash (the shebang of run_job_server.sh is likely #!/bin/bash), using a bash array is the most robust way to handle list of arguments with potential spaces. If it must remain a string, we should ensure JVM_ARGS is initialized to an empty string at the top of the script to prevent unbound variable errors if set -u is used.

mypid=$!
if kill -0 $mypid >/dev/null 2>&1; then
echo $mypid >> $pid
Expand Down
Loading