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

Switch to JDK 22 in the runtime #21161

Merged
merged 2 commits into from
Mar 25, 2024
Merged

Switch to JDK 22 in the runtime #21161

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
069fe40
Install Java 22 in a docker image
wendigo Mar 21, 2024
90f9aac
Run all tests with Java 22
wendigo Mar 21, 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
2 changes: 1 addition & 1 deletion .github/actions/setup/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: "Verify checked out commits and setup Java"
inputs:
java-version:
description: "Java version to setup"
default: 21
default: 22
cache:
description: "Cache Maven repo (true/false/restore)"
default: restore
Expand Down
5 changes: 2 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ jobs:
fail-fast: false
matrix:
include:
- { java-version: 21, cache: 'true', cleanup-node: 'false' }
- { java-version: 22, cache: 'restore', cleanup-node: 'true' }
- { java-version: 22, cache: 'true', cleanup-node: 'false' }
timeout-minutes: 45
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -519,7 +518,7 @@ jobs:
with:
cache: restore
cleanup-node: ${{ format('{0}', matrix.modules == 'plugin/trino-singlestore') }}
java-version: ${{ matrix.jdk != '' && matrix.jdk || '21' }}
java-version: ${{ matrix.jdk != '' && matrix.jdk || '22' }}
- name: Maven Install
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
Expand Down
4 changes: 4 additions & 0 deletions core/trino-server-rpm/src/main/rpm/preinstall
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,13 @@ if ! check_if_correct_java_version "$JAVA_HOME"; then
java_found=false
for candidate in \
/usr/lib/jvm/java-21-* \
/usr/lib/jvm/java-22-* \
/usr/lib/jvm/zulu-21 \
/usr/lib/jvm/zulu-22 \
/usr/lib/jvm/temurin-21 \
/usr/lib/jvm/temurin-21-* \
/usr/lib/jvm/temurin-22 \
/usr/lib/jvm/temurin-22-* \
/usr/lib/jvm/default-java \
/usr/java/default \
/ \
Expand Down
3 changes: 3 additions & 0 deletions core/trino-server-rpm/src/test/java/io/trino/server/rpm/ServerIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Copy link
Contributor

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?

Copy link
Member

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'

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it on purpose?

Copy link
Contributor Author

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

Copy link
Member

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

Copy link
Contributor

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)?

Copy link
Contributor

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

Copy link
Contributor

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

}

private void testInstall(String temurinReleaseName, String javaHome, String expectedJavaVersion)
Expand Down
2 changes: 1 addition & 1 deletion ...-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/EnvironmentOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public final class EnvironmentOptions
public String launcherBin;

@Option(names = "--trino-jdk-version", paramLabel = "<trino-jdk-version>", description = "JDK to use for running Trino " + DEFAULT_VALUE)
public String jdkProvider = "temurin21";
public String jdkProvider = "temurin22";

@Option(names = "--jdk-tmp-download-path", paramLabel = "<jdk-tmp-download-path>", defaultValue = "${env:PTL_TMP_DOWNLOAD_PATH:-${sys:java.io.tmpdir}/ptl-tmp-download}", description = "Path to use to download JDK distributions " + DEFAULT_VALUE)
@Nullable
Expand Down
Loading