Java 21 compatibility

[gianm]

Java 21 is the next LTS release, expected September 2023. We'd like to officially support this.

It isn't out yet, but we can start with Java 20.

[clintropolis]

I'll share my notes since I've been slowly working my way through this!

Equalsverifier based tests fail with:

Java 20 (64) is not supported by the current version of Byte Buddy which officially supports Java 18 (62) - update Byte Buddy or set net.bytebuddy.experimental as a VM property

which is resolved by manually adding the newest version of byte-buddy to the pom (I also updated both equalsverifier and mockito to latest versions which also have transient dependencies on byte-buddy, but that wasn't enough to fix on its own).

Before updating easymock we also see some failures of this form

[ERROR] org.apache.druid.segment.writeout.FileWriteOutBytesTest.writeBufferSmallerThanCapacityShouldIncrementSizeCorrectly  Time elapsed: 0 s  <<< ERROR!
java.lang.IllegalArgumentException: Unsupported class file major version 64

which go away after updating easymock, however I am still seeing a failure on the latest version of easymock:

[ERROR] org.apache.druid.query.aggregation.constant.LongConstantBufferAggregatorTest.testAggregate  Time elapsed: 0.018 s  <<< ERROR!
java.lang.IllegalArgumentException: Could not create type
    at org.easymock.bytebuddy.TypeCache.findOrInsert(TypeCache.java:170)
    at org.easymock.bytebuddy.TypeCache$WithInlineExpunction.findOrInsert(TypeCache.java:399)
    at org.easymock.internal.ClassProxyFactory.createProxy(ClassProxyFactory.java:136)
    at org.easymock.internal.MocksControl.createMock(MocksControl.java:108)
    at org.easymock.internal.MocksControl.createMock(MocksControl.java:81)
    at org.easymock.IMocksControl.mock(IMocksControl.java:44)
    at org.easymock.EasyMock.mock(EasyMock.java:70)
...
Caused by: java.lang.IllegalStateException: java.lang.IncompatibleClassChangeError: class org.easymock.mocks.ByteBuffer$$$EasyMock$1 cannot inherit from sealed class java.nio.ByteBuffer

which is as far as i've got so far, I think maybe easymock hasn't been updated to be cool with java 20, as https://github.com/easymock/easymock/releases has an older version of bytebuddy than i added in my tests to our pom

[gianm]

For LongConstantBufferAggregatorTest specifically, I think we can replace buffer = EasyMock.mock(ByteBuffer.class) with buffer = null. It's just verifying that the LongConstantBufferAggregator is not calling any methods on buffer. Can't call methods on null 😉

[gianm]

Want to make a branch on apache/druid that various of us can commit to? Once things are buildable someone can make a PR from it. At that point I would suggest running most tests on 8 (oldest supported), 17 (latest LTS supported), and 20.

[clintropolis]

Yeah, will try to do later today :+1:

fwiw druid itself seems to run fine in java 20, i've been using it for all of my dev work running a local cluster for a couple of months now (though disclaimer i mainly use indexers instead of middle managers since easier to debug stuff), so i think it is just test problems.

I let some more tests run while i was eating lunch and the next failure is in druid-server:

[ERROR] org.apache.druid.curator.CuratorModuleTest.exitsJvmWhenMaxRetriesExceeded  Time elapsed: 0 s  <<< ERROR!
java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release
    at java.base/java.lang.System.setSecurityManager(System.java:429)
    at org.junit.contrib.java.lang.system.ProvideSecurityManager.before(ProvideSecurityManager.java:39)

and later in druid-sql are some failures like:

[ERROR] org.apache.druid.sql.calcite.rule.DruidLogicalValuesRuleTest$GetValueFromLiteralSimpleTypesTest.testGetValueFromLiteral[BIGINT, class java.lang.Long]  Time elapsed: 0 s  <<< FAILURE!
java.lang.AssertionError: 
Unable to mock the literal for test.
Exception: java.lang.NoSuchFieldException: value
    at org.junit.Assert.fail(Assert.java:89)
    at org.apache.druid.sql.calcite.rule.DruidLogicalValuesRuleTest$GetValueFromLiteralSimpleTypesTest.makeLiteral(DruidLogicalValuesRuleTest.java:109)
    at org.apache.druid.sql.calcite.rule.DruidLogicalValuesRuleTest$GetValueFromLiteralSimpleTypesTest.testGetValueFromLiteral(DruidLogicalValuesRuleTest.java:91)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
    at java.base/java.lang.reflect.Method.invoke(Method.java:578)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)

[gianm]

For DruidLogicalValuesRuleTest, we can remove the mocks from makeLiteral and make real literals instead:

    private static RexLiteral makeLiteral(Comparable<?> val, SqlTypeName typeName)
    {
      return (RexLiteral) new RexBuilder(DruidTypeSystem.TYPE_FACTORY).makeLiteral(
          typeName == SqlTypeName.DECIMAL && val != null ? new BigDecimal(String.valueOf(val)) : val,
          DruidTypeSystem.TYPE_FACTORY.createSqlType(typeName),
          false
      );
    }

[gianm]

The CuratorModuleTest case exitsJvmWhenMaxRetriesExceeded relies on NoExitSecurityManager to trap System.exit. Security managers are being removed gradually from Java 18+ (https://openjdk.org/jeps/411). The test will need to be rewritten or deleted.

IMO a good option would be to adjust CuratorModuleTest to subclass CuratorModule and override shutdown(lifecycle). The overridden method should set an AtomicBoolean in the subclass rather than calling System.exit(1). Then, the test case should check the AtomicBoolean rather than using ExpectedSystemExit.

[xvrl]

I was able to make unit tests pass in #15014. The biggest issue I see is that we need mockito 5.x to run tests with Java 21, however that version no longer supports Java 8. We might be able to do some workaround where we run tests with different mockito versions and restrict ourselves to the shared APIs, but that is also somewhat tricky since mockito-inline is now merged into mockito-core.

[clintropolis]

The biggest issue I see is that we need mockito 5.x to run tests with Java 21, however that version no longer supports Java 8

Now that we've dropped support for Hadoop 2, maybe we can consider doing the same for Java 8? Afaik that was the primary reason we needed to keep supporting it... though we'd need to decide on a new minimum version.

[xvrl]

IMO we should only target LTS Java versions, so 11 should be the new minimum version

[gianm]

Current versions of Kafka and Spark still support Java 8; I'd rather not be on the vanguard of dropping support, unless we get something big in return.

Kafka has deprecated it for removal in Kafka 4.0. Maybe we can do something similar?

[GWphua]

Wonder how this is going, is Druid ready to allow Java 21 without DRUID_SKIP_JAVA_CHECK? 😄