issue #608: stabilise SegmentOwnershipTransferTest waitFor deadline#622
Merged
eolivelli merged 2 commits intoMay 20, 2026
Merged
Conversation
Bump the per-cycle deadline in the file-local waitFor helper from 10s to 30s and thread a String desc through so a timeout failure pinpoints which predicate did not stabilise. The 10s budget proved too tight on contended CI runners (observed in run 25856862885 on PR #554) — the SegmentAssignmentWatcher's single-thread dispatch executor must process several ZK watcher fires plus an index re-list per transition, and 30s matches the deadline used by comparable polling helpers elsewhere in the indexing-service test suite (OptimizerMergeAdoptionRecallTest, IndexOptimizerSessionExpiredTest, OptimizerTaskRegistryIntegrationTest). Healthy local runs still complete in under a second per cycle.
- Call out that mvn checkstyle:check must NOT be invoked directly: with no project-level config the plugin falls back to Sun defaults and prints thousands of bogus violations (Spotless replaced Checkstyle in issue #544 / PR #554). - Reorder the pre-PR validation command so spotbugs:check runs after install, matching .github/workflows/ci.yml. spotbugs analyses the freshly-compiled .class files install writes; the previous ordering could leave it inspecting stale (or no) classes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #608.
Changes
herddb-indexing-service/src/test/java/herddb/indexing/segment/SegmentOwnershipTransferTest.java— raise the deadline in the file-localwaitForhelper from 10 s → 30 s and add aString descparameter so a timeout failure identifies the exact predicate. The 10 s budget proved too tight on contended CI runners (observed in run 25856862885, PR issue #544: replace Maven Checkstyle with Spotless #554) where the watcher's single-thread dispatch executor has to process several ZK watcher fires plus an index re-list per transition. 30 s matches the deadline used by comparable polling helpers elsewhere in the suite (OptimizerMergeAdoptionRecallTest,IndexOptimizerSessionExpiredTest,OptimizerTaskRegistryIntegrationTest). Healthy runs still complete each cycle in well under a second.CLAUDE.md(small unrelated doc fix surfaced while running pre-PR validation) — warn thatmvn checkstyle:checkmust not be invoked directly (it falls back to Sun defaults and reports thousands of bogus violations since Spotless replaced Checkstyle in Move from checkstyle to spotless #544/issue #544: replace Maven Checkstyle with Spotless #554), and reorder the pre-PR command sospotbugs:checkruns afterinstall, matching.github/workflows/ci.yml.Tests
SegmentOwnershipTransferTestpass locally with the new helper.repeatedTransfersFireTerminalEventsExactlyOncePerCyclerun 3× in isolation: 3 / 3 green, ~0.83 s each.mvn -B spotless:check apache-rat:check install -DskipTests spotbugs:check -Pci) green across all modules.This change does not touch any production code path — it only relaxes a brittle test timing budget and improves the failure message — so no new test was added (a "should not flake" test on a healthy local box would always pass and provide no signal).
🤖 Implemented by the `pr-worker` agent.