Skip to content

ZOOKEEPER-4993: Lower "Attempting to delete candidate container..." logging to DEBUG - #2430

Open
Pyolar wants to merge 1 commit into
apache:masterfrom
Pyolar:ZOOKEEPER-4993
Open

Pyolar wants to merge 1 commit into
apache:masterfrom
Pyolar:ZOOKEEPER-4993

Conversation

@Pyolar

@Pyolar Pyolar commented Aug 1, 2026

Copy link
Copy Markdown

This change lowers the candidate container deletion log from INFO to DEBUG to avoid excessive log volume while preserving the constructor configuration log at INFO.

A regression test verifies that the candidate deletion message is emitted at DEBUG level.

Jira: https://issues.apache.org/jira/browse/ZOOKEEPER-4993

Tests:

  • mvn -pl zookeeper-server -am -Dtest=CreateContainerTest -Dsurefire.failIfNoSpecifiedTests=false test
  • mvn -pl zookeeper-server -am -DskipTests compile checkstyle:check spotbugs:check
  • mvn verify spotbugs:check checkstyle:check -Pfull-build -Dsurefire-forkcount=4 reached 3,239 tests. It reported 7 unrelated OCSP/SSL failures in ClientSSLRevocationTest and QuorumSSLTest; all CreateContainerTest tests passed.

@Pyolar

Pyolar commented Aug 1, 2026

Copy link
Copy Markdown
Author

/retest

@Pyolar

Pyolar commented Aug 1, 2026

Copy link
Copy Markdown
Author

Jenkins build #1 reached compilation and failed only in two unrelated tests: ServerCnxnFactoryTest.testShedConnections_SmallPercentageRoundsToZero{FactoryType}[2] (probabilistic 1% shedding returned 1 instead of the test expected 0) and QuorumRestartTest.testRestartingLeaderMultipleTimes (30-second leader-election timeout). The PR-specific CreateContainerTest passed in that run. Focused local validation also passes on this commit: ServerCnxnFactoryTest 10/10, QuorumRestartTest 3/3, and CreateContainerTest 14/14. No source changes are needed for PR #2430; please rerun the Jenkins check.

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

You don't need to write unit test to cover a log level change.

@Pyolar

Pyolar commented Oct 8, 2026

Copy link
Copy Markdown
Author

@anmolnar Removed the unit test as suggested. The PR now only contains the one-line log level change in ContainerManager.java.

@gmazza

gmazza commented Oct 17, 2026

Copy link
Copy Markdown

@Pyolar I appreciate your efforts to get this super-simple matter fixed, but I'm not fully sure the Zookeeper team is dealing in good faith with us. A PR should not have been needed for this type of change, but once given, the unneeded test could easily have been taken out before merging, and I suspect that had your PR not had the unit test, they would have insisted upon it having one, i.e., no way to win. We tried, but it's been eleven-plus months, so I cancelled the ticket.

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