Claim full support for Java 17. #14384
Conversation
No production code has changed, except the startup scripts. Changes: 1) Allow Java 17 without DRUID_SKIP_JAVA_CHECK. 2) Include the full list of opens and exports on both Java 11 and 17. 3) Document that Java 17 is both supported and preferred. 4) Switch some tests from Java 11 to 17 to get better coverage on the preferred version.
There was a problem hiding this comment.
sgtm, 👍 (after CI, didn't look into why it is failing)
I was using java 17 with no issue for quite some time in most of my local development until fairly recently where I have switched to java 20 (which has some dependency problems for some tests such as the equalsverifier stuff which maybe needs to be updated to be cool).
docs/operations/java.md
Outdated
| @@ -85,29 +80,22 @@ these JVMs is given by `druid.indexer.runner.javaOptsArray` or `druid.indexer.ru | |||
| a line like the following: | |||
|
|
|||
| ``` | |||
| druid.indexer.runner.javaOptsArray=["-server","-Xms1g","-Xmx1g","-XX:MaxDirectMemorySize=1g","-Duser.timezone=UTC","-Dfile.encoding=UTF-8","-XX:+ExitOnOutOfMemoryError","-Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager","--add-exports=java.base/jdk.internal.ref=ALL-UNNAMED","--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED","--add-opens=java.base/java.lang=ALL-UNNAMED","--add-opens=java.base/java.io=ALL-UNNAMED","--add-opens=java.base/java.nio=ALL-UNNAMED","--add-opens=java.base/jdk.internal.ref=ALL-UNNAMED","--add-opens=java.base/sun.nio.ch=ALL-UNNAMED"] | |||
| druid.indexer.runner.javaOptsArray=["-server","-Xms1g","-Xmx1g","-XX:MaxDirectMemorySize=1g","-Duser.timezone=UTC","-Dfile.encoding=UTF-8","-XX:+ExitOnOutOfMemoryError","-Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager","--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED","--add-exports=java.base/jdk.internal.ref=ALL-UNNAMED","--add-opens=java.base/java.nio=ALL-UNNAMED","--add-opens=java.base/sun.nio.ch=ALL-UNNAMED","--add-opens=java.base/jdk.internal.ref=ALL-UNNAMED","--add-opens=java.base/java.io=ALL-UNNAMED","--add-opens=java.base/java.lang=ALL-UNNAMED","--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED"] | |||
There was a problem hiding this comment.
i wonder if we should modify the code to always append these options for java > 8
There was a problem hiding this comment.
bin/run-java does that, so it does happen if you set druid.indexer.runner.javaCommand=bin/run-java (which the default configs do). Having the ForkingTaskRunner do it also seems legit though.
There was a problem hiding this comment.
I modified ForkingTaskRunner in this way and updated the main PR description.
Well, Java 20 will be next 🙂 |
|
Raised #14502 for Java 21. |
* Claim full support for Java 17. No production code has changed, except the startup scripts. Changes: 1) Allow Java 17 without DRUID_SKIP_JAVA_CHECK. 2) Include the full list of opens and exports on both Java 11 and 17. 3) Document that Java 17 is both supported and preferred. 4) Switch some tests from Java 11 to 17 to get better coverage on the preferred version. * Doc update. * Update errorprone. * Update docker_build_containers.sh. * Update errorprone in licenses.yaml. * Add some more run-javas. * Additional run-javas. * Update errorprone. * Suppress new errorprone error. * Add exports and opens in ForkingTaskRunner for Java 11+. Test, doc changes. * Additional errorprone updates. * Update for errorprone. * Restore old fomatting in LdapCredentialsValidator. * Copy bin/ too. * Fix Java 15, 17 build line in docker_build_containers.sh. * Update busybox image. * One more java command. * Fix interpolation. * IT commandline refinements. * Switch to busybox 1.34.1-glibc. * POM adjustments, build and test one IT on 17. * Additional debugging. * Fix silly thing. * Adjust command line. * Add exports and opens one more place. * Additional harmonization of strong encapsulation parameters.
|
@gianm Do we need to update java.version, maven.compiler.source and maven.compiler.target to Java 17? |
No production code has changed, except the startup scripts.Some production Java code has changed:
Add opens and exports in ForkingTaskRunner, rather than relying on
bin/run-java. I expect this would be more robust.Minor changes to fix or suppress issues noticed by the errorprone upgrade.
Script, doc, test changes:
Allow Java 17 without DRUID_SKIP_JAVA_CHECK.
Include the full list of opens and exports on both Java 11 and 17.
Document that Java 17 is both supported and preferred.
Switch some tests from Java 11 to 17 to get better coverage on the
preferred version.
Update errorprone to a version that supports Java 17.
Fixes #12838.