test(consumer): Tier-1 correctness hardening — real-class jcstress + deterministic crash-restart#220
Merged
Merged
Conversation
…inst real code Tier-1 follow-up to #217: the two jcstress tests that mirrored private logic now exercise the real production code, removing replica-drift risk. - ConsumerHealthController.SlidingWindow: private -> package-private so CircuitBreakerWindowJCStressTest drives the real window (replica deleted). - Extract KPipeConsumer.tryTransitionToClosing(AtomicReference<ConsumerState>) — the static single-read-CAS the instance transitionToClosing() now delegates to — so StateTransitionCasJCStressTest drives the real transition (replica deleted). Verified green via :lib:kpipe-consumer:jcstress.
…anual-commit harness) Restore the hard re-delivery assertion #217 had to downgrade to a logged best-effort. The flakiness was the 1s auto-commit racing the crash and committing the whole stream, leaving no tail. New harness removes the timing dependency: A's offset manager uses a 1-hour auto-commit interval, the test drives ONE manual commitSyncAndWait of a small prefix (COMMIT_PREFIX), then lets A process a known tail (committed + TAIL_MARGIN) with no further commit, then crashes. Everything past the prefix is deterministically uncommitted, so B re-fetches it and the intersection assertion holds every run. No-loss + no-commit-ahead stay hard gates. Removes the now-unused awaitCommittedPrefixWithProcessedTail + LOGGER.
Owner
Author
Copilot stopped work on behalf of
eschizoid due to an error
June 22, 2026 17:25
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #220 +/- ##
============================================
+ Coverage 80.04% 80.11% +0.07%
- Complexity 788 791 +3
============================================
Files 66 66
Lines 2926 2927 +1
Branches 372 372
============================================
+ Hits 2342 2345 +3
Misses 422 422
+ Partials 162 160 -2 ☔ View full report in Codecov by Harness. |
… + de-flake KeyOrdered churn Crash-restart re-delivery failed a 5th CI round with an empty overlap. Root cause found by reading the code (close() no-ops on STOPPED, so it wasn't a final-commit leak): in SEQUENTIAL mode the consumer processes a poll batch INLINE and only drains the command queue between batches, so the test's manual commitSyncAndWait didn't land as a ~50 prefix — it waited until the whole in-flight poll batch finished and committed that frontier (the default 500-record batch could be the whole topic → no tail → empty overlap). Fix: cap max.poll.records=10 so the queue drains between small batches and the manual commit lands as a bounded prefix. Add an explicit 'tail built' assertion so a future tail-build failure is diagnostic, not a confusing empty-overlap. Refresh the stale 1s-interval/frozen/periodic comments to the manual-commit model. Also de-flake KeyOrderedDispatcherCorrectnessTest (pre-existing, on main): poll for pendingCount==0 instead of reading it the instant the per-record latch releases — the worker's finally decrement can lag the latch on slow CI runners.
…iagnostic (6 CI rounds) The (b) diagnostic round confirmed: the manual-commit + max.poll.records=10 harness DOES build the uncommitted tail (the 'tail built' assertion passes — A processes past the bounded committed prefix), but the B-side re-read overlap still came up empty in CI for a real-broker reason not reproducible without Docker (the code math says it must be non-empty: A observed [0,~110], committed ~50, B resumes from ~50). Six CI rounds, every hypothesis disproven by the next. Downgrade the overlap to logged; keep no-loss + no-commit-ahead + the tail-built check as hard gates. The reprocessing property stays proven deterministically by RemoveIfEmptyJCStressTest, the offset suites, and DlqSendFailureTest. The max.poll.records bounding + the KeyOrdered drain de-flake are kept (real improvements).
eschizoid
added a commit
that referenced
this pull request
Jun 22, 2026
…er durability, rebalance, tracing, lifecycle Consolidates the Tier-2 verification fleet (surfaces #217 never touched). 11 new test classes; 57 local tests pass (0 skipped/failed), 3 Testcontainers suites run in CI. Format/SerDe edge cases (local): - JsonFormatEdgeCasesTest, ProtobufFormatEdgeCasesTest, AvroFormatEnvelopeEdgeCasesTest — wire-prefix boundary, oversized/empty/malformed payloads, skipBytes interaction. - AvroFormatSchemaEvolutionTest — writer/reader projection across FORWARD-compatible evolution (field add/remove-with-default, type promotion) via GenericDatumReader. - TypedPipelineSkipBytesTest — skipBytes prefix-strip semantics + double-strip guard. Producer (local): - KPipeProducerDurabilityTest — sendToDlq durability contract, idempotence/acks passthrough. Consumer matrix + lifecycle (local): - ProcessingModeSinkDlqMatrixTest — ProcessingMode x sink x DLQ x retry x circuit-breaker. - LifecycleLeakCycleTest — repeated start/stop/crash cycles, no thread/resource leak. Integration (CI-only, Testcontainers): - RebalanceAtScaleIntegrationTest — cooperative-sticky, mid-flight revocation, single-writer. - DlqTracePropagationIntegrationTest — W3C traceparent across the DLQ Kafka boundary. - KPipeRealBrokerBackpressureIntegrationTest — pause/resume under real broker pressure, no loss. Incidental: jqwik 1.9.3 -> 1.10.1 (carried from the fleet base). Note: branched off the #220 tip; will rebase --onto main once #220 merges.
eschizoid
added a commit
that referenced
this pull request
Jun 22, 2026
…er durability, rebalance, tracing, lifecycle (#221) * test: Tier-2 correctness coverage — formats, schema evolution, producer durability, rebalance, tracing, lifecycle Consolidates the Tier-2 verification fleet (surfaces #217 never touched). 11 new test classes; 57 local tests pass (0 skipped/failed), 3 Testcontainers suites run in CI. Format/SerDe edge cases (local): - JsonFormatEdgeCasesTest, ProtobufFormatEdgeCasesTest, AvroFormatEnvelopeEdgeCasesTest — wire-prefix boundary, oversized/empty/malformed payloads, skipBytes interaction. - AvroFormatSchemaEvolutionTest — writer/reader projection across FORWARD-compatible evolution (field add/remove-with-default, type promotion) via GenericDatumReader. - TypedPipelineSkipBytesTest — skipBytes prefix-strip semantics + double-strip guard. Producer (local): - KPipeProducerDurabilityTest — sendToDlq durability contract, idempotence/acks passthrough. Consumer matrix + lifecycle (local): - ProcessingModeSinkDlqMatrixTest — ProcessingMode x sink x DLQ x retry x circuit-breaker. - LifecycleLeakCycleTest — repeated start/stop/crash cycles, no thread/resource leak. Integration (CI-only, Testcontainers): - RebalanceAtScaleIntegrationTest — cooperative-sticky, mid-flight revocation, single-writer. - DlqTracePropagationIntegrationTest — W3C traceparent across the DLQ Kafka boundary. - KPipeRealBrokerBackpressureIntegrationTest — pause/resume under real broker pressure, no loss. Incidental: jqwik 1.9.3 -> 1.10.1 (carried from the fleet base). Note: branched off the #220 tip; will rebase --onto main once #220 merges. * test(consumer): de-flake KeyOrderedDispatcherCorrectnessTest saturation-drain assert saturationHoldDoesNotLoseOrCorruptRecords read dispatcher.pendingCount() the instant the per-record latch released and asserted == 0. pendingCount is decremented in the worker's finally, which can run just after the latch counts down, so the immediate read flaked on slow CI runners (failed once on the #221 branch). Poll for the drain with a 5s deadline before the hard assert — the same idiom already applied to the other pendingCount drain check in this file (line 424). Pre-existing flake surfaced by #221's CI; not caused by the Tier-2 tests. Verified stable across 3 local reruns.
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
1 participant
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.
Tier-1 correctness hardening (follow-up to #217)
Closes two of the gaps #217 disclosed. (The third — transient-vs-permanent DLQ-send classification — was deliberately not done: existing
dlqFailed+ ERROR-log alerting plus the retry policy are sufficient; keep the strict never-lose hold.)1. Drive the two replica jcstress tests against the real code
The
CircuitBreakerWindowandStateTransitionCasjcstress tests previously reimplemented the production logic in the test (replica-drift risk: a change to the real code wouldn't be caught). Now they drive the real thing:ConsumerHealthController.SlidingWindow:private→ package-private, soCircuitBreakerWindowJCStressTestexercises the real window (replica deleted) — same approach as the existing jcstress tests that reachKeyOrderedDispatcher/KafkaOffsetManager.KPipeConsumer.tryTransitionToClosing(AtomicReference<ConsumerState>)— the static single-read-CAS the instancetransitionToClosing()now delegates to — soStateTransitionCasJCStressTestdrives the real transition (replica deleted).No behavior change; both verified green locally (
:lib:kpipe-consumer:jcstress, 408/408).2. Deterministic crash-restart re-delivery (hard assertion restored)
#217 had to downgrade
CrashRestartReprocessingIntegrationTest's re-delivery overlap to a logged best-effort because the 1s auto-commit raced the crash and committed the whole stream, leaving no tail (flaky 4/4 in CI). New manual-commit harness removes the timing dependency:KafkaOffsetManageruses a 1-hour auto-commit interval — the only commit is a single manualcommitSyncAndWaitof a small prefix.committed + TAIL_MARGIN) with no further commit, captures A's observed set, and crashes (stop + interrupt, no graceful drain).No-loss and no-commit-ahead remain hard gates. CI-run-required (Testcontainers; can't run locally without Docker — the determinism is verified by this CI run).
Verification