DanielLeens commented on code in PR #11545:
URL: https://github.com/apache/seatunnel/pull/11545#discussion_r4227035282
##########
seatunnel-core/seatunnel-starter/src/main/bin/seatunnel.sh:
##########
@@ -109,6 +109,32 @@ do
fi
done
+# SeaTunnel requires Java 11 or newer. Fail fast with an actionable message
instead of letting a
+# JDK 8 launcher abort on the JDK 9+ module flags below with a cryptic
"Unrecognized option" error.
+JAVA_MAJOR_VERSION=$(java -version 2>&1 | awk -F '[".]' '/version/ {print ($2
== "1") ? $3 : $2; exit}')
+if [[ -n "$JAVA_MAJOR_VERSION" && "$JAVA_MAJOR_VERSION" -lt 11 ]]; then
+ echo "Error: SeaTunnel requires Java 11 or newer, but Java
${JAVA_MAJOR_VERSION} was detected. Point JAVA_HOME/PATH at a Java 11+ JDK." >&2
+ exit 1
+fi
+
+# These JDK module flags are mandatory on Java 11+: Hazelcast needs reflective
access to JDK
+# internals and the Kerberos krb5.conf reload needs the jgss export. They are
appended here, not
+# only shipped in the config/jvm_*_options templates, so an in-place upgrade
that preserves an old
+# config directory (mounted Docker volume, K8s ConfigMap) cannot silently drop
them. Appending a
+# flag twice is harmless, so config files that already carry them stay
compatible.
+for module_flag in \
+ "--add-opens=java.base/java.lang=ALL-UNNAMED" \
+ "--add-opens=java.base/java.net=ALL-UNNAMED" \
+ "--add-opens=java.base/java.nio=ALL-UNNAMED" \
Review Comment:
Thanks @goutamadwant, you are right, and I verified it against `dev`.
`seatunnel.cmd` and `seatunnel-cluster.cmd` only read
`config/jvm_client_options` / `jvm_master_options` / `jvm_worker_options` /
`jvm_options` and `JvmOption`; neither has the Java-version check or the
module-flag fallback that this PR added to `seatunnel.sh` and
`seatunnel-cluster.sh`. So a Windows user who upgrades to Java 17 and keeps an
existing `jvm_*_options` still starts without the flags.
I should also be upfront that I knew the `.cmd` launchers were not covered
and did not call that out clearly in the PR description. That was a gap in how
I reported it, not something I had decided was fine.
Your Doris/StarRocks reproduction is a useful concrete consequence
(`java.nio.Buffer.address` inaccessible on Java 17 without
`--add-opens=java.base/java.nio=ALL-UNNAMED`), which matches what the
Arrow-based `connector-lance` tests hit in this PR's CI.
This PR is already merged, so I will fix it in a follow-up PR that adds the
same Java-version pre-check and the same flag fallback (skipping flags the
config already carries) to both batch launchers, and link it here. One limit up
front: I have no Windows environment, so I can only check the batch logic by
reading it, and the PR will say that instead of claiming a Windows end-to-end
run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
