Skip to content

ZOOKEEPER-5012: guard zkDb close in QuorumPeer shutdown#2345

Merged
anmolnar merged 7 commits intoapache:masterfrom
seekskyworld:fix/5012-quorumpeer-shutdown-npe
Feb 19, 2026
Merged

ZOOKEEPER-5012: guard zkDb close in QuorumPeer shutdown#2345
anmolnar merged 7 commits intoapache:masterfrom
seekskyworld:fix/5012-quorumpeer-shutdown-npe

Conversation

@seekskyworld
Copy link
Contributor

Issue: https://issues.apache.org/jira/browse/ZOOKEEPER-5012

Summary

  • Guard zkDb close in QuorumPeer.shutdown when initialization did not finish.
  • Add a QuorumPeerTest regression for shutdown without zkDb.

Motivation

  • During restart, shutdown can run before zkDb is initialized; avoid NPE while preserving cleanup.

Validation

  • mvn -pl zookeeper-server -Dtest=QuorumPeerTest#testShutdownWithoutZkDbDoesNotThrow test

@seekskyworld seekskyworld force-pushed the fix/5012-quorumpeer-shutdown-npe branch from 9620c67 to 0703e20 Compare January 21, 2026 02:19
@seekskyworld seekskyworld changed the base branch from branch-3.9 to master January 21, 2026 02:20
Copy link
Contributor

@PDavid PDavid left a comment

Choose a reason for hiding this comment

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

Many thanks for the fix, looks good to me. 👍

Just added a small idea.

@seekskyworld
Copy link
Contributor Author

@PDavid I agree with you. It looks great

seekskyworld and others added 3 commits February 18, 2026 14:31
Co-authored-by: Dávid Paksy <paksydavid@gmail.com>
Signed-off-by: seekskyworld <djh1813553759@gmail.com>
Signed-off-by: seekskyworld <djh1813553759@gmail.com>
@seekskyworld
Copy link
Contributor Author

@PDavid I made two changes: replaced assertDoesNotThrow with a JUnit‑compatible try/catch to keep the “no exception” assertion, and added the missing fail static import so the test compiles.

Signed-off-by: seekskyworld <djh1813553759@gmail.com>
public void testShutdownWithoutZkDbDoesNotThrow() throws Exception {
QuorumPeer peer = new QuorumPeer();
peer.shutdown();
assertDoesNotThrow(peer::shutdown);
Copy link
Contributor

@PDavid PDavid Feb 18, 2026

Choose a reason for hiding this comment

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

Thanks for trying this out. 👍

I think this should have worked, just you have to add a static import for org.junit.jupiter.api.Assertions.assertDoesNotThrow next to the already existing static imports, e.g.:

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;

Signed-off-by: seekskyworld <djh1813553759@gmail.com>
@seekskyworld
Copy link
Contributor Author

@PDavid You're right. I didn't notice that the import wasn't included in the proposal. It has now been corrected.

Signed-off-by: seekskyworld <djh1813553759@gmail.com>
Copy link
Contributor

@anmolnar anmolnar left a comment

Choose a reason for hiding this comment

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

lgtm.

@anmolnar anmolnar merged commit 2f0d22b into apache:master Feb 19, 2026
16 checks passed
@anmolnar
Copy link
Contributor

Merged to master branch. Thanks @seekskyworld !

@seekskyworld
Copy link
Contributor Author

@anmolnar @PDavid I have also submitted four PRS here. Maybe you can take a look

@anmolnar
Copy link
Contributor

@anmolnar @PDavid I have also submitted four PRS here. Maybe you can take a look

Links?

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.

3 participants

Comments