Repository navigation
fix(core) astubbs#370: register the offset before the record is published to its shard - #450
Merged
Merged
Conversation
… its shard
PartitionState#maybeRegisterNewPollBatchAsWork added a record's container to
its shard first and put its offset into incompleteOffsets second, so between
the two the record was selectable by any shard scan while the state that
makes its completion valid did not yet hold it. A completion landing in that
gap tripped PartitionState#onSuccess's `assert removedFromIncompletes` - the
one shape that fires it with no double delivery at all. Without -ea it was
worse than a crash: the removal silently removed nothing, the second
registration then put the already-completed offset INTO the incomplete set,
and nothing could ever complete it again because the shard had removed its
container on success - the commit frontier for that partition sat below it
for the life of the assignment, and the next rebalance redelivered everything
above it.
The order is now register, then publish. Both maps are concurrent, so the
put into incompleteOffsets happens-before the shard's put, which happens-
before any scanner's get: a completion is valid whichever thread selects the
record, and no thread model is required to make it so.
Why it was latent, and why the smaller candidate was taken. On the shipped
engine registration and completion both reach PartitionState from the control
thread's mailbox drain (AbstractParallelEoSStreamProcessor#
processWorkCompleteMailBox calls WorkManager#registerWork and
WorkManager#handleFutureResult from one loop; the broker-poll thread's
registerWork only posts to the mailbox), so no completion could enter the
gap - an accident of today's callers, which the direct-pull engine's worker
selectors would end. The issue offered two settlements: swap the lines, or
state and assert the control-thread invariant. The swap removes the gap
outright, at no cost, and nothing depended on the container existing before
the offset (ProcessingShard#addWorkContainer reads no PartitionState offset
state; the take-side checks read epoch, allowedMoreRecords and the succeeded
mark, none of which the swap moves). A runtime owning-thread assertion was
deliberately NOT added: the invariant, derived from callers rather than from
the javadoc, is refuted by callers this repo keeps on purpose -
PartitionStateLincheckTest drives onSuccess from two threads to reach the
very assert, and WorkManagerLincheckTest drives handleFutureResult from
Lincheck threads against a registration made on the test thread. A guard
would silence what those harnesses measure and would also have to come out
again for the direct-pull engine. The thread model is written on the method
instead, dated, with what still relies on a single writer (the plain longs,
owned by jcstress-poc's SeenSucceededOrderingProbes).
Proof. PartitionStateRegistrationOrder370Test installs, through the
PCModule#createShardManager DI seam, a ShardManager whose armed insert does
what a concurrent scanner would: selects the container it has just published
through the real selection path and completes it through handleFutureResult
before returning to the registration loop. RED on the unfixed order:
[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0 <<< FAILURE! -- in bz.stub.parallelconsumer.state.PartitionStateRegistrationOrder370Test
[ERROR] PartitionStateRegistrationOrder370Test.aCompletionTheInstantARecordIsPublishedIsAValidCompletion <<< FAILURE!
java.lang.AssertionError
at bz.stub.parallelconsumer.state.PartitionState.onSuccess(PartitionState.java:279)
at bz.stub.parallelconsumer.state.PartitionStateManager.onSuccess(PartitionStateManager.java:349)
at bz.stub.parallelconsumer.state.WorkManager.onSuccessResult(WorkManager.java:226)
at bz.stub.parallelconsumer.state.WorkManager.handleFutureResult(WorkManager.java:433)
at ...PartitionStateRegistrationOrder370Test$CompletingOnPublishShardManager.addWorkContainer(...)
at bz.stub.parallelconsumer.state.PartitionState.maybeRegisterNewPollBatchAsWork(PartitionState.java:377)
at bz.stub.parallelconsumer.state.PartitionStateManager.maybeRegisterNewRecordAsWork(PartitionStateManager.java:367)
at bz.stub.parallelconsumer.state.WorkManager.registerWork(WorkManager.java:145)
GREEN with only the two lines swapped, the seam still firing (the test
asserts it fired, so a dead seam cannot pass by never running), and the
unarmed control test green on both sides. The control test pins that the
double leaves the ordinary path untouched.
The assert's javadoc now names the three shapes that have fired it and the
test pinning each, so the next reader arriving from it does not re-derive
the list. The inflight note bug-370-... is removed: the defect it tracked no
longer exists, and what outlives it - the invariant and the thread model -
lives on the method. The Lincheck open-items note no longer lists this as a
harness target.
Same defect class, other instances - a container reachable before the state
that validates its completion. In PartitionState and ShardManager: no other
hit on master. Checked and dismissed: ProcessingShard#addWorkContainer offers
the selection claim only after workMap.put, the right order for that pair;
ShardManager#onFailure re-queues a container whose offset never left
incompleteOffsets; WorkManager#onSuccessResult removes the offset before the
container leaves the shard, and the SUCCEEDED state refuses a claim in that
window (#335); initStateFromOffsetData loads incompletes before any
container exists. One hit off master: draft PR #410's
restoreCompletedButUncommittedWork replays records with the same
publish-then-register order, on the control thread under the producer write
lock - noted there rather than fixed here.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019SVm2cT6ZgUyPMim9CukYK
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)
|
[superseded - a quarantined test changed outcome] 🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its No quarantined test changed outcome since the previous push. 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 #450 +/- ##
============================================
+ Coverage 81.70% 82.53% +0.83%
- Complexity 1324 1480 +156
============================================
Files 84 95 +11
Lines 4597 5165 +568
Branches 492 505 +13
============================================
+ Hits 3756 4263 +507
- Misses 648 707 +59
- Partials 193 195 +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 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 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: 70a88bf, 27211b9, c5dde06, 92364a1, 867c407, 7a8dd92, df3bd71, 440b9bd, 11e0500 Since the previous push: ratio 0.865 -> 1.008, share 1.729 -> 1.704, rate 63219 -> 69842 (+10.5%). One push of difference sits inside this test's measured spread - read it as movement, not as a result. Updated for |
Test-only. The fix itself - the register-then-publish swap in PartitionState#maybeRegisterNewPollBatchAsWork - and both javadoc blocks are untouched, as is every assertion in the test. Two changes, both in PartitionStateRegistrationOrder370Test: - The one-element array holding the shard manager the module builds is now a plain field. The array read as the effectively-final capture box a lambda needs, and implied one was required here; it is not. The write happens in a method body of the anonymous PCModuleTestEnv subclass, and an instance field of the enclosing class is assignable from there - the "effectively final" restriction covers captured locals, not fields, and there is no forward reference either way because the field is declared above the module. The field now carries a line saying why it exists at all: shardManager() hands back the base type, and the seam has to be armed from the test. - The fixture guard both tests open with - fetch the installed seam, assert it is not null - was written out twice with the same message. It is now installedShardManager(), called from both. A drifted copy of a guard whose whole job is to stop the test measuring nothing is the copy worth not having. Deliberately NOT changed, from the three reviewer passes: - The explicit `fired` boolean was proposed for removal as derivable from `completed != null`. Skipped: the flag is the test's liveness guard - it is set the moment the seam fires, before the internal assertions and before `completed` is written, so it stays true on a path where `completed` never gets set. Collapsing it would make "did the seam run" depend on the completion path reaching its last line, which is a weaker guard than the one the class javadoc argues for on purpose. - Extracting the ConsumerRecords/EpochAndRecordsMap/registerWork idiom into a shared ModelUtils helper. Skipped as out of scope, not as wrong: most tests in the state and internal packages already hand-roll it, so it is a pre-existing clone class rather than one this branch introduces (the duplicate-code gate reports no new PMD CPD clone from this PR), and the existing ModelUtils#registerOneRecordAndTakeIt is not a drop-in - it builds its own record and takes the work after registerWork returns, which is exactly the ordering this test cannot use. Left for docs/refactoring.md as a repo-wide tidy. The efficiency pass returned nothing: the swap is the same two independent O(1) map writes reordered, and neither callee reads what the other writes. Verified: PartitionStateRegistrationOrder370Test 2/2 green, the core unit suite green, and bin/check-all.sh green. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019SVm2cT6ZgUyPMim9CukYK
…d the repeat in #410 Documentation only, from the review pass on this PR. No code change: the register-then-publish swap, the test and every assertion are untouched. Four items. 1. The cleared-suspicion block on maybeRegisterNewPollBatchAsWork handed the plain-long question to jcstress-poc's SeenSucceededOrderingProbes without saying whether anything runs it. Nothing does: jcstress-poc is absent from the root pom's <modules> list, so no reactor build reaches it, and no workflow and no bin/ script names it - `grep -rn jcstress .github/ bin/` returns nothing (verified before writing it). The engine AGENTS.md's cleared-suspicion rule requires the reopening condition to say plainly when no gate would catch it, and this one would not, so it now does. 2. The same block named offsetHighestSeen as the only plain-long residue. It is the harmless one. offsetHighestSucceeded is also plain, onSuccess read-modify-writes it, and getOffsetHighestSequentialSucceeded returns it directly whenever incompleteOffsets is empty - so a stale read of it is an offset committed to the broker, not bookkeeping. The dirty field's own javadoc already records jcstress measuring exactly that staleness, and what closes it is that field's volatile, not anything on this method. Both longs are now named, with which one has teeth. 3. docs/inflight/test-lincheck-lane-open-items.md retired the registration race with "No schedule can reach a container whose offset is absent". Too broad: WorkManager#onSuccessResult removes the offset (pm.onSuccess) before the container leaves its shard (sm.onSuccess), so a scanner in that window sees exactly that pair. The claim is now scoped to the registration path, with the post-completion window named and pointed at what actually refuses a claim there - the container's SUCCEEDED state (#335), which is candidate 1 in that same list. The retirement reasoning is unchanged and still holds. 4. docs/refactoring.md gains two lines. Under state/PartitionState.java: draft #410's restoreCompletedButUncommittedWork replays each record as addWorkContainer then addNewIncompleteRecord - the publish-then-register order this PR removed - and must take the same swap when it lands over this. It was called out in the defect-class sweep in the fix commit and in the PR body, but only there, where nobody editing that method would find it. Under the state package's white-box harness heading: the ConsumerRecords / EpochAndRecordsMap / registerWork idiom hand-rolled per test, which the simplify pass deliberately skipped as a pre-existing clone class, with why registerOneRecordAndTakeIt is not a drop-in and what a registerOneRecord-shaped sibling would have to do instead. Verified: PartitionStateRegistrationOrder370Test and WorkClaimStateMachineTest 12/12 green, bin/check-all.sh 16 passed 0 failed, bin/check-issue-refs.sh clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019SVm2cT6ZgUyPMim9CukYK
astubbs
marked this pull request as draft
September 7, 2026 02:16
…fset-before-work-container
🧪🔒 Quarantine Lane Report
🔴 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 |
astubbs
marked this pull request as ready for review
September 7, 2026 04:45
Owner
Author
|
@claude review this |
|
Claude finished @astubbs's task in 4m 47s —— View job Review: fix(core) #370: register the offset before the record is published to its shard
Verdict: LGTM. Verified, not just read. What I checked and confirmed
Minor observations (non-blocking)
No correctness, safety, or test-coverage issues found. The fix is minimal, the reasoning in the PR body holds up against the actual code (not just against itself), and the regression test demonstrably reproduces the bug and is fixed by the change. |
astubbs
added a commit
that referenced
this pull request
Sep 7, 2026
Master moved 21 commits under this branch in two days. Three touch the rider's own files and none argues with it: #450 registers a record's offset before publishing it to its shard (PartitionState, adjacent to but not inside the commit path this branch changed); #452 makes the codec's encodingCounters a ConcurrentHashMap (the encode-path static this branch's tryToEncodeOffsets calls through); #106 walks only the offsets where completion state changes when no encoder needs every offset (the overhead table's domain is unchanged by it, and its tests pass on the merge). Two text conflicts, both in append-only records, both resolved in master's favour plus this branch's additions: config/infer-known-findings.txt keeps master's retirement of every THREAD_SAFETY_VIOLATION identity (#433) and this branch's three BUFFER_OVERRUN_L1 identities; docs/refactoring.md keeps master's rewritten thread-safety entry for the codec (#452 replaced the "not thread-safe" heading and body) and this branch's two entries above it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkRtYPnDuAPLEmHFYSnAiJ
astubbs
added a commit
that referenced
this pull request
Sep 7, 2026
Picks up the batched per-PR jobs (#457) whose `scan: repo` and `static: analysis` contexts master's ruleset now requires, plus #450's offset-registration ordering in the same shard code this branch's put-back touches. One conflict, docs/inflight/ci-codecov-flags-not-like-for-like.md: both sides appended a dated sighting; both kept, this branch's 2026-09-03 one first. Master's text naming this PR in two notes is marked post-merge-correct rather than rewritten - dated records of runs against this branch while it was open, which read as history once it lands. State and ArchUnit tests green on the merged tree from fresh reports: RetryQueueRequeueWindowTest, RetryQueueRebalancePathTest, RetryQueueTest, RetryQueueIteratorConfinementTest, ShardManagerTest, ShardPopulationRaceTest, ArchitectureTest, RebalanceCallbackRuleControlTest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rEzrWYFr6oEzy6porczd3
astubbs
added a commit
that referenced
this pull request
Sep 7, 2026
…encing-brainstorm Fifteen commits on top of the sixteen the previous merge took, three conflicts: - PartitionState: master's javadoc on onSuccess(long) - the three closed shapes of its assert - with this branch's cross-reference to the container overload beneath it. - TransactionalClaim: master's 2026-09-07 scope note ends C14; this branch's C15 follows it. C4 and C9 move to REFUTED as master decided (#436); C15 is untouched by that finding. - The flake ledger: master's rows where both sides have one (the registration-race guard is now quarantined on master, #440), plus the three rows only this branch carries. One inherited instruction is NOT applied here: #450 asks this branch's replay loop to take the same register-then-publish swap it made in maybeRegisterNewPollBatchAsWork, and records that in the refactoring backlog. The commit after this one does it, with the pin. The paragraph in master's revoke-drain note that names this PR's stacking is attested post-merge: it records where #408 sat when the split was decided. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrpH51xNDodaajE4P2nhFg
astubbs
added a commit
that referenced
this pull request
Sep 7, 2026
…rd to its shard, as #370 established on the poll path #450 swapped maybeRegisterNewPollBatchAsWork to register-then-publish and left this branch an instruction in the refactoring backlog: the replay loop repeats the old order. It did. A scanner that selects and completes a replayed container the instant it is reachable through its shard completed an offset the partition had not yet re-registered, and onSuccess's assert fired from inside the replay - latent for the same reason it was latent on the poll path, one control thread doing both, and the same defect the day there is a second selector. Pinned the way #450 pinned its half: a shard manager installed through the module's seam that selects and completes the container on publish. RED on the old order (the AssertionError escaped the replay, observed before the swap), GREEN after, with the seam checked to have fired so a test that never raced cannot pass by accident. The backlog line that asked for this is retired. Also in this commit, because it was staged with it: the Infer annotations survey note is retired. #433 did what it proposed and more - confined the retry-queue iterator, showed the lastCommitTime confinement would have been a false declaration (the poll thread writes it too), measured every other annotation in the jar, and put the verdicts in the engine's AGENTS.md with the one open item in its own note. Two records of the same question would drift. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrpH51xNDodaajE4P2nhFg
6 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #370.
Description
PartitionState#maybeRegisterNewPollBatchAsWorkpublished a record to its shard before registering its offset as incomplete; it now registers first, so a completion is valid whichever thread selects the record.Between the two calls the container was reachable by every shard scan while
incompleteOffsetsdid not hold its offset. A completion landing in that gap tripsPartitionState#onSuccess'sassert removedFromIncompletes- the one shape that fires it with no double delivery at all. Without-eait is worse than a crash: the removal silently removes nothing, the second registration then puts the already-completed offset into the incomplete set, and nothing can ever complete it again because the shard removed its container on success. The commit frontier for that partition sits below it for the life of the assignment, and the next rebalance redelivers everything above it.Which candidate, and why. The issue offered two settlements: swap the two lines, or state and assert the control-thread invariant that happened to keep the gap unreachable. This PR takes the swap. It removes the gap outright rather than defending the accident that closed it: both maps are concurrent, so the put into
incompleteOffsetshappens-before the shard's put, which happens-before any scanner's get - no thread model required. The issue's precondition for the swap was met first: nothing depends on the container existing before the offset.ProcessingShard#addWorkContainerreads noPartitionStateoffset state, and the take-side checks (couldBeTakenAsWork,checkIfWorkIsStale,isBlockingProgress) read the epoch,allowedMoreRecordsand the succeeded mark, none of which the swap moves. ThegetShardManager()javadoc had described the register-first order all along ("if it's been successfully added to our tracked state"); the code now matches it.The thread model, derived from callers, and why it is written rather than asserted. The one production route into registration is
AbstractParallelEoSStreamProcessor#processWorkCompleteMailBoxon the control thread - the broker-poll thread'sregisterWorkonly posts to the mailbox - and the same drain loop callshandleFutureResult, so on the shipped engine registration and completion never overlap. That is now on the method, dated, per the engineAGENTS.md's cleared-suspicion rule. A runtime owning-thread guard was deliberately not added, followingdocs/solutions/architecture-patterns/a-query-must-never-mutate-derive-thread-safety-from-callers.md: enumerating callers refutes the invariant on purpose.PartitionStateLincheckTestdrivesonSuccessfrom two threads to reach this very assert, andWorkManagerLincheckTestdriveshandleFutureResultfrom Lincheck threads against a registration made on the test thread. A guard would silence what those harnesses measure and would have to come out again for the direct-pull engine, whose worker selectors are what make this reachable. The swap is the invariant that survives every one of those callers. What still assumes a single writer - the plainlongsaddNewIncompleteRecordwrites - is named on the method and owned byjcstress-poc'sSeenSucceededOrderingProbes.Proof.
PartitionStateRegistrationOrder370Testinstalls, through thePCModule#createShardManagerDI seam (the same oneRegistrationRaceStaleResidentITuses), aShardManagerwhose armed insert does what a concurrent scanner would: selects the container it has just published through the real selection path and completes it throughhandleFutureResult, before returning to the registration loop. Same shape asWorkManagerStaleCheckDoubleLookupTest's racing double - the interleaving played out by hand on one thread, where a timing test would need a second engine to reach it. RED on the unfixed order, onorigin/masteratfb5ea9346:GREEN with only the two lines swapped, the seam still firing (the test asserts that it fired, so a dead seam cannot pass by never running). The unarmed control test is green on both sides and pins that the double leaves the ordinary path untouched. The control arm is the diff itself: one term changed, the outcome flipped.
Where the knowledge went. The assert's javadoc now names the three shapes that have fired it - the claim (#335), the swapped state after a staleness check (#346), and this one - with the test pinning each, so the next reader arriving from it does not re-derive the list. The inflight note
bug-370-...isgit rm'd perdocs/inflight/AGENTS.md: the defect it tracked no longer exists, and what outlives it lives on the method.docs/inflight/test-lincheck-lane-open-items.mdno longer lists this as a harness target - no schedule can reach a container whose offset is absent, so there is nothing left for a model checker to explore there.Prior art, as the checks returned it.
node bin/inflight.mjs prior-art maybeRegisterNewPollBatchAsWork addNewIncompleteRecord addWorkContainer removedFromIncompletes 370: the note itself and its pre-rename twin onbench/share-groups-arm; #335 (which extracted the issue), #346 and #345 as the other assert sightings; no branch carrying either candidate - a scan of every local andorigin/ref for aPartitionState.javain whichaddNewIncompleteRecord(aRecord)precedes the shard add found none.note drifton the inflight note: nothing divergent.codecov test WorkClaimStateMachineTest: ten recorded test names, too many for one question, and not narrowed further - the class is the test pattern here, not the test under investigation. Merged PRs touchingPartitionState: #349, #347, #346, #344, #337 - none reorders registration. Issues--state all: only #370 itself and #139 (the thread-safety umbrella).Collisions, noted rather than dodged. Draft #410 touches
PartitionState(addsrestoreCompletedButUncommittedWork,UncommittedCompletions, anonSuccess(WorkContainer)overload) - its diff does not touch the lines swapped here, so the merge is a hunk apart; but its replay loop repeats the publish-then-register order this PR removes (see below). Draft #437 touchesShardManagerand adds aRacingSeamWorkManagerdouble; no overlap with the seam here, which is aShardManagersubclass in the test's own file. Concurrent agents onWorkManager/PartitionStateManagercounters and on the assignment path: this PR changes neither file.Other instances of the same defect - a container reachable before the state that validates its completion. Grepped
PartitionStateandShardManagerfor everyaddWorkContainer,incompleteOffsets.put/removeand retry-queue add, plus every caller ofPartitionState#onSuccess:maybeRegisterNewPollBatchAsWork.restoreCompletedButUncommittedWorkreplays each record asaddWorkContainerthenaddNewIncompleteRecord- the same order, on the control thread under the producer write lock, so latent for the same reason. Its author should swap it when that PR merges over this one.ProcessingShard#addWorkContaineroffers the selection claim only afterworkMap.put- the right order for that pair (fix(core): own the shard's available-count spend with a compare-and-set, instead of inferring it #373).ShardManager#onFailurere-queues a container whose offset never leftincompleteOffsets.WorkManager#onSuccessResultremoves the offset before the container leaves the shard, and in that window a claim is refused by theSUCCEEDEDstate (fix(core): make the work claim one atomic transition on a per-attempt identity #335).initStateFromOffsetDataloads incompletes at construction, before any container exists.PartitionStateManager#onPartitionsAssignedputs the state in its map before any registration can look it up.Checklist
docs/features/- N/A - internal correctness fix, no user-facing featurePartitionStateRegistrationOrder370Test, red-then-green as abovedocs/inflight/working note (pr-/branch-) started at the PR's first commit - N/A - a single-commit PR whose every finding is in this body and on the method; nothing here thatghcannot showce-simplifyandce-code-reviewlocally - both ran: the simplify pass landed asb40969258(test-only, with what it deliberately did not change), and the review pass's findings are applied ine1515d32f(documentation-only)🤖 Generated with Claude Code
https://claude.ai/code/session_019SVm2cT6ZgUyPMim9CukYK