Repository navigation
test(chaos) confluentinc#857: the eager sync stall is a bound the processor count crosses, not a defect - #478
Conversation
…cessor count crosses, not a defect WITHDRAWS the "fourth open item" in docs/inflight/bug-857-family.md - an eager PERIODIC_CONSUMER_SYNC stall said to reproduce with two recorded seeds, every time, on trees carrying the #29 fix, mechanism unknown. It has no mechanism because there is nothing there. THE CONTROL ARM. Seed 4709156528562690268 - the grid's seed B, and the one of the two that no recovery diagnostic had ever been run against - replayed on 745b1f6 with -Dchaos.diagnoseStallRecovery=true, one term changed: 12 processors no observations true peak 134.5s / 125.2s drained 8 processors fires, 2 of 2 peak is bound+cadence drained Same tree, same seed, same test sources, byte-identical apart from JAVA_TOOL_OPTIONS=-XX:ActiveProcessorCount=8. All four runs drained to inFlight=0 with uniqueConsumed=250000 and zero violations. The processor count alone decides whether the symptom appears. The grid's own confound list named that cap and dismissed it - "the controls passing under the same cap argue it does not manufacture the stall" - which is the reading these runs reverse. Read the peaks correctly: recordLagStagnation re-arms the partition it fires on, so a crossing run's peak is bound plus sampler cadence and carries no severity. Only the runs that did NOT fire have a real peak, and there it sat fifteen to twenty-five seconds under. THE GRID WAS ALSO NOT A ONE-TERM A/B. ConsumerManager.poll refreshes the pause cache at ENTRY and at exit on the grid's DEFECT arm (438b09d); on its FIXED arm (b8a335b) only the exit refresh is there. Exit-only is the shape docs/solutions/performance-issues/paused-poll-wakeup-lost-to-stale-pause-cache-2026-09-01.md owns, fixed two weeks after the grid ran by d2690c5 - not an ancestor of b8a335b. So the grid measured #29's lock change plus a second, since-fixed difference in the subsystem that governs how fast a backlog is re-fetched. WHAT WAS REFUTED, and it is the more interesting half. The prediction was that restoring exit-only refresh would push the 12-processor arm over the bound. It did not: peak 125.2s and a 90.4s drain against the unpatched arm's 134.5s and 102.7s, no observations. The negative control makes that readable - ConsumerManagerPauseCacheTest#pausedPartitionCacheIsFreshDuringThePollItDescribes fails with the entry call removed and passes with it restored, so the patch really did restore the defect. That solution doc measured its 4-10x collapse in PERIODIC_TRANSACTIONAL_PRODUCER; it does not reach this eager PERIODIC_CONSUMER_SYNC recovery at this shape. The second term is therefore proof the grid was never one-term, not an explanation of its fixed-worse-than-defect asymmetry - which stays unexplained, and which the control arm makes uninteresting. No product code changed and no test was touched, so there is no red-first test to show: the finding is that an existing detector's finding is a speed number. The regression test that would matter is the correlated per-shard gate docs/inflight/test-per-shard-liveness-has-no-gate.md already specifies, and that note's own bar - a red control first - is deliberately not met here. WHAT SURVIVES. The per-shard liveness gap: INSTANCE_STALL is per-instance, so a watermark frozen by a commit that never landed, on an instance whose other shards keep completing, is covered by nothing that gates. Every run here drained with full key coverage, which is what a false positive of that shape looks like. Ledger corrections land where each fact lives: bug-857-family.md's fourth item (the numbers and the accounting), test-857-revoke-under-work-sightings.md (the grid itself and its withdrawn headline), docs/testing.md (one line, because a reader meets the Class 2 interpretation there first). The reusable method failure is written up as docs/solutions/best-practices/a-master-versus-branch-ab-measures-the-whole-branch-2026-09-08.md. Co-authored-by: Claude Opus <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
✅ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
No new clones introduced by this PR. Powered by astubbs/duplicate-code-cross-check |
[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 Updated for Superseded by a newer quarantine lane report. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #478 +/- ##
============================================
+ Coverage 82.47% 82.92% +0.45%
- Complexity 1558 1571 +13
============================================
Files 96 96
Lines 5380 5383 +3
Branches 535 537 +2
============================================
+ Hits 4437 4464 +27
+ Misses 750 728 -22
+ Partials 193 191 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
🟢 Throughput — OKThis branch measured about 6% faster 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 9 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: 65e11e3, e05b399, ee3d8a9, 890aa53, fd3f913, 21e8728, 9302a89, c2d35c0, ad482ec Since the previous push: ratio 1.022 -> 1.056, share 1.797 -> 1.738, rate 68407 -> 74356 (+8.7%). One push of difference sits inside this test's measured spread - read it as movement, not as a result. Updated for |
…thdrawn, rejoin is expressible Two of the burn-down's open items closed on 2026-09-08, each by its own agent, and the note records both as things that happened rather than things to do. The eager-mode PERIODIC_CONSUMER_SYNC stall (the confluentinc#857 family's fourth open item) is withdrawn by #478: four replays of the recorded seed on today's master drained completely with zero loss, and the "stall" was the Class 2 timing bound, whose crossing flips with the processor count at a fixed seed and tree. The grid it was opened on was also never a one-term A/B. Nothing to ship or to name; what survives is the per-shard liveness gap, already tracked and deferred. Whether "rejoin" after producer fencing is expressible in the lifecycle is known by a read of the #472/#474/#410 diffs against the engine's ownership rules: it is, and the question dissolves, because the instance never leaves the group - rejoin reduces to abort, replace and replay on the control thread under the write lock. The stack respects the @GuardedBy ledger, thread confinement and the lock pair. What it leaves is review-sized: #420's territory, one wire-level test nobody wrote, and the plan's one open question about declining the lock during a rebalance. None of it changes the tier 2 decision. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
|
@claude review this |
|
Claude finished @astubbs's task in 3m 15s —— View job Review complete
This is a clean, well-substantiated docs-only correction. Summary of what I checked and found: Verified independently (not just taken on the PR's word):
One inconsistency worth fixing before merge, low severity: Nothing else stood out. The commit message and PR body are consistent with the diff, the checklist's |
…he fourth item as open The automated review on #478 caught the one inconsistency this branch left: the family note top marker, written by the 2026-09-08 vetting sweep, still said the fourth and fifth items were open, and this branch withdraws the fourth. The marker now says so, and names why. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…merge-checked The line names #478 as the PR that withdrew the fourth item, which stays true after it lands; the self-reference gate cannot tell that on its own. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
🧪🔒 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 |
…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>
One conflict, the family note top vetted marker: this branch corrected it to say the fourth item is withdrawn, and #485 appended its proposed closure of the revoke-path deadlock line to the same marker. Both survive: master line, with the fourth-item correction re-applied. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
… both PROPOSED closes and the withdrawal in one marker #486 landed its PROPOSED close of the fifth item on the same vetted marker line this branch edits to withdraw the fourth. Resolved by taking master's marker and re-applying this branch's one clause - the fourth item is withdrawn by #478 - so the marker now carries #485's deadlock-line close, #486's fifth-item close and this withdrawal. The fifth item's heading takes master's RESOLVED form; the CORRECTED section this branch adds is unchanged. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…ster The fourth 857 item's withdrawal landed as a squash on 2026-09-08, after taking #486's marker change from master so the register's vetted line carries both PROPOSED closes and the withdrawal together. The burn-down records it merged in both places it is named. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
The replay queue in the lead paragraph is drained into PRs, so it now points at them and at the still-open list rather than describing work as queued. Three gaps the resolved items exposed are added so the release note cannot paper over them: the per-shard liveness blind spot that #478 leaves standing, the one unconditional by-key shard removal #483's sweep reported and nobody fixed, and the flake register's remaining rows, one still undiagnosed, which gate a green master rather than the scope. Claude-Session: 460f7df9-dcc2-4b00-a9f9-62f3a2c6d5e4 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…or a busy pool - it is an eventual certainty Review feedback on #487 said the previous close understated the finding: under retry-forever the held poison population only grows, so the latch is not a possibility to be weighed but something any long-lived instance reaches. Checked against the existing logs first, then measured with a fourth arm rather than asserted. The reviewer's conclusion is CONFIRMED and their arithmetic is a correct UPPER BOUND; the ordering claim inside it is refuted, in the direction that makes the defect worse. THE ARITHMETIC, CHECKED AGAINST ARMS 1-3 BEFORE ADOPTING IT. parkedForRetry is not a property of the population - by Little's law it is retry-throughput times retryDelay. Three independent confirmations in logs already taken: parkedForRetry has median 135 and hard max 140 in all three arms while inShards ranges 549 to 17,103, a 31x population change with an unchanged parked count; the failure rate is 133 / 133.5 / 129.5 per second in those same three arms against a C/U ceiling of 140/s; and arm 1's predicted unparked of 549-140=409 is exactly its observed minimum. So unparked = P - (retry throughput * retryDelay) latch when unparked > targetAmountOfRecordsInFlight * loadingFactor and since retry throughput cannot exceed maxConcurrency/userFnDuration, the subtracted term is bounded by maxConcurrency*retryDelay/userFnDuration = 140 here. P only grows. The latch is therefore unavoidable, with a computable ceiling of 140+42 = 182 held records at these defaults. ARM 4 - failureFraction 0.01, ten minutes, everything else arm 1's. PREDICTED, written before the run: the gate oscillates early and then stops permanently (the discriminator, since arms 1 and 2 never oscillated at all); successes flow for minutes; parked tracks P then pins at 140; the latch arrives late but arrives, at P between 168 and 182, around 5.6-6.1 minutes. OUTCOME: the shape is confirmed, the numbers are not, and the miss matters. The gate DID oscillate - 264 false readings against exactly four in each of arms 1 and 2, all of those at startup - and then stopped: the last false is inShards=71 - parkedForRetry=29 = 42 vs target(14)*loadingFactor(3) =42, the boundary exactly, after which the final unbroken true run is 8m57s. Successes rose 992 -> 1,971 -> 2,946 -> 3,902 and then froze at 3,902 for the remaining nine minutes. But it latched at P=98, about 64 seconds in - not 182 at six minutes - because its retry throughput settled at 30.6/s, so its parked term was ~30 rather than 140. A SLOWER RETRY SERVICE LATCHES THE GATE SOONER, since fewer records are in back-off and more therefore read as workable. 182 is a ceiling, not an estimate. SATURATION IS NOT A PRECONDITION, and this is the correction worth carrying. At arm 4's latch the pool was doing 30.6 failures/s - about 3 of its 14 workers, 22% utilisation, against arm 1's 95%. The instance stopped fetching from the broker while 78% IDLE. What limits the retry cadence to ~3.2s per record against a static 1s delay is NOT measured - confirmed static, no retryDelayProvider is set and WorkContainer#computeRetryDueAt has no progressive backoff - and it is now the first arm in the scenario's list, because the latch point is a function of that number. HEAD-OF-LINE BLOCKING GAINS THE ROLE IT ACTUALLY HAS. It is still not what latches the gate (arm 2 latches with no ordering constraint; arm 1 held one record per key). It is what stops the residue draining afterwards: arm 4's inShards fell 103 -> 98 and then sat at exactly 98 for 6,201 consecutive evaluations, nothing retiring for nine minutes. Those 98 are ~40 poison plus ~58 healthy records queued behind poisoned heads on their own keys - eleven idle workers beside 58 deliverable records they were not allowed to reach. TWO MORE MECHANISMS RULED OUT WITH EVIDENCE, both things a reader would otherwise suspect. A dead poll thread (#477's class): the soak reported no finding in any of the four arms, so the poller is alive and paused. A lost wakeup from a stale pause cache: maybeWakeupPoller() is gated on !isSufficientlyLoaded(), so with the gate continuously true no wakeup is ever attempted - its "Found Poller paused" line appears zero times in every arm, and nothing can lose a wakeup never sent. AND A DISTINCTION THE MERGE MADE NECESSARY. #478 withdraws an eager-sync stall as "a bound the processor count crosses, not a defect". Do not collapse this note into that one: that symptom DRAINED and needed a processor-count cap to appear, while this one never drains and is reached at 22% utilisation with no cap at all. The failureFraction=0 arm is withdrawn as superseded - arm 4 answers what it was aimed at with a live workload rather than an empty one. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
Serves #119 (confluentinc#857).
Description
Withdraws the "fourth open item" in
docs/inflight/bug-857-family.md- an eagerPERIODIC_CONSUMER_SYNCstall recorded as reproducing with two seeds, every time, on trees carrying#29's fix, with the mechanism unknown. It has no mechanism because there is
nothing there. Docs only: no product code and no test is touched.
The control arm
Seed
4709156528562690268- the replay grid's seed B, and the one of its two seeds that no recoverydiagnostic had ever been run against - replayed on
745b1f6a5(master plus everything through#466),
ChaosRevokeUnderWorkIT,-Dchaos.diagnoseStallRecovery=true, oneterm changed between the arms:
inFlight=0, every keyinFlight=0, every key-XX:ActiveProcessorCount=8)inFlight=0, every keyinFlight=0, every keySame tree, same seed, same test sources, byte-identical invocation apart from
JAVA_TOOL_OPTIONS.All four drained to
inFlight=0withuniqueConsumed=250000and zero violations. The processorcount alone decides whether the symptom appears. The grid's own confound list named that cap and
dismissed it - "the controls passing under the same cap argue it does not manufacture the stall" -
which is the reading these runs reverse.
The peaks are only measurements in the runs that did not fire:
recordLagStagnationre-arms thepartition it fires on, so a crossing run's peak is the bound plus sampler cadence and encodes no
severity - the same arithmetic the ledger already records about its ~154s constant.
The grid was also not a one-term A/B
ConsumerManager.pollrefreshes the pause cache at ENTRY and at exit on the grid's DEFECT arm(
438b09d9b); on its FIXED arm (b8a335b05) only the exit refresh is there. Exit-only is the shapedocs/solutions/performance-issues/paused-poll-wakeup-lost-to-stale-pause-cache-2026-09-01.mdowns,fixed two weeks after the grid ran by
d2690c57f, which is not an ancestor ofb8a335b05.What was refuted - the more interesting half
The prediction was that restoring exit-only refresh would push the 12-processor arm over the bound.
It did not: peak 125.2s and a 90.4s drain, no observations, both better than the unpatched arm.
ConsumerManagerPauseCacheTest#pausedPartitionCacheIsFreshDuringThePollItDescribesfails with theentry call removed and passes with it restored, so the patch really did restore the defect - the
negative control is what makes the null result readable. That solution doc measured its 4-10x
collapse in
PERIODIC_TRANSACTIONAL_PRODUCER; it does not reach this eager recovery at this shape.So the second term proves the grid was never one-term, and does not explain its
fixed-worse-than-defect asymmetry, which stays unexplained and which the control arm makes
uninteresting.
What survives
The per-shard liveness gap, unchanged:
INSTANCE_STALLis per-instance, so a watermark frozen by acommit that never landed, on an instance whose other shards keep completing, is covered by nothing
that gates.
docs/inflight/test-per-shard-liveness-has-no-gate.mdowns it, including the red controla replacement detector must have first - a bar this PR deliberately does not try to meet.
Where each correction landed
docs/inflight/bug-857-family.md- the fourth item marked withdrawn, with the numbers and theaccounting.
docs/inflight/test-857-revoke-under-work-sightings.md- the grid itself, and its withdrawnheadline claim.
docs/testing.md- one paragraph, because a reader meets the Class 2 interpretation there first.docs/solutions/best-practices/a-master-versus-branch-ab-measures-the-whole-branch-2026-09-08.md-the reusable method failure.
Limits, stated
Non-reproduction is the weak direction; this is four runs on one laptop, where the grid ran on a
32-core box capped to eight. A chaos seed fixes the conductor's schedule and never the
poll-versus-control interleaving, which is why the deadlock itself was settled on a deterministic
probe rather than by replay.
Checklist
N/Adocs/features/-N/A - no user-facing behaviour changes; this is a ledger correctionN/A - no product code changed. The finding is that an existing detector's finding is a speed number, so there is no behaviour to pin. The test that would matter is the correlated per-shard gate docs/inflight/test-per-shard-liveness-has-no-gate.md already specifies, and that note's own bar - a red control first - is not met here.docs/inflight/working note (pr-/branch-) started at the PR's first commit -N/A - the PR's entire content is a correction to two existing inflight notes; a third note describing it would be a second copy of what the diff saysce-simplifyandce-code-reviewlocally -N/A - docs only, ce-simplify has no code to work on; will ask for @claude review on the PR instead