Skip to content

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

Open
seekskyworld wants to merge 7 commits intoapache:masterfrom
seekskyworld:fix/5012-quorumpeer-shutdown-npe
Open

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

Conversation

@seekskyworld
Copy link

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
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
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
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>
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.

2 participants