Skip to content
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

Support building with Java 21 #10474

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Support building with Java 21 #10474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e66bd0
Support building with Java 21
findepi Jun 10, 2024
27e9f55
Add spotlessApply stub for JDK 21
findepi Jul 10, 2024
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
4 changes: 2 additions & 2 deletions .github/workflows/delta-conversion-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
jvm: [8, 11, 17]
jvm: [8, 11, 17, 21]
env:
SPARK_LOCAL_IP: localhost
steps:
Expand Down Expand Up @@ -98,7 +98,7 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
jvm: [8, 11, 17]
jvm: [8, 11, 17, 21]
env:
SPARK_LOCAL_IP: localhost
steps:
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/flink-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,15 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
jvm: [8, 11, 17]
jvm: [8, 11, 17, 21]
flink: ['1.17', '1.18', '1.19']
exclude:
# Flink 1.17 does not support Java 17.
- jvm: 17
flink: '1.17'
# Flink 1.17 does not support Java 21.
- jvm: 21
flink: '1.17'
env:
SPARK_LOCAL_IP: localhost
steps:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/hive-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
jvm: [8, 11, 17]
jvm: [8, 11, 17, 21]
env:
SPARK_LOCAL_IP: localhost
steps:
Expand Down Expand Up @@ -96,7 +96,7 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
jvm: [8, 11, 17]
jvm: [8, 11, 17, 21]
env:
SPARK_LOCAL_IP: localhost
steps:
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/java-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
jvm: [8, 11, 17]
jvm: [8, 11, 17, 21]
env:
SPARK_LOCAL_IP: localhost
steps:
Expand Down Expand Up @@ -92,7 +92,7 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
jvm: [8, 11, 17]
jvm: [8, 11, 17, 21]
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
Expand All @@ -105,7 +105,7 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
jvm: [8, 11, 17]
jvm: [8, 11, 17, 21]
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
Expand Down
9 changes: 8 additions & 1 deletion .github/workflows/spark-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,16 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
jvm: [8, 11, 17]
jvm: [8, 11, 17, 21]
spark: ['3.3', '3.4', '3.5']
scala: ['2.12', '2.13']
exclude:
# Spark 3.5 is the first version not failing on Java 21 (https://issues.apache.org/jira/browse/SPARK-42369)
# Full Java 21 support is coming in Spark 4 (https://issues.apache.org/jira/browse/SPARK-43831)
- jvm: 21
spark: '3.3'
- jvm: 21
spark: '3.4'
env:
SPARK_LOCAL_IP: localhost
steps:
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Community discussions happen primarily on the [dev mailing list][dev-list] or on

### Building

Iceberg is built using Gradle with Java 8, 11, or 17.
Iceberg is built using Gradle with Java 8, 11, 17, or 21.

* To invoke a build and run tests: `./gradlew build`
* To skip tests: `./gradlew build -x test -x integrationTest`
Expand Down
11 changes: 10 additions & 1 deletion baseline.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,16 @@ subprojects {
apply plugin: 'com.palantir.baseline-reproducibility'
apply plugin: 'com.palantir.baseline-exact-dependencies'
apply plugin: 'com.palantir.baseline-release-compatibility'
apply plugin: 'com.diffplug.spotless'
// We need to update Google Java Format to 1.17.0+ to run spotless on JDK 8, but that requires dropping support for JDK 8.
if (JavaVersion.current() == JavaVersion.VERSION_21) {
task spotlessApply {
doLast {
throw new GradleException("Spotless plugin is currently disabled when running on JDK 21 (until we drop JDK 8). To run spotlessApply please use a different JDK version.")
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for adding that msg, that's going to be helpful

}
}
} else {
apply plugin: 'com.diffplug.spotless'
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we can merge the PR without having spotless enabled unfortunately

Copy link
Member Author

Choose a reason for hiding this comment

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

I wish this was avoidable. I do not know of a way to run same formatting rules under Java 8 and Java 21 and that's we support today.
I do believe that being able to test and actually testing with 21 is incremental improvement over current state. I see value in adding even partial support for 21. In particular, this PR identifies a problem with scala 2.12.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you bring this topic up on the Dev mailing list to see what others think about adding JDK21 support without being able to run spotless?

Copy link
Member

Choose a reason for hiding this comment

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

I think the comment is not accurate: we need to update Google Java Format to 1.17.0+ to run spotless on JDK21, but it breaks JDK8.
I'm more in favor of jumping to JDK21 with spotless/GJF 1.17.0+, as soon as we don't have any module requiring JDK8.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jbonofre we can't upgrade the GJF as long as we support JDK8 because upgrading the GJF version will produce different results depending on the underlying JDK it is executed with and so CI actions (and locally) will fail.

Copy link
Member

Choose a reason for hiding this comment

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

That's my point: I would postpone the JDK21 update right now, and jump directly to JDK21 without supporting JDK8 anymore.
I think it will be very difficult to support both JDK21 and JDK8 from spotless/GJF standpoint. So, I would directly jump to new version when possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Dropping JDK8 support means dropping Hive support (which I'm totally fine with) but we need to properly communicate which Iceberg version will drop Hive support. My understanding was that we were planning to drop Hive support with Iceberg 2.0

Copy link
Member

Choose a reason for hiding this comment

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

Yes exactly. It's what I proposed while ago in the Iceberg 2.0 discussion thread. So, I propose:

  • on 1.x (including 1.6.0 release currently in preparation), we keep JDK8
  • for 2.x, we merge this PR and jump to JDK21

I can reply on the Iceberg 2.0 thread if you want or Piotr can start a new thread about JDK21 on Iceberg 2.0.
I would love to have feedback on the Iceberg 2.0 thread 😄

Copy link

@BsoBird BsoBird Jul 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Dropping JDK8 support means dropping Hive support (which I'm totally fine with) but we need to properly communicate which Iceberg version will drop Hive support. My understanding was that we were planning to drop Hive support with Iceberg 2.0

Sir. Maybe the iceberg-hive module should be maintained by the hive community in the hive project. Because hive 3.X will EOL sooner or later.

Or just keep the catalog support.

Copy link
Contributor

Choose a reason for hiding this comment

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

@BsoBird newer versions of Hive are maintained by the Hive community but the Iceberg community still maintains an older version of Hive

}

pluginManager.withPlugin('com.palantir.baseline-checkstyle') {
checkstyle {
Expand Down
6 changes: 3 additions & 3 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ if (JavaVersion.current() == JavaVersion.VERSION_1_8) {
} else if (JavaVersion.current() == JavaVersion.VERSION_11) {
project.ext.jdkVersion = '11'
project.ext.extraJvmArgs = []
} else if (JavaVersion.current() == JavaVersion.VERSION_17) {
project.ext.jdkVersion = '17'
} else if (JavaVersion.current() == JavaVersion.VERSION_17 || JavaVersion.current() == JavaVersion.VERSION_21) {
project.ext.jdkVersion = JavaVersion.current().getMajorVersion().toString()
project.ext.extraJvmArgs = ["--add-opens", "java.base/java.io=ALL-UNNAMED",
"--add-opens", "java.base/java.lang.invoke=ALL-UNNAMED",
"--add-opens", "java.base/java.lang.reflect=ALL-UNNAMED",
Expand All @@ -86,7 +86,7 @@ if (JavaVersion.current() == JavaVersion.VERSION_1_8) {
"--add-opens", "java.base/sun.security.action=ALL-UNNAMED",
"--add-opens", "java.base/sun.util.calendar=ALL-UNNAMED"]
} else {
throw new GradleException("This build must be run with JDK 8 or 11 or 17 but was executed with JDK " + JavaVersion.current())
throw new GradleException("This build must be run with JDK 8 or 11 or 17 or 21 but was executed with JDK " + JavaVersion.current())
}

tasks.withType(AbstractArchiveTask).configureEach {
Expand Down
4 changes: 2 additions & 2 deletions jmh.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
* under the License.
*/

if (jdkVersion != '8' && jdkVersion != '11' && jdkVersion != '17') {
throw new GradleException("The JMH benchmarks must be run with JDK 8 or JDK 11 or JDK 17")
if (jdkVersion != '8' && jdkVersion != '11' && jdkVersion != '17' && jdkVersion != '21') {
throw new GradleException("The JMH benchmarks must be run with JDK 8 or JDK 11 or JDK 17 or JDK 21")
}

def flinkVersions = (System.getProperty("flinkVersions") != null ? System.getProperty("flinkVersions") : System.getProperty("defaultFlinkVersions")).split(",")
Expand Down
2 changes: 1 addition & 1 deletion site/docs/contribute.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ settle disagreements or to force a decision.

## Building the Project Locally

Iceberg is built using Gradle with Java 8, 11, or 17.
Iceberg is built using Gradle with Java 8, 11, 17, or 21.

* To invoke a build and run tests: `./gradlew build`
* To skip tests: `./gradlew build -x test -x integrationTest`
Expand Down
8 changes: 8 additions & 0 deletions spark/v3.3/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") {
implementation project(':iceberg-parquet')
implementation project(':iceberg-arrow')
implementation("org.scala-lang.modules:scala-collection-compat_${scalaVersion}:${libs.versions.scala.collection.compat.get()}")
if (scalaVersion == '2.12') {
// scala-collection-compat_2.12 pulls scala 2.12.17 and we need 2.12.18 for JDK 21 support
Copy link
Contributor

Choose a reason for hiding this comment

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

does this also apply for Scala 2.13?

Copy link
Member Author

Choose a reason for hiding this comment

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

apparently not, i think the Ci would detect that

Copy link
Contributor

Choose a reason for hiding this comment

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

do you remember what the exact error msg was here?

Copy link
Member Author

Choose a reason for hiding this comment

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

i don't, i should have kept a note

implementation 'org.scala-lang:scala-library:2.12.18'
}

compileOnly libs.errorprone.annotations
compileOnly libs.avro.avro
Expand Down Expand Up @@ -136,6 +140,10 @@ project(":iceberg-spark:iceberg-spark-extensions-${sparkMajorVersion}_${scalaVer

dependencies {
implementation "org.scala-lang.modules:scala-collection-compat_${scalaVersion}:${libs.versions.scala.collection.compat.get()}"
if (scalaVersion == '2.12') {
// scala-collection-compat_2.12 pulls scala 2.12.17 and we need 2.12.18 for JDK 21 support
implementation 'org.scala-lang:scala-library:2.12.18'
}
implementation libs.roaringbitmap

compileOnly "org.scala-lang:scala-library"
Expand Down
8 changes: 8 additions & 0 deletions spark/v3.4/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") {
implementation project(':iceberg-parquet')
implementation project(':iceberg-arrow')
implementation("org.scala-lang.modules:scala-collection-compat_${scalaVersion}:${libs.versions.scala.collection.compat.get()}")
if (scalaVersion == '2.12') {
// scala-collection-compat_2.12 pulls scala 2.12.17 and we need 2.12.18 for JDK 21 support
implementation 'org.scala-lang:scala-library:2.12.18'
}

compileOnly libs.errorprone.annotations
compileOnly libs.avro.avro
Expand Down Expand Up @@ -137,6 +141,10 @@ project(":iceberg-spark:iceberg-spark-extensions-${sparkMajorVersion}_${scalaVer

dependencies {
implementation("org.scala-lang.modules:scala-collection-compat_${scalaVersion}:${libs.versions.scala.collection.compat.get()}")
if (scalaVersion == '2.12') {
// scala-collection-compat_2.12 pulls scala 2.12.17 and we need 2.12.18 for JDK 21 support
implementation 'org.scala-lang:scala-library:2.12.18'
}
implementation libs.roaringbitmap

compileOnly "org.scala-lang:scala-library"
Expand Down
8 changes: 8 additions & 0 deletions spark/v3.5/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") {
implementation project(':iceberg-parquet')
implementation project(':iceberg-arrow')
implementation("org.scala-lang.modules:scala-collection-compat_${scalaVersion}:${libs.versions.scala.collection.compat.get()}")
if (scalaVersion == '2.12') {
// scala-collection-compat_2.12 pulls scala 2.12.17 and we need 2.12.18 for JDK 21 support
implementation 'org.scala-lang:scala-library:2.12.18'
}

compileOnly libs.errorprone.annotations
compileOnly libs.avro.avro
Expand Down Expand Up @@ -137,6 +141,10 @@ project(":iceberg-spark:iceberg-spark-extensions-${sparkMajorVersion}_${scalaVer

dependencies {
implementation("org.scala-lang.modules:scala-collection-compat_${scalaVersion}:${libs.versions.scala.collection.compat.get()}")
if (scalaVersion == '2.12') {
// scala-collection-compat_2.12 pulls scala 2.12.17 and we need 2.12.18 for JDK 21 support
implementation 'org.scala-lang:scala-library:2.12.18'
}
implementation libs.roaringbitmap

compileOnly "org.scala-lang:scala-library"
Expand Down