spark-test commented on code in PR #59897:
URL: https://github.com/apache/spark/pull/59897#discussion_r4407600506
##########
.github/workflows/build_and_test.yml:
##########
@@ -25,7 +25,7 @@ on:
java:
required: false
type: string
- default: 17
+ default: 21
Review Comment:
**Non-blocking (P2):** Moving this default to 21 silently disables
`HiveExternalCatalogVersionsSuite`. The suite is `@SlowHiveTest` and runs in
the 'hive - slow tests' entries here and in `maven_test.yml` with `java: ${{
inputs.java }}`. `PROCESS_TABLES.testingVersions` is non-empty only when
`Utils.isJavaVersionAtMost17` (lines 264-266), and `test("backward
compatibility")` calls `runSparkSubmit` only when it is non-empty (line 250).
No master job can run on Java 17 any more, so every run logs 'Skip tests
because old Spark versions don't support Java 21' and passes without testing
anything. Master loses its automated check that tables and metadata written by
released Spark versions stay readable through `HiveExternalCatalog`, a main 5.0
upgrade path. Released 4.x lines run on Java 21, so the gate can select the
latest release of each line that supports the running JDK instead of returning
nothing.
**Verification:**
- **Regression:** Running the Hive external catalog backward-compatibility
suite on Java 21 exercises at least one prior release line, instead of logging
the Java 21 skip and passing vacuously.
- **Compatibility:** Release lines that cannot run on the current JDK, such
as 3.5.x on Java 21, are excluded, so the suite does not fail spuriously.
##########
pom.xml:
##########
@@ -121,8 +121,8 @@
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <java.version>17</java.version>
- <java.minimum.version>17.0.11</java.minimum.version>
+ <java.version>21</java.version>
+ <java.minimum.version>21</java.minimum.version>
Review Comment:
**Non-blocking (P2):** `R/pkg/DESCRIPTION:13` still declares
`SystemRequirements: Java (>= 17, < 26)`, and SparkR's `checkJavaVersion()`
(`R/pkg/R/client.R`) reads its lower bound from that field before
`sparkR.session()` launches the backend. With the minimum now 21, a SparkR user
on Java 17 passes the guard. The `spark-submit` launcher, compiled for release
21, then fails with `UnsupportedClassVersionError`, and SparkR ends with the
misleading `JVM is not ready after 10 seconds` instead of its explicit 'Java
version ... is required' error. The package metadata also misstates the
minimum. The previous baseline bump moved this field in a same-day companion
change, SPARK-45284; `Java (>= 21, < 26)` restores the guard.
**Verification:**
- **Regression:** With a Java 17 runtime, starting a SparkR session fails at
SparkR's Java version check with the explicit minimum-21 message, not with a
backend launch failure.
- **Compatibility:** With Java 21 and Java 25 runtimes, the SparkR Java
version check passes and sessions start as before.
##########
docs/index.md:
##########
@@ -34,7 +34,7 @@ source, visit [Building Spark](building-spark.html).
Spark runs on both Windows and UNIX-like systems (e.g. Linux, Mac OS), and it
should run on any platform that runs a supported version of Java. This should
include JVMs on x86_64 and ARM64. It's easy to run locally on one machine ---
all you need is to have `java` installed on your system `PATH`, or the
`JAVA_HOME` environment variable pointing to a Java installation.
-Spark runs on Java 17/21/25, Scala 2.13, Python 3.11+, and R 4.0+ (Deprecated).
+Spark runs on Java 21/25, Scala 2.13, Python 3.11+, and R 4.0+ (Deprecated).
Review Comment:
**Nit (P3):** `docs/running-on-yarn.md` still contradicts this line. Line 36
says 'Apache Spark requires at least Java 17 since 4.0.0' and asks for a
separate JDK only when Hadoop is older than 3.5.0, and the JDK example at lines
1072-1078 points `JAVA_HOME`, `spark.yarn.appMasterEnv.JAVA_HOME`, and
`spark.executorEnv.JAVA_HOME` at `/opt/openjdk-17`. A user who follows that
page on a cluster whose nodes run Java 17 gets an ApplicationMaster and
executors that cannot load Spark 5.0's Java 21 classes. The baseline sentence,
the Hadoop condition, and the example path need to reflect Java 21.
See **Shared repair plan 1** in the review body.
##########
.github/workflows/benchmark.yml:
##########
@@ -28,11 +28,10 @@ on:
default: '*'
jdk:
type: choice
- description: 'JDK version: 17, 21, or 25'
+ description: 'JDK version: 21 or 25'
required: true
- default: '17'
+ default: '21'
options:
- - '17'
Review Comment:
**Nit (P3):** Without the JDK 17 option, nothing can regenerate the
unsuffixed baseline files. `BenchmarkBase.scala:55` still suffixes result files
only when `version > 17`, so every run this workflow can launch writes `-jdk21`
or `-jdk25` files. The 126 committed unsuffixed `*-results.txt` files, 125 of
which report a JDK 17 VM (for example `KryoBenchmark-results.txt`), stay frozen
as JDK 17 snapshots that still look like the default baseline, and the
scaladocs that say results go to `benchmarks/<Name>-results.txt` describe files
no supported run writes. When the baseline last moved, SPARK-45281 changed this
threshold from `> 8` to `> 17` and companion changes refreshed and pruned the
result files; moving it to `> 21` follows the same pattern.
**Verification:**
- **Behavior:** Regenerating a benchmark on JDK 21 writes the unsuffixed
result file, and regenerating it on JDK 25 writes the jdk25-suffixed file.
- **Inspection:** No committed unsuffixed benchmark result file names a JDK
17 VM in its header, and no orphaned jdk21-suffixed duplicates remain.
##########
pom.xml:
##########
@@ -121,8 +121,8 @@
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <java.version>17</java.version>
- <java.minimum.version>17.0.11</java.minimum.version>
+ <java.version>21</java.version>
Review Comment:
**Nit (P3):** The Kubernetes integration-test dev script still overrides
this property to 17.
`resource-managers/kubernetes/integration-tests/dev/dev-run-integration-tests.sh:41`
defaults `JAVA_VERSION="17"`, and line 145 passes
`-Djava.version=$JAVA_VERSION` to its `build/mvn install ... -am` build. That
replaces `java.version=21` in `maven.compiler.release`, the javac `--release`
argument, `requireJavaVersion`, and the `enforceBytecodeVersion` ceiling, while
the scalac `-release` literal further down stays 21. A default developer run
therefore compiles Java sources for release 17 and Scala sources for release
21, which matches neither the shipped build nor anything CI runs. Whether the
release-17 bytecode ceiling also rejects the release-21 reactor jars was not
verified without a Maven build. A default of 21 keeps the script consistent
with the POM.
**Verification:**
- **Behavior:** With default arguments, the Kubernetes integration-test dev
build compiles Java and Scala classes for the same release 21 target as the
shipped build.
##########
.github/workflows/build_maven.yml:
##########
@@ -17,7 +17,7 @@
# under the License.
#
-name: "Build / Maven (Scala 2.13, JDK 17)"
+name: "Build / Maven (Scala 2.13, JDK 21)"
Review Comment:
**Nit (P3):** This name is already used by `build_maven_java21.yml`, and
after the default change the two workflows are the same build. Apart from cron
(`0 13` vs `0 14`, every two days), they differ only in
`build_maven_java21.yml`'s explicit `java: 21`, which is now `maven_test.yml`'s
default. So master runs two identical full Maven JDK 21 test jobs under one
display name, and the README master table shows both badges. The branch
schedulers dispatch both files with `--ref branch-*`, so neither can simply be
deleted. Turning master's `build_maven_java21.yml` into a no-op placeholder
like `build_scala213.yml`, with a distinct display name and without the
duplicate README badge, removes the duplicate run and keeps branch dispatch and
badges working.
See **Shared repair plan 2** in the review body.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
