Skip to content

ZOOKEEPER-5039: Raise to min JDK 17, also build, test with JDK25#2376

Merged
anmolnar merged 19 commits into
apache:masterfrom
PDavid:ZOOKEEPER-5039-mockito-upgrade
Jun 24, 2026
Merged

ZOOKEEPER-5039: Raise to min JDK 17, also build, test with JDK25#2376
anmolnar merged 19 commits into
apache:masterfrom
PDavid:ZOOKEEPER-5039-mockito-upgrade

Conversation

@PDavid

@PDavid PDavid commented Apr 23, 2026

Copy link
Copy Markdown
Contributor
  • Replaced maven.compiler.source + maven.compiler.target with a single maven.compiler.release=17
  • Removed maven.compiler.release property from m2e profile (it now inherits from main properties).
  • Removed jdk-release-flag Maven profile as it is not required anymore (maven.compiler.release=17 makes sure our build is compatible with Java 17).
  • Upgrade Mockito to 5.23.0 for JDK25: As of Mockito 5.x, the mockito-inline mock-maker became the default, so mockito-inline as a separate artifact is no longer needed.
  • Switch GH: Action workflows and Jenkins builds to JDK 17 and 25
  • Upgrade spotbugs to support Java 25
    • Upgraded spotbugs-maven-plugin to 4.9.3.0 and annotations to 4.9.3.
    • Overrode ASM to 9.9.1 in the plugin dependencies to support JDK 25 (class file version 69).
    • Added newly found bug patterns to excludeFindBugsFilter.xml - these are all pre-existing issues in the codebase that the newer SpotBugs detects but the old version couldn't.
  • Upgrade burningwave mockdns to 0.27.2 to support JDK25
  • Fix PrometheusMetricsProviderConfigTest: Exception message differs now a bit.
  • Upgrade JDK version in dev Dockerfile

@PDavid

PDavid commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

OK, Mockito 5 requires Java 11 or higher. It will not work with Java 8.
Mockito 4 supported Java 8 but that does not support Java 25.

Do we have to support Java 8 on ZooKeeper master branch?

@kgeisz kgeisz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@anmolnar

Copy link
Copy Markdown
Contributor

I think we can drop Java 8 support on the master branch. Please follow the discussion on the dev@ list.

@PDavid PDavid force-pushed the ZOOKEEPER-5039-mockito-upgrade branch from a72fee0 to e413971 Compare May 7, 2026 09:17
@PDavid PDavid changed the title ZOOKEEPER-5039: Upgrade Mockito to 5.23.0 for JDK25 ZOOKEEPER-5039: Raise to min JDK 17, also build, test with JDK25 May 7, 2026
@PDavid

PDavid commented May 7, 2026

Copy link
Copy Markdown
Contributor Author

It seems only changing the Jenkins file did not worked out:

org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
WorkflowScript: 33: Tool type "jdk" does not have an install of "jdk_1.17_latest" configured - did you mean "jdk_1.8_latest"? @ line 33, column 13.
           jdk "jdk_1.17_latest"
               ^

https://ci-hadoop.apache.org/job/zookeeper-precommit-github-pr/job/PR-2376/5/console

Wrote to ASF Slack builds channel if thy can install / configure JDK 17 / JDK 25 for us.

@anmolnar

anmolnar commented May 7, 2026

Copy link
Copy Markdown
Contributor

It seems only changing the Jenkins file did not worked out:

org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
WorkflowScript: 33: Tool type "jdk" does not have an install of "jdk_1.17_latest" configured - did you mean "jdk_1.8_latest"? @ line 33, column 13.
           jdk "jdk_1.17_latest"
               ^

https://ci-hadoop.apache.org/job/zookeeper-precommit-github-pr/job/PR-2376/5/console

Wrote to ASF Slack builds channel if thy can install / configure JDK 17 / JDK 25 for us.

Might be (just guessing) jdk_17_latest and jdk_25_latest.

@PDavid

PDavid commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

SnapshotAndRestoreCommandTest.testSnapshotAndRestoreCommand_streaming failed in the GH: Action build. It succeeds for me locally so I guess it is just a flake here.

@anmolnar anmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lgtm. Please update and test Java version in dev/docker/Dockerfile as well.
Thanks!

Comment thread dev/docker/Dockerfile Outdated
#

FROM maven:3.8.4-jdk-11
FROM maven:3.9.6-eclipse-temurin-17

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since the old jdk and openjdk tags are deprecated, we should use the newer vendor-specific tags. Use Eclipse Temurin since we already use it in our GH: Actions workflows.

Also update Maven to 3.9.6 - it includes several improvements.

@PDavid PDavid requested a review from anmolnar May 11, 2026 06:59

@anmolnar anmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm.

@eolivelli @ctubbsii PTAL

@phunt

phunt commented May 19, 2026

Copy link
Copy Markdown
Contributor

-1 - the documentation needs to be updated. Things like https://zookeeper.apache.org/doc/current/zookeeperAdmin.html - but please review all the docs to ensure covered.

Also the release notes (generated from JIRA) need to include this change, see : https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=135860428#HowToReleaseusingmavenreleaseplugin-Createreleasenotesonreleasebranch

@phunt

phunt commented May 19, 2026

Copy link
Copy Markdown
Contributor

fwiw claude(sonnet) says:

Documentation Files

  zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md — The main change. Update the Required Software section (~line 128) to set JDK 17 as the minimum instead of JDK
   8. Also review the TLS cipher suites section (~lines 1742/1899) which discusses Java 8 vs Java 9+ behavior — some of that context may no longer be relevant.

  README.md — Remove the caveat at ~line 42 about needing JDK 8 update u211 or later when compiling with Java 1.8.

  zookeeper-docs/src/main/resources/markdown/zookeeperStarted.md — No version text directly, but it links to zookeeperAdmin.md for requirements, so verify that reference
  still makes sense after the above change.

Build and CI files (not prose docs, but need to match):

  pom.xml — Update maven.compiler.source, maven.compiler.target, and maven.compiler.release from 1.8/8 to 17.

  .github/workflows/ci.yaml — Remove or replace the full-build-jdk8 matrix profile; make JDK 17 the primary.
  .github/workflows/e2e.yaml — Update jdk: [8, 11] to include 17 instead of 8.

The single most impactful documentation edit is zookeeperAdmin.md line ~128 — that's the canonical "required software" statement users and operators reference. The pom.xml
and CI workflow changes are mechanical but required to make the claim true.

Comment thread excludeFindBugsFilter.xml
Comment on lines +11 to +26
<Bug pattern="EI_EXPOSE_REP"/>
<Bug pattern="EI_EXPOSE_REP2"/>
<Bug pattern="CT_CONSTRUCTOR_THROW"/>
<Bug pattern="AT_STALE_THREAD_WRITE_OF_PRIMITIVE"/>
<Bug pattern="AT_NONATOMIC_64BIT_PRIMITIVE"/>
<Bug pattern="AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE"/>
<Bug pattern="PA_PUBLIC_PRIMITIVE_ATTRIBUTE"/>
<Bug pattern="DCN_NULLPOINTER_EXCEPTION"/>
<Bug pattern="US_USELESS_SUPPRESSION_ON_METHOD"/>
<Bug pattern="US_USELESS_SUPPRESSION_ON_CLASS"/>
<Bug pattern="US_USELESS_SUPPRESSION_ON_FIELD"/>
<Bug pattern="DMI_RANDOM_USED_ONLY_ONCE"/>
<Bug pattern="SS_SHOULD_BE_STATIC"/>
<Bug pattern="NP_UNWRITTEN_FIELD"/>
<Bug pattern="SING_SINGLETON_HAS_NONPRIVATE_CONSTRUCTOR"/>
<Bug pattern="MS_EXPOSE_REP"/>

@PDavid PDavid May 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually this suppression list are very broad. They are including real concurrency bugs (AT_STALE_THREAD_WRITE_OF_PRIMITIVE, AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE) and design issues (CT_CONSTRUCTOR_THROW, EI_EXPOSE_REP). Should we create a follow-up Jira to triage
and fix these incrementally?

Or should we suppress these by class so that these are not suppressed everywhere? We have 198 class/pattern combinations across these 16 patterns.

Comment thread .github/workflows/e2e.yaml Outdated

ZooKeeper runs in Java, release 1.8 or greater
(JDK 8 LTS, JDK 11 LTS, JDK 12 - Java 9 and 10 are not supported).
ZooKeeper runs in Java, release 17 or greater

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we define which ZooKeeper versions drop JDK8/11 support?

@PDavid PDavid May 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good question. 👍

I guess the current master (3.10.x) drops the JDK8/11 support for sure.

@anmolnar wrote this on the dev list:

“… we could make a leap and make JDK 17 the minimum runtime and compile versions for the master branch.

Once the change is merged to master, we'll backport it to branch-3.9 as follows:

  • minimum JDK for building: 17
  • minimum JRE for running: 8 (no change) “

See: https://lists.apache.org/thread/xrd4ct4lv6hqd7mb2dqtx8v1nkzy7kjq

So I guess only 3.10.x drops JDK8/11 support then. 🤔

Do you all think I get it right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's correct. Java 17 supposed to be the minimum required runtime and compile version for 3.10.x (or whatever version is going to be cut from master).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@anmolnar what's the criteria for a major version? Is dropping support for older JDKs something that users might expect to be in a 4.x release? (It doesn't affect me as all my infra is ready for this change already, I'm mostly curious)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I remember when JDK7 got dropped, it was also a minor version.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest rather discussing everything related to this change on the mailing list. It's probably going to be a 3.10.0 cut soon once this change is landed.

@anmolnar anmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

-1

I cannot execute gpg command inside the docker container.

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-gpg-plugin:1.6:sign (sign-release-artifacts) on project parent: Unable to execute gpg command: Error while executing process. Cannot run program "gpg": error=2, No such file or directory -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
andor@317a015b04a9:/Users/andor/work/zkrelease/zookeeper$ gpg
bash: gpg: command not found

@anmolnar

Copy link
Copy Markdown
Contributor

You need to add gnupg package to Dockerfile to be installed during provisioning.

Another problem is that Maven 3.9.x doesn't seem to be compatible with ZooKeeper. I got:

[ERROR] Unsupported policy: never
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
andor@1c56bb3be996:/Users/andor/work/zkrelease/zookeeper$ mvn -version
Apache Maven 3.9.6 (bc0240f3c744dd6b6ec2920b3cd08dcc295161ae)
Maven home: /usr/share/maven
Java version: 17.0.11, vendor: Eclipse Adoptium, runtime: /opt/java/openjdk
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "6.8.0-50-generic", arch: "aarch64", family: "unix"

when trying to run mvn clean install -Pfull-build -Papache-release -DskipTests.

Please investigate.

@anmolnar

Copy link
Copy Markdown
Contributor

FROM maven:3.8.8-eclipse-temurin-17 works fine for me.

private volatile long totalGcExtraSleepTime = 0;
private final AtomicLong numGcWarnThresholdExceeded = new AtomicLong(0);
private final AtomicLong numGcInfoThresholdExceeded = new AtomicLong(0);
private final AtomicLong totalGcExtraSleepTime = new AtomicLong(0);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

FYI: These were needed to fix the spotbugs issue:

[ERROR] High: Increment of volatile field org.apache.zookeeper.server.util.JvmPauseMonitor.numGcInfoThresholdExceeded in org.apache.zookeeper.server.util.JvmPauseMonitor$JVMMonitor.run() [org.apache.zookeeper.server.util.JvmPauseMonitor$JVMMonitor] At JvmPauseMonitor.java:[line 203] VO_VOLATILE_INCREMENT
[ERROR] High: Increment of volatile field org.apache.zookeeper.server.util.JvmPauseMonitor.numGcWarnThresholdExceeded in org.apache.zookeeper.server.util.JvmPauseMonitor$JVMMonitor.run() [org.apache.zookeeper.server.util.JvmPauseMonitor$JVMMonitor] At JvmPauseMonitor.java:[line 200] VO_VOLATILE_INCREMENT

@PDavid PDavid requested review from anmolnar, tamaashu and txangel June 17, 2026 10:22

@anmolnar anmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

gpg is still missing from docker

Comment thread dev/docker/Dockerfile
PDavid added 2 commits June 22, 2026 11:27
As of Mockito 5.x, the mockito-inline mock-maker became the default, so mockito-inline as a separate artifact is no longer needed.
PDavid and others added 14 commits June 22, 2026 11:27
- Upgraded spotbugs-maven-plugin to 4.9.3.0 and annotations to 4.9.3.
- Overrode ASM to 9.9.1 in the plugin dependencies to support JDK 25 (class file version 69).
- Added newly found bug patterns to excludeFindBugsFilter.xml - these are all pre-existing issues in the codebase that the newer SpotBugs detects but the old version couldn't.
to fix test issue:

Caused by: io.github.toolfactory.jvm.util.ObjectProvider$BuildingException: Exception occurred while retrieving the implementation of class io.github.toolfactory.jvm.function.catalog.ConstructorInvokeFunction (jvm architecture: x64, jvm version: 25, jvm vendor: Eclipse Adoptium)
Exception message differs now a bit.
Also update Maven to 3.9.6 - it includes several improvements.

Since the old `jdk` and `openjdk` tags are deprecated, we should use the newer vendor-specific tags. Use Eclipse Temurin since we already use it in our GH: Actions workflows.
- Replaced maven.compiler.source=17 + maven.compiler.target=17 with a single maven.compiler.release=17
- Removed maven.compiler.release property from m2e profile (it now inherits from main properties).
Replace volatile long fields with AtomicLong to ensure atomic
read-modify-write operations for increment and add.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
so that gpg command can be used by Maven.
@PDavid PDavid force-pushed the ZOOKEEPER-5039-mockito-upgrade branch from 76daf8a to 21a1c02 Compare June 22, 2026 10:50
@PDavid PDavid requested a review from anmolnar June 22, 2026 12:42

@anmolnar anmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add Maven enforcer rules to minimum Java and Maven versions just like in HBase.

Please see pull request: PDavid#1

@PDavid

PDavid commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Add Maven enforcer rules to minimum Java and Maven versions just like in HBase.

Please see pull request: PDavid#1

Great idea, thanks, will do that.

@PDavid

PDavid commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

CnxManagerTest.testWorkerThreads test failed in the GH Action build:

[ERROR]   CnxManagerTest.testWorkerThreads:511 Tue Jun 23 09:09:46 UTC 2026 Incorrect number of Worker threads for sid=0 expected 4 found 2 ==> expected: <null> but was: <Tue Jun 23 09:09:46 UTC 2026 Incorrect number of Worker threads for sid=0 expected 4 found 2>

It is successful for me locally on this branch:

$ mvn test -Dtest=CnxManagerTest -pl zookeeper-server

...

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.zookeeper.server.quorum.CnxManagerTest
[INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.496 s - in org.apache.zookeeper.server.quorum.CnxManagerTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:32 min
[INFO] Finished at: 2026-06-23T12:02:04+02:00
[INFO] ------------------------------------------------------------------------
 ws1test3@FHTL1T3  ~/projects/upstream/zookeeper   ZOOKEEPER-5039-mockito-upgrade  

@PDavid PDavid requested a review from anmolnar June 23, 2026 10:03

@anmolnar anmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm.

@tamaashu tamaashu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe [3.5.9, 3.6.3, 3.7.0, nightly] should be fixed, but it's not a blocker issue. Can be done separately.

Comment thread .github/workflows/e2e.yaml Outdated
@phunt

phunt commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

lgtm - I built with jdk21 and it built fine. +1.

@PDavid

PDavid commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

org.apache.zookeeper.test.ReconfigTest.testRemoveAddTwo test failed in the Jenkins build but it is successful for me locally on this branch:

$ mvn test -Dtest=ReconfigTest -pl zookeeper-server

...

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.zookeeper.test.ReconfigTest
[INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 245.043 s - in org.apache.zookeeper.test.ReconfigTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  04:13 min
[INFO] Finished at: 2026-06-24T11:11:39+02:00
[INFO] ------------------------------------------------------------------------

@anmolnar anmolnar merged commit 12d4cb4 into apache:master Jun 24, 2026
17 checks passed
@anmolnar

Copy link
Copy Markdown
Contributor

Merged to master branch. Thanks @PDavid and the reviewers!

@PDavid

PDavid commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Many thanks everyone for your reviews and help! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants