-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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.
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
Switch to JDK 22 in the runtime #21161
Conversation
Failures so far:
|
8faa202
to
862eae8
Compare
...n/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/metadata/ShardCleaner.java
Outdated
Show resolved
Hide resolved
core/trino-server-rpm/src/test/java/io/trino/server/rpm/ServerIT.java
Outdated
Show resolved
Hide resolved
6cc1630
to
31bbbf0
Compare
87d7a64
to
311c5bf
Compare
@electrum PTAL for raptor changes |
are both still accurate? |
@findepi updated |
311c5bf
to
9255aec
Compare
Just rebased |
9255aec
to
90f9aac
Compare
@@ -76,6 +76,9 @@ public void testInstallUninstall() | |||
// Release names as in the https://api.adoptium.net/q/swagger-ui/#/Release%20Info/getReleaseNames | |||
testInstall("jdk-21.0.2+13", "/usr/lib/jvm/temurin-21", "21"); | |||
testUninstall("jdk-21.0.2+13", "/usr/lib/jvm/temurin-21", "21"); | |||
|
|||
testInstall("jdk-22+36", "/usr/lib/jvm/temurin-22", "22"); | |||
testUninstall("jdk-22+36", "/usr/lib/jvm/temurin-22", "22"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expectedJavaVersion
parameter in testUninstall
is not used, and Error Prone fails the build for me locally on this. Why isn't it failing on CI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because it skips some mdules -
trino/.github/workflows/ci.yml
Lines 199 to 200 in 0714cae
$MAVEN ${MAVEN_TEST} -T 1C clean verify -DskipTests ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \ | |
-pl '!:trino-docs,!:trino-server,!:trino-server-rpm' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it on purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's fix both issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure why though. It's been there since beginning - 7949e41
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we thought that there's no Java code in that module (like it's the case for docs and rpm modules)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, I now realized that this is the RPM module :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But yeah, I too would assume that there's no Java code here
Just a note -- jdk22 is a short term release; depending on that is going to cause issues in some enterprise environments that only stick to LTS versions of java. Only a problem if jdk22 features begin to get baseline, which will result in forcing longish upgrade cycles for enterprise customers. |
@covalesj we are aware of that. However the benefits outweigh these issues. We will continue to stay with the latest Java release going forward. What JDK is used is mostly an implementation detail. Especially in container-based deployments there is no impact. For non-container based deployments users will just have to choose to stay with old Trino and old Java .. or go with new features, more performance, timely security fixes, bug fixes and overall improvements all around. Also keep in mind that the "support" window for Java release is completely variable between vendors and you can get support for any release. We are using Eclipse Temurin as an open source project for our testing and inclusion in the container. |
Release notes: use Java 22 in the docker image.