Repository navigation
Conversation
…duplicating it
The duplicate-code check had been reporting this on every push since 2026-08-13 and
nobody acted, including me - I noticed the reports early and carried on. Both files are
this branch's own (April), so it is duplication this PR introduced, which AGENTS.md
says to remove before merge.
ShardManagerStaleContainerTest and EpochAndRecordsMapRaceTest had each built the same
fixture independently: identical fields and an identical setup() differing only in
whether partitions were assigned. Now extracted to
BrokerlessWorkManagerTestBase, in the parent package so both subpackages can reach it.
The one difference is preserved as an explicit opt-in rather than smoothed away,
because it is load-bearing in BOTH directions and a shared setup that guessed would
break one of them:
ShardManagerStaleContainerTest needs the epoch-0 assignment - it has to have work to
make stale against.
EpochAndRecordsMapRaceTest must NOT assign, because the race it reproduces is
precisely a poll arriving BEFORE onPartitionsAssigned.
So the base does nothing by default and a subclass overriding assignPartitionsIfWanted()
says so, rather than every subclass inheriting an assignment it may be trying to avoid.
Both intents are stated in the base's javadoc and at each override, so the next reader
does not have to infer which way round it goes.
Also what the repo asks for generally: extend the shared helpers rather than growing a
parallel copy - a drifted copy of setup logic has already been a flaky-CI source here.
7 tests across the two classes pass unchanged; full unit suite 386, 0 failures.
…ed claim as unmeasured Capture from a planning conversation, so the reasoning survives the session rather than being re-derived. test-vertx-non-200-handling-untested.md - HIGH VertxTest.handleHttpResponseCodes is a disabled stub whose body is assertThat(true).isFalse(), and the 2026-08-08 audit judged it a "deletion candidate - there is nothing here to restore". That reads the body and misses the purpose. An empty or trivially-false test behind @disabled is being used as a TODO that carries a name and a location: it names the missing coverage and puts the reminder where the test belongs. Deleting it removes the reminder and leaves the gap. Stated by the repo owner: "the whole point of those empty tests was for me to remind myself that something needs testing that I haven't had time to do yet." So the audit's verdict is right about the body and wrong as an instruction, and the sweep that skipped it is the actual miss. Non-200 handling is untested in the vertx module while that module has live defects, so this is filed high. test-progressbar-width-needs-a-machine-assertion.md ProgressBarTest.width is the one disabled test that documents itself - a human eyeball check. Rather than delete it, split it: render to a string at a known width and assert the geometry (total width, truncation, narrow-width behaviour), and keep the visual demo as something a human runs deliberately, tagged out of the gating lanes. The assertion answers "is the rendering correct"; the demo answers "does it look right". Today neither runs. test-largenumberofinstances-residual-failures-unmeasured.md, plus the javadoc corollary That test's javadoc attributes its residual ~10% failure rate to the Kafka group coordinator rather than to PC. That is asserted and never measured: no experiment separates "the coordinator cannot converge at this churn" from "PC has a defect that only appears at this churn", and both present identically as instances alive with an empty assignment. It matters because the obvious response to a flaky stress test is to reduce the churn until it passes - and if any of the residual is ours, that hides a defect instead of removing a confound. Exactly what #68 did to confluentinc#857: uncontended brokers made the suite green and left the deadlock untouched for four months. The javadoc now carries that corollary and a TODO(refactor) pointing at the note, so the next person to reach for the parameters meets the caveat first. A control arm - the same churn against a plain KafkaConsumer group with no PC in the path - is what would settle it.
…i-consumers-bug' into bugs/857-paused-consumption-multi-consumers-bug-rename
…issing Corrects the durable record after e31b3e5 established that the fourth ChaosRevokeUnderWork sighting - listed here as sighting 7 - did have a replay seed all along: 4709156528562690268. The consolidated write-up carried the original claim, that no seed existed because the console log truncated, and used it as the closing example of why seeds must move into the AMBIENT PROBE AUTOPSY block. The example survives and gets sharper, because the claim itself was wrong in the same way one level up: "the log is truncated" was a statement about the RETRIEVAL ROUTE, not about the run. The job-log route returned 2207 lines ending at 05:47 for a job that ran to 05:52; the archived attempt returns 6266 lines with both autopsy blocks, BUILD FAILURE and all three scenario seeds. Also records the second trap that sits with it, which is new: after a re-run, the same job id resolves to the LATEST attempt's window - so re-running a job to "get a better log" silently changes what the query returns. Name the attempt explicitly: gh api repos/.../actions/runs/<run-id>/attempts/1/logs That makes four distinct ways a CI log has misled this investigation in two days: the job-log route truncating, GitHub truncating the stream server-side, a console that parsed as a clean run while the artifact carried the violation, and now a re-run silently redirecting the query. Each was read as a fact about the system rather than about the tooling. All twelve seeds - failing and control-arm - are now in the durable record. The claim that none has ever been replayed is unchanged and still accurate: the seventh now has one to replay, which it did not appear to before.
…e first seed that arrived with it A fifth sighting, from #296's CI: the same eager CLASS2_STALL signature, same 154s against a 150s bound, in PERIODIC_CONSUMER_SYNC - the only mode where #29's AB-BA cycle can close. What is new is the width, 37 violations and 52 frozen partitions against the handful the earlier entries record. It is the first sighting whose replay seed arrived with the violation rather than needing a second retrieval route. #296 moved the seed inside the AMBIENT PROBE AUTOPSY block, which is captured into the failsafe XML and uploaded as an artifact, so console truncation can no longer take it. The fourth sighting's correction records the incident that motivated that change. Two things worth more than the sighting itself. The eager arm has now failed on FIRST ATTEMPT at two of this branch's three heads, with no main-source file differing between them - the range touches docs, three chaos test classes, AmbientProbeExtension and its unit test only. That is a higher rate than "intermittent" implies. And re-running hides it. Both failing runs now report success at the run level, because both were re-run into green; the failures survive only in attempt 1. A re-run also draws a NEW seed, so a green re-run is a different experiment, not a retraction of the failure - which is exactly the inference this file exists to stop people making. The preamble's seed tally is corrected to match: five sightings, four with a reproducer, none replayed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… arguments An A/B measurement attributed a test flake to this branch and then to this specific cost, and the attribution is mine: replacing the pausedForThrottling volatile boolean with a derived consumer.paused() call made three log statements expensive, because their argument is now a live consumer call rather than a field read. SLF4J does not evaluate the format string when the level is off, but it does evaluate the ARGUMENTS, so all three ran on every loop iteration regardless of level. In production that is a synchronized consumer call per poll-loop pass, for logging usually switched off. In tests it is worse: each goes ThreadConfinedConsumer check -> Mockito spy interception -> synchronized MockConsumer, several times per iteration, on the poll thread specifically. That last part is why it showed up as a flake. ParallelEoSStreamProcessorTest.processInKeyOrder fills its input from a doAnswer on the POLL thread, then waits on control-thread loop cycles - nothing orders the two, so under CPU oversubscription the control loop can turn before the poll thread has been scheduled at all, and the input sanity check sees 0 records. Slowing the poll thread widened that window. Measured, matched-pair with both arms running simultaneously to control for load: 5 failures in 31 runs on this branch, 0 in 31 on master. All three commit modes hit, always the same [sanity check input data] signature. I had already guarded exactly this pattern one level down, in ThreadConfinedConsumer.checkThread, and missed these three one level up. Evidence for the fix itself is weaker than the evidence for the cause, and worth stating plainly: 0 failures in 8 runs under amplified parallelism, which at a 16% rate is not significant on its own. The guards are correct independently - an unguarded expensive log argument is a defect whatever it does to this test - so they land on that basis, not on 8 runs. The underlying harness weakness is master-side and NOT fixed here: asserting on input after a fixed number of control-loop cycles, when the input arrives on another thread. The same pattern appears in inFlightMessagesCommittedIfProcessedDuringShutdown, executorThreadsInterruptedOnShutdownTimeout and the JStream tests, and wants an event-based wait rather than a cycle count. Recorded in docs/inflight/test-untracked-ci-flakes.md.
…ed offsets, not the method that once carried them RebalanceEoSDeadlockTest is upstream confluentinc#548's regression guard for the confluentinc#541 transactional-mode rebalance deadlock. It proved "the revoke path committed" by overriding commitOffsetsThatAreReady() and counting a latch when the override ran on pc-broker-poll. This branch's revoke path commits via the private tryCommitOffsetsOnRevoke() instead, so the commit still happened but the latch never counted: measured, the test passed 5/5 on the defective build and failed 5/5 on the fixed one - a textbook inverted instrument (see docs/solutions/workflow-issues/ prove-the-problem-exists-before-writing-the-fix.md). It is why the Integration Tests lane was red on this PR. What the defect actually was, from the record (upstream 2738fb3): in PERIODIC_TRANSACTIONAL_PRODUCER, pc-control takes the producer transaction write lock (A, via maybeAcquireCommitLock -> preAcquireOffsetsToCommit) and then the commit mutex (B, in commitOffsetsThatAreReady); the pre-fix revoke callback on pc-broker-poll entered the commit path B-first, then A inside retrieveOffsetsAndCommit. AB-BA: the poll thread wedged in the rebalance callback (or died on ProducerManager's cross-thread write-lock ConcurrentModificationException) and the revoked partitions' completed work was never committed. The confluentinc#548 fence: the revoke callback waits out any in-flight transactional commit (the isTransactionCommittingInProgress spin - its predicate is "lock A held"), and isRebalanceInProgress stops control starting a new commit cycle mid-rebalance. The repaired detector observes behaviour instead of an internal method name: - Deterministic overlap: each pc-control commit cycle dwells 4s between acquiring A and touching B; the revoke callback waits (bounded, asserted via overlapForced so the test cannot silently go vacuous) for a fresh dwell before proceeding - the exact pre-fix fatal window, every run. - The outcome: the group's committed offsets for the revoked partitions, read via AdminClient at callback entry and again at callback return, must advance strictly during the revoke window - by the waited-out control commit or by the revoke path's own commit. Post-revoke processing is slowed 500x so redelivery cannot re-advance the offsets before the read. - Bounded revoke (30s), no recorded failure cause, and post-rebalance progress complete the guard. Mode stays PERIODIC_TRANSACTIONAL_PRODUCER deliberately: confluentinc#541 is a transactional-mode defect - lock A only exists there. That also bounds the scope: this test does NOT exercise the confluentinc#857 AB-BA cycle, whose second edge lives in ConsumerOffsetCommitter and only exists in the consumer-commit modes; Rebalance857CommitSyncDeadlockProbeIT covers that. Both arms measured, byte-identical test on each. Defect arm = this head with the confluentinc#548 fence disabled behind a temporary hardcoded boolean (spin and gate short-circuited; flag deleted before this commit), per the red-black-switch technique: fixed arm 10/10 pass, defect arm 10/10 fail, each failure naming the frozen committed offsets and each run logging the predicted "write lock already held by another thread" abandonment. Also fixes an accidental aliasing the upstream test always had: setupTopic("output-topic") overwrites the inherited topic field, so PC consumed from and produced back into the "output" topic in a feedback loop. The output topic is now genuinely separate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QiZYbruWmTFGkfepmZn2er
…data not loop cycles Two changes, one cause: an A/B measurement attributed a test flake to this branch (5 failures in 31 runs, 0 in 31 on master, matched-pair with both arms running simultaneously so load was controlled rather than assumed). 1. THE COST I INTRODUCED Deriving pause state from consumer.paused() instead of mirroring it in a volatile boolean is what makes the eager/cooperative distinction irrelevant - but it turned a field read into a live consumer call, and managePauseOfSubscription plus the three methods below it were making three to five of them per loop iteration on the poll thread. Three were in unguarded log ARGUMENTS, which Java evaluates before SLF4J ever checks the level, and two more were functional. The paused set is now read ONCE per pass and threaded down as a parameter. A parameter rather than a field on purpose: a stack local cannot outlive the pass, so there is no invalidation to get wrong and nothing that can go stale between iterations - which was precisely the failure mode of the mirror this replaced. It is also strictly fewer calls than before in the resume path, which used to fetch the set a second time to act on it. Drain and close keep a no-arg overload; they run once per lifecycle, not per iteration. 2. THE ACTUAL RACE, WHICH IS OLDER AND NOT MINE Reducing the cost did NOT fix the flake - it still fired once in four full suites - and that is the useful half of the result. ParallelEoSStreamProcessorTest.processInKeyOrder fills `polled` from a doAnswer on the POLL thread, then waits via awaitForOneLoopCycle(), which counts CONTROL thread cycles. Nothing orders the two: the control loop turns about every 100ms whether or not the poll thread has ever completed a poll, so under CPU oversubscription the assertion can run before the poll thread has been scheduled at all and sees exactly 0 records - all-or-nothing, because the MockConsumer hands over the whole batch at once. Now waits for the data instead. 4 consecutive full suites green afterwards, against 1 failure in 4 before; n is small, so the fix stands on being correct rather than on the count. The same assert-after-N-cycles pattern appears in inFlightMessagesCommittedIfProcessedDuringShutdown, executorThreadsInterruptedOnShutdownTimeout, queuedMessagesNotProcessedOrCommittedIfSubmittedDuringShutdown and the JStream tests - master-side hardening, tracked in docs/inflight/test-untracked-ci-flakes.md rather than swept into this PR.
…ministic correctness gate and a capacity measurement
MultiInstanceRebalanceTest.largeNumberOfInstances answered two questions at
once, badly: "does PC handle rebalance churn correctly?" (a correctness
question - must pass 100%) and "how much churn can PC survive?" (a capacity
question - a rate is a legitimate output). One test cannot answer both, which
is how it ended up documenting "80%+ pass rate" as an acceptance criterion - a
measurement wearing a test's clothes, in a repo whose rule is that a flake
fails the build with no retry.
What changed, all in the one test class:
- Parameterised the scenario into a Scenario builder (instance count, message
count, pre-produce fraction, poll delay, assignor, churn style, stall
window, ceiling) so one implementation - runScenario() - drives every
profile. The four pre-existing profiles map onto it with identical
parameters and identical behaviour, including the legacy ProgressTracker
stall detector (11 consecutive progress-free 1s checks), so the capacity
profiles' measured baseline is undisturbed.
- NEW gating correctness profile scriptedChurnRoundsCompleteWithoutStall (no
performance tag): 3 instances / 8 partitions / 30k records, cooperative
assignor, same code paths as the capacity profile. Deterministic by
construction rather than by dilution - a scaled-down copy of a probabilistic
test would just fail at 1% instead of 20%, rarely enough to be dismissed as
noise. Every probabilistic ingredient is removed, not reduced:
* churn is a scripted, event-anchored schedule (4 rounds of synchronous
stop -> assert survivors progress -> restart -> assert progress again,
round-robin victims, no Math.random anywhere);
* all records pre-produced and acked before churn starts (no producer
race);
* the assertion is PROGRESS, not completion-within-T: the consumed count
must advance within ProgressProbe.NO_PROGRESS_WINDOW (the chaos suite's
NO_PROGRESS model, reused not reinvented, as a sliding watermark - the
5-minute ceiling exists only to stop a runaway build). A slow machine
keeps passing; a genuine stall fails inside the window with a full
instance-state dump;
* uncontended broker via resetKafkaContainer() - contention is a confound
for the correctness arm. The #68 caveat is written into the
javadoc: uncontended hid the confluentinc#857 deadlock when applied to
everything, so it is right for this arm and deliberately wrong for the
capacity arm, which stays on the shared container.
Determinism evidence: 17/17 consecutive local passes (16 solo runs, ~45-53s
each, plus one inside a full-class run; 2026-08-18, Docker/TestContainers).
- largeNumberOfInstances keeps today's parameters and churn EXACTLY (verified:
a run under -Dincluded.groups=performance started all 12 instances on 80
partitions, fired 204 chaos rounds, and reproduced the documented residual
signature - instances alive, assignedPartitions=0, no PC failure cause).
Its javadoc now frames the pass rate as the profile's measured output
rather than an acceptance criterion, names the correctness twin, and keeps
the corollary that the "it's Kafka's fault" attribution is asserted, never
measured. Churn is deliberately untouched:
docs/inflight/test-largenumberofinstances-residual-failures-unmeasured.md
owns that investigation and its baseline, and gets a dated update noting
the split.
Defect-class sweep: "pass rate" as an acceptance criterion appears in no
other test across the modules (grep over parallel-consumer-*/src) - this was
the only instance.
Verification: full class green in gating mode (7/7 executions: both vanilla
parameterized profiles across all three orders, plus the new scripted
profile); TestConventionsArchTest green; bin/check-issue-refs.sh and
bin/check-copyright-headers.sh clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QiZYbruWmTFGkfepmZn2er
…p in its ledger docs/features/seeded-chaos-replay.yaml The suite's most distinctive property had no user-facing record: every chaos run is driven by a recorded seed, so a failure seen once can be re-run as the same scenario. docs/testing.md documents how to use it and testing-evidence.yaml mentions seeds in passing, but nothing states the capability itself - and it is the strongest available answer to "are the reliability claims tested or asserted". The boundary that matters most is written into the record rather than left to be learned: a seed fixes the SCENARIO - the sequence of stops and starts, and the workload - not the thread interleaving. So a reproduction is strong evidence and a non-reproduction is weak. Without that, "replayed it, did not reproduce, therefore fixed" is the obvious and wrong conclusion. docs/inflight/test-chaos-ledger-does-not-check-ordering.md The ledger already checks no-loss and bounded-duplicates, and ProgressProbe checks liveness - already recorded in docs/testing.md and testing-evidence.yaml, so not restated here. What is missing is ORDERING: "key concurrency without losing per-key order" is why this library exists, and the suite that hammers it hardest never asserts it. Recorded as small, with the reason it is not trivial. The recording exists - the ledger already receives allConsumed - but a naive "offsets per key must increase" would false-positive, because under at-least-once with rebalances an earlier offset legitimately reappears after a later one when uncommitted work is redelivered. The check has to bound ordering per key, per instance, per assignment epoch, and getting that boundary wrong gives a detector that fires on correct behaviour - worse than none, in a suite whose value depends on a RED meaning something.
…i-consumers-bug' into bugs/857-paused-consumption-multi-consumers-bug-rename
… the fix does not close the eager stall Two recorded seeds replayed for the first time, 16 local runs with the arms interleaved and the chaos sources byte-identical across them. Both seeds reproduce on BOTH arms: 11 of 12 seeded attempts failed, including six of six on the branch carrying #29's deadlock fix. The documented asymmetry makes this the strong direction - reproduction is strong evidence, non-reproduction weak - so this is not a result that a further run can soften. The one seeded pass is a seed fixing the scenario and not the interleaving, which is the behaviour the asymmetry predicts. What lifts it above a replay artefact is the control. Fresh random seeds passed three of four in the same window, so the recorded seeds are genuinely enriched rather than the box being unable to run the scenario - and the fourth control FAILED on the fixed arm from a fresh seed, reaching CLASS2_STALL with no replay machinery involved. It does NOT establish that the residual stall is the AB-BA cycle, and the file says so. The cleanest failing run has partition 22 committed-frozen for 100s of the QUIET phase - group STABLE, heartbeats flowing - against the scenario's own ~40s worst-case legitimate recovery. An anomaly by the test's own calibration, mechanism unattributed; the named next experiment is a fixed-arm failing run with periodic thread dumps. Two traps recorded so they are not re-learned: the ~154s figure is bound plus probe cadence, an artefact of the detector rather than a defect fingerprint; and the recurring per-partition fingerprints are workload-determined, appearing under both seeds, both arms and the control, so fingerprint identity must not be read as seed identity. The preamble's "no seed here has ever been replayed" is corrected rather than left to read as current. Confounds are recorded rather than resolved, including that the invocation ran the eager scenario alone where CI runs the whole chaos group in one JVM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ttribution, and retire the 154s fingerprint The replay spends the best evidence this family had for tying the eager chaos sightings to #29's AB-BA cycle, so three records that leaned on it are corrected where the claim lives rather than left to read as current. Mode-compatibility said the cycle COULD close in PERIODIC_CONSUMER_SYNC, and that was carrying weight it cannot support now the stall reproduces six times out of six with the fix applied. The mode table itself is untouched and still correct: it narrows the candidates, which is a different job from naming one. The second correction is the more useful one, because it was wrong before the replay and nobody had noticed. Two sightings were argued to be the same failure partly because both reported ~154s - but that is the 150s bound plus the probe's poll cadence, so every stall of this class reports it. The agreement was the detector's, not the defect's. The sameness claim now rests on same-test, same-arm, same-violation-class plus the arithmetic control arm, which is weaker than what was written and still sufficient. The family ledger gains the residual as a fourth open item, and its opening count is marked a floor rather than a total. This does not weaken the case for #29's fix, which is proven on its own instrument at 60/60 to 0/60; it removes a symptom that fix was never shown to own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s-bug-rename Both sides are fully renamed (254 and 244 bz/stub files, zero io/confluent), so this is the ordinary case rather than the pairing hazard AGENTS.md warns about. One conflict, in the log-truncation write-up, and it needed a side from each. Master's wording is right and is the fix this branch had just tripped over independently: the autopsy banner is cited as AMBIENT PROBE AUTOPSY rather than wrapped in the === decorations, because the real emitted line carries the test name between them, so the decorated form greps to nothing. This branch's citation path is right, because the split moved those two truncation incidents out of the family ledger into test-857-churn-storm-async-stalls.md - confirmed by date, the churn-storm file's fourth and fifth are 2026-08-12 and 2026-08-18 against the ledger's 2026-08-13. AbstractParallelEoSStreamProcessor auto-merged with both sides having changed its close and interrupt handling, which is exactly the case where a clean merge is not evidence, so the result was read rather than trusted. The two changes are in different methods and compose: #209 hardens work submission against a dead pool, this branch's awaitTermination loop records the interrupt in a local and restores it once after the loop. Re-ran this branch's own guards against the merged class - CloseInterruptLivelockTest, ThreadConfinedConsumerTest, BrokerPollSystemCooperativeRebalancePauseTest, ArchitectureTest and OutForProcessingCounterDriftProbeTest, 16 tests, all green. Also drops a forward-reference that the merge revealed had already been satisfied: tryCommitOffsetsOnRevoke told a future reader to interpolate describeWithRootCause "when it lands with #267", on the line above the one already doing it, since this branch cherry-picked ThrowableUtils. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… killed at the moment of detection Adds an off-by-default diagnostic mode to the revoke-under-work scenario, -Dchaos.diagnoseStallRecovery=true, to settle a question the gating configuration structurally cannot ask. The scenario's arithmetic asserts that "a REAL Class 2 stall is unbounded" and the probe's javadoc records its RED calibration as still open. Neither claim has ever been tested, and the reason is the instrument rather than the opportunity: failFast aborts the quiet phase on the first violation and QUIET_CAP gives up at five minutes, so every observation ends the instant the stall is confirmed. From that data a wedged partition and a merely slow one are indistinguishable - which is precisely the discrimination now needed, since the first seed replays showed the stall reproducing on both arms and left its mechanism unattributed. In this mode the quiet phase keeps waiting after a violation, up to twenty minutes, logging the consumption count each poll. The recovery curve is the measurement: flat to the cap means unbounded, so lost state - a partition paused and never resumed, or a lost wakeup - while a creeping count means bounded, so starvation or fairness, a different defect class needing a different hunt. It cannot turn a red run green, and the javadoc says so where someone reaching for it will read it: assertScenarioSlos still asserts the probe's violations are empty whichever way the wait ends, so a run that trips the probe still fails. It fails having recorded what happened next. With the property absent the gating configuration is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…er never checked The correctness ledger checked that no record is lost and that duplicates stay bounded. It never checked ordering - the guarantee the library exists for. Adds KeyOrderLedger and W5, ChaosKeyOrderIT, the first chaos scenario to run ProcessingOrder.KEY. The other two run UNORDERED over a unique key per record, so a per-key ordering assertion over them is doubly vacuous: no guarantee to keep, and no key with two records in it. The window is the whole difficulty. A naive "offsets per key must increase" fires on correct behaviour, because at-least-once redelivery after a revoke is the contract working. The guarantee asserted is: within one PC INCARNATION, one PARTITION, one assignment EPOCH and one KEY, records are executed one at a time in ascending offset order. Each component excludes one class of legitimate re-processing, and the epoch is read off the record's own WorkContainer through public API (PollContext#streamInternal), not from a counter at execution time - so a heavy record still in flight when its partition is revoked keeps the old epoch and cannot be mistaken for the new window's work. Two violations follow: LEDGER_KEY_ORDER (an offset regression) and LEDGER_KEY_CONCURRENCY (two deliveries of one key in flight at once). The second is not decoration - the UNORDERED control arm produced 857 overlaps and one order regression, so an offset-order-only detector would have called that arm green. Non-vacuity is enforced, not hoped for. A window holding one delivery asserts nothing, so check() reports LEDGER_ORDER_VACUOUS when no window held two, and logs comparedDeliveries - the count actually compared against a predecessor (measured 85,433 of 90,401 on a live run). KeyOrderLedgerIT pins both directions against constructed histories: a genuine regression is caught, and redelivery after revoke, redelivery to another instance, a restart of the same instance, and a heavy straggler overlapping the next epoch are all passed. Calibration cost two real workload artifacts, both now checked rather than commented. HEAVY_EVERY being a multiple of KEY_SPACE puts the entire heavy tail on key k-0, serialising every dwell onto one shard and pinning that partition's commit watermark for the run - measured as a 154s lag stagnation that reads as a Class 2 stall; heavyRecordsMustNotAllShareOneKey is the check. And a dwell longer than the gap between rebalances chains indefinitely under at-least-once, which is why HEAVY_SLEEP is 3s here and not W1's 45s. The conductor ticks slower than W1 for a stated reason: at 500-1500ms membership changes arrive faster than an EAGER rebalance completes, so the group never reaches Stable and the dwell probe's continuous clock passes its bound with no member misbehaving. No probe threshold was touched. Test-side only. ManagedPCInstance gains a RecordListener carrying the PC incarnation id, because a restart resets the epoch counter and a window scoped to the instance id would span two unrelated lifetimes. Live verification: the full chaos suite is green (5 tests), W5 at 90k records reporting orderRegressions=0 overlaps=0 with probe peaks well inside every bound (dwell 4.4s/15s, lag stagnation 22s/150s).
… into test/chaos-ledger-ordering
…ing gap's docs to their new owner Merges the branch head, where resolveSeed() now returns a ChaosSeed carrying its own replay command, and the seed is lifted into the autopsy block. Also records the prior art the ordering ledger did NOT reuse, in both directions. KafkaTestUtils#checkExactOrdering is the repo's only existing per-key ordering assertion, and it has no ordering window because it cannot need one - a MockConsumer, one instance, no rebalance, drained before checking, so no record can be redelivered. In that world it asserts something strictly stronger than the chaos ledger does (a gapless +1 value sequence of exactly the produced size, which doubles as a loss/duplicate check), and scoping it to a window would weaken it for its own caller. It also fails on the first duplicate under churn, so its javadoc now names its validity domain and points at KeyOrderLedger, rather than leaving a rebalance test to reach for it and read the false RED as a product bug. docs/inflight/test-chaos-ledger-does-not-check-ordering.md is deleted - its work landed - and the seeded-chaos-replay feature doc, which cited it as the open gap, now states what the ledger checks and who owns the window definition. docs/testing.md gains the ledger's two halves and the workload-artifact lesson; the Phase 2+ roster records that KEY ordering, its top-ranked unexplored Class 2 lever, has now been tried and is spent as a trigger. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…the hardware, and stop the gate from following The capacity profiles measured at whatever size someone once had a box for - 500k records, 80 partitions, 12 instances, welded into builder literals. That is too big for a laptop and too small for the 32-core runner, and in both cases it is silently the wrong experiment rather than a slow or fast one. -Dperf.scale=<n> now multiplies each capacity profile's own baseline, so their relative proportions hold and one flag moves all three. Absent, it is 1.0 and every historical number is reproduced exactly. The point is not convenience. A capacity profile is a MEASUREMENT, and a measurement that can only be taken at one size cannot answer the question test-largenumberofinstances-residual-failures-unmeasured.md is about: whether the residual ~10% is the group coordinator failing to converge or a PC defect. A rate that does not move with scale is hard to explain as "Kafka cannot converge at this size". That is weaker than the bare-consumer control arm that note asks for and does not replace it, but it costs a flag instead of a harness. The correctness profiles deliberately cannot read the factor, and that is enforced rather than asked for. A scale factor is exactly the probabilistic ingredient the deterministic gate was split out to remove, so scaling it would reintroduce by the back door the property the split exists to guarantee - and the failure would be silent, since the gating lane never sets the property. A run-time guard fails any correctness profile that reads the scale, deciding on the calling TEST method's own annotations so an intermediate helper cannot launder it. onlyCapacityProfilesMayScale proves the guard in both directions on real method names, including that a non-test helper is walked past rather than judged, and ends by reading the scale from itself - an untagged test - to confirm the throw happens end to end rather than only in the predicate. It is untagged on purpose: the invariant is a property of the GATING profiles, so checking it only in the performance lane would leave it unchecked exactly where it matters. It needs no broker work of its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ption-multi-consumers-bug-rename
…the defect the probe hunts Experiment 1, run under the diagnostic mode added for it. Seed A replayed with the quiet phase watching instead of aborting at the first violation: the run drained completely, 252421 of 250000 expected records with every key covered and no timeout. Legitimate recovery took 281s from storm end. The probe fires at 150s of per-partition stagnation and the gating run's failFast destroys the wait at the first violation, around 154s. So every observation of this signature to date has been taken at 154s of a drain that needed 281s. Fifty-three partitions eventually tripped the bound - four of them while already recovering - and all fifty-three finished. The probe's own javadoc says what it hunts: "a REAL Class 2 stall is unbounded, so the bound still catches it with 50s margin". A bounded stall is by that definition not that defect, so these sightings are the detector firing on slow legitimate recovery. That explains the replay grid without a surviving bug. A false positive does not care which arm it runs on, which is why both reproduced; the universal ~154s is bound plus poll cadence; and a green re-run is one whose interleaving happened to finish under 150s. The calibration arithmetic under-counts redelivery chaining. It reasons worst legit freeze = storm + one HEAVY_SLEEP + slack = 100s, which assumes a heavy record is redelivered once, but a 60s storm at 300-1000ms ticks can revoke it mid-dwell repeatedly and each link adds another dwell. The file had already named this exact failure mode at 90s/45s and retuned to 60s/20s to escape it; the retune fixed the single-chain case and left the multi-chain case. NOT a licence to retune the bound, and the record says so. One seed, one run, on a box that caps every JVM to 8 of 32 processors - which slows the drain and inflates the 281s, so an uncontended runner may fit under 150s and make this contention rather than calibration. The next step is seed B and an uncontended repeat, measuring the legitimate recovery time, not adjusting until green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he CPU arm settled Experiment 1 extended with the two arms that discriminate its result, plus the control arm they point at. Seed B under the diagnostic mode drained in 104s with ZERO violations, against seed A's 281s and 53. Seed A re-run with the JVM's processor cap lifted, 32 cores instead of 8, took 290s with 54 violations - indistinguishable from the capped run. So the contention hypothesis is dead: seed A's schedule intrinsically needs ~285s of quiet drain against a 150s bound, and seed B's needs 104s and never trips it. The scenario's arithmetic predicted 100s worst case and is out by roughly three times, for seed A's draw. The mechanism is now visible in the log rather than inferred. During the flat stretches the poll thread reports "Subscriptions are paused: true" and "Got 0 records" - back-pressure, working - while the in-flight batch is the deliberately heavy records sleeping HEAVY_SLEEP each. A partition's commit watermark cannot advance past an incomplete record, and stops are closeDontDrainFirst, so a storm tick catching a heavy in flight abandons it and at-least-once redelivers it for another dwell. Chain enough links and the watermark is legitimately pinned past the bound. ChaosRevokeUnderWorkDrainIT tests exactly that chain. Drain-only stops let the in-flight heavy finish and commit before the member leaves, so the chain cannot form; RESTART and JOIN_NEW keep their W4 weights so the churn RATE is comparable and the manner of leaving is the only variable. Duplicates measure the same thing from the other side - the no-drain arm produced 2421 of 250000, and a draining close should produce few or none. Either outcome is informative, and the javadoc says so: a clean drain arm makes the stagnation an artifact of abandoned work against the bound, while a red one kills the leading explanation and would be the first real Class 2 lead in this family. The weights hook it needs carries the warning with it. W4 excludes drains deliberately, because a drain opens the Class 1 zombie window that can mask the Class 2 mechanism, so an override changes which failure class the scenario can see. A variant that overrides it is answering a different question, not running the same test more gently, and its GREEN says nothing about the no-drain arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd put the prior art where it gets read Two fixes for two ways the last experiment misled its own author. The diagnostic mode logged only totalConsumed, which is incremented at the END of the user function. A fleet sitting inside HEAVY_SLEEP therefore reads as a flat line while it is fully busy, and that is indistinguishable from a fleet that is genuinely stuck - so the flat stretches in the seed A run were reported as possible stalls when the poll thread was concurrently logging "Subscriptions are paused: true" and "Got 0 records", which is back-pressure working exactly as designed. A counter is now incremented at ENTRY as well, and the mode logs started, consumed and the difference. In-flight work is the quantity that makes a flat completion count interpretable. The second is the prior-art miss. The scenario's own javadoc records that a recovery diagnostic was already run at the 90s/45s shape and already established that the freeze resolves and the backlog completes. That was re-derived in August 2026 by an agent that had run all six of AGENTS.md's prior-art checks correctly - because those six search plans, solutions, inflight notes, PRs and issues, and not one of them reaches a class javadoc. So the checklist gains a seventh row naming the javadoc, with the grep that finds them, and a bullet saying plainly that a test's javadoc is prior art the six commands cannot find. That helps someone who reads the checklist. The stronger fix is that the diagnostic mode now prints the pointer itself, when it starts, naming what the earlier run established and what the remaining question actually is - so the prior art arrives at the moment it is about to be repeated rather than waiting in a file to be opened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… rules The harness doc argued that a rule which must not be missed cannot live only in a document someone chooses to open. The same layer carries KNOWLEDGE, and that was never written down, so it kept being solved case by case or not at all. The distinction worth stating: a rule protects the repo from the agent and you can test whether it fires. Knowledge protects the agent from wasted work, and nothing goes red when it fails to arrive - the cost is paid silently, as an experiment repeated or a symptom misread. There is no feedback telling you the delivery was needed, so it has to be decided when the code is written. Two incidents a day apart are the worked examples, both recorded there. An agent ran all six of the prior-art checks correctly and still repeated an experiment whose result was recorded in a class javadoc, because not one of those six reaches a javadoc. And /tmp on the dev box hit 99%, which surfaced as a chaos log truncated mid-run - read as a test ending early, at real diagnostic cost - and as another session's watcher dying on ENOSPC, with neither agent able to see the cause. So the pre-commit hook gains a disk advisory: silent below 75%, and otherwise naming the filesystem and saying that tidying an agent's own /tmp scratch files is part of finishing a task rather than something only the repo gets. It blocks nothing - a full disk is not a defect in the change being committed, and a gate that blocks on it would be disabled within a day. Verified in both directions: silent at 32%, fires when the threshold is lowered. Silence below the threshold is the design rather than a detail. Advisory output competes for attention with every rule in AGENTS.md, which already names attention as the scarce resource, and a notice that prints every time is one an agent learns to skip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… name the timeout that silently caps a diagnostic watch Two things learned while running the recovery experiment, recorded where the next person meets them. The suite cannot model a CRASH. Every stop the conductor performs is an orderly close - stop() and stopAsync() call closeDontDrainFirst(), STOP_DRAIN calls closeDrainFirst() - so the member always leaves the group and a rebalance starts at once. A crashed process stops heartbeating with its assignment still held and nothing happens until the session timeout, which is the shape most confluentinc#857 reports describe. The family's most-reported condition is the one no scenario in the suite can generate. It is parked rather than queued because the fleet shares one JVM by design: the consumed and started counters, allConsumed and the ordering ledger's history are in-memory structures every instance writes to, so killing a process loses exactly the evidence a crash scenario exists to examine. The note records Antony's route through that - separate JVMs appending to files that a central chaos admin tails, so kill -9 is exact and the killed instance's evidence survives it - plus the two traps that route has: not on the dev box's /tmp, which is a shared 32GB tmpfs that chaos logs filled to 99% this week, and flush per record or the ledger under-reports precisely the records the crash is about. It also says why now is not the time. The open question is a Class 2 stall no scenario has reproduced in nine seeds, and the evidence points at detector calibration rather than a defect, so a multi-JVM harness is a large build aimed at a hypothesis that is currently losing. Separately, DIAGNOSTIC_QUIET_CAP's javadoc now names the ceiling above it. The scenario classes carry @timeout(600), so a quiet cap beyond about six minutes cannot be reached - JUnit kills the test mid-observation and the run then reads as one that stopped for its own reasons rather than one that was cut off. The default is twenty minutes, so the trap is armed out of the box. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s the detector meeting the workload Seed A held constant and run through four arms, each killing a different explanation. Unbounded stall: dead, the run drains fully in 281s. Machine contention: dead, 32 CPUs instead of 8 gave 54 violations and 290s. Abandoned in-flight work: dead, drain-only stops gave 45 violations against 53 and left 2007 duplicates against 2421. The cooperative arm names the cause. Same seed, same workload, same churn rate, and its storm phase ended having consumed 239808 records against the eager arm's 193715 - about 46000 more in the same sixty seconds - with ZERO violations, and it passes. Cooperative rebalancing keeps assignments; eager revokes every partition from every member on any membership change and restarts whatever heavy work was in flight. So each storm tick restarts in-flight heavies, each restart costs another HEAVY_SLEEP, a partition's commit watermark cannot advance past the incomplete record, and enough links pin it past the 150s bound legitimately. That also explains why the drain arm barely moved: draining the DEPARTING member's work cannot prevent a revocation that takes everyone's. This was root-caused once before, in July, at the 90s/45s shape - the scenario javadoc says so - and the response was to retune to 60s/20s so that 60+20+20 sat under the bound. That arithmetic assumes ONE restart. What is measured here is what several cost, and 281s is not near 100s, so the retune fixed the case it was derived from and left the rest. The eager sightings in this family should no longer be read as confluentinc#857 evidence. #29 was never implicated by more than mode-compatibility and the replay already spent that. Deliberately NOT concluded: that the bound should be raised. The honest options are to raise it with the multi-restart arithmetic written down, to shorten HEAVY_SLEEP so chains cost less, or to accept that the eager arm cannot host a Class 2 hunt and let the cooperative arm own it. That is the suite owner's calibration call, and the one way it must not be made is by nudging a threshold until a run goes green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…x, and name the probe's category error The fourth cell, cooperative with draining stops, closes the matrix on seed 4734674029169027864. Eager/no-drain 53 violations and 2421 duplicates, eager/drain 45 and 2007, cooperative/no-drain 0 and 405, cooperative/drain 0 and 369. The assignor accounts for essentially all of it and the stop mode is second-order in both rows, which is what duplicates being a product of REVOCATION rather than departure predicts: eager takes every partition from every member on any membership change, so the abandoned work was never the leaver's to drain. The clearest number is the last row's storm-end count. Cooperative with draining stops had consumed 249788 of 250000 before the quiet phase even began, against the eager arm's 193715 under the identical disturbance schedule. The matrix also makes a design fault plain, and Antony named it: the suite exists to establish CORRECTNESS, and CLASS2_STALL asserts a TIMING bound as a proxy for liveness. Every cell completed with nothing lost, so all four are correct; three fail only the 150s clock. A committed offset is silent while a long record runs, so a busy fleet and a wedged one are indistinguishable to a watermark-stagnation detector - and the diagnostic mode's new inFlight counter shows the eager arm holding 20-29 records in flight with consumed advancing every poll throughout the window it was called stalled. test-class2-probe-asserts-timing-not-correctness.md records the critique and the replacement: gate on completion, loss, bounded duplicates, ordering and a real liveness check such as in-flight never sitting at zero while a backlog exists - which detects a wedge and cannot fire on slowness - and REPORT recovery time and stagnation peaks instead of gating on them. It also says why raising the bound is not the fix: the July recalibration already did that once, from an arithmetic that assumed one restart of an in-flight heavy, and measured reality with several is 281s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…emo app an experiment to run Three records, one measurement. The assignor x stop-mode matrix over 250,000 records produced duplicate counts of 2421 eager with close(), 2007 eager draining, 405 cooperative with close(), 369 cooperative draining - a 6x spread that users currently have no way to learn except by hitting it. The README template gains a "reducing duplicate replay" section under the offset-map discussion, because that is where a reader already asking about replay will be. It says plainly that neither setting is specific to this library - both are ordinary consumer behaviour - and orders them by effect: the assignor governs work abandoned on EVERY member, since eager revokes every partition from everyone on any membership change, while the close mode only governs the instance actually shutting down. close() is closeDontDrainFirst(), which is the part people do not expect. The table is captioned as what it is. These come from the chaos suite, which stops and restarts members every few hundred milliseconds for a minute with deliberately slow records in flight, built to be far more hostile than production so effects like this are visible at all. The caption says not to read them as expected rates, and that the relative ordering is what carries over. It also repeats that replay cannot be prevented across failure, so the settings reduce how often the cost is paid, not whether it must be handled. The rebalance-behaviour feature record gets the same facts as boundaries, and the README rendered from the template. The demo-app ideation note gets the owner's addition: the demo should RUN these experiments and show results live, which is direction 3 - the bring-your-own-topic what-if machine - given an engine. The chaos harness becomes a UI toggle rather than a fork, since chaos-on is the dramatic demo and chaos-off against a user's own topic is the honest one; the diagnostic mode's per-poll consumed/started/ inFlight series is already the right shape to render, and inFlight is what makes work-in-progress visible rather than only throughput. Runs may be parallel or sequential, with the caveat recorded that sequential is the fair comparison because concurrent cells contend - a confound already measured here - so the choice must be explicit in the UI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… produced Seven candidates, all from an instrument being wrong rather than the product, which is why they generalise past this investigation. The headline one: the chaos suite judged PC from OUTSIDE, through committed offsets read by an admin client, while WorkManager and ShardManager answer the same questions directly and pc.getWm() is public. A test that infers internal state from an external signal will eventually infer it wrongly. Also recorded: measure both ends of any counter used to judge liveness, since a completion count alone cannot separate "nothing is finishing" from "nothing is happening"; assert the property and report the timing, rather than gating a correctness suite on a duration; and granularity is part of a liveness check's correctness, because the existing NO_PROGRESS probe has exactly the right shape but is fleet-wide, so one wedged shard hides behind seventy-nine healthy ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch has carried its knowledge in TOPIC-scoped files - bug-857-family, the per-mode sighting ledgers, the solutions write-up - and never had a PR-scoped one. The April investigation log was retired in 69a670d into exactly those files, which kept the settled knowledge and left the live threads with nowhere to live. So the compounding ideas from today's measurement work were briefly written into next-candidates.md, which is the ranked READY PICKS list and the wrong home for un-triaged observations; they move here and the pointer is removed rather than left to erode what makes that list useful. The note records what the PR is - the AB-BA deadlock fix, 60/60 failures before and 0/60 after - and, at more length, what it is NOT. The eager ChaosRevokeUnderWorkIT sightings were its leading candidates on the strength of PERIODIC_CONSUMER_SYNC being the one mode where the cycle can close, and they are neither this defect nor probably any defect. It also carries the open threads, which is the part that will earn its keep: what is in flight, the three honest options for the 150s bound with a note that nudging it until green is not among them, the ordering ledger gating only in the chaos lane, the fact that the four-cell matrix now quoted in the README is one seed and one run per cell, and the merge mechanics nobody has done - no review requested, no merge strategy recommended, duplication reports unread. Append to it as the work goes rather than reconstructing at the end. Several things today survived only because the owner asked at the right moment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6 tasks done
…the control thread (#466) In PERIODIC_TRANSACTIONAL_PRODUCER mode the revocation-time commit ran inline on the broker-poll thread and never drained the controller's work mailbox, so it could publish a transaction whose committed offsets omitted records that transaction contained: output committed, input offset not, and the partition's next owner reprocessed the input and produced the output again. #436 diagnosed it, refuted claims C9 and C4 of the transactional claim register, and quarantined the proof with a passing control arm. This is the fix. What changed for a user: exactly-once holds across a rebalance in transactional mode. RebalanceEoSDeadlockTest reads the output topic with a read_committed consumer after the revoked partitions return and fails on a repeated result; against the old code it found three to five duplicated results in about a hundred and ten, five runs of five, and reads zero five of five with the fix. C9 and C4 read PROVED again with their one documented exception, the proof left quarantine and carries both claims, and the README caution is gone. The fix. In transactional mode onPartitionsRevoked no longer commits on the poll thread. It posts a request, wakes the control loop with a mailbox message (never an interrupt, which a periodic commit's timed lock wait would read as a shutdown), and waits, bounded by commitLockAcquisitionTimeout. The control loop takes the request at the top of a pass and runs its ordinary sequence - write lock, flush, drain, fence, commit - then completes the request; a pass that throws fails it; a request the control thread has already taken is waited through to completion so truncation strictly follows the commit; the close serves whatever is pending at its own commit and the callback declines at once while the instance is closing. Waiting on the control thread is safe in this mode and only this mode: the deadlock behind confluentinc#857 is the control thread blocking on the poll thread, and a transactional commit needs nothing from it. The consumer-commit modes keep the inline tryLock commit. The fence. With the drain alone the broker-level check still read two or three duplicates per run: once the served commit released the write lock, a worker parked on the produce lock resumed with a record of a partition about to be truncated. PartitionState.fenceForRevocation is set by the served pass inside the write lock, for the assignment epoch the request was posted with, so a late pass cannot fence a re-assignment; the produce wrapper checks it right after the produce lock in both transactional modes and refuses with PCRetriableException. Rejected before writing code, by experiment: the position recorded at #436's merge prep was to decline the commit unconditionally. Two arms in ProducerManagerTest showed that a revoke which commits nothing leaves the output in the open transaction for the next commit to publish without its offset - the same defect through a different door. Declining is the deadline fallback, logged at WARN with that cost named, never the fix. Also: processWorkCompleteMailBox declares @ThreadConfined to the control thread with a runtime assertion; the confluentinc#548 sleep spin and its ArchUnit exemption are gone; the revoke-drain inflight note is retired into docs/solutions/logic-errors/the-revoke-path-commit-did-not-drain-the-mailbox-2026-09-07.md; STRATEGY.md's account of the register is brought current. Collides with #408 on tryCommitOffsetsOnRevoke; whichever lands second resolves it. Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
6 tasks done
astubbs
added a commit
that referenced
this pull request
Sep 8, 2026
…on to #408 #466 merged while the burn-down was being written: the revoke-path commit now drains the work mailbox first in transactional mode, its proof left quarantine with it, and the README caution is gone. The burn-down ticks it and moves it from "fixed in the queue" to "fixed on master". Its commit body records a collision with #408 on tryCommitOffsetsOnRevoke, to be resolved by whichever landed second. That is now #408, so its tier 2 line says so - it was already the one PR in the queue needing work rather than a merge. The #473 line no longer orders itself after #466; it clears the two quarantine entries that remain, which the release guard blocks on. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Sep 8, 2026
Each of these notes carried a PROPOSED vetting marker from the 2026-09-07/08 sweep. The owner accepted each proposal as stated; this applies them, and every claim was re-checked against the tree at this HEAD before the edit, not against the marker's own reading. Each surviving note's marker now records what was applied and what was verified. bug-shared-collections-across-the-poll-boundary.md - closed. Both named defects are fixed on master: PCMetrics.registeredMeters is a final LinkedHashSet with every add, walk and remove under @synchronized("metersLock") and close() walking it through the never-throwing removeQuietly (758744b, #120 / #57), and RemovedPartitionState.READ_ONLY_EMPTY_SET is Collections.emptySortedSet() (6aae299, #267). The file is kept rather than removed because static-infer-findings.md and core-139-public-api-thread-safety-contract.md both cite it; its "Live on master" headline is corrected and the branch section reduced to what is still true of fix/concurrent-collection-sweep, which still exists on origin with no PR. bug-torn-read-family.md - shrunk to the racing-double unification, the next hunt iteration and the out-of-family pointers. All three candidates are fixed and verified in the tree, so their dossiers were a FIXED narrative; they are replaced by a summary plus a history pointer, with candidate 1's bootstrapPhase-versus-dirty disjointness argument kept because it is the safety argument for the encoder's single-sample read and is recorded nowhere else. bug-177-commit-response-timeout-unreproduced.md and bug-857-mirror-attributions-unconfirmed.md - each shrunk to its #175 half. #177 was closed as completed on 2026-09-01, one second after #399 merged, with no comment naming either candidate mechanism - the outcome both notes existed to prevent. That is recorded once, in the attributions note, which owns the attribution question; the other cross-references it. Candidate 3 is closed: #29 and #57 have merged and PCMetrics.removeQuietly now swallows everything meterRegistry.remove throws. The filename keeps #177 so citations of it survive. bug-stale-sweep-iterator-evicts-fresh-replacement.md - shrunk to the remove-by-key defect, which is live: removeStaleWorkContainersFromShard still removes by key, so a fresh container the controller put at that offset is what leaves. The mechanism sentence is re-anchored on removeWorkAtOffset, and the #373 counter/stall block is removed - #336 (3e668a4) closed it and the note's own last paragraph already contradicted its middle. bug-857-transactional-revoke-wait.md - re-premised on #466. The unbounded spin is gone: in transactional mode the revoke callback posts to the control thread and waits bounded by commitLockAcquisitionTimeout (commitOnRevokeViaTheControlThread). What survives is whether that bound is right against confluentinc#803's complaint that it burns max.poll.interval.ms, which #408 holds along with its RebalanceEoSDeadlockTest amendment. Four stale citations repaired: the "no open PR addresses it" claim, the dead fix/bound-revoke-transaction-wait branch, the renamed core-recoverable-producer-fencing.md, and the two file:line citations, replaced with greppable anchors. bug-retry-queue-write-lock-on-the-rebalance-path.md - the defect is intact and stays the subject; "not started, and no design agreed" is replaced with #431, the open fix, which RetryQueue.remove's own javadoc already names as the tryLock() path being added. bug-wedged-after-poisoned-transaction.md - merged into bug-poisoned-transaction-not-aborted-while-running.md, the survivor the owner chose, and removed. Folded in: the answered recovery question and its answer (isTransactionOpen() is producerState.equals(BEGIN), so lazyMaybeBeginTransaction never opens a replacement and nothing in band aborts the poisoned one), the observed wedge, the two mitigations, and the freeze-behaviour test suggestion. Two citations repointed at the survivor; the third, in process-candidate-ranking.md, names the merge itself and is correct as written. bug-857-family.md - shrunk and retyped as a register, which is what its own closing sections describe it as; the impact stays stall. The revoke-path deadlock's mechanism section is retired per the file's own criterion now that #29 has merged; the "reproducer cannot settle anything" section is replaced with what the rewritten RebalanceEoSDeadlockTest and its purpose-built sibling Rebalance857CommitSyncDeadlockProbeIT establish; and the fifth item's claim that ProgressTracker.withDiagnostic has never been wired is corrected - MultiInstanceRebalanceTest now supplies describeFleet. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W511R2TZFAqEiqgS5yogTv
…e first sweep of the notes for v6 (#476) The road to v6 needs the in-flight notes vetted - which are still real, and in what order to read them - and nothing recorded when a note was last confirmed against the tree. git log could not say: the package rename of 2026-08-26/27 rewrote every file, so last-touched was the same day for nearly all of them, and the only way to know which of the open notes were still true was to read all of them. Tooling (bin/inflight.mjs): - `vet`: every open note on the baseline (or --ref a branch), partitioned on a new `<!-- inflight-vetted: YYYY-MM-DD - what was checked -->` marker, ordered by the index's group order and then oldest first by first-added date, each annotated with the cheap staleness signals a script can see - every cited fork number settled, the filename's number settled, a cited path or symbol that no longer resolves, a stated delete-when condition. A signal is a reason to open the note, never a verdict; the tool closes nothing. --area splits a sweep between agents. - `rank` orders the rows inside each impact bucket oldest first rather than alphabetically (operator ruling on finding the buckets were in filename order). The session-start index keeps its path order. - firstAddedDates in bin/lib/git.mjs, one git log for every note's first-added date; symbolsPresent tokenises the source in-process (112ms against 4.8s for the git grep it replaced); numbersByValue caches issue and PR states for a day, with the kind, since the two share a counter. - bin/check-inflight-tags.sh validates the marker's shape and names a `>` inside it as the cause. Self-tests for the gate and eleven negative-controlled checks in bin/test-inflight.mjs. Procedure: docs/grooming.md owns the sweep - when to run one, how it is split by area, the dispatch prompt, what comes back and where it is consolidated, and the traps the first sweep met. Routed from the root AGENTS.md table, the "Vetting a note" section of docs/inflight/AGENTS.md (the per-note contract: the marker, the five outcomes, the owner gate), the release procedure, and vet's own output. The sweep: six agents, one per area. Every open note on master now carries a stamp or a state. Owner-gated notes - a bug at stall or worse, the release record, a public-API contract - were not changed by the sweep; each carried a PROPOSED marker with the evidence, consolidated and ranked in process-candidate-ranking.md. The owner's pass on 2026-09-08 accepted every proposal: three closes, one merge (bug-wedged-after-poisoned-transaction into its sibling), one retype (bug-857-family is a register), and the shrinks and corrections, applied by three further agents; #139 was ruled out of v6 scope and its note deferred with its false volatility premise corrected. The register keeps the agents' reading of what gates v6, stated with their confidence. Overtaken by master mid-sweep: #466 landed the revoke-path drain fix, deleting one note the sweep had read as a v6 gate and removing the spin another described; both are re-premised here. Two out-of-area passages the sweep proved stale are corrected in place - the ruleset removals in docs/ci.md and the Jackson comment in .github/dependabot.yml - and docs/refactoring.md drops an entry describing a defect the tree no longer has. Rejected: a numeric priority field. Impact is the coarse order, a second axis would compete with it and rot the way the next- prefix did, and the v6 cut is the deferred state. Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Sep 8, 2026
…hecklist #476 vetted every open note and touched five this branch also edits. Three conflicted and each is resolved on the side that reflects the later decision: - release-0600-blockers.md: deleted here, modified there. Kept deleted; the two corrections the sweep made to it are folded into the burn-down's tag-day checks. One of them is only half right and is recorded as such: release.yml does build a notes file from the changelog (#72), but its exact heading match fails on the "(unreleased)" suffix and falls back to generated notes, which is what #199 fixes. The sweep's "the MDC gap is real" is refuted on the tree - MdcPropagation captures and restores the caller's context since #205 - so the burn-down keeps that correction. - release-experimental-module-records.md: the 2026-09-07 deferral wins over the sweep's vetted marker, per the per-note contract (a deferred note carries the state, not the marker). - release-when-is-v6-good-enough.md: the sweep's dated "the date passed" paragraph and its stale vetted marker ("the question is still unanswered") are dropped; the rewritten note records the decisions and names the failure mode in its own words. Also carried from the sweep: the citation to the poisoned-transaction wedge note is repointed to the sibling #476 merged it into; the burn-down now points at the sweep's own "what gates v6" list in process-candidate-ranking.md and records where the two disagree (the sweep reads the poisoned pair as not gating, and the batchSize validate() bound as the cheapest real fix, which could ride in tier 1); and the #44 exception is narrowed - #466 bounded the revoke wait, so what #408 still owns is declining rather than waiting. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
5 of 6 tasks
astubbs
added a commit
that referenced
this pull request
Sep 8, 2026
…s, and narrow the named exception Three edits from the owner's 2026-09-08 pass over the burn-down: - The vetting sweep's "what gates v6" reading moves out of process-candidate-ranking.md and into the burn-down, attributed as the agents' reading with their stated confidence, so the release has one note to burn down. Where it disagrees with the tiers the tiers now say so in the same place: the poisoned-transaction pair, the bound on the revoke wait, and the batchSize validation bound. The ranking note keeps a pointer and the ranking of everything else. - Every known unknown is now being pursued, each by its own agent on its own branch. The note records the one scheduling decision that matters: code-shaped questions run in parallel, replay-shaped ones one at a time, because several chaos or soak replays on one machine produce the starvation artefacts they exist to rule out. The replay order is written down so it survives the session. - The release claim's named exception narrows. #466 replaced the unbounded transactional revoke wait with one bounded by commitLockAcquisitionTimeout; what #408 still adds is declining rather than waiting. The release note says "bounded, not yet declined", which is what a user on that mode will actually experience. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…moves only the container it inspected (#468) BREAKING CHANGE: RecordContext equality changes. Its Lombok @EqualsAndHashCode covers the WorkContainer it wraps, so two contexts built from different containers for one record no longer compare equal - a Set, a Map key or an equals check over record contexts that used to collapse them now keeps both. Narrow in practice: ConsumerRecord does not override equals either, so two contexts from two different polls were never equal; what changes is two contexts over the same ConsumerRecord instance. WorkContainer itself is public in modifier only and is not part of the surface a user is expected to hold. THE DEFECT. The poller's stale sweep asked each occupant whether it was stale and then removed it by KEY, with removeWorkAtOffset(entry.getKey()). The staleness answer is about one container; the removal is about one offset. Between the two statements the occupant can change, and it does: the sweep runs on the broker-poll thread inside a rebalance callback, while addWorkContainer's stale-replacement branch runs on the controller, and nothing orders them. The harm is a LOST RECORD. The fresh container is evicted while PartitionState still carries its offset as incomplete, so nothing selects it and nothing completes it - the commit high-water mark for that partition cannot advance past it until the partition is re-polled. The accounting half was already closed (#336, #373), so the counters settled correct while the record was being lost: correct counters are not evidence that the right object left. It is the same fresh-replacement-at-one-offset class as the confluentinc#909 write-up, reached from the other side. THE FIX IS IN THE VALUE TYPE, NOT THE REMOVAL. Map.remove(key, value) is the JDK's compare-and-remove, and it decides "still mapped to the value I inspected" with equals - so what that call MEANS is a property of the value type and not of the map. WorkContainer.equals was topic, partition and offset only, so a stale container and its fresh replacement compared equal and no map API could express which of the two was meant. The equals/hashCode pair is deleted, equality is reference identity, and workMap.remove(offset, inspected) is a true compare-and-remove: it evicts the container the sweep inspected or nothing. Nothing evicted is a correct outcome, not a failure: the replacement won the offset, so the call retires nothing and REPORTS nothing. ShardManager.removeStaleContainers feeds the returned list to the retry queue, and #437 pinned that the queue removal is reached only through a real shard removal; reporting a container this call did not remove would take the retry entry at coordinates the fresh container now owns. getWorkIfAvailable's last-resort stale removal has the same shape and is written the same way, with a dated cleared-suspicion note saying why the race is not reachable there today and what would reopen it. REJECTED, and why each would have shipped looking correct: - computeIfPresent with an identity check in the remapping function. ConcurrentSkipListMap commits the removal through doRemove(key, v), which RE-READS the node value and gates on value.equals(reRead) before its compare-and-set, so with offset-only equality the identity test does not survive to the commit. Reproduced: a put landing inside the remapping function left the map EMPTY under offset-only equals and kept the replacement under identity equals. It narrowed the window to a few instructions inside the JDK and closed nothing. Under identity equality the same arm shows the gate declining directly: the remapping function runs TWICE, the first pass planting the replacement and asking for a removal that doRemove refuses, the second pass being handed the replacement. That is recorded at the site rather than asserted, since an implementation reaching the same gate without looping would be equally correct. - A ProcessingShard.Residency token as the map value, overriding neither equals nor hashCode. An earlier revision of this branch shipped it. It bought exactly one correct removal and left the value type wrong, so the next value-conditional site would have had to remember to wrap. Replaced before merge on the maintainer's call: 0.6.0.0 is the breaking release and already carries a Breaking section. - A remove-then-put-back repair, and a claim the sweep takes before removing which the writer must wait out. Both reintroduce a window, one in the map and one on the controller. THE OBJECTIONS THAT HAD QUEUED IDENTITY EQUALITY AS FUTURE WORK, each checked and each fallen: Comparable only RECOMMENDS consistency between compareTo and equals; SortedSet and SortedMap REQUIRE it, and nothing in main code puts a raw container in either - RetryQueue sorts by its own WorkContainerSortKey and de-duplicates by its own WorkContainerKey, so compareTo and its comparator are unchanged with the inconsistency stated in the javadoc. The one collection keyed on containers elsewhere, ExternalEngine.holdingDispatchPermit, has been an IdentityHashMap-backed set since #342, so identity equality makes the container agree with that code rather than changing it. The public break is RecordContext, stated above. THE OTHER HALF OF THE CONTRACT IS ENFORCED, NOT WRITTEN DOWN. Nothing puts a raw container in a sorted collection today, and both RetryQueue and ProcessingShard.workMap are one refactor away from being written the other way round with nothing going red. WorkContainerIsNeverInASortedCollectionArchTest reads every field, return type and parameter in main code whose declared type is a sorted set, sorted map or priority queue and whose FIRST type argument - the element-or-key position for all of them - is WorkContainer, so a container in a sorted map's value position passes. Proved red on main code with a temporary TreeSet<WorkContainer> field on ProcessingShard, which reported that field and not the workMap declared beside it. A standing positive control, in the RebalanceCallbackRuleControlTest shape, hands a hand-imported fixture to the real rule object so the rule cannot go quietly blind. Its limits are in the test javadoc: a body-local collection is invisible, the comparator is invisible so a legitimate Comparator-taking construction is still reported, and it covers core's classpath - measured, no main source outside core declares any of those types. EVIDENCE. ShardStaleSweepReplacementEvictionTest drives the seam deterministically rather than by racing: ShardSeamTestBase's spied PartitionStateManager lands the controller's replacement at an exact instruction inside the sweep. The defect arm was red against master. Restoring the old coordinate-based equals/hashCode and changing nothing else sends the defect arm and the premise arm red - "expected specific instance ... but was: null" and "expected not to be" the same coordinates - while the control arm stays green, as it must: a sweep with nothing racing it never has to say which of two containers it meant. Restoring removeWorkAtOffset(entry.getKey()) in the sweep sends exactly the defect arm red. The premise arm, twoContainersAtOneOffsetMustNotBeInterchangeable, asserts the equality contract directly and is the tripwire for a reintroduced coordinate-based equals. TREE-WIDE SWEEP for what identity equality breaks: one test assertion, ShardManagerTest's isNotEqualTo between containers at different offsets, became vacuous and is restated against RetryQueue.WorkContainerKey, which is what the queue keys on. PollContext.getByTopicPartitionMap's Set de-duplication becomes a no-op with no observable change, said in an @implNote at the site. Checked and found empty: no ArgumentCaptor or Mockito matcher over a container, no Truth containment assertion over containers except on the same instance, no container as a Map key or Set element in any test helper, and every Lincheck harness returns primitives from its operations in stress mode. SAME-DEFECT-CLASS SWEEP over core main, with a correction to this branch's own earlier result: an earlier commit reported holdingDispatchPermit as a real unfixed hit and opened an inflight note for it. That defect does not exist - the sweep matched the declared type Set<WorkContainer> and stopped, and a collection's semantics live in its initialiser, not its declaration. The note is deleted and the lesson is in the solutions write-up. Dismissed with reasons: ProcessingShard.onSuccess, removeWorkAtOffset from the revocation path, RetryQueue, the metrics and counter maps, OffsetSimultaneousEncoder.activeEncoders, MDC.remove. CI SIGHTING RECORDED, NOT DIAGNOSED. Chaos Pain Suite shard 4 went red once on ChaosChurnStormIT's ZOMBIE_MEMBER/REBALANCE_BLOCKED arm with maxInstanceStall=0ms; that line has an open ledger on master, and seed 1053013618367208111 is appended to docs/inflight/test-857-churn-storm-async-stalls.md because the seed dies with the CI log. Nothing on the rebalance path reads WorkContainer.equals, and the shard was green on every later push. DOCS. docs/inflight/bug-stale-sweep-iterator-evicts-fresh-replacement.md is resolved and deleted, its mechanism and rejected alternatives migrated to docs/solutions/logic-errors/a-by-key-removal-cannot-say-which-container-it-meant-2026-09-07.md, cross-linked with the confluentinc#909 write-up. docs/refactoring.md's queued-breaking-change section now states that an entry is deleted in the PR that lands it, because the commit message carries the release-note content and the section lists only what is still queued; the entries marked DONE there are removed, and the two citations that pointed at this branch's own entry - WorkContainer's class javadoc and the solutions write-up - name this commit as the record instead. docs/inflight-tool.md used the resolved note as the worked example for the vet command; the example now names a surviving note with the same data-loss signal, bug-run-length-plausibility-ceiling.md. Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
…he stall its first runs found (#471) #175 (confluentinc#809) has reported `Timeout waiting for commit response` for months and had never been reproduced, by anybody. docs/inflight/bug-177-commit-response-timeout-unreproduced.md says so in its own title and names the experiment that would change that. This is that experiment, the opt-in lane it runs in, and its first two runs - which found a different stall instead. THE SCENARIO. CommitResponseTimeoutSoakIT transcribes the application code #177's reporter posted in full - that report is closed and is kept only as the source of the shape, since it is the cheapest route to the wedged-poller question #175 still poses: KEY ordering over a thousand distinct keys, PERIODIC_CONSUMER_SYNC at a one-second commit interval, maxConcurrency 14, a ~100ms user function, and half the records throwing. One instance, no churn - the report involves no rebalance, which is the deliberate difference from every other scenario in the chaos package. The load-bearing choice is that a poisoned record throws on EVERY attempt, not on about half of them. The reporter's flag is a per-record header set on i % 2, so a marked record never succeeds; under KEY ordering it head-of-line blocks its key's shard and pins that partition's committed offset for the rest of the run. That is what makes "runs for a while and then exits" an ACCUMULATION rather than a startup race; a per-attempt coin flip would let every record eventually succeed and the thing being hunted would never build up. The per-attempt shape is a separate arm. It asserts exactly one thing - that no commit-response timeout occurs - and wires no ProgressProbe, because this workload pins its own commit watermark by design and the Class 2 lag detector would observe continuously while saying nothing about the question. Classification is already in the product: since #204 a message naming the poll thread's death is #100's class and a bare timeout is the poller wedged-but-alive, carrying PollThreadStallDiagnosis's BLOCKED-vs-SLOW verdict. The scenario adds the half the product cannot cover - a full JVM thread dump at the moment of detection. THE LANE. The soak tag is new and is in no suite at all: it is in the pom's excluded.groups and in every gating wrapper. It could not join chaos, whose scenarios are calibrated detectors that gate every PR and whose local recipe would pick a half-hour class up. docs/testing.md gains the lane, what a soak result is (a rate under conditions, never a verdict), where it gets recorded, and that this lane is not bin/soak-test.sh, which repeats a short test under CPU contention to measure a flake rate. THE RUNS, recorded as rates with conditions in the inflight note and the scenario's Calibration status. Two thirty-minute runs, zero timeouts - and neither is a sighting-ledger entry, because in both the assertion could not have failed after the first minute: failureFraction 0.5 (#177's reporter's) seed 3747722682837130843 90,000 produced, 451 succeeded, 237,006 failed, no findings failureFraction 0.03 seed 5055695573431537469 90,000 produced, 2,372 succeeded, 81,114 failed, no findings WHAT THEY ACTUALLY MEASURED. Successes froze inside the first minute of each run and never moved again, while the producer kept publishing and the failure count climbed at a rate that then held exactly constant. A constant retry rate against a frozen success count means no new record is being taken as work at all: the instance has stopped, not merely slowed. A stalled instance cannot reach the exception being hunted - only PartitionState#onSuccess marks a partition dirty, onFailure is an explicit no-op, and the control loop gates commits on isDirty, so with no success anywhere no commit request is enqueued and ConsumerOffsetCommitter#commitAndWait, the sole thrower of the message, is never entered. A green assertion here cannot tell "no timeout occurred" from "no commit was attempted". This is the dirty asymmetry upstream-tell-809-833-the-hang-is-fixed.md names for this very workload. The 0.03 arm is the control that makes the fraction the wrong knob rather than a guess: it bought about 1.5 extra bursts and stalled identically. Duration is the wrong knob too. ONE MECHANISM ELIMINATED, ONE NAMED. Offset-encoding back pressure is eliminated: PartitionState#updateBlockFromEncodingResult logs every transition and neither message appears once in either log. The named, untested candidate is the load gate - WorkManager#isSufficientlyLoaded compares inShards minus parkedForRetry against the in-flight target, and inShards counts records queued BEHIND a blocked shard head while only the failing head is parked. That is the silent-stall shape the gate's own comment names against confluentinc#857. Stated as a hypothesis; the next arm reads the gate's own DEBUG line and settles it in one run. The stall may be the better lead than the timeout: confluentinc#833's reporter showed pc_processed_records_total FLAT across the window in which their timeout fired, which is this state, not a busy one. TWO DEFECTS IN THIS BRANCH'S OWN FIRST COMMIT, both silent. The soak tag had been added to the comment above excluded.groups, to the javadoc and to docs/testing.md, and not to excluded.groups itself, so a six-hour scenario sat in the GATING integration lane - measured, not inferred: this PR's own Integration Tests job ran 35 minutes with the scenario's banner in its log. QuarantinedAnnotationContractTest could not have caught it, since its list checks compare the pom against the wrappers and a tag missing from both is consistent; it now also asserts membership of the named opt-in lanes (pomExcludesEveryLaneTooLongToGateAMerge). And the documented run command could not run: -am builds the parent module first and failsafe fails the reactor there before core is reached, the trap bin/chaos-test.sh's header already owns. Both copies of the recipe now pass -Dfailsafe.failIfNoSpecifiedTests=false and say to read the scenario's banner out of the log before believing a green, since that flag lets a run selecting nothing exit 0. WHAT MASTER SETTLED WHILE THIS WAS OPEN, reconciled in the merge rather than left contradictory. The inflight vetting sweep shrank the note to the #175 half, closed the throwing-metrics candidate against the tree now that #29 and #57 have merged, and recorded the AB-BA wedge in upstream-175-sporadic-commit-timeouts.md as fixed by #29. So this branch's earlier reading that candidate 2 was "owned by #29, not unowned" is withdrawn: candidate 2, the poller wedged but alive, stays the open one with no named mechanism inside it - which is why the intake stall these runs found is the more promising of the two leads. Arm 3 of what to run next no longer nominates a "better wedge candidate"; it survives as the closest recorded configuration to the live report. The note's opening sentence that no reproduction attempt existed is corrected to name this one. Static analysis on lines this PR wrote: StringCharset fixed with StandardCharsets.UTF_8; the deprecated RandomUtils.nextInt() deliberately kept, because it is the topic-naming pattern in three sibling chaos scenarios and changing one of four leaves the odd one out, so docs/refactoring.md carries one item covering all four. Prior art recorded in the note: the unmerged branch test/177-commit-response-timeout, which confirmed the #100 trigger reproduces when the catch is removed and records one falsified experiment (moving brokerPollSubsystem.supervise() earlier in controlLoop() does not help). Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
…nswers, not when it is sent (#470) Under PERIODIC_CONSUMER_ASYNCHRONOUS - the shipped default commit mode - AbstractOffsetCommitter marked the partition clean on the line after commitOffsets() returned. commitSync and the transactional producer return only once the broker has answered, so that marking is a statement about something that already happened. commitAsync does not: it returns as soon as the request is handed to the client and answers later through an OffsetCommitCallback. So the offsets were recorded as durable at the moment of SENDING, and the callback was logged and nothing else, carrying a "todo keep work in limbo until async response is received?" from years ago. WHAT THAT COST. Marking clean ends the story: nothing is dirty afterwards, collectCommitDataForDirtyPartitions() returns empty, and no further commitAsync is ever issued for those offsets. A callback arriving with a RetriableCommitFailedException had nothing left to retry, and a callback that never arrived was indistinguishable from success. The broker's committed offset stayed behind PC's belief, silently and permanently, and the pc.partition.latest.committed.offset gauge read an offset the broker never had, so whoever owned the partition next resumed from a position the broker never recorded. Silent loss, in the default mode. THE FIX, inside the async branch. AbstractOffsetCommitter gained commitOffsetsReturnsOnlyOnceAcknowledged(), true by default, and marks success inline only when it holds; onOffsetCommitSuccess became protected so a committer answering false can call it on its own acknowledgement. ConsumerOffsetCommitter returns isSync() from it, and its async branch registers onAsyncCommitAnswered: success records the offsets, a failure logs and leaves them dirty for a later request to carry, and a dropped callback does the same by doing nothing at all. A failure is a DEFERRAL and lands where the synchronous path's rejections already land - logged, not fatal, offsets still marked as needing a commit - which is the option commitDeferringOnRebalance()'s javadoc argues for, reached from the other direction. Only an acknowledgement advances the state. THE HAZARD THE FIX CREATES, AND THE ONE RULE FOR IT. Deferring the clean mark is what makes two async commits able to be in flight at once, so an answer can arrive for a request a later one has partly overtaken. The whole of the handling is one rule inside PartitionState: getCommitDataIfDirty remembers the offer it makes, WHOLE, as the OffsetAndMetadata it handed out; onOffsetCommitSuccess records every acknowledgement - the answer is true, the broker committed up to it, and recordCommittedOffset keeps the higher of the two when answers arrive out of order so the gauge cannot walk backwards - and marks the partition clean only when what was acknowledged equals that offer. Whole rather than the offset alone, because completing a record above the lowest incomplete one changes the encoded metadata and moves the offset not at all, so two requests can be in flight at the same offset with different metadata; compared on offsets, the older answer would clean the partition and the newer request's metadata - the only record that the higher record is done - would never be re-sent if that request failed. Comparing the whole object is exact, since commitAsync hands the callback the very map it was given and the transactional path calls onOffsetCommitSuccess inline with the map it just collected. Every edge lands on the dirty side, which costs at most one extra commit and cannot under-report; a partition state rebuilt by a rebalance has no offer recorded, so a stale acknowledgement cannot clean it either. The committer keeps no record of what it has in flight, and that is the point. Two earlier revisions put the decision in the committer - a sequence number that ignored any answer that was not the latest, throwing away a fact the broker had established, and then a per-partition in-flight map with a second success method plumbed through four classes - each a copy of state the partition already held. Both were replaced on the maintainer's call by the one rule above; the per-partition outcome they implemented now falls out, because each partition made its own offer and each partition decides. setClean's existing commit-window protocol is untouched, and the counter protocol landing separately for the commit-window pair is deliberately not pulled in. THE FAILURE LINE IS ONE LINE, and its level is the classification: can a later request fix this? Transient - RebalanceInProgressException, CommitFailedException, RetriableException - is a WARN saying the partitions stay dirty and are committed when a later request is acknowledged. Anything else is an ERROR saying every later request fails identically, the committed offset stops advancing and the retries will not succeed without intervention; logging that at WARN with the deferral promise attached would tell the operator to wait for something that will not happen. The classification is the synchronous path's own - commitDeferringOnRebalance() catches exactly those two exceptions and lets the rest escape - so the two modes agree on which failures are routine. Neither level escalates: the async mode has no commit budget and cannot reach the commit-failure seam (#317, #352), whose options validation rejects a handler under this mode, and throwing from a broker-poll-thread callback strands every waiting committer. The #168 (confluentinc#629) bound applies to both lines: offsets summarised, the map one level down at DEBUG, each level with its own text budget so neither can become the door the offset map comes back through. THE FIELD'S THREAD MODEL. offerLastMadeForCommit is written in the commit path and read back in onOffsetCommitSuccess by the same thread in every commit mode - the broker-poll thread under both consumer modes, the control thread under the transactional mode, which #466 made uniform - with Consumer#close() flushing a pending callback as the one hand-over, carrying a happens-before edge. It is plain, for the reason the dirty field's javadoc gives from jcstress measurements, and docs/refactoring.md records it with stateChangedSinceCommitStart as one lifecycle to be declared together if anyone does the thread-confinement work. Only the latest offer is kept, assigned rather than raised to a maximum, because maybeTruncateBelowOrAbove's downward reset on a bootstrap poll would leave a maximum unmatchable and the partition dirty forever. EVIDENCE. Two red tests landed first, one per way a sent request never becomes a durable commit: MockConsumerAsyncCommitCallbackFailsTest and MockConsumerAsyncCommitCallbackDroppedTest, asserting the broker's own committed offset because every weaker assertion passes on the defect. Both control arms flip exactly the new predicate: forced true, both async tests fail with commit attempts stalling at two of three withheld acknowledgements and the sync control passes; forced false, the sync control fails because attempts never stand still and the async tests pass. That third arm is why MockConsumerSyncCommitMarksCleanOnReturnTest exists. PartitionStateAcknowledgedCommitOffsetTest pins the rule on a real PartitionState, including the same-offset-different-metadata case that was red before the offer was held whole, and ConsumerOffsetCommitterOverlappingAsyncCommitTest drives the three orderings through the committer over a real WorkManager with two captured callbacks - the MockConsumer answers inline, so it can never reach the overlap. Reverting only the PartitionState rule sends four of those seven red, each naming the partition marked clean at the older offset while the higher offer was still outstanding. OTHER INSTANCES of the class - state advanced on request rather than on acknowledgement - checked and dismissed: the produce path blocks on the send futures before the batch is marked succeeded; the transactional commit is blocking, so the base default is right for it; ConsumerManager.commitSync is blocking and already refuses to return normally when close aborted its retry loop; Vert.x, Reactor and Mutiny all mark success from the completion signal, with ExternalEngine suppressing the dispatch-time marking for exactly this reason. None found. DOCS. The inflight note recording the defect is retired to docs/solutions/logic-errors/an-async-commit-was-recorded-on-send-not-on-acknowledgement-2026-09-07.md, which records both committer-side designs this replaced so nobody re-proposes one. Three verbatim await-and-read blocks are hoisted into MockConsumerTestBase with their deadlines as arguments, narrowing that class's no-hoisted-assertions decision where it is made; the commit-attempt assertion stays per file because the families assert opposite things with it. docs/inflight/issue-response-248.md carries the draft response the directory requires and is deleted when it is posted. The upstream manifest entry is updated at the lifecycle transition. This does not close #248: the reporter's symptom was a throughput collapse with endless retries, and nothing here reproduced that; the fix removes the silent-loss path the report exposed. The chaos ledger's three async-mode stalls recorded as explained by no known defect now have a candidate, explicitly not an attribution. Upstream-Issue: confluentinc#203 Upstream-PR: confluentinc#345 Forwarded: no Applied-Upstream: no Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
…ut, not by waiving them (#473) Clears the quarantine registry by fixing what its entries were about, not by waiving them. With the third entry fixed on master by #466 in the meantime, no @Quarantined annotation remains in the tree, and the release workflow's "no release while tests are quarantined" block clears for 0.6.0.0. RegistrationRaceStaleResidentIT was quarantined by #440 on a sighting ledger with no mechanism: it failed for a fortnight across unrelated branches, always on its own SETUP guard ("control thread must reach the mid-loop pause point (offset 25)"), never on the confluentinc#909 assertion it exists to make. The mechanism is the test's staging, not PC. The test saturates the pipeline by gating the user function so nothing ever completes, which makes whatever stage 1 produced permanent; stage 1 produced four records more than the out-for-processing target, so records in shards sat permanently over WorkManager.isSufficientlyLoaded()'s threshold, the poller paused the partition for back pressure, and neither resume path can fire while the gate is closed. The stage-2 records carrying the pause point were never fetched. It passed at all only by racing a consumer.poll() already in flight when the gate closed. Control arms, predictions stated before running: waiting on the load gate did NOT reproduce it (the gate closing is not the partition pausing - the refuted arm located the window); waiting on the pause reproduced the exact CI signature every time; producing exactly the target passes every time; and with stale replacement disabled the repaired test still goes red on the 909 signature, naming the same 25 keys the original write-up recorded. The fix derives the stage size from the buffer size so the two cannot drift, asserts the intake precondition before the second produce instead of leaving it mute, and reports the intake state after the wait so a future failure says which of "slow broker" and "records never fetched" it was. Rejected: a bigger buffer (same knife-edge), producing everything up front (saturation could never be established), and loosening the guard (which would have hidden that the records could not arrive at all). MultiInstanceRebalanceTest.largeNumberOfInstances leaves the registry for a different reason: nothing about it was repaired, because nothing is broken. #444 measured its residual failure as the Kafka consumer group protocol under the profile's churn rate, so its legitimate output is a pass rate, and a required check that fails one run in fifteen for a reason no change to PC can move gates nothing. All three capacity profiles now carry @tag("capacity"), excluded by the required Performance Tests lane and by the pom's default exclusion and the CI wrappers' copies of it, and still selected by the scheduled experiment runners - verified by what each lane actually selected, not by the flags looking right. The two profiles that had no scheduled runner get one on the same weekly cadence, sharing the iteration loop in bin/lib/chaos-experiment-common.sh so the classifiers cannot drift, and the workflow's collect step scans their tally directories. A latent bug the tag split would have introduced silently is fixed on the way: getAnnotation(Tag.class) returns null once @tag is repeated, which would have reclassified every capacity profile as a correctness gate. The deterministic correctness twin, scriptedChurnRoundsCompleteWithoutStall, is untouched and keeps those code paths gated. The residual-failures note, which said its own subject had been acted on, is migrated to docs/solutions/test-flakiness/ and deleted with every citation repointed; the solved flake leaves the open flake register for its own write-up. Not done, and named: the capacity rate is still a per-batch tally, not compared against a rolling median. Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
wins the transactional revoke path Master landed #466 (the revoke-path commit hands itself to the control thread), #468 (WorkContainer equality is identity) and #471, and this PR went CONFLICTING. #466 is a different answer to the question this branch answered: in transactional mode onPartitionsRevoked no longer commits on the poll thread at all - it posts a request, wakes the control loop through the mailbox, and waits, bounded by commitLockAcquisitionTimeout; the confluentinc#548 spin is gone; and it refutes this branch's decline by experiment (a revoke that commits nothing leaves its output in the open transaction for the next commit to publish without the offset - the same duplicate through a different door). Master's own re-premising of docs/inflight/bug-857-transactional-revoke-wait.md says what is left for this PR: not the absence of a bound, but whether the bound is the right value - five minutes, against a max.poll.interval.ms it can exceed. So the resolution is master's on that path, per file: - AbstractParallelEoSStreamProcessor: master's onPartitionsRevoked, commitOnRevokeViaTheControlThread and the no-argument consumer-commit tryCommitOffsetsOnRevoke replace this branch's parameterised decline, its performCommit extraction and its post-catch wake (moot: the served commit runs on the control thread, which recovers itself); commitOffsetsThatAreReady is master's again; the mailbox loop keeps master's wake-up message skip in front of #410's first-failure try; one of two identical assertOnControlThread helpers (#410's and master's) is kept - master's, which names the new design. - RebalanceEoSDeadlockTest: master's whole file. Its unamended assertion that committed offsets advance inside the callback holds again by construction under #466, and it now reads the output topic at read_committed for duplicates; this branch's decline amendment is superseded. - ArchitectureTest: master's whole file (#465). This branch's interface-hop widening does not merge onto it; its note now records that the blind spot is still open on master and the widening is to be re-applied on top of #465 as its own change. - ProducerManagerTest: master's revoke-request tests and this branch's five revocation tests, both kept; PartitionState: master's onSuccess(long) javadoc with #410's ledger cross-reference folded in; TransactionalClaim: master's scope note plus #410's C15. - config/infer-known-findings.txt, bug-857-family.md, core-recoverable-producer-fencing.md: master; bug-wedged-after-poisoned-transaction.md: master's deletion (the grooming sweep); the three vetted notes keep master's markers and this branch's concurrency label; test-untracked-ci-flakes keeps master's rows and this branch's three later sightings. - ProducerRecoveryTest's revoke-path fence test is retargeted to the served-commit shape: the fence now fires on the control thread inside the served pass, the callback returns promptly on the failed pass, nothing is stranded, and the replacement is built. Its wake assertion is gone with the wake. What is red, on purpose: Revoke857TransactionalWaitProbeIT, 5/5, with the callback at 19.2s of a 20s in-flight dwell against its 10s poll-interval budget - and never the 79s starvation the spin produced. That is the measurement of #466's bound, and it is this PR's remaining acceptance test, not a broken instrument. What is now dead main code, held for the owner's call: the three ProducerManager revocation lock helpers and the DeclineCountingProducerManager instrument, which count a decline the transactional path no longer makes. Verified: ProducerRecoveryTest, ProducerManagerTest, ProducerManagerDetectionTest, ArchitectureTest and the convention rules, PartitionStateAbortedTransactionReplayTest, TransactionalClaimCoverageTest; RebalanceEoSDeadlockTest 5/5, ProducerFencingRecoveryIT 2/2. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PVL2FEJ645T76PbybEBUZ6
…3-bound-transactional-revoke-wait The base this PR is stacked on moved again - its own master merge and a re-cut into five rungs - and GitHub reads the PR against that base. Four conflicts: the infer ratchet takes #410's one new entry (the lane is the judge); the revoke-drain note takes master's retirement into docs/solutions; the flake ledger's rows were identical on both sides; the transactional claim register keeps master's newer wording of the same scope note ahead of #410's C15. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PVL2FEJ645T76PbybEBUZ6
and #473 Two conflicts. PartitionState: #470 counts an async commit as committed only when the broker answers the latest offer, and #410's ledger trims on commit success; joined so the ledger snapshots before the offer is made and is trimmed only on the answer to that offer - a stale answer to an older offer leaves the newer snapshot in place, so a replay can only over-replay, never under. The flake ledger takes master's updated rows and #473's retirement of the section it fixed, and keeps this branch's three later sightings. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PVL2FEJ645T76PbybEBUZ6
[superseded - the quarantine lane is now empty] 🧪🔒 Quarantine Lane ReportThe superseded report, collapsed because it no longer applies
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its Since the previous push: Updated for Superseded by a newer quarantine lane report. |
🧪🔒 Quarantine Lane ReportThe quarantine lane is empty - no Any earlier row on this PR asking for a Lane: non-gating; rules: see the Quarantine Audit check. No quarantined test changed outcome since the previous push. Updated for |
…ller collects what it leaves (#481) WHAT CHANGED FOR A USER: a rebalance can no longer be delayed by however long the control loop's retry-queue scan takes, in any ordering or commit mode. `RetryQueue.remove` takes an unbounded, FAIR `writeLock().lock()`, and every Kafka rebalance callback reached it on the broker-poll thread - by direct call through `ShardManager.removeWorkFromShardFor` on revoke and lost, and by the method reference `.map(retryQueue::remove)` through `ShardManager.removeStaleContainers` on all three, assigned included. The controller thread holds the matching READ lock for a whole scan (`RetryQueue.iterator()` hands it out; `ShardManager.getLowestRetryTime` closes it only after a full iteration), and a fair lock queues an arriving writer behind that scan rather than interleaving. The wait sits inside `consumer.poll()` with the whole group waiting, spent out of `max.poll.interval.ms`. No cycle was found: an unbounded wait, not a deadlock. No fork issue exists; found by the confluentinc#857 defect-class sweep (#29). THE FIX: THE POLL THREAD NEVER TOUCHES THE RETRY QUEUE. The callbacks remove from the SHARDS only - a `ConcurrentHashMap` of `ConcurrentSkipListMap`s, no lock to wait for and so nothing to decline. `ShardManager.purgeDepartedRetryEntries()` then collects, on the controller thread where waiting is allowed, every retry-queue entry whose container `ProcessingShard.isResident` says has left: scan under the read lock, close the iterator, remove the collected entries under the write lock in one `removeAll`. The invariant, stated once at the purge: a retry-queue entry with no resident container is garbage the controller collects; it may exist for at most one control-loop tick; the callbacks are free to create it. Where it runs: at the top of `ShardManager.getWorkIfAvailable`, the one point the control loop reaches unconditionally every pass in both RUNNING and DRAINING, and before `drain()` reads `getNumberOfWorkQueuedInShardsAwaitingSelection` in the same pass - the consequence that matters, because #437 established that a queue-only orphan surviving that figure holds a draining close open to its timeout. The bound is one tick in every ordering mode: the purge scans the queue rather than the shards, so the ordered modes' shard-scan break cannot delay it. #468 landed while this was open and settles by construction what this branch argued: `WorkContainer` equality is now identity, so the controller-side stale branch's compare-and-remove evicts only the container it inspected, and the purge's by-key removal no longer rests on the single-writer argument alone. Why scan and remove may be two steps: departure is monotonic (`isResident` is reference identity and nothing re-inserts the same instance), and the queue has exactly one writer - `RetryQueue.add` is `@ControllerThreadOnly`, its only production caller is `ShardManager.onFailure`, and the purge runs on that same thread. A second writer is what would reopen this, and both sites now say so: a `false` from a removal there can only mean one exists. THE DESIGN IT SUPERSEDES, NAMED. #431 kept the poll thread on the queue and made it DECLINE: `tryRemove`, abandon the paired shard removal on refusal, and after #437 a second ask after the shard removal with a put-back on refusal. It was correct and proven, and it lost on machinery: a second non-blocking entry point keyed differently from the blocking one, a queue reference threaded into a second `ProcessingShard` method, two asks per removal, and a retirement bound that varied by ordering mode. Also rejected: reusing #466's control-thread hand-off, which has the poll thread WAIT on a future - right for a revocation commit, wrong here, because the purge needs no hand-off at all. COUNTERS DURING THE WINDOW, RE-MEASURED. Between a callback's shard removal and the purge, `getWorkableRecords` reads one LOW - the safe direction: `isSufficientlyLoaded()` reads false and the poller fetches sooner; high would have paused it with nothing to process. `WorkManagerTest`'s revocation arm pins the direction. #437's post-add residency confirmation in `ShardManager.onFailure` becomes belt-and-braces and is kept this round, its javadoc saying so. THE STATIC GATE GOES GREEN ON MERIT. `ArchitectureTest.rebalanceCallbacksMustNotBlock` (widened in #465 to follow method references and `@ControllerThreadOnly`) carried eighteen exemptions for exactly these reaches. With the set emptied and the callbacks still reaching the queue it reports every route; with the callbacks off the queue it is green and the set stays empty. The set, its `root => target` keying and the argument for keeping it remain - an exemption is how a newly found reach is recorded as a defect on the books. EVIDENCE, AS SHAPE AND REPRODUCE COMMANDS. `RetryQueueRebalancePathTest` holds the read lock the way the controller does - a live iterator on another thread - and drives the real callback on a thread named `broker-poll`: red on unfixed code, both callbacks parked on the write lock until the deadline; green after, with the departed entry collected on the next controller pass, a queue-only entry no longer holding drain open, the ordered-mode arm collected on the same tick, and a still-resident entry left alone as the control. Ablations: deleting the purge call reds arms across three classes; inverting the residency predicate reds the same plus the control. Two arms from #437's window test are re-stated rather than deleted, because the sweep they described stopped existing. The contended harness was then folded into the shared `BlockedThreadAsserter` (a `returnsWithin` idiom that returns the fact rather than asserting it, so a caller can hold the contention while it reads state), re-proved red by restoring the queue removal to both callback paths. The three `DLS_DEAD_LOCAL_STORE` findings this branch's own lines introduced are cleared by using the values - logged, or a `void` return - rather than parking them. Reproduce: `bin/ci-unit-test.sh`; `bin/lincheck-test.sh`; `bin/check-all.sh`; `./mvnw -Pci -pl parallel-consumer-core -am test -Dtest='RetryQueueRebalancePathTest,RetryQueueRequeueWindowTest,WorkManagerTest,ArchitectureTest'`; `bin/check-pr-analysis-surfaces.sh 481`. RECORDED, NOT APPLIED. The purge is the first unconditional full scan of the retry queue on the control loop; both simplify reviewers converged on gating it behind a departure signal (a monotonic `LongAdder` in `RecordPopulation`'s shape). Not done here: it would put poll-thread-written state back on the rebalance path, and the cost is unmeasured - `DispatchScanMeter` is the precedent for metering first. Filed as `docs/inflight/perf-retry-queue-purge-scan-is-unmetered.md` with that order stated. RECORDS. `docs/solutions/runtime-errors/retry-queue-write-lock-on-the-rebalance-path.md` records both designs. Retired: `docs/inflight/bug-retry-queue-write-lock-on-the-rebalance-path.md`, `pr-431-must-pair-its-queue-removal-with-the-shard-removal.md`, and the branch note that guarded the superseded PR while it was still open. Corrected beside their dated text: the ArchUnit rules note (the set is empty on merit), the RetryQueue coverage note, the shard-displacement note (its harm is now bounded by one tick), the runtime-ownership-guard note (the single-writer premise makes that guard more load-bearing), two notes that named #431 as the fix, and the orphan-window write-up (its assertion stays as written; the confirmation is belt-and-braces). Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
🟢 Throughput — OKThis branch measured about 3% slower than master, on the one test this measures. That is INSIDE this test's own run-to-run spread of about 17%, so read it as a reading and not as a result - re-running the same commit moves it by about as much.
Allowable range 🟢 ≥ 0.70 · 🟡 0.50–0.70 (about a 30% loss) · 🔴 < 0.50 (about a 50% loss) What the numbers mean, and what they cannot tell youThe one that gets misread. Why a shape and not a rate. A rate depends on which runner you drew. A shape does not: every test here processes a fixed number of records, so a runner twice as slow doubles the subject and the controls together and leaves their ratio alone. That is the whole trick, and it is why the reported rate is shown last and labelled as this machine only. Reading the comparison. By conservation, not by correction. Every test in this lane processes a fixed number of records, so within one run the ratio of one test's time to another's is invariant under machine speed — a runner twice as slow doubles both terms and leaves the ratio alone. There is no machine-index correction to be wrong, because nothing needed correcting. Per-method times, not class times. A class time is Reference is the median of 6 recent What this still cannot do. It removes machine-to-machine variance. It does not remove this test's own run-to-run variance, measured at about 30% on a single unchanged commit while its controls stayed within 5%. That is a property of the test, not of the comparison, and no arithmetic here can touch it — which is why the reference is a median and the bounds are deliberately coarse. 🟡 means look at this; only 🔴 is outside the measured spread. Runs used: 890aa53, fd3f913, 21e8728, 9302a89, c2d35c0, ad482ec Since the previous push: ratio 1.144 -> 0.967, share 1.668 -> 1.929, rate 75354 -> 71204 (-5.5%). One push of difference sits inside this test's measured spread - read it as movement, not as a result. Updated for |
…s decided (#482) The register's most-sighted flake, undiagnosed since 2026-08-18: `simpleBatchTest` timing out at 30s with `Expected size: 3 but was: 4` across `ReactorBatchTest`, `MutinyBatchTest` and `VertxBatchTest`, always on the KEY parameter, always on a branch whose diff could not have caused it. THE CAUSE ESTABLISHED: expectation-versus-input, the third of the three readings the register kept live. Not contention, and not a batching defect - the library is correct and is unchanged by this commit. Under KEY ordering a shard is per key and `ProcessingShard.getWorkIfAvailable` breaks out of its scan after one record when `isOrderRestricted()`, so a shard yields at most one record per work-retrieval round; `makeBatches` then partitions each round into chunks of batchSize. The batch count is therefore decided by how many records sit on DISTINCT keys. `KafkaTestUtils.sendRecords` draws keys with replacement from a hundred-key pool, so five records can land three to a key - and `expectedNumOfBatches` is `ceil(records / batchSize)`, computed from the record count alone and blind to the draw. Three records on one key cannot share a batch without breaking the ordering guarantee KEY exists to provide, so four batches is right and the expectation is wrong. HOW IT WAS ESTABLISHED - three arms, KEY ordering, batchSize 2, five records, everything but the key distribution held identical, predictions stated first: A control, random keys as shipped predicted ~all 3 20/20 gave 3 B forced 3-way collision 7,7,7,8,9 predicted 4 20/20 gave 4, 2+1+1+1 C forced distinct keys 1,2,3,4,5 predicted 3 20/20 gave 3, 2+2+1 `2+1+1+1` is the shape every captured sighting payload carried. Supplementary arms: 7,7,8,8,9 gave 3 three times and 4 twice - the same input, two counts, which is what proves no exact expectation can survive a colliding draw; 7,7,7,7,8 gave 4 five times; 7,7,7,7,7 gave 5 five times. RED THEN GREEN: pointing `simpleBatchTest` itself at 7,7,7,8,9 fails 1 of its 3 parameters in 31.7s with the CI signature character for character - ConditionTimeoutException, alias 'expected number of batches', 30 seconds, `Expected size: 3 but was: 4`. With this fix, green. THE FIX KEEPS THE EXACT ASSERTION and removes the randomness from the input instead. `KafkaTestUtils.sendRecordsWithDistinctKeys` draws WITHOUT replacement - the distribution stays arbitrary, the collision cannot happen - and the exact-count assertion is now actually justified rather than loosened. That would leave KEY behaving like UNORDERED for that test, so the KEY-specific guarantee gets a test of its own rather than being lost: `keyOrderNeverBatchesTwoRecordsOfOneKey` sends a deliberate 3-way collision and asserts what is deterministic about it - every record delivered exactly once, no batch over batchSize, and no batch holding two records of one key. Net coverage rises: the behaviour that broke the old expectation was never tested at all. Rate check, as corroboration rather than proof: five draws from a hundred keys collide three ways about once in a thousand, only KEY is exposed, four classes run it per unit lane - so roughly one run in 250 should carry one, which is the order of magnitude six sightings over three weeks predicts. A contention reading never had to explain why UNORDERED and PARTITION were spared entirely. OTHER INSTANCES OF THE CLASS - an exact count asserted over an input whose distribution the test randomises. Every `sendRecords` caller and every `Math.ceil` in test code was read; none found. Checked and dismissed: `batchFailureTest` (same ceil, but `hasSizeGreaterThanOrEqualTo` - a collision can only add batches); `averageBatchSizeTest`, `TransactionalBatchProduceTest`, `MdcContextPropagationTest` and `OffsetEncodingBackPressureUnitTest` (all UNORDERED, where every record is its own shard); `TransactionMarkersTest` (PARTITION, and its own comment already says `// just so we dont need to use keys`); `BitSetEncoder` (main code, unrelated ceiling arithmetic). Register row retired to the header's fixed-and-out list; the payloads, the arms and the residual are migrated to docs/solutions/test-flakiness/a-randomised-key-draw-decided-a-batch-count-the-test-computed-from-the-record-count-2026-09-08.md Co-authored-by: Claude Opus (1M context) <noreply@anthropic.com>
…, then fenced or redesigned (#469) PartitionState carried three fields that cross the broker-poll / control thread boundary. #349 fenced one of them, dirty, after measuring its window, and deliberately left the other two alone for want of a measurement. This measures them with jcstress and then gives them DIFFERENT fixes, because the measurement says they are different defects. The rule that falls out of it: volatile is the fix for a one-writer field, not for a shared one. THE MEASUREMENT, in jcstress-poc/, in the shape CommitPathVisibilityProbes already used for the dirty pair. Rates are per raced pair on one workstation, not production incidence, and the recorded figures are in each class's Calibration status block. BackPressureFlagVisibilityProbes measures allowedMoreRecords, written on the broker-poll thread inside the commit path's encode and read on the control thread in couldBeTakenAsWork. The plain field with its real surrounding ConcurrentSkipListMap accesses fires; with the neighbours stripped it fires about fifty times more often - so the map touch one statement earlier suppresses the window fiftyfold and does not close it, the same incidental-fencing shape the dirty pair measured; the volatile arm is FORBIDDEN at zero. What the arms do not show, said in the javadoc: this field's harm is unbounded staleness - a stale false admits nothing, so nothing succeeds, nothing re-dirties the partition and the encode never runs again - and "never" is not an outcome a bounded run observes. They measure the fence, not the stall. CommitWindowLostUpdateProbes measures stateChangedSinceCommitStart, written by BOTH threads, with the arbiter asserting the invariant itself: a completion landing inside the commit window must leave the partition dirty unless the commit covered it. As shipped it fires at around 1.6e-3. The arm that decides the design is the flag made volatile as well - the obvious next step, and the one the inflight note had recorded as next: statistically unmoved, because only one of the three mechanisms breaking the invariant is a visibility effect. The check-then-act in the clean-marking step and the lost update between two writers fire at interleaving rates on sequentially consistent hardware, and no modifier addresses either. VolatileStateChangedFlagAcrossTheCommitWindow exists solely to hold that line; re-run it before proposing the modifier again. The monotone-count protocol arm is FORBIDDEN at zero, with a known weakness stated below. THE TWO FIXES. allowedMoreRecords is volatile: one writer, so a release/acquire edge is the whole fix. Its javadoc carries the figures, the arm name and the direction of the crossing - poll thread writes, control thread reads, the OPPOSITE direction to the commit-path fence, so that fence never covered it. dirty and stateChangedSinceCommitStart are gone. In their place a monotone AtomicLong completionCount, the completionCountCommitted a successful commit publishes, and the held completionCountBeingCommitted. "Is this partition dirty" is a comparison of two values rather than a flag somebody clears, which removes the defect rather than narrowing it: no check-then-act is left in the clean-marking step and no write either thread can lose to the other. The release/acquire edge #349 measured is preserved: incrementAndGet is a strictly stronger release than the volatile store it replaces, and the counter load on the commit path is the paired acquire, so the plain longs it published stay plain for the reason that PR gave. The held count is volatile because which thread runs the commit cycle varies by mode, so a thread-confinement claim would have been a lie. COMPOSED WITH #470, which merged first and changed the same protocol from the other side: the partition remembers the commit offer it makes WHOLE and marks itself clean only on the broker's acknowledgement of that offer. The two rules do not overlap. getCommitDataIfDirty samples the completion count once, stashes it, and remembers the offer; onOffsetCommitSuccess records every acknowledged offset and publishes the stashed count as committed only when the acknowledged offer equals the last one made. The offer decides WHETHER an answer ends the commit window; the count decides HOW MUCH it covered. Under the async mode the acknowledgement is no longer part of the collect-and-commit unit, so a later cycle can overwrite both fields while an earlier request is outstanding, which is safe because only the latest offer's answer publishes anything. THE NAMES FOLLOW THE DESIGN. setDirty() is now recordCompletion(): nothing sets a flag any more, the completing thread stamps a new version on the partition, and the count is a version stamp rather than a quantity - its only use is inequality with what the last acknowledged commit covered. isDirty() carries the definition the code had been paraphrasing three ways: dirty means a completion has landed that no acknowledged commit has covered. The comparison lives once, in isDirtyAt(long): isDirty() passes a fresh load, and getCommitDataIfDirty() passes the sample it then stashes, because the collecting cycle must publish the very count it TESTED - a completion landing between two loads would be published as covered while its offset need not be in the metadata this cycle encodes. That burnt commit cycle is the defect this change closes. THE RED PROOF. PartitionStateCommitWindowSeamTest drives, deterministically, the one interleaving the old protocol lost a completion to: inside the clean-marking step, between its read of the flag and its write of dirty. Master offered no override point between those two instructions, so PartitionState.onCommitWindowClosing() was added - an overridable no-op in the one-shot-armed seam shape of RacingSeamWorkManager, with firing tracked separately from arming so a test that forgot to arm cannot pass while asserting nothing. The review searched for an existing hook at that instruction and found none. Redness is shown twice: a permanent control arm reverts only the protocol and asserts the loss, so the defect stays runnable after the fix removed it; and transiently, reverting the protocol in PartitionState while keeping the seam turns the pinning arm red. Stashing the whole change does not produce that, because it removes the seam too. PartitionStateDirtyFlagFenceTest becomes PartitionStateCrossThreadFieldFenceTest, pinning the modifiers on all three fields, which is the move its own failure message asked for. KNOWN WEAKNESS, RULED ON RATHER THAN FIXED. The protocol arm's FORBIDDEN zero is unreachable by construction: the flag arms are seeded dirty so the poll actor always enters the commit window, while the protocol arm starts clean and enters the window only once it has already observed the completion, and the AtomicLong acquire that lets it in publishes everything the release wrote. The recorded run corroborates it - the flag arms reached all four outcomes, the protocol arm only two. Seeding it dirty is not a fix, for the same release/acquire reason; a real power check would need a deliberately broken protocol variant as a negative control. The maintainer ruled that the control would not be built and the weakness documented instead, so its zero is cited as "no anomaly observed" and never as "the window was measured shut". The argument this change rests on is the plain-versus-volatile pair, which none of this affects. Every other FORBIDDEN arm in the module earns its zero from a paired arm of the same state shape that fires; this is the only arm whose fixed version has a different state shape from its comparator. THE PROBE ARMS SHARE THEIR SCAFFOLDING and keep only the measured term apart, answering the duplicate-code check. The rule: a member may be hoisted only if it contains no access to the arm's measured field. The map and its setup, the neighbouring writes, the instrumentation payload, the encode read and the arbiter's result mapping moved into a package-private base class; the measured field's own declaration per @State class and each actor's literal access sequence stay in the arm, so plain versus volatile on one line is the term under test and the JIT compiles the shape production has. jcstress 0.16 permits a @State class to inherit - read out of its annotation processor, whose inheritance ban binds @Result classes only - and jcstress -l lists every arm after the change. The recorded rates are deliberately not overwritten with the re-run's. A LEDGER THAT WAS WRONG, and nothing went red: docs/refactoring.md's AT_STALE_THREAD_WRITE_OF_PRIMITIVE entry listed two fields while SpotBugs named six, three of them in PartitionState - the class #349 was fencing at the time - and one, ProgressTracker.highestRoundCountSeen, that no ledger carried. The lane runs spotbugs:check with failOnError=false, so the finding annotates and never blocks. The repair is the command that re-derives the membership, the spotbugs step's own line verbatim, not a better list. DOCS. The two docs/inflight/bug-* notes for these fields are retired, their durable content in docs/solutions/logic-errors/volatile-is-the-fix-for-a-one-writer-field-not-a-shared-one-2026-09-07.md, and the engine AGENTS.md ledger carries the rule. docs/inflight/test-jcstress-probe-module-open-items.md carries the vacuous-arm line item and the generalisation that a green FORBIDDEN tick is worth nothing unless some arm shows the outcome is reachable at all. Two local-only test failures seen while verifying are recorded in docs/inflight/test-untracked-ci-flakes.md with their same-base control arms; the row for ReactorPCTest.concurrencyTest no longer rules this change out as a contributor, because setAllowedMoreRecords(true) fires on two paths and the fence publishes unblocking sooner too. The master merges kept the retired notes deleted over the vetting sweep's stamps, which asserted the fields were still unfenced and unmeasured - precisely what this change falsifies. Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
…n no run can answer, and a control arm settles it instead (#485) The revoke-path commit deadlock's write-up carried two contradictory verification statuses: a 2026-08-31 header block saying the fix was proven by a controlled A/B, and a body section titled "the part that matters" saying Unproven. The second is the state of #29 as an unmerged draft on 2026-08-07, and it is the one a reader greps to. Six capture sections in the family register cite it, each closing by asking for the same replay. That replay cannot answer the question. Each capture identifies the defect by the poll thread being BLOCKED on an AtomicBoolean monitor; #29 replaced that monitor with a ReentrantLock, and a thread waiting on a lock parks rather than blocking. So the answer is "no BLOCKED frame" whether the cycle is closed or wide open. Measured, not argued. The deadlock was deliberately restored at 6aab3ff - master plus everything through #466, whose change to the revoke path the 2026-08-31 A/B predates - by the single term commitLock.tryLock() -> commitLock.lock(), and Rebalance857CommitSyncDeadlockProbeIT run against it. arm assignor result declines commit timeouts fix eager 5/5 pass 6 0 fix cooperative 5/5 pass 5 0 pre-fix control eager 5/5 FAIL 0 65 All 80 diagnoses from the failing control read WAITING on a ReentrantLock$NonfairSync held by pc-control, through tryCommitOffsetsOnRevoke under onPartitionsRevoked. Not one read BLOCKED; not one named an AtomicBoolean. A future capture must be matched by the method pair and the holder, never by thread state or lock type, or a genuine reproduction gets mis-filed. The declines and uncontended columns count log lines, one per execution of the revoke fork, which the eager fix cell settles from inside the grid: six declines cannot be a per-repetition count of five repetitions. Two seeds replayed as falsification attempts, and the pair is instructive: 7728704565782280867 ChaosRevokeUnderWorkKeyOrderIT eager pass 182 uncontended, 1 decline 2867310537409227917 ChaosRevokeUnderWorkCooperativeIT coop pass 54 uncontended, 0 declines The second is VOID by this repo's own standard - the window never opened, so its green is uninformative - and it is visible only because both branches of the revoke fork now log at INFO. One void in two is the base rate this method has. The remaining four seeds were deliberately not run; a third green buys nothing. Also recorded: the JVM's own findDeadlockedThreads() cannot see this cycle even fully closed, because its other edge is a LinkedBlockingQueue.poll rather than a lock. That is why the probe exists instead of an assertion on the deadlock detector. Defect-class sweep - a document identifying a concurrency defect by a signature that is an artefact of the implementation, so the signature dies when the implementation changes: one further instance fixed (the A/B soak harness note, the highest-traffic one, now says the seeds could never have answered the question), the family register's literal dump signatures corrected, and six candidates checked and dismissed as ordinary prose about a stuck thread rather than a fingerprint. Prior art that shaped the method rather than being rediscovered: the family register's standing "stop replaying seeds" rule and the 2026-08-31 control-armed replay that came back void, and #478's finding that a master-versus-branch A/B measures the whole branch. The dated Unproven section is superseded in place, not rewritten, per docs/citations.md. PROPOSED, owner-gated: the revoke-path deadlock line may now be closed. Not #119, which is a symptom bucket, and not the register, which still holds open items that reproduce on trees carrying this fix. Co-authored-by: Claude Opus (1M context) <noreply@anthropic.com>
, #481, #482, #485 Two conflicts, and one silent auto-merge artefact that only the compiler found. PartitionState. #469 replaced the pair of booleans with a version counter, deleting stateChangedSinceCommitStart; #410's ledger snapshots at the same point. Joined by taking master's counter whole - the single completionCount load, tested and stashed as completionCountBeingCommitted - and moving the ledger's snapshotForCommit() inside that same branch, still ahead of createOffsetAndMetadata(), so the ledger is still snapshotted before the offer is made and trimmed only on the answer to that offer. The artefact: restoreCompletedButUncommittedWork() still called setDirty(), which #469 deleted, and git merged both sides clean because neither touched the other's lines. It is recordCompletion() now - the replay has moved the partition's state on, so it stamps a new version and isDirty() derives the rest. PartitionStateAbortedTransactionReplayTest 7/7 and PartitionStateCommitWindowSeamTest 3/3 cover the join. The flake ledger takes both sides' rows and master's retirement of the simpleBatchTest section, which #482 fixed - expectation-versus-input, not the library. config/infer-known-findings.txt takes master's version whole. The "1 NULLPTR_DEREFERENCE AbstractParallelEoSStreamProcessor.processWorkCompleteMailBox" line this branch carried was a merge artefact: #346 added it, #466 retired it in the same change that reshaped the mailbox loop, and an earlier merge here re-took #410's older side. The static lane was red on it. Re-ran infer locally on the merged tree - 9 findings, all known, none new. Verified: ProducerManagerTest 22/22, ProducerRecoveryTest 15/15, the PartitionState family, the RetryQueue family, WorkManagerTest, ArchitectureTest, TestConventionsArchTest, TransactionalClaimCoverageTest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Sep 8, 2026
Two more lines of the burn-down land on master on 2026-09-08: the shard-displacement orphan is known unreachable (#483, whose review round widened the regression test to every ordering mode), and the fifth 857 item is the group protocol (#486, carrying the PROPOSED close for the owner). The release-note draft said #408 adds the decline. It no longer does: #466 refuted declining as the fix by experiment, so the decline is only the deadline fallback, and what #408 now carries is the measurement of the bound and the held decline seam - the reading its reconciliation with master recorded today. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Sep 8, 2026
…PR that carries it The three lines the release note still has to name now read like the rest of the burn-down: a box, the PR in front, and what closes the box. A ticked box means the v6 action for that line is done, not that the defect is closed - the revoke wait is ticked because its release-note sentence is written and its fix is #408 after v6; the INSTANCE_STALL line waits on #488, whose load arm has now measured the starvation reading from the direction the idle replays never could; the intake stall waits on #487, whose soak arms are instrumented and predicted but not yet run. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
6 tasks done
astubbs
added a commit
that referenced
this pull request
Sep 18, 2026
… not decline the lock #408 refuted its own decline design after #466 moved the revocation commit off the poll thread; what remains is bounding the delegated wait under max.poll.interval.ms. The two posted comments claimed decline was the fix and were edited today; the tracker row now matches. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XS64Xttx4vF5datYh7fmk
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.
Closes #44 (confluentinc#803).
depends on #262
depends on #410
Stacked on #410 (recoverable producer fencing, #225), decided 2026-09-02. That PR is the plan and the implementation, and it settled the one design question this PR had left open. Its branch is merged in here and is this PR's base; the reconciliation its plan asked of this branch is done and listed below. When #410 merges and its branch is deleted, GitHub retargets this PR to
masteron its own.The design question, and how #410 settled it
Decline was chosen here over bounding the lock holder because deadlining the holder meant aborting a transaction while
ProducerFencedExceptionwas fatal. #410 removes that objection - and removes the reason to want a wait at all. Its plan's KTD11 makes the revoke commit a third detection site for a recoverable condition, recorded and declined on the poll thread, and states that once recovery bounds the write-locked region to abort, close, drain and replay, neither a bounded wait nor deadlining the holder is needed for liveness. Both stay viable and both are deliberately not taken. What is left is a trade - a bounded wait buys fewer redeliveries on a transactional rebalance, at the price of an ArchUnit exemption with a stated bound, with the starvation measurement below still standing against it - and decline stays.RebalanceEoSDeadlockTestis amended accordingly. It asserted that the revoked partitions' committed offsets had advanced by the time the callback returned, which held on master because of the unbounded spin: the callback waited the control commit out. It now asserts what the confluentinc#541 guard needs plus the new contract - the callback returns inside a quarter of the dwell it was timed into, and the window was resolved rather than skipped (declined, counted with the probe's instrument now shared inDeclineCountingProducerManager, or committed inline if the dwell had already ended). The forced overlap and its vacuity guard are unchanged, as are survival and liveness. Evidence in the table below, including the negative control.Reconciliation with #410, done here (its plan's KTD11)
ProducerFencedException | InvalidProducerEpochExceptionrethrow intryCommitOffsetsOnRevokeis deleted: on the PC-built path the commit path converts both intoProducerInvalidatedException, which the generic catch logs and the control thread recovers from on its next pass, and the deprecated producer-instance path keeps master's behaviour, which that PR defers by design (R19). A correction to this PR's own earlier account: commit afd7874 added that rethrow on the claim that master failed the instance loudly at this site. It did not. Master's revoke catch was already the generic WARN and master'scommitTransactionthrew raw into it, so deleting the rethrow restores master exactly. Found by the review's validator; recorded indocs/inflight/bug-857-transactional-revoke-wait.md.notifySomethingToDo()after releasing both locks when the manager reports it is replacing. Pinned by a new unit test inProducerRecoveryTestthat drivesonPartitionsRevokedagainst a PC-built producer fenced for that commit only, and that failed on exactly that wait before the call existed. Negative control: with the deleted rethrow restored, the fence escapes the callback and the test errors on it.ProducerManagermerged clean; the three lock helpers coexist with recovery'sbeginReplacement, as that PR's own reconciliation note predicted. The two test managers that subclass it now build with the replacement source, the wayPCModuledoes.ArchitectureTestand the test-convention rules green,ProducerFencingRecoveryIT1/1, the amendedRebalanceEoSDeadlockTest5/5 at 32-34ms with one decline each, the probe's defect arm 5/5.A trap for whoever ever re-argues the bounded wait: the ArchUnit widening here adds
Lock.tryLock(long, TimeUnit)and its siblings toBLOCKING_CALLS, so a bounded-wait revoke would trip the rule. The rule cannot distinguish a 2s bound from the 5-minute one that caused confluentinc#803, so that site would need an explicit exemption with a "bounded well undermax.poll.interval.ms" justification, or the rule a way to see the bound.STATE AFTER #466, 2026-09-08. Master merged in (merge commit on this branch). #466 moved the transactional revocation commit off the poll thread:
onPartitionsRevokednow posts a request, the control thread runs the commit, and the callback waits bounded bycommitLockAcquisitionTimeout(5 min default); the confluentinc#548 spin is gone; and it refuted this PR's decline by experiment (a revoke that commits nothing defers a duplicate). Master's re-premising ofdocs/inflight/bug-857-transactional-revoke-wait.mdstates what is left for this PR: the bound is five minutes, and confluentinc#803's complaint is precisely that such a bound exceedsmax.poll.interval.ms.What this PR now is, and what it is not:
max.poll.interval.ms, with the timeout fallback fix(core): the revoke-path commit drains first, by handing itself to the control thread #466 already has (decline at WARN, the deferred-duplicate cost named). PC cannot read the consumer'smax.poll.interval.ms, so the bound needs an option or a derivation - the owner's call.Revoke857TransactionalWaitProbeITis the acceptance test and is red on this head, 5/5: the callback waits out a 20s in-flight commit (19.2s) against a 10s poll-interval budget - and never the 79s starvation the spin produced.RebalanceEoSDeadlockTestamendment, the fencing-rethrow deletion and its wake (the served commit runs on the control thread, which recovers itself), and the ArchUnit widening (test(core): the rebalance-callback rule sees method references and a declared contract #465 rewrote the rule; the interface-hop blind spot is recorded as still open indocs/inflight/static-archunit-walk-was-blind-through-interface-hops.md).ProducerManagerrevocation lock helpers, their five unit tests, and theDeclineCountingProducerManagertest instrument - they count a decline the transactional path no longer makes.RemovedPartitionState.onOffsetCommitSuccess, the docs corrections, the probe (re-purposed as above), the retargeted fence-on-served-revoke test inProducerRecoveryTest.RECONCILED AND CURRENT, 2026-09-08. Master merged again (#469, #481, #482, #485). Nothing about the design moved: #466's delegated commit stands, and this branch carries it. What changed is that the branch now builds, passes, and is green on every gate it can be green on.
The title is stale and should be changed before merge. It still says "a revocation declines the transaction lock instead of waiting on it", which is the design #466 refuted and this branch no longer implements. On squash-merge that title becomes the commit subject and feeds the release notes. Something like
fix(core) confluentinc#803: measure the delegated revoke wait against max.poll.interval.msdescribes what is actually here.What the second master merge cost, and it is worth reading before the next one.
PartitionStateconflicted where #469 replaced the two booleans with a version counter and #410's uncommitted-completions ledger snapshots at the same point; resolved by taking master's counter whole and movingsnapshotForCommit()inside its branch, still ahead of the offer. Then a silent artefact the conflict markers never showed:restoreCompletedButUncommittedWork()still calledsetDirty(), which #469 deleted, and git merged both sides clean because neither had touched the other's lines. Only the compiler found it. It isrecordCompletion()now.Reds on this head, and what each one is:
Check PR Dependencies— expected, and not fixable here. feat(core) astubbs#225: recover the producer when the broker invalidates it, rather than dying #410 is open by design; this PR is stacked on it.review: human LGTM— expected. No human has approved it.Integration Tests—Revoke857TransactionalWaitProbeIT, 5/5, and the red IS the defect. The callback holds the poll thread 19.2s of a 20s in-flight dwell against a 10smax.poll.interval.msbudget. That is Transactional Producer instance gets timeout getting commit lock while second instance starts confluentinc/parallel-consumer#803 reproduced against the current design, not a broken instrument and not a flake. It is deliberately not quarantined:docs/quarantined-tests.mdis empty by design and a non-empty registry blocks the release guard, so quarantining this to go green would take the v6 cut with it - which would revisit a scope decision this PR does not own. The probe's javadoc now says all of this at the point of use; it previously claimed both arms were expected to pass, which had been false since the fix(core): the revoke-path commit drains first, by handing itself to the control thread #466 reconciliation.repo: hygiene— one line left, in a file this PR must not touch. The self-reference gate was red on ten lines; nine are fixed. The tenth isdocs/inflight/process-candidate-ranking.md:76("whether that bound is right is what is left, and fix(core) confluentinc#803: a revocation declines the transaction lock instead of waiting on it #408 holds it"), which docs(release) astubbs#197: v6 is a bug release, overdue - the burn-down to cut it #475 owns and deletes, so it resolves itself when that PR merges and this branch takes master again. Expect the gate to fire on docs(release) astubbs#197: v6 is a bug release, overdue - the burn-down to cut it #475's new mentions of this PR inrelease-0.6.0.0.mdandrelease-v6-scope.mdat that point; they are already written in permanent PR-citation terms, so they need markers rather than rewrites.static: analysis— FIXED. The infer ratchet carried1 NULLPTR_DEREFERENCE AbstractParallelEoSStreamProcessor.processWorkCompleteMailBox, which fix(core): staleness checkpoint 3 checks and acts on one state lookup, closing a stall and a wrong-commit route #346 added and fix(core): the revoke-path commit drains first, by handing itself to the control thread #466 retired in the same change that reshaped the mailbox loop; an earlier merge here re-took feat(core) astubbs#225: recover the producer when the broker invalidates it, rather than dying #410's older side of the file.config/infer-known-findings.txtis master's again. Re-ran infer locally on the merged tree: 9 findings, all known, none new.Where the open question is written down.
docs/inflight/bug-857-transactional-revoke-wait.mdgains "The measurement, and what it does not settle": the 19.2s result; what #466 genuinely did fix (starvation across successive transactions - the old spin took 79s out of the same 20s dwell, because a 1s commit interval let the control thread re-take the lock as fast as it dropped it); why declining unconditionally is not the answer; and that choosing a bound needs an option or a derivation PC cannot read off the consumer, which is the owner's call.The description below is the history of the decline design and is kept as the record of what was tried and why; it no longer describes the code on this branch.
Description
The revoke path declines the producer transaction lock instead of waiting on it.
onPartitionsRevokedheld the poll thread in two waits with no deadline, and every tracking note named only the first:while (isTransactionCommittingInProgress()) Thread.sleep(100), arriving with Fix deadlock between pc-control and pc-broker-poll threads where partitions are revoked confluentinc/parallel-consumer#548, which spins for as long as any thread holds the producer transaction lock; andcommitLockAcquisitionTimeout— five minutes by default.The second is the one the user's report actually threw from. Its stack trace is
TimeoutException: Timeout getting commit lock (which was set to PT5M)raised insideonPartitionsRevoked; that message has exactly one throw site; and the reported timeline matches it exactly — rebalance at 0, eviction atmax.poll.interval.ms(180s), the throw atcommitLockAcquisitionTimeout(300s). Fixing only the spin would have left the report reproducible, so the plan of record — "delete the ArchUnit exemption and the rule goes green" — was not a sufficient acceptance criterion on its own.Why decline, and not deadline the holder
The standing candidate for weeks was to bound the control thread that owns the transaction. Two independent reasons killed it, and the first is measured rather than argued:
ProducerFencedExceptionis still fatal (Producer fencing kills the instance; it should abort and rejoin, like Kafka Streams' TaskMigratedException #225). Declining performs no transaction operation at all, so that issue is not on its critical path.Declining costs a replay: offsets stay dirty and travel to the new assignee. That is at-least-once doing its ordinary job — already the published contract for the consumer lane, and what Kafka Streams falls back to (
closeDirtyAndRevive) when its own inline revoke commit exhaustsmax.block.ms. Streams can commit inline because oneStreamThreadowns consumer and producer; PC cannot without confluentinc#200 (#142), which stays open as the successor.Stacked on #29, and why the two had to be unified
Both branches independently wrote a
tryCommitOffsetsOnRevoke, for different locks, and they are complementary rather than alternatives:commitLock— the consumer lane's AB-BA cycle (Paused consumption across multiple consumers confluentinc/parallel-consumer#857).Neither subsumes the other. The control thread takes the producer write lock in
maybeAcquireCommitLock()before it takescommitLock, so there is a window wherecommitLock.tryLock()succeeds and the transaction lock is already gone — and proceeding into it lands on exactly the five-minute wait this removes. Order is load-bearing:commitLockfirst because this method releases it itself; the transaction lock second because it is handed to the commit path, whosepostCommit()releases it in afinally.The ArchUnit rule is repaired, and only then does removing the exemption mean anything
The
Thread.sleepentry inKNOWN_BLOCKING_VIOLATIONS— the acceptance criterion #29 wrote for this issue — is deleted. On its own that would have been a false pass:BLOCKING_CALLSlisted only unbounded primitives, while the call confluentinc#803 threw from is a timed acquire, and the rule's own message ("Decline (tryLock) rather than wait") reads as iftryLockwere the cure. Timed overloads are now denied; the no-argtryLock(), which is the cure, deliberately is not.One ArchUnit defect remains open and is recorded rather than fixed: the transitive walk resolves the declared member, so a call through an interface-typed field never descends into the implementation (
committeris declaredOffsetCommitter). That did not hide this defect — the revoke path reachesProducerManagerthrough a concrete reference — but the blind spot is real and unmeasured.Evidence
Both arms, five repetitions each, shared broker,
-Dpc.log.level=info:RebalanceEoSDeadlockTest, amended (4s forced dwell)ProducerRecoveryTest, fence on the revoke-path commit (PC-built producer)The decline count is what makes the green mean anything: a fixed callback returns in milliseconds because it declined, otherwise indistinguishable from a run where no commit was ever in flight — the trap that left the sibling fix looking unproven for four months.
Also on the merged tree (#410's branch, which carries master as of #417): core unit suite 662 run / 2 failures, both the pre-existing
AmbientProbeExtensionTestheadroom flake tracked indocs/inflight/test-ambient-probe-headroom-red-on-master.mdand not in any file this PR touches;ArchitectureTestgreen with the exemption removed;bin/check-all.shclean. Three unit tests pin the seam in seconds rather than a three-minute broker run, and each was verified by negative control — breaking the decline makes two of them fail on their own messages; restoring the five-minute acquire turns the ArchUnit rule red.Housekeeping for the merger
origin/fix/857-bound-transactional-revoke-waitshould be deleted when this merges. It is the docs-only predecessor of this branch, superseded entirely; this branch was renamed tofix/803-...because Paused consumption across multiple consumers confluentinc/parallel-consumer#857 is the other defect, in the mutually exclusive commit mode, and the branch name was the last place still conflating them.fix/bound-revoke-transaction-wait(the empty branch the notes mention) no longer exists locally or on the remote — nothing to do.A factual correction carried in here
#44 is not "the only issue upstream ever labelled a verified bug" — a couple of dozen carry that label. The claim had propagated into several notes, a probe javadoc and one of this branch's own commit messages; it is corrected everywhere except that commit message, which cannot be changed in place.
Checklist
docs/features/—rebalance-behaviour.yaml's transactional boundary no longer says the wait is unboundedRevoke857TransactionalWaitProbeITis the acceptance test for the bound and is red on purpose, with its javadoc saying so at the point of use; the unit tests for the superseded decline seam are kept and named as dead-but-held belowpr-note would duplicate what already exists:docs/inflight/bug-857-transactional-revoke-wait.mdowns the defect, the measurement and the open decision, and is updated in this PR rather than copiedce-simplifyandce-code-reviewlocally, and acted on bothSecond pass, 2026-09-02, after the master and #410 merges (
ce-simplifythree reviewers,ce-code-reviewsix reviewers plus an independent validator): the probe's module now extends the shared counting module instead of re-wiring it; a P1 claiming the deleted rethrow regressed the instance path was invalidated by the validator against master's code (see the correction above); the surviving P1, that nothing tested the revoke-path fence trace, produced the test above and the wake-up fix it found. Residual risks the review recorded and this PR does not act on: the deadlock test's clock starts after a baseline admin read inside the dwell, so a read slower than ~3s would let a restored spin pass the bound (measured reads are far faster); the decline counter is cumulative per manager; and no test exercises the revoke path's acquire-and-commit-inline arm on a broker.What the local passes changed, since a tick alone says nothing.
ce-simplify(three reviewers) found the duplicated commit body and the unused Truth subject, and its efficiency pass cleared the hot path.ce-code-review(eight reviewers) found two real defects — fatal producer fencing being downgraded to a WARN, and javadoc documenting the wrong method in two files — plus the finding that matters most:ArchitectureTest's blocking-call walk stopped at the declared member, so it could not see through the interface-typedcommitterfield into the commit implementation at all. Widening it surfaced three pre-existing blocking reaches fromonPartitionsRevoked, one of them the confluentinc#857 AB-BA edge itself; all three are now tracked exemptions with an owner note.A P0 was raised (declining orphans produced records, breaking exactly-once) and disproved — declining performs no transactional action, and the in-flight transaction commits its own offsets atomically. But investigating it found a real, pre-existing gap one step over: the revoke path commits without draining the work mailbox, while the control loop drains under the write lock first. Recorded in
docs/inflight/core-revoke-commit-skips-the-work-mailbox-drain.mdrather than fixed blind.Known outstanding, 2026-09-08: the reds above, and nothing else the gates can see. Historical, and superseded by two master merges since - the
dups: similaritycheck failed at +11.4 against amax_increaseof 10, because the newProducerManagerTestcases reuse the latch-and-assert shape ofProduceLockReleaseTest. Left red by decision on 2026-09-02: at 47.6% it is under the tool's ownwarn_aboveof 50 and jscpd finds no clones. Master has since deletedProduceLockReleaseTest(#262 folded it intoProducerManagerTest), so the comparison changed with the master merge; re-read the check on the current head before acting on it.