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.

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

[DO NOT MERGE YET] Require Java 21 to run Trino #17520

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions .github/actions/setup/action.yml
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: 17
default: 21
cache:
description: "Cache Maven repo (true/false/restore)"
default: true
Expand Down Expand Up @@ -36,14 +36,16 @@ runs:
- uses: actions/setup-java@v3
if: ${{ inputs.java-version != '' }}
with:
distribution: 'temurin' # use same JDK distro as in Trino docker images
distribution: 'zulu' # use Zulu temporary
java-version: ${{ inputs.java-version }}
- name: Cache and Restore local Maven repo
id: cache
if: ${{ format('{0}', inputs.cache) == 'true' }}
uses: actions/cache@v3
with:
path: ~/.m2/repository
path: |
~/.m2/repository
/tmp/pt_java_downloads
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: |
${{ runner.os }}-maven-
Expand Down
7 changes: 3 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ env:
# used by actions/cache to retry the download after this time: https://github.com/actions/cache/blob/main/workarounds.md#cache-segment-restore-timeout
SEGMENT_DOWNLOAD_TIMEOUT_MINS: 5
CI_SKIP_SECRETS_PRESENCE_CHECKS: ${{ secrets.CI_SKIP_SECRETS_PRESENCE_CHECKS }}
PTL_TMP_DOWNLOAD_PATH: /tmp/pt_java_downloads

# Cancel previous PR builds.
concurrency:
Expand All @@ -56,8 +57,7 @@ jobs:
fail-fast: false
matrix:
java-version:
- 17
- 20
- 21
timeout-minutes: 45
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -566,7 +566,6 @@ jobs:
- lib/trino-filesystem-s3
- lib/trino-hdfs
- { modules: core/trino-main }
- { modules: core/trino-main, jdk: 20 }
- { modules: lib/trino-filesystem-s3, profile: cloud-tests }
- { modules: lib/trino-filesystem-azure, profile: cloud-tests }
- { modules: plugin/trino-accumulo }
Expand Down Expand Up @@ -635,7 +634,7 @@ jobs:
- uses: ./.github/actions/setup
with:
cache: restore
java-version: ${{ matrix.jdk != '' && matrix.jdk || '17' }}
java-version: ${{ matrix.jdk != '' && matrix.jdk || '21' }}
- name: Cleanup node
# This is required as a virtual environment update 20210219.1 left too little space for MemSQL to work
if: matrix.modules == 'plugin/trino-singlestore'
Expand Down
3 changes: 1 addition & 2 deletions core/docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
#
FROM ghcr.io/airlift/jvmkill:latest AS jvmkill

# Use Eclipse Temurin as they have base Docker images for more architectures.
FROM eclipse-temurin:17-jdk
FROM openjdk:21-slim

RUN \
set -xeu && \
Expand Down
2 changes: 1 addition & 1 deletion core/docker/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Builds the Trino Docker image
EOF
}

ARCHITECTURES=(amd64 arm64 ppc64le)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Restore when JDK 21 ppc64 images are available

ARCHITECTURES=(amd64 arm64)
TRINO_VERSION=

while getopts ":a:h:r:" o; do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,45 +16,23 @@
import com.google.common.util.concurrent.ForwardingListeningExecutorService;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import jakarta.annotation.Nullable;

import java.lang.invoke.MethodHandle;
import java.lang.reflect.Method;
import java.time.Duration;
import java.util.Collection;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

import static com.google.common.base.Throwables.throwIfUnchecked;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static io.trino.util.Reflection.methodHandle;
import static java.util.Objects.requireNonNull;

public class DecoratingListeningExecutorService
extends ForwardingListeningExecutorService
implements ListeningExecutorService
{
// TODO remove after requiring Java 19+ for runtime.
private static final @Nullable MethodHandle CLOSE_METHOD;

static {
Method closeMethod;
try {
closeMethod = ExecutorService.class.getMethod("close");
}
catch (NoSuchMethodException e) {
closeMethod = null;
}
CLOSE_METHOD = closeMethod != null
? methodHandle(closeMethod)
: null;
}

private final ListeningExecutorService delegate;
private final TaskDecorator decorator;

Expand Down Expand Up @@ -194,21 +172,10 @@ public boolean awaitTermination(Duration duration)
return super.awaitTermination(duration);
}

// TODO This is temporary, until Guava's ForwardingExecutorService has the method in their interface. See https://github.com/google/guava/issues/6296
//@Override
@Override
public void close()
{
if (CLOSE_METHOD == null) {
throw new UnsupportedOperationException("ExecutorService.close has close() method since Java 19. " +
"The DecoratingListeningExecutorService supports the method only when run with Java 19 runtime.");
}
try {
CLOSE_METHOD.invoke(delegate());
}
catch (Throwable e) {
throwIfUnchecked(e);
throw new RuntimeException(e);
}
delegate.close();
}

public interface TaskDecorator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,7 @@ else if ("Mac OS X".equals(osName)) {

private static void verifyJavaVersion()
{
Version required = Version.parse("17.0.3");

Version required = Version.parse("21");
if (Runtime.version().compareTo(required) < 0) {
failRequirement("Trino requires Java %s at minimum (found %s)", required, Runtime.version());
}
Expand Down
10 changes: 5 additions & 5 deletions core/trino-server-rpm/src/main/rpm/preinstall
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ check_if_correct_java_version() {
# candidate for JAVA_HOME).
JAVA_VERSION=$(java_version "$1")
JAVA_MAJOR=$(echo "$JAVA_VERSION" | cut -d'.' -f1)
if [ "$JAVA_MAJOR" -ge "17" ]; then
if [ "$JAVA_MAJOR" -ge "21" ]; then
echo "$1" >/tmp/trino-rpm-install-java-home
return 0
else
Expand All @@ -34,8 +34,8 @@ check_if_correct_java_version() {
if ! check_if_correct_java_version "$JAVA_HOME"; then
java_found=false
for candidate in \
/usr/lib/jvm/java-17-* \
/usr/lib/jvm/zulu-17 \
/usr/lib/jvm/java-21-* \
/usr/lib/jvm/zulu-21 \
/usr/lib/jvm/default-java \
/usr/java/default \
/ \
Expand All @@ -55,9 +55,9 @@ if [ "$java_found" = false ]; then
+======================================================================+
| Error: Required Java version could not be found |
+----------------------------------------------------------------------+
| Please install JDK 17. On RHEL/CentOS, use java-17-openjdk-devel. |
| Please install JDK 21. On RHEL/CentOS, use java-21-openjdk-devel. |
| |
| You can also download an OpenJDK 17 build, such as Zulu Community: |
| You can also download an OpenJDK 21 build, such as Zulu Community: |
| >>> https://www.azul.com/downloads/zulu-community/ <<< |
| |
| NOTE: This script will attempt to find Java whether you install |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,8 @@
@Test(singleThreaded = true)
public class ServerIT
{
private static final String BASE_IMAGE_PREFIX = "eclipse-temurin:";
private static final String BASE_IMAGE_SUFFIX = "-jre-centos7";

@Test(dataProvider = "rpmJavaTestDataProvider")
public void testInstall(String rpmHostPath, String javaVersion)
@Test(dataProvider = "rpmJavaDockerTestDataProvider")
public void testInstall(String rpmHostPath, String dockerImage, String javaVersion)
{
String rpm = "/" + new File(rpmHostPath).getName();
String command = "" +
Expand All @@ -70,7 +67,7 @@ public void testInstall(String rpmHostPath, String javaVersion)
// allow tail to work with Docker's non-local file system
"tail ---disable-inotify -F /var/log/trino/server.log\n";

try (GenericContainer<?> container = new GenericContainer<>(BASE_IMAGE_PREFIX + javaVersion + BASE_IMAGE_SUFFIX)) {
try (GenericContainer<?> container = new GenericContainer<>(dockerImage)) {
container.withExposedPorts(8080)
// the RPM is hundreds MB and file system bind is much more efficient
.withFileSystemBind(rpmHostPath, rpm, BindMode.READ_ONLY)
Expand All @@ -87,8 +84,8 @@ public void testInstall(String rpmHostPath, String javaVersion)
}
}

@Test(dataProvider = "rpmJavaTestDataProvider")
public void testUninstall(String rpmHostPath, String javaVersion)
@Test(dataProvider = "rpmJavaDockerTestDataProvider")
public void testUninstall(String rpmHostPath, String dockerImage, String javaVersion)
throws Exception
{
String rpm = "/" + new File(rpmHostPath).getName();
Expand All @@ -98,7 +95,7 @@ public void testUninstall(String rpmHostPath, String javaVersion)
"/etc/init.d/trino start\n" +
// allow tail to work with Docker's non-local file system
"tail ---disable-inotify -F /var/log/trino/server.log\n";
try (GenericContainer<?> container = new GenericContainer<>(BASE_IMAGE_PREFIX + javaVersion + BASE_IMAGE_SUFFIX)) {
try (GenericContainer<?> container = new GenericContainer<>(dockerImage)) {
container.withFileSystemBind(rpmHostPath, rpm, BindMode.READ_ONLY)
.withCommand("sh", "-xeuc", installAndStartTrino)
.waitingFor(forLogMessage(".*SERVER STARTED.*", 1).withStartupTimeout(Duration.ofMinutes(5)))
Expand All @@ -119,12 +116,14 @@ public void testUninstall(String rpmHostPath, String javaVersion)
}

@DataProvider
public static Object[][] rpmJavaTestDataProvider()
public static Object[][] rpmJavaDockerTestDataProvider()
{
String rpmHostPath = requireNonNull(System.getProperty("rpm"), "rpm is null");

// It's expected that the provided docker images are Centos7-based, so that
// the rpm installation method is common.
return new Object[][]{
{rpmHostPath, "17"},
{rpmHostPath, "19"}};
{rpmHostPath, "openjdk:21-rc-oraclelinux7", "21"}};
}

private static void assertPathDeleted(GenericContainer<?> container, String path)
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@
<air.check.skip-pmd>true</air.check.skip-pmd>
<air.check.skip-jacoco>true</air.check.skip-jacoco>

<project.build.targetJdk>17</project.build.targetJdk>
<project.build.targetJdk>21</project.build.targetJdk>
<air.java.version>17.0.4</air.java.version>
<air.modernizer.java-version>8</air.modernizer.java-version>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import static com.google.common.base.Preconditions.checkArgument;
import static io.trino.tests.product.launcher.env.EnvironmentContainers.COORDINATOR;
import static io.trino.tests.product.launcher.env.jdk.BuiltInJdkProvider.BUILT_IN_NAME;
import static java.util.Locale.ENGLISH;
import static picocli.CommandLine.Option;

Expand Down Expand Up @@ -58,7 +57,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 = BUILT_IN_NAME;
public String jdkProvider = "temurin21-rc";
wendigo marked this conversation as resolved.
Show resolved Hide resolved

@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