fix(core) astubbs#119: the intake load gate says so when it has latched with nothing retiring - #497
Conversation
… nothing retiring #487 measured the intake stall and left the cheap mitigation written up but not built: any instance that retries forever and meets any poison at all ends with every partition paused, workers churning on records that never retire, and nothing whatsoever in the log - the state was exported only as the pc.partitions.paused gauge, and its low-poison arm reached it while 78% idle and looking healthy. This is that mitigation. WHAT IT ADDS. One observation and one log line. The gate's decision, the poller's pausing, the retry service and every counter are untouched: isSufficientlyLoaded() returns exactly what it returned before, and the poll thread's route into it through shouldThrottle() observes nothing at all. The control loop's once-per-pass call becomes isSufficientlyLoadedReportingLatch(pausedPartitions), which takes ONE reading of the shards and uses it for both the wakeup decision and the report - so the report can never print an equation the decision was not made on, the same trap ShardManager#getWorkableRecords already exists to close for the DEBUG line. THE SIGNAL IS CONSECUTIVE PASSES, NOT ELAPSED TIME. A timing bound would be a threshold argument nobody can win, and it would need a clock this class does not otherwise read. Both operands were already there: the gate's own reading, and RecordPopulation#getRetiredTotal, which is monotonic and is exactly "no record left a shard, by any route". Nothing new is counted. WHY A HUNDRED. Chosen from the loop's two cadences rather than from a target wall-clock time, because when nothing retires the pass rate differs by two orders of magnitude between the state being reported and the state that must not be. Latched, every held record fails on every attempt, so results arrive in the mailbox continuously and each pass returns at once - #487's low-poison arm measured 6,201 gate evaluations across a nine-minute unbroken latch, about 87ms a pass, so a hundred passes is about nine seconds, and the state it reports is permanent so a few seconds late costs nothing. Healthy inside a long user function, the mailbox is empty and each pass blocks for the commit interval, five seconds by default - so a hundred passes is over eight minutes in which not one record anywhere retired. The pass count therefore gives the healthy case roughly fifty times the grace it gives the latch, which no single elapsed-time bound can do. PC puts no ceiling on a user function, which is why the report is a WARN and not an exception. ONCE, THEN QUIET, THEN ONCE ON RECOVERY. The WARN fires on the pass that reaches the count and not again; the INFO fires when either clause stops holding, and re-arms the WARN so a second latch is not swallowed by the first. The WARN names inShards, parkedForRetry, workable, target times loading factor, and the paused partition count - scalars only, no collection interpolated, so it cannot be truncated past the point where it stops identifying the event. THREAD MODEL. The three counters are written and read only by observeLoadGateLatch, whose one production caller is maybeWakeupPoller on the control thread. They deliberately carry NO @ThreadConfined and no owning-thread assertion, which is this repo's usual pairing: the harness drives controlLoop from more than one thread inside a single test (BlockedThreadAsserter in ProducerManagerTest), so the assertion would fire on a legitimate caller - and a diagnostic that can kill the consumer is a worse trade than one that can miscount. No decision reads these fields. Recorded in docs/refactoring.md's non-volatile ledger so a later "fix" with volatile has the argument in front of it. RED FIRST, BOTH CLAUSES. Two sabotage arms, each restored: with the report removed entirely (master's behaviour) the two positive tests fail on their assertions rather than on compilation, and the control test still passes, which is what a negative control should do. Dropping only the "nothing retired" clause flips exactly the other two - the healthy-instance test now sees a WARN naming 300 held records, and the recovery INFO never arrives. Green with both restored. BrokerPollSystem gains getPausedPartitionCountForBackPressure(), and isSubscriptionsPausedForBackPressure() is now expressed in terms of it rather than repeating the same cache read. Serves #119 (confluentinc#857); closes neither - the fix that bounds the failures rather than the buffer is #149's dead-letter queue, and the note stays open on it. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
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 |
…arries it The number was written before the PR existed and guessed one low. It is #497, not 495. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
…ter the merge Both lines cite #497 in the past tense, which is how they are meant to read once it has landed, so they take the gate's `post-merge: checked` marker rather than a rewrite. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
|
@codex review this |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #497 +/- ##
============================================
+ Coverage 82.67% 83.26% +0.59%
- Complexity 1586 1604 +18
============================================
Files 96 96
Lines 5444 5475 +31
Branches 549 554 +5
============================================
+ Hits 4501 4559 +58
+ Misses 746 723 -23
+ Partials 197 193 -4
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 1% 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 10 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: a37d148, e8bd2cb, 1743297, 4bc6e7a, b62c310, c381310, c79424a, 9c67c89, f1aa5eb, eb9fdb0 Since the previous push: ratio 1.028 -> 1.01, share 1.799 -> 1.8, rate 74775 -> 63096 (-15.6%). One push of difference sits inside this test's measured spread - read it as movement, not as a result. Updated for |
|
LoadGateReading is read by one private method and returned to nobody - isSufficientlyLoadedReportingLatch hands back a boolean. Public was the reflex, and it would have added a type, its getters and Lombok's generated equals/hashCode/toString to a library surface that has to keep them. A diagnostic's internal shape is not API. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 918bc4c313
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e the release Owner review on #498, five changes: - The opening no longer teases three roadmap items in a clause. A new "What comes next" section lists the queue by state - implemented and on an open PR (fencing recovery, virtual threads, self-tuning concurrency, global rate limiting, Streams and Connect previews, the multi-language sidecar and in-process clients, the commit-failure seam, the health check, residence time, the dashboard, offset density, the direct-pull engine, the docs site, the API gate); designed but not built (the dead-letter queue, batch failure attribution, the poll-path error seam, micro-batching, bounded buffers, the Java 17 baseline); and the 1.0 API settlement. The list is drawn from docs/data/roadmap.yaml and the open PR list, and says so; previews are marked as previews per the announcement note's rule. - Breaking is rewritten as nested bullets: one line of consequence per change, one sub-bullet per thing a reader must do or know, no paragraphs. Same content, about half the words. - The Fixes subsection "Records lost or duplicated with nothing in the logs" is "Priority 1: data loss and duplicates". - The intake-stall limitation no longer says "silently": #497 adds a WARN when the gate has stayed latched with nothing retiring, and is in tier 1 for this release. An AsciiDoc comment beside the bullet records the tag-day dependency: if #497 has not merged, the WARN sentence comes out and "silently" goes back. - A "The size of this release" section quantifies the gap from 0.5.3.2, upstream's last published release: merged PRs, main and test Java lines added and removed with rename detection, and new files, main against test. These are figures a command can produce, which docs/merge-checklist.md warns against; they are here because the section is frozen at the tag and the release is the one place a point-in-time number is the point. The commands are in a comment beside them, and recomputing them is a tag-day check named in the PR body. Lines over 120 columns from the rewrite were rewrapped. The rendered Markdown was regenerated with #199's converter in strict mode and its self-test passes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XS64Xttx4vF5datYh7fmk
Codex review on #497, three findings taken. A FALSE ALL-CLEAR (P2). The recovery line read "Record intake has resumed" whenever the latch cleared, and one of the two ways it clears is a record retiring while the gate still reads loaded - the poller is still paused, and an operator told intake had resumed would stop watching an instance that is still not fetching. The reviewer found it through the recovery test itself, which clears with 399 records still held against a threshold of two. There are now two statements, each greppable on its own: an unloaded gate is the all-clear and says so, and a retirement under a loaded gate says the poller stays paused. The recovery test asserts the distinction, and gains an arm that drains below the threshold so the other branch is exercised rather than assumed. ONE OWNER FOR THE THREAD MODEL (P1). docs/refactoring.md's ledger entry restated the reasoning it had just named observeLoadGateLatch's javadoc as owning. It is now the pointer plus the work item, which is all a ledger owes. THE DRAFT ISSUE RESPONSE (P1). docs/inflight/AGENTS.md requires the note mapped to an issue to carry an unposted draft before its PR merges, and there is no exemption for a family mirror - what a family mirror changes is the SCOPE of the draft, not whether one exists. docs/inflight/issue-response-119.md is written to be appended to the mirror's Fork status section, not to replace it, and says in as many words that the issue stays open on the original deadlock. REJECTED, WITH THE ARGUMENT IN THE JAVADOC (P2): moving the observation after the mailbox drain. The suspicion is that a completion queued but not yet drained is invisible, so the count climbs through work that did retire. It is not: the observation sits at the same point in every pass, so the window between two readings spans a whole pass, drain included, and no retirement falls between them. What the position costs is one pass of latency out of a hundred, self-correcting on the next pass. Both remedies are worse than that. A second gate reading per pass buys it with an O(n) fair-lock acquisition on the hottest path in the engine, and reintroduces the trap ShardManager#getWorkableRecords exists to close - the report would print an equation the poller decision was not made on. Gating on an empty mailbox would suppress the WARN in exactly the state it exists for, because the latched workload's failure results flow through that same mailbox continuously. Recorded as a dated cleared suspicion on observeLoadGateLatch, with what would reopen it. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
…ighten Breaking, name what comes next, and size the release Owner review on #498, in one commit. THE SOURCE-COMPATIBILITY CLAIM WAS FALSE, in four places. The opening paragraph said "no method signature changed, but two identifiers did", and the coordinates bullet under Breaking said "no signature changed, so nothing else in your code moves" - while the bullets beneath them listed a renamed exception, a removed method, a new exception type on the commit failure surface, changed protected signatures on the controller, a changed stream contract, and identity equality on RecordContext. The claim had been copied from src/docs/README_TEMPLATE.adoc, which carried it twice (the fork summary and the Upgrading section), written when the rename was the only change and never revisited. All four sites now say: for most users the upgrade is the pom and the imports, it is not source-compatible beyond that, the offset format is unchanged so a consumer group upgrades in place, and the API changes are the short list under Breaking - which the README names, so a reader knows what to look for before following the link. README.adoc is regenerated from the template with the asciidoc-template plugin. WHAT COMES NEXT replaces a one-clause teaser. A new section lists the queue by state, drawn from docs/data/roadmap.yaml and the open PR list: implemented and on an open PR (fencing recovery and the two transactional fixes it unlocks, virtual threads, self-tuning concurrency, global rate limiting, the Streams and Connect previews, the multi-language sidecar and in-process clients, the commit-failure seam, the health check, residence time, the dashboard, offset density, the direct-pull engine, the docs site, the API gate); designed but not built (the dead-letter queue, batch failure attribution, the poll-path error seam, micro-batching, bounded buffers, the Java 17 baseline); and the 1.0 API settlement. Previews are marked as previews, per the announcement note's rule. BREAKING is nested bullets: one line of consequence per change, one sub-bullet per thing a reader must do or know. Same content, half the words. THE PRIORITY-1 FIXES SUBSECTION is named "Priority 1: correctness", for the property rather than the failure. THE INTAKE-STALL LIMITATION no longer says "silently": #497 adds a WARN when the gate has stayed latched with nothing retiring, and is in tier 1 for this release. An AsciiDoc comment beside the bullet records the tag-day dependency - if #497 has not merged, the WARN sentence comes out and "silently" goes back. THE SIZE OF THIS RELEASE is a new section quantifying the gap from 0.5.3.2, upstream's last published release: merged PRs, main and test Java lines added and removed with rename detection, and new files, main against test. These are figures a command can produce, which docs/merge-checklist.md warns against; they are here because the section is frozen at the tag and the release is the one place a point-in-time number is the point. The commands are in a comment beside them, and recomputing them is a tag-day check named in the PR body. The rendered Markdown was regenerated with #199's converter in strict mode and its self-test passes; every line of the section is within 120 columns. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XS64Xttx4vF5datYh7fmk
… is on master #496 merged after this branch was cut. Its subject carries the breaking marker and its body states the user-visible change: zero, a negative or null batch size now throws at construction where it used to start a consumer that processed nothing, or died in an ArithmeticException when messageBufferSize was set. The bullet says that, and the one thing a deployment has to know - a property resolving to zero now fails to start. The tier 1 item that still has no bullet is #497, the gate-latch warning, which is open; its tag-day line in the PR body stands. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XS64Xttx4vF5datYh7fmk
… it turned up THE CONFIRMATION. One run of CommitResponseTimeoutSoakIT's KEY arm against the shipped WARN - same seed 3747722682837130843, failureFraction 0.5, same workstation, shortened to PT4M because the latch arrives in the first second and this is a confirmation rather than a re-derivation, with the gate logger at info so the report is visible without the per-tick DEBUG equation. The arm reproduced: succeeded=451, which is arm 1's number and #471's thirty-minute number. Exactly ONE WARN in the whole run, about ten seconds after the banner - a hundred passes at the latched cadence #487 measured, as designed - and no clear line, because the latch never cleared. Its operands agree with the arms that derived them: inShards=549 (arm 1's pinned population), parkedForRetry=138 (inside the band arms 1-3 measured), all twenty partitions paused. Written into the scenario's own Calibration status block, which owns these numbers. THE SIGHTING. The Lincheck lane cancelled at its 20-minute budget twice on this branch, with every other job in both runs green. It is the lane, not the branch, and the control arm is a documentation-only branch that succeeded at 19m58s the same afternoon - it cannot change what the model checker explores. A third branch took 12m55s. Against a recorded 7m42s baseline, which side of the bound a branch lands on is the runner it drew. Also ruled out directly: no harness in the lane reaches the changed code - WorkManagerLincheckTest's operations are handleFutureResult and the revoke/reassign pair, and no harness mentions the intake gate. Recorded in the lane's own note because a CI log expires and the evidence that a red was infrastructure does not survive it. The decision it needs - a larger budget, fewer iterations on the arm that dominates the wall clock, or a split - is left open there. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
…anch The sighting cited `fix/119-gate-latch-warning` and said "this branch broke Lincheck". A branch stops existing when it lands, so both read as nothing to whoever finds the note later; the PR number is the durable anchor. Reworded to #497 throughout, in the past tense, with the gate's post-merge attestation around the section. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
The tier 1 line still read "no PR yet". #497 has been open as a draft since the item was decided, with the Claude review answered and the Codex review in progress. The box stays open until it merges. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
… CI keeps only the testing lanes #496 merged after this branch was cut. Its subject carries the breaking marker and its body states the user-visible change: zero, a negative or null batch size now throws at construction where it used to start a consumer that processed nothing, or died in an ArithmeticException when messageBufferSize was set. The bullet says that, and the one thing a deployment has to know - a property resolving to zero now fails to start. Build & CI, on owner review, is cut back to the lanes that say how the library is tested and analysed; it ends at the CVE scan and the hygiene gates. The tier 1 item that still has no bullet is #497, the gate-latch warning, which is open; its tag-day line in the PR body stands. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XS64Xttx4vF5datYh7fmk
…ad cancelled twice The fourth data point, and the one that settles the sighting: same branch, same harnesses, nothing the lane can see changed between the cancels and the pass - which lands it three seconds inside the twenty-minute bound, beside the documentation-only branch at 19m58s. The outcome is decided by the runner the job drew, not by the diff. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
|
@claude review this |
|
Claude finished @astubbs's task in 2m 45s —— View job Review
I read this against the actual current tree (not just the diff shown to Codex against No new issues found. This is a clean, well-scoped mitigation. Specifics:
Nothing to add beyond what Codex and astubbs already resolved in review. This looks ready from a correctness standpoint, modulo the still-open checklist item ( |
… not "any poison" Owner review point, 2026-09-09. "Retry-forever plus any poison at all" was inherited from #487 and overstates the result. A SINGLE record that never succeeds does not latch the gate and cannot. The gate is inShards minus parkedForRetry against target times loading factor: one held record, minus one parked while it waits out its back-off, is nowhere near a threshold of tens. Its offset map encodes a single gap compactly, the commit sits below it, and the instance runs indefinitely with that record retrying beneath a healthy stream that keeps retiring. What latches the gate is a non-zero FRACTION of a live stream that never succeeds, and both properties that make it inevitable are about the population rather than any one record: healthy records retire and leave the shards, these do not, so their share of what is held rises monotonically while the stream keeps arriving, and the parked term subtracted from it is bounded by throughput rather than population. "Any fraction" is the correct claim and still a strong one - the measured arm reached it at 1%. Corrected in place in the note, which is the live record rather than a dated one, with a dated section saying what was corrected and why. #487's own text on master is left alone; the soak scenario's Calibration status block carries the correction as a note against its verdict rather than a rewrite of it, because a dated record of runs may not be edited to match today's reading - none of its measurements change, only the claim drawn from them narrows. Every arm it ran was a fraction (0.5, then 0.01). Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
…hold's own bound is corrected Four review passes over the branch diff - correctness, reliability, testing and an adversarial one. Correctness and the reuse/quality/efficiency passes returned nothing to fix. What follows is what the other three found. A FALSE ALL-CLEAR THE EARLIER WORDING COULD HAVE GIVEN. The trigger reads RecordPopulation#getRetiredTotal, which rises when a record leaves a shard by ANY route - success, revocation, or a stale container being swept, as its own javadoc says. For the trigger that is exactly right: a revocation really does drain the shards and really does unlatch the gate. For the message it was not. "Records are retiring again" and "intake has resumed" would both have been emitted by an instance whose partitions were just revoked, with nothing having succeeded - an all-clear delivered at the precise moment an operator or the group coordinator is intervening in the stall. Both lines now report what is measured, records leaving the shards, and both carry the same tail naming the three routes and saying it is not on its own evidence that processing recovered. Pinned by a new arm that latches, revokes with zero successes, and asserts the clear line does not claim recovery. THE PASS COUNT IS CALIBRATED AGAINST ONE COMMIT INTERVAL, AND THE JAVADOC SAID OTHERWISE. The healthy-case bound is the commit interval, not five seconds: under PERIODIC_TRANSACTIONAL_PRODUCER the default is DEFAULT_COMMIT_INTERVAL_FOR_TRANSACTIONS, two orders of magnitude shorter, so the healthy grace collapses to roughly the same order as the latched cadence and a fully-loaded transactional instance inside a long user function can be reported. The "fifty times" claim holds at the ordinary default and nowhere else. Corrected in place, with the false-alarm window named. The TRIGGER is deliberately not narrowed: the line already prints parkedForRetry, which this state holds continuously and a merely-slow instance reads as zero, so an operator can separate them from the report itself - and narrowing on that term would change what fires and wants its own measured arm, since a transient zero would suppress a real latch. Left open in the note with both candidates. A TEST THAT COULD NOT SEE ITS OWN MUTANT. Deleting the count reset from the recovery branch, leaving the flag reset, survived all three tests: the re-arm arm asserted the cumulative WARN count after a full batch, which is satisfied whether the second report arrives on pass 100 or on pass 1. It now drives one pass short and asserts the count is unchanged first. Confirmed as sabotage arm 3 - the mutant fires the second WARN at pass 101 and the new assertion catches it - and arm 4 confirms the revocation arm, which goes red when the honest tail is dropped from the all-clear branch. Also: consecutiveLatchedPasses is a long. It keeps counting past the threshold because the clear line reports how long the latch lasted, so an int would eventually wrap and print a negative duration; the widening costs nothing. And the paused-partition count is labelled "as of the last poll" wherever it is printed - it is a per-poll cache, and a WARN reading pausedPartitions=0 beside text asserting the poller is paused is a contradiction an operator should not have to resolve at 3am. Not taken, reported instead: the observation runs only while the state is RUNNING, so a graceful shutdown that hangs in DRAINING - a plausible shape of this very defect - is not reported. Widening the guard is a decision about what the diagnostic is for, not a mechanical fix, so it is the owner's. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
|
@claude review this Context for the pass, so it does not re-derive what has already been settled. This adds one observation and one log line to the record-intake load gate: a WARN when the gate has read loaded across Already reviewed and addressed, so please spend the pass elsewhere unless you think one of these is wrong:
Two things are deliberately open and named in the body rather than fixed, and a second opinion on either is worth more than a fresh sweep:
Worth attacking hardest: whether the pass count can false-alarm on any healthy configuration I have not named, and whether the two clear branches are exhaustive and correctly assigned. |
|
Claude finished @astubbs's task in 5m 39s —— View job Review
Not re-litigating the settled items (Codex's five, The two clear branches: exhaustive and correctly assigned
That's exhaustive over the boolean pair, and the assignment is right: real recovery is defined purely by A false-alarm window the javadoc doesn't name, alongside the transactional one
Net effect: a healthy, ordered, backlog-holding instance with one ordinary transient retry pending can run its passes at roughly the 1-second retry-delay cadence rather than the assumed 5-second commit-interval cadence, collapsing the ~50x healthy grace to roughly ~10x — well short of "eight minutes," on stock defaults, no transactional mode required. It's a narrower and cheaper-to-trigger version of the already-documented collapse, via a mechanism the javadoc and I haven't run this as an experiment — it's a static trace of Everything else
|
…guration at all From the automated review on #497, and it is the better half of the finding the previous commit only got halfway. getTimeToBlockFor() has two branches and the threshold's derivation described one of them. The commit-interval branch is the one already corrected. The other is taken whenever isWorkInFlightMeetingTarget() is false - dispatch below full concurrency, which is the ORDINARY state under KEY or PARTITION ordering whenever fewer keys are active than maxConcurrency allows, not a misconfiguration - and any record at all is in retry back-off. The pass then blocks for min(commitInterval, max(defaultMessageRetryDelay, lowestScheduled)), which at the stock one-second retry delay is a one-second cadence rather than five. A backlog-holding ordered instance with one transient retry pending therefore has roughly a hundred seconds of grace, not eight minutes, on stock defaults with no transactional mode involved. So the eight minutes is the best case, not the bound, and both windows are now named where the constant is defined and in the note. The discriminator is unchanged and still works in both: the report prints parkedForRetry, which the state it exists for holds continuously and a merely-slow instance reads as zero. Recorded as a static trace of the two branches rather than a measured arm, and said so in both places - it wants a calibration run before anything is decided on it, which is the same bar every other claim about this mechanism was held to. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
|
Taken, in I had corrected the derivation for the commit-interval branch and stopped there, which left the paragraph still implying that the commit interval is the healthy bound. It is not:
So with one record in back-off the pass blocks for Carried over your framing that this is a static trace of the two branches and not a measured arm, in both places, because that is the bar every other claim about this mechanism was held to - a calibration run comes before anything is decided on it. The discriminator is unchanged and still applies to both windows: the report prints On the two branches being exhaustive and correctly assigned - agreed, and your reading of why |
#499 diagnosed the Lincheck lane's timeouts more fully than this branch's sighting did and settled the decision it left open: the cap goes from 20 to 60, and the cause is ordinary hosted-runner speed variance against a fixed, uninterruptible budget - measured across every branch on the day, docs-only ones included, and reversing itself on a clock rather than on a commit. The only conflict was the note both touched. Resolved by keeping 499's section whole - it owns the cause and the ruling - and reducing this branch's sighting to what it adds and 499 could not have: the same branch on BOTH sides of the bound, three cancels and one 19m57s pass with nothing changed that the lane can see, which states the same diagnosis from one branch instead of across many. The open decision this branch recorded is marked as settled by that ruling rather than left standing. Also inherited: #496 rejects a batch size below one, and #446 lifts the v6 announcement plan onto master. Neither touches the record-intake gate. Co-Authored-By: Claude Opus <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
The gate-latch warning is on master, which ticks the last tier 1 box. The line records what the reviews settled: the trigger is consecutive passes rather than elapsed time, the soak confirmation at #487's seed, the two design calls left for the owner, and the correction that the latch takes a fraction of never-succeeding records rather than one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
…es (#498) The release-time rewrite of CHANGELOG.adoc's `== 0.6.0.0` section, so that it is the published release notes rather than the working text it had been since the fork. docs/releasing.md says the section for the release being cut is generated at release time, replacing what is there; there is no generator in bin/, so this is that generation, done by hand from the first-parent commit log since the fork point (every fix, fix!, feat, feat! and deps body read in full) and from the release documents: the v6 burn-down note on #475, docs/inflight/release-0.6.0.0.md, docs/data/roadmap.yaml and the open PR list. It is the one deliberate exception to "a PR never adds a changelog entry", which exists so this rewrite can be written as a set. What the section now says, in order: the fork and its coordinates, the stability-release argument, and that upgrading is the pom and the imports for most users but is not source-compatible beyond that; the size of the release since 0.5.3.2, upstream's last published release; Breaking, as nested bullets - the coordinates and Java packages, the commit-budget exception, JStream blocking until close, the metadata-policy default, the batchSize bound, the exception rename and removed listener getter, identity equality for RecordContext, the two controller narrowings, the Mutiny Java 17 floor; Fixes in three subsections - priority 1 correctness, consumption stopped after a rebalance (the confluentinc#857 story: the mechanisms closed, the detector lines demoted to timing proxies, the one unattributed chaos-only arm), other fixes; Known limitations, stated so the release claims no more than it can show; What comes next, by state - implemented and on an open PR, designed and not yet built, toward 1.0; Dependencies re-read against the poms; Examples; and Build & CI, the lanes that say how the library is tested and analysed, with Fray named as the next concurrency lane. Claims removed or corrected from the old text: the "source-compatible" claim, which the Breaking list itself contradicted, removed here and from src/docs/README_TEMPLATE.adoc in two places, README.adoc regenerated; the wrong upstream attribution on the null-epoch fix that #217 asked to be dropped; "upstream's last release 0.5.3.3" corrected to 0.5.3.2 published; the Reactor version, which said 3.8.6 while the pom says 3.8.7; the self-hosted lane described as per-PR; the quarantine state; two counts restated as shape; two upstream bullets folded into the fork entries that carry them. The heading loses its "(unreleased)" suffix, so release.yml's exact heading match now finds the section. The release page body is still posted by hand on the day from the converter's Markdown, per the burn-down's tier 3; #199 follows the tag. Tag-day checks this leaves: #497 must be merged, or the WARN sentence in the intake-stall limitation comes out and "silently" goes back (an AsciiDoc comment beside the bullet says the same); and the figures under "The size of this release" are recomputed with the commands in the comment beside them. Also touched: docs/inflight/release-0.6.0.0.md gains the settled release-condition wording at the same insertion point #475 amends it, whichever merges second keeps the settled paragraph; docs/releasing.md no longer says the section's generation is undecided. Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
…uilt The review caught the confluentinc#857 section calling the gate-latch warning "still to be built" after tier 1 had ticked it as #497. The ranking note had the same shape twice for the batchSize bound, which merged as #496. All three now name the PR that landed each. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD
…ter dropped The version sections were rendered by #199's converter, which does not emit AsciiDoc line comments - so every `//` block inside a version section was silently lost in the conversion. Only the preamble's one comment survived, because the preamble was converted by hand. The automated review found one of the four. Grepping the defect class rather than the symptom - `grep -n '^//'` against `3e66041e3^:CHANGELOG.adoc`, which is the last byte-identical copy - finds twelve comment lines in six blocks: - The preamble's `git log --pretty` recipe. Already present, hand-converted. - The `// only show TOC if this is the root document` note. Correctly gone: it annotated the `ifndef::github_name[]` / `toc::[]` directives, which have no Markdown equivalent and were themselves dropped on purpose. - Regenerate at the tag, above `### Breaking`: the exact commands that recompute the size-of-this-release figures. This is the one the review flagged. - TAG-DAY, in Known limitations: what to do to the latched-gate bullet if #497 had not merged before the tag. - There is no 0.5.3.4 release, at the end of the 0.6.0.0 section: why a version number is missing from the file. - The upstream release-tag URL under `## v0.4.0.0`, a source note on a pre-fork section. The last four are restored as HTML comments, in the positions they held in the AsciiDoc. Nothing rendered changes: comment syntax is the only difference, and content parity is unmoved at 29 `## ` headings, 58 `### ` headings and 342 bullets. The TAG-DAY block is indented two spaces so it sits inside the list item it annotates. A `<!--` at column zero between two bullets is an HTML block, which ends the list and starts a new one - a gap AsciiDoc line comments do not create, and one that would have shown up in the release body. DECISION FOR THE MAINTAINER, deliberately not taken here. Three of these now live inside the `## 0.6.0.0` section, and this PR makes that section the release body verbatim - so they ship into the v0.6.0.0 release notes. They do not render, but they are readable in the body source, which the old converter route never exposed. Regenerate-at-the-tag and TAG-DAY are the maintainer-only two; moving them to docs/releasing.md instead is a one-line change if you would rather the release body carried none of them. Restoring in place is the lossless default and is what the review suggested first. `awk` extraction rehearsed over the modified file: 0.6.0.0 is 631 lines, 0.5.3.3 is 7, v0.4.0.0 is 15 - all non-empty, so the fail-loud guard is not tripped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XS64Xttx4vF5datYh7fmk
…t - #497 merged The note told the tag-day operator what to do to the intake-stall bullet if #497, the gate-latch warning, had not merged before the tag: remove the sentence about the WARN and put "silently" back. #497 merged on 2026-09-09, so the sentence stands and the note is dead text. It did not render, but it sat in the release body's source, where a reader would trip over an instruction that no longer applies. The bullet it annotated is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XS64Xttx4vF5datYh7fmk
…ut it (#475) 0.6.0.0 is a bugs-only stability release, and it is overdue: the fork has carried the fixes for upstream's most-reported defects for months while the release waited on features. This note is the source of truth for cutting it - the owner's decisions, the merge queue from those decisions to the tag, every open question, and the checks that make the published artefacts true on the day. #197 is the tracking handle and its body points here; nothing is maintained on the issue. THE DECISIONS, 2026-09-07 and confirmed since. The bar is the stability release and nothing else; Streams and Connect move to the next-0x horizon in the roadmap data; the producer-recovery stack is outside v6. The release claim carries two named exceptions rather than waiting on them: the transactional revoke wait (#44, bounded since #466, not yet declined) and, from 2026-09-09, the poisoned-transaction wedge, both in the transactional producer mode only. The merge queue closed on 2026-09-09; later finds are 0.6.0.x unless they are data loss on a default configuration. THE BURN-DOWN, recorded as each merge landed. Tier 1, the self-contained fixes, is complete: the last two to join were the batchSize bound (#496) and the gate-latch warning (#497), both decided v6-sized on the day the queue closed. Tier 3, the plumbing, has the changelog section finalised as the release notes and the claim amended (#498) and the release page body posted verbatim from CHANGELOG.md by release.yml (#501, closing #199); what remains is the tag-day checks, the drafted issue responses, and the tag. A can-follow list names what is deliberately not v6. WHAT THE RELEASE NOTE SAYS ABOUT THE confluentinc#857 FAMILY, each line with the PR that settled it: the revoke-path deadlock proven by control arm, the eager stall withdrawn as a timing bound that flips with the processor count, the fifth item measured as the consumer-group protocol under churn rather than PC, the poller death fixed, the instance-stall sightings classified as worker saturation from the load side. The intake stall #471 found has its verdict from #487: the record-intake load gate is what stops intake, head-of-line blocking is not why, and any instance that retries forever while a fraction of its stream never succeeds latches eventually at a computable threshold, idle or not. There is no gate fix; the fix bounds the failures (#149's dead-letter queue), and until then #497 makes the state visible. One arm stays unattributed and is named as such. DATA LOSS AND DUPLICATES: the bug-162 replay branch refuted and the false truncation warning fixed (#494, closing #162). KNOWN UNKNOWNS, split in two so nothing is papered over: what is still unknown at the cut - the shard half of the per-shard liveness blind spot, the flake rows kept open with reasons, the maturity claim - and, under its own heading, the unknowns made known on 2026-09-08 and how each was settled. TAG-DAY CHECKS, folded in from the retired blockers note: master green with the lanes known to lie named, the churn scenario's no-progress window settled by replay and widened in #499 with the rebalance-dwell bound named as that class's survivor, the Lincheck lane's timeout raised against runner-speed variance, the rename named in both groupId and packages, the README's trademark wording claiming nothing it does not have (#495), and the changelog section as the release notes since #498, posted as the release body by release.yml since #501. ONE CHANGELOG EDIT, on the owner's decision of 2026-09-10: the "size of this release" table of merged-PR and line counts is removed. Measured on a branch, carrying its own re-measure instruction, stale from the next merge on; the notes make their claim through the fixes they name. Also here: a ci- note from this PR's own last review round - the file-refs gate reads a token as a path only with two segments, so the changelog rename left this branch-only note naming the old file with nothing to go red, and the note records the allow-list that would close it; the vetting sweep's reading and every open bug note's disposition, moved into the ranking note where the tiers override them; a dated survey of upstream items with no fix and no response as its own deferred note; the refactoring registry's codec entry corrected for what #480 did and did not change; and the confluentinc#546 manifest entry marked merged. Two notes retired with their content migrated: the blockers register and the merge-order plan for a far larger v6. The question this note began as, "when is v6 good enough?", was answered on 2026-09-08 and the file renamed. Serves #197; closes nothing. The tracker closes when the tag is cut. Co-authored-by: Claude Fable 5.1 (1M context) <noreply@anthropic.com>
Serves #119 (confluentinc#857) - it does not close it.
Description
#487 measured the record-intake stall and left the cheap mitigation
written up but not built. Under retry-forever plus any non-zero fraction of records that never
succeed, an instance ends with every partition paused, its workers churning on records that never
retire, and nothing in the log - the state was exported only as the
pc.partitions.pausedgauge. In #487's low-poison arm - that PR's measurement, not a claim about
every instance - the instance reached it with eleven of its fourteen workers idle, looking healthy
from outside. This builds that mitigation: one observation and one log line.
It takes a fraction, not one bad record. A single record that never succeeds is one held minus
one parked against a threshold of tens, so it never crosses; its offset map encodes one gap
compactly, the commit sits below it, and the instance runs indefinitely with that record retrying
underneath a healthy stream. What latches the gate is the share of held records that never leave
rising while the stream keeps arriving - healthy ones retire, these do not.
No semantic change
The gate's decision, the poller's pausing, the retry service and every counter are untouched.
WorkManager#isSufficientlyLoaded()returns exactly what it returned before, and the broker-pollthread's route into it through
shouldThrottle()observes nothing at all. The control loop'sonce-per-pass call in
maybeWakeupPollerbecomesisSufficientlyLoadedReportingLatch(pausedPartitions),which takes one reading of the shards and uses it for both the wakeup decision and the report -
so the report can never print an equation the decision was not made on, which is the same trap
ShardManager#getWorkableRecords()already exists to close for theDEBUGline.The signal is consecutive passes, not elapsed time
Both operands were already there: the gate's own reading, and
RecordPopulation#getRetiredTotal(),which is monotonic and is exactly "no record left a shard, by any route". Nothing new is counted, and
no clock is read. A timing bound would be a threshold argument nobody can win - see
docs/solutions/best-practices/a-timing-bound-used-as-a-correctness-gate-manufactures-its-own-evidence.md.Why 100 passes
Chosen from the loop's two cadences, not from a target wall-clock time, because when nothing retires
the pass rate differs by two orders of magnitude between the state being reported and the state that
must not be:
continuously and each pass returns almost at once. As measured by
test(soak) astubbs#119: any retry-forever instance eventually stops fetching, at a threshold you can compute #487's low-poison arm - 6,201 gate evaluations across a nine-minute
unbroken latch, about 87ms a pass - 100 passes is roughly nine seconds, and the state is
permanent, so being a few seconds late costs nothing.
getTimeToBlockFor(), bounded by the commit interval (five seconds by default). 100 passes is thenover eight minutes in which not one record anywhere in the instance retired.
So the pass count gives the healthy case roughly fifty times the wall-clock grace it gives the
state it is hunting, which no single elapsed-time bound can do. PC puts no ceiling on a user
function, which is why the report is a WARN and not an exception. The full derivation lives on
WorkManager#LATCHED_PASSES_BEFORE_WARNING.Two known false-alarm windows, both found by review and both named in that javadoc - eight minutes
is the best case, not the bound.
getTimeToBlockFor()has two branches:PERIODIC_TRANSACTIONAL_PRODUCERthedefault is
DEFAULT_COMMIT_INTERVAL_FOR_TRANSACTIONS, two orders of magnitude shorter, so thehealthy grace collapses to the same order as the latched cadence.
isWorkInFlightMeetingTarget()is false - dispatch below full concurrency, the ordinary stateunder
KEY/PARTITIONordering - and any record is in retry back-off, the pass blocks formin(commitInterval, max(defaultMessageRetryDelay, lowestScheduled)): a one-second cadence onstock defaults, so about a hundred seconds of grace rather than eight minutes. Recorded as a static
trace of the two branches, not a measured arm.
The report carries its own discriminator in both cases - it prints
parkedForRetry, which this stateholds continuously and a merely-slow instance reads as zero - so narrowing the trigger on that term
is deliberately not done here: it changes what fires and wants its own measured arm. Left open in
the note, with both candidates.
Once, then quiet, then once on recovery
The WARN fires on the pass that reaches the count and not again. The clear fires when either clause
stops holding, and re-arms the WARN so a second latch is not swallowed by the first. The WARN names
inShards,parkedForRetry,workable,target * loadingFactorandpausedPartitions- scalarsonly, no collection interpolated, per
docs/inflight/bug-unbounded-log-lines.md.The clear is two statements, not one, because the two ways it clears are not the same news. A
gate that has gone unloaded means intake can really resume and says so; a record leaving the shards
while the gate still reads loaded means the poller is still paused, and reporting that as "intake
has resumed" would be a false all-clear. Codex review found this through the recovery test itself,
which clears with 399 records still held against a threshold of two.
And neither line claims processing recovered. The trigger reads
getRetiredTotal(), which riseswhen a record leaves a shard by any route - success, revocation, or a stale container being swept.
That is right for the trigger (a revocation really does drain the shards and unlatch the gate) and
wrong for a message, because revoking a stalled instance is exactly what an operator or the group
coordinator does to one. Both lines now report what is measured and name the three routes. Pinned
by an arm that latches, revokes with zero successes, and asserts the clear does not claim recovery.
It logs on the
WorkManagerlogger deliberately: both test logback profiles already pin that loggername for the load gate (
pc.loadgate.log.level), so the WARN reaches a soak run without a loggingchange, and the recovery INFO reaches it too under the integration profile's
infodefault.Thread model
The three counters are written and read only by
observeLoadGateLatch, whose one production calleris
maybeWakeupPolleron the control thread. They deliberately carry no@ThreadConfinedand noowning-thread assertion, which is this repo's usual pairing: the harness drives
controlLoopfrommore than one thread inside a single test (
BlockedThreadAsserterinProducerManagerTest), so theassertion would fire on a legitimate caller - and a diagnostic that can kill the consumer is a worse
trade than one that can miscount. No decision reads these fields; the worst a foreign caller costs is
one emitted or missed WARN. Recorded in
docs/refactoring.md's non-volatile ledger so a later "fix"with
volatilehas the argument in front of it.Red first, on both clauses
Two sabotage arms, each restored afterwards:
rather than on compilation; the control test still passes, which is what a negative control should
do.
sees a WARN naming 300 held records, and the recovery INFO never arrives.
Green with both restored. Full local unit suite green, and the recovery test gained an arm that
drains below the threshold so the unloaded branch is exercised rather than assumed.
Review
Codex reviewed commit
918bc4c31at the owner's request: five findings, four taken, one rejectedwith the argument recorded as a dated cleared suspicion on
observeLoadGateLatch. Every thread hasan in-thread reply and is resolved. The rejected one asked for the observation to move after the
mailbox drain; the window between two consecutive readings already spans a whole pass, drain
included, so nothing is invisible - what the position costs is one pass of latency out of a
hundred, and gating on an empty mailbox would suppress the WARN in exactly the state it exists for,
because the latched workload's failure results flow through that same mailbox continuously.
Also
BrokerPollSystemgainsgetPausedPartitionCountForBackPressure(), andisSubscriptionsPausedForBackPressure()is now expressed in terms of it rather than repeating thesame cache read.
Open, for the owner to decide - not taken here
The diagnostic is silent outside
RUNNING.maybeWakeupPoller'sstate == RUNNINGguard wasright for its original job (do not wake a poller you are not running) and now also gates the
observation. A graceful shutdown that hangs in
DRAININGbecause the buffer will not drain is aplausible shape of this very defect, and there the WARN can never fire. Widening the guard - keeping
RUNNINGforwakeupIfPaused()and observing under the existingisIdlingOrRunning()- is adecision about what the diagnostic is for, not a mechanical fix, so it is left for you.
Checklist
docs/inflight/bug-119-load-gate-counts-blocked-work-as-available.md(the"Make the latch loud" bullet is now done and cites this PR; the note stays open on
confluentinc#310: Add a dead letter queue (DQL) implementation #149), and
docs/refactoring.md's non-volatile ledgerdocs/features/- not a new file. This is anew operator-visible surface on an existing capability, so the record that owns it,
docs/features/backpressure-and-broker-liveness.yaml, gains aboundariesentry naming the WARNand the INFO.
docs/features/README.mdsays a feature page is for a new capability, not for everychange to one, and a second file would have split the same capability across two records.
WorkManagerTest: the report fires once at WARN with theoperands, a loaded instance whose records retire never trips it, and the recovery INFO fires once
and re-arms the WARN
docs/inflight/working note (pr-/branch-) started at the PR's first commit - N/A - thework's note already exists and owns this item:
bug-119-load-gate-counts-blocked-work-as-available.md,on master since test(soak) astubbs#119: any retry-forever instance eventually stops fetching, at a threshold you can compute #487. A
pr-note would be a second copy of it. Thedirectory's other obligation is met:
docs/inflight/issue-response-119.mdcarries the unposteddraft response, scoped to append to the mirror's Fork status rather than replace it, since
confluentinc#857: Paused consumption across multiple consumers #119 mirrors a whole family and stays open on the original deadlock.
ce-simplifyandce-code-reviewlocally.ce-simplify: one reuse finding (the operandtail is spelled out in each log statement rather than shared) - rejected, because collapsing it
would trade per-field SLF4J arguments for one opaque token and make each line unsearchable in
source; zero quality findings, zero efficiency findings (one gate read per pass confirmed
unchanged, log arguments confirmed unevaluated when they do not fire).
ce-code-review:correctness clean; testing found a mutant that survived all three tests (deleting the count reset
from the recovery branch) - fixed, with the sabotage arm run; reliability found the false
all-clear on a revocation - fixed; adversarial found the threshold's healthy bound is the
commit interval rather than five seconds - javadoc corrected, trigger deliberately unchanged,
and that the diagnostic is silent outside
RUNNING, which is a design call left for the owner anddescribed below.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Xoi3HYae8pjsEatuNFKieD