From 8ca66cccee7c5cdd59a3722e58504430626f7c3e Mon Sep 17 00:00:00 2001
From: Varun Thacker <varun@vthacker.in>
Date: Fri, 22 Sep 2023 14:45:55 -0700
Subject: [PATCH] Upgrade to java 21 (#678)

* Upgrade built to java 21

* use corretto

* upgrade Mockito

* explicitly define bytebuddy definition

* use new fmt-maven-plugin version

* use com.spotify.fmt:fmt-maven-plugin

* whitespace

* comments

* bytebuddy
---
 .github/workflows/benchmark.yml                  |  6 +++---
 .github/workflows/maven.yml                      |  8 +++++---
 Dockerfile                                       |  4 ++--
 benchmarks/pom.xml                               |  2 +-
 kaldb/pom.xml                                    | 16 +++++++++++++---
 .../writer/kafka/KaldbKafkaConsumerTest.java     |  1 +
 6 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml
index 365c2a39eb..3fed0150e2 100644
--- a/.github/workflows/benchmark.yml
+++ b/.github/workflows/benchmark.yml
@@ -23,11 +23,11 @@ jobs:
     if: github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'benchmark')
     steps:
       - uses: actions/checkout@v3
-      - name: Set up JDK 17
+      - name: Set up JDK 21
         uses: actions/setup-java@v3
         with:
-          java-version: '17'
-          distribution: 'temurin'
+          java-version: '21'
+          distribution: 'corretto'
           cache: 'maven'
       # - name: Download benchmark data
       #  # AWS cli can be used if we want to host separate data - https://stackoverflow.com/questions/59166099/github-action-aws-cli
diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml
index 94b1dad89b..7c61ac6fb6 100644
--- a/.github/workflows/maven.yml
+++ b/.github/workflows/maven.yml
@@ -19,11 +19,13 @@ jobs:
 
     steps:
     - uses: actions/checkout@v3
-    - name: Set up JDK 17
+    - name: Set up JDK 21
       uses: actions/setup-java@v3
       with:
-        java-version: '17'
-        distribution: 'temurin'
+        java-version: '21'
+        #  Currently, LTS versions of Eclipse Temurin (temurin) are cached on the GitHub Hosted Runners.
+        #  When temurin releases Java 21 we can start using that again
+        distribution: 'corretto'
         cache: 'maven'
     - name: Ensure code is formatted
       run: mvn -B -Dstyle.color=always com.spotify.fmt:fmt-maven-plugin:check --file kaldb/pom.xml
diff --git a/Dockerfile b/Dockerfile
index 711afc0ba0..8c5c5d5bf4 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,8 +1,8 @@
-FROM maven:3.9-amazoncorretto-17 as build
+FROM maven:3.9-amazoncorretto-21 as build
 COPY . /work/
 RUN cd /work; mvn package -DskipTests
 
-FROM amazoncorretto:17
+FROM amazoncorretto:21
 COPY --from=build /work/kaldb/target/kaldb.jar /
 COPY --from=build /work/config/config.yaml /
 ENTRYPOINT [ "java", "-jar", "./kaldb.jar", "config.yaml" ]
diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml
index 3097a87c21..2df9830aea 100644
--- a/benchmarks/pom.xml
+++ b/benchmarks/pom.xml
@@ -50,7 +50,7 @@
             <plugin>
                 <groupId>com.spotify.fmt</groupId>
                 <artifactId>fmt-maven-plugin</artifactId>
-                <version>2.20</version>
+                <version>2.21.1</version>
                 <configuration>
                     <sourceDirectory>src/main/java</sourceDirectory>
                     <testSourceDirectory>src/test/java</testSourceDirectory>
diff --git a/kaldb/pom.xml b/kaldb/pom.xml
index 6d07041faa..7d07bb262e 100644
--- a/kaldb/pom.xml
+++ b/kaldb/pom.xml
@@ -426,10 +426,13 @@
             <version>3.24.2</version>
             <scope>test</scope>
         </dependency>
+        <!-- Note: next time we upgrade this, check if mockito-core depends on 1.14.8+ version of bytebuddy -->
+        <!-- if it does then we don't need to define bytebuddy explicitly anymore -->
+        <!-- Run mvn dependency:tree to verify. armeria also has a runtime dependency on bytebuddy so we need to check that as well -->
         <dependency>
             <groupId>org.mockito</groupId>
             <artifactId>mockito-core</artifactId>
-            <version>5.3.1</version>
+            <version>5.5.0</version>
             <scope>test</scope>
         </dependency>
         <dependency>
@@ -438,6 +441,13 @@
             <version>5.2.0</version>
             <scope>test</scope>
         </dependency>
+        <!-- Define bytebuddy 1.14.8 explicitly which support Java 21 -->
+        <dependency>
+            <groupId>net.bytebuddy</groupId>
+            <artifactId>byte-buddy</artifactId>
+            <version>1.14.8</version>
+            <scope>test</scope>
+        </dependency>
 
         <!-- BlobFs test dependencies -->
         <dependency>
@@ -572,7 +582,7 @@
                             <version>${error.prone.version}</version>
                         </path>
                     </annotationProcessorPaths>
-                 </configuration>
+                </configuration>
             </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
@@ -629,7 +639,7 @@
             <plugin>
                 <groupId>com.spotify.fmt</groupId>
                 <artifactId>fmt-maven-plugin</artifactId>
-                <version>2.20</version>
+                <version>2.21.1</version>
                 <configuration>
                     <sourceDirectory>src/main/java</sourceDirectory>
                     <testSourceDirectory>src/test/java</testSourceDirectory>
diff --git a/kaldb/src/test/java/com/slack/kaldb/writer/kafka/KaldbKafkaConsumerTest.java b/kaldb/src/test/java/com/slack/kaldb/writer/kafka/KaldbKafkaConsumerTest.java
index aba7ff2284..b8725fec15 100644
--- a/kaldb/src/test/java/com/slack/kaldb/writer/kafka/KaldbKafkaConsumerTest.java
+++ b/kaldb/src/test/java/com/slack/kaldb/writer/kafka/KaldbKafkaConsumerTest.java
@@ -223,6 +223,7 @@ public void testConsumeMessagesBetweenOffsets() throws Exception {
       // Assign doesn't create a consumer group.
       assertThat(kafkaServer.getConnectedConsumerGroups()).isEqualTo(0);
     }
+
     // TODO: Test batch ingestion with roll over. Not adding a test, since this functionality is
     // not needed by the recovery indexer yet.