Repository navigation
fix(gateway): a crash mid boot-redelivery no longer sends a second unmarked copy of the reply (salvage #70700) - #120450
Merged
Merged
Conversation
sweep_recoverable claimed a dead-owner 'pending' row by re-stamping the owner and spending an attempt but left state='pending', and the boot redelivery path never marked it attempting before adapter.send. A boot killed after the platform accepted that plain resend (before mark_delivered) left the row 'pending', so the next boot resent it UNMARKED: a silent duplicate reply. The claim UPDATE now moves every claimed row to 'attempting' in the same owner-stamp CAS; needs_marker still reads the pre-claim state, so a never-claimed pending row is redelivered plainly once and any later copy carries RECOVERED_MARKER. A pending row that an older build already claimed (attempts > 0) is also marked, since it may have been sent.
Two invariants on the boot outbox sweep, both red on the previous ledger: - a pending row whose boot redelivery was interrupted after the platform accepted it (or that an older build claimed) is redelivered with the recovered marker, never a second plain copy; - a boot-claimed failed row is not re-claimable by the runtime reconnect sweep while the boot send is in flight (it used to stay 'failed' and owned by this process, so a reconnect could send it a second time). Also corrects the startup-claim comment in _obligation_adapter and the messaging docs bullet on mid-send recovery.
૮ >ﻌ< ა ci reviewran on 209911e — fix(gateway): release a boot claim whose adapter vanished be
|
alt-glitch
added
type/bug
…atch The boot sweep now moves every claimed row to attempting. If the platform went fatal between the claim and the send (restart notification, flood sleep), _obligation_adapter skipped the row without releasing it, so it sat in attempting owned by this live process: the reconnect sweep only takes failed rows and the boot sweep skips live owners, so the reply waited for the next restart (and then carried a false duplicate marker). Release any undispatched claim, not only runtime ones. Found by independent review of #120450.
teknium1
added a commit
that referenced
this pull request
Sep 23, 2026
…y cells A strict xfail on a gap whose fix is an open PR turns main red the moment that fix merges (XPASS), and the non-strict ones guarded nothing. Each gap now has a probe (tests/e2e/core/delivery/_pending_fixes.py) that reproduces the defect's mechanism on the tree under test in a throwaway interpreter; expect_gap() applies the strict xfail only while the probe still reproduces it, so the cell becomes a plain test once the fix is in the tree, whatever the merge order. Covered: #120314, #119970 (soak), #120315, #120377, #120444 (C12) and #120450 (cell 5). Each probe was checked against every fix head: it flips on its own PR and on no other. C12 cells made deterministic (identical outcome on every run): - long_split streams the whole reply as one chunk; long_streamed and stream_timeout_first_send pace chunks so each lands in its own consumer tick. The five former coin-flip xfails are now two plain cells and three strict #120315 gap cells. - sent_ack_lost waits until the answer is persisted before the kill, so it pins the #120377 recovery; the streamed-before-persisted order is its own cell (stream_accepted_unpersisted, a strict live gap with a stalled provider stream). - zzz_unclean_restart compares director.resumes against a snapshot taken before its kill instead of requiring it empty: crash cells on their own homes may legitimately resume. - the whole-run audit skips the reconnect replay only while #120444's gap is open.
teknium1
added a commit
that referenced
this pull request
Sep 23, 2026
…y cells A strict xfail on a gap whose fix is an open PR turns main red the moment that fix merges (XPASS), and the non-strict ones guarded nothing. Each gap now has a probe (tests/e2e/core/delivery/_pending_fixes.py) that reproduces the defect's mechanism on the tree under test in a throwaway interpreter; expect_gap() applies the strict xfail only while the probe still reproduces it, so the cell becomes a plain test once the fix is in the tree, whatever the merge order. Covered: #120314, #119970 (soak), #120315, #120377, #120444 (C12) and #120450 (cell 5). Each probe was checked against every fix head: it flips on its own PR and on no other. C12 cells made deterministic (identical outcome on every run): - long_split streams the whole reply as one chunk; long_streamed and stream_timeout_first_send pace chunks so each lands in its own consumer tick. The five former coin-flip xfails are now two plain cells and three strict #120315 gap cells. - sent_ack_lost waits until the answer is persisted before the kill, so it pins the #120377 recovery; the streamed-before-persisted order is its own cell (stream_accepted_unpersisted, a strict live gap with a stalled provider stream). - zzz_unclean_restart compares director.resumes against a snapshot taken before its kill instead of requiring it empty: crash cells on their own homes may legitimately resume. - the whole-run audit skips the reconnect replay only while #120444's gap is open.
teknium1
added a commit
that referenced
this pull request
Sep 23, 2026
…y cells A strict xfail on a gap whose fix is an open PR turns main red the moment that fix merges (XPASS), and the non-strict ones guarded nothing. Each gap now has a probe (tests/e2e/core/delivery/_pending_fixes.py) that reproduces the defect's mechanism on the tree under test in a throwaway interpreter; expect_gap() applies the strict xfail only while the probe still reproduces it, so the cell becomes a plain test once the fix is in the tree, whatever the merge order. Covered: #120314, #119970 (soak), #120315, #120377, #120444 (C12) and #120450 (cell 5). Each probe was checked against every fix head: it flips on its own PR and on no other. C12 cells made deterministic (identical outcome on every run): - long_split streams the whole reply as one chunk; long_streamed and stream_timeout_first_send pace chunks so each lands in its own consumer tick. The five former coin-flip xfails are now two plain cells and three strict #120315 gap cells. - sent_ack_lost waits until the answer is persisted before the kill, so it pins the #120377 recovery; the streamed-before-persisted order is its own cell (stream_accepted_unpersisted, a strict live gap with a stalled provider stream). - zzz_unclean_restart compares director.resumes against a snapshot taken before its kill instead of requiring it empty: crash cells on their own homes may legitimately resume. - the whole-run audit skips the reconnect replay only while #120444's gap is open.
teknium1
added a commit
that referenced
this pull request
Sep 23, 2026
…y cells A strict xfail on a gap whose fix is an open PR turns main red the moment that fix merges (XPASS), and the non-strict ones guarded nothing. Each gap now has a probe (tests/e2e/core/delivery/_pending_fixes.py) that reproduces the defect's mechanism on the tree under test in a throwaway interpreter; expect_gap() applies the strict xfail only while the probe still reproduces it, so the cell becomes a plain test once the fix is in the tree, whatever the merge order. Covered: #120314, #119970 (soak), #120315, #120377, #120444 (C12) and #120450 (cell 5). Each probe was checked against every fix head: it flips on its own PR and on no other. C12 cells made deterministic (identical outcome on every run): - long_split streams the whole reply as one chunk; long_streamed and stream_timeout_first_send pace chunks so each lands in its own consumer tick. The five former coin-flip xfails are now two plain cells and three strict #120315 gap cells. - sent_ack_lost waits until the answer is persisted before the kill, so it pins the #120377 recovery; the streamed-before-persisted order is its own cell (stream_accepted_unpersisted, a strict live gap with a stalled provider stream). - zzz_unclean_restart compares director.resumes against a snapshot taken before its kill instead of requiring it empty: crash cells on their own homes may legitimately resume. - the whole-run audit skips the reconnect replay only while #120444's gap is open.
teknium1
added a commit
that referenced
this pull request
Sep 23, 2026
…y cells A strict xfail on a gap whose fix is an open PR turns main red the moment that fix merges (XPASS), and the non-strict ones guarded nothing. Each gap now has a probe (tests/e2e/core/delivery/_pending_fixes.py) that reproduces the defect's mechanism on the tree under test in a throwaway interpreter; expect_gap() applies the strict xfail only while the probe still reproduces it, so the cell becomes a plain test once the fix is in the tree, whatever the merge order. Covered: #120314, #119970 (soak), #120315, #120377, #120444 (C12) and #120450 (cell 5). Each probe was checked against every fix head: it flips on its own PR and on no other. C12 cells made deterministic (identical outcome on every run): - long_split streams the whole reply as one chunk; long_streamed and stream_timeout_first_send pace chunks so each lands in its own consumer tick. The five former coin-flip xfails are now two plain cells and three strict #120315 gap cells. - sent_ack_lost waits until the answer is persisted before the kill, so it pins the #120377 recovery; the streamed-before-persisted order is its own cell (stream_accepted_unpersisted, a strict live gap with a stalled provider stream). - zzz_unclean_restart compares director.resumes against a snapshot taken before its kill instead of requiring it empty: crash cells on their own homes may legitimately resume. - the whole-run audit skips the reconnect replay only while #120444's gap is open.
teknium1
added a commit
that referenced
this pull request
Sep 23, 2026
… gaps with no fix PR #120314, #120377, #120444 and #120450 are on main: their PROBES entries, every expect_gap naming them and the gap_open(120444) audit branch go, so those cells are plain tests again (two of those probes read source text, which the suite must not do). Cell 5's README row no longer claims a strict xfail. The two static strict xfails with no probe and no fix PR (STREAM_ACK_LOST_GAP, STREAM_CRASH_AFTER_ACCEPT_GAP) become run-time xfails via _pending_fixes.known_failure: only the final assertions run under it, after every wait (restart, catch-up, settle) has succeeded, and only an AssertionError matching the gap's own signature XFAILs; any other failure stays red and a fixed tree simply passes. The whole-run audit now skips only tokens whose cell actually XFAILed this run.
teknium1
added a commit
that referenced
this pull request
Sep 23, 2026
…y cells A strict xfail on a gap whose fix is an open PR turns main red the moment that fix merges (XPASS), and the non-strict ones guarded nothing. Each gap now has a probe (tests/e2e/core/delivery/_pending_fixes.py) that reproduces the defect's mechanism on the tree under test in a throwaway interpreter; expect_gap() applies the strict xfail only while the probe still reproduces it, so the cell becomes a plain test once the fix is in the tree, whatever the merge order. Covered: #120314, #119970 (soak), #120315, #120377, #120444 (C12) and #120450 (cell 5). Each probe was checked against every fix head: it flips on its own PR and on no other. C12 cells made deterministic (identical outcome on every run): - long_split streams the whole reply as one chunk; long_streamed and stream_timeout_first_send pace chunks so each lands in its own consumer tick. The five former coin-flip xfails are now two plain cells and three strict #120315 gap cells. - sent_ack_lost waits until the answer is persisted before the kill, so it pins the #120377 recovery; the streamed-before-persisted order is its own cell (stream_accepted_unpersisted, a strict live gap with a stalled provider stream). - zzz_unclean_restart compares director.resumes against a snapshot taken before its kill instead of requiring it empty: crash cells on their own homes may legitimately resume. - the whole-run audit skips the reconnect replay only while #120444's gap is open.
teknium1
added a commit
that referenced
this pull request
Sep 23, 2026
… gaps with no fix PR #120314, #120377, #120444 and #120450 are on main: their PROBES entries, every expect_gap naming them and the gap_open(120444) audit branch go, so those cells are plain tests again (two of those probes read source text, which the suite must not do). Cell 5's README row no longer claims a strict xfail. The two static strict xfails with no probe and no fix PR (STREAM_ACK_LOST_GAP, STREAM_CRASH_AFTER_ACCEPT_GAP) become run-time xfails via _pending_fixes.known_failure: only the final assertions run under it, after every wait (restart, catch-up, settle) has succeeded, and only an AssertionError matching the gap's own signature XFAILs; any other failure stays red and a fixed tree simply passes. The whole-run audit now skips only tokens whose cell actually XFAILed this run.
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
3 participants
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.
A gateway killed while its boot sweep is redelivering a stored reply no longer makes the next boot send a second, unmarked copy of that reply: any copy after the first is labeled with the "♻️ Recovered reply" marker.
Salvages #70700 by @fangliquanflq (authorship kept on the fix commit), rebased onto the current ledger (
gateway/run.py→gateway/run_startup.pysplit) and trimmed.Changes
gateway/delivery_ledger.py::sweep_recoverable: the claim UPDATE moves every claimed row to'attempting'in the same owner-stamp CAS (it used to do that only for flood rows).needs_markerstill reads the pre-claim state, so a never-claimedpendingrow goes out plain exactly once. Apendingrow withattempts > 0, left by an older build that already claimed and maybe sent it, is also marked.failedrow used to stayfailedand owned by the live process while its boot send was in flight. The runtime reconnect sweep (sweep_failed_for_runtime, which takesfailedrows) could then claim it and send it again. The row is nowattemptingfor the whole send, so the boot claim stays exclusive.mark_attemptingcall right beforeadapter.sendin the redelivery loop. Both callers of_redeliver_claimed_obligations(the boot sweep and the runtime sweep) now hand over rows that are alreadyattempting, so the call was a redundant write._obligation_adapterand the messaging docs bullet on mid-send recovery (website/docs/user-guide/messaging/index.md).Root cause
sweep_recoverablespent an attempt and re-stamped the owner of apendingrow without moving it toattempting. A crash after the platform accepted the resend therefore still looked like "never sent" to the next boot.Validation
Live repro: before: on
origin/main@ 03544a7, the cell-5 real-process harness from #120344 (tests/conformance/persistence/test_cell5_delivery_outbox_exactly_once.py) was run with--runxfail. It uses a realstate.db, the realGatewayRunnerclaim and redeliver halves, a journalingBasePlatformAdapterthat fsyncs each accepted send, and a real SIGKILL inside boot 1's plain resend. It fails with'cell5 reply 0' (pending): 2 UNMARKED copies reached the platform — a silent duplicate (roles=['boot-1', 'boot-2']). After: the same harness on the fix passes all 6 kill-point cases (6 passed in 216s), including concurrent reboots, where claims still sum to exactly one per obligation.pendingrow, then rebootpendingrow left by an older build withattempts=1failedrow, then adapter reconnect during the boot sendpendingrowTests (2 invariants in
tests/gateway/test_delivery_ledger.py; each was red with theorigin/mainledger and is green with the fix):test_redelivery_after_an_earlier_boot_claim_is_marked[killed_inside_send|older_build_left_pending]: drives the realGatewayRunner._redeliver_pending_obligations. Boot 1's send is interrupted after acceptance (CancelledError, so nomark_delivered), and the next boot's copy must beRECOVERED_MARKER + reply.test_boot_claimed_row_is_not_reclaimed_by_runtime_sweep_mid_send: after a boot claim,sweep_failed_for_runtimereturns nothing.No existing assertions changed.
scripts/run_tests.sh tests/gateway/: 8665 passed and 12 failed at host load 100–220. None of the 12 are delivery-ledger tests. 6 of them fail the same way on a pristineorigin/mainworktree, and the other 6 pass when their files are re-run alone. The delivery, restart-resume and flood-invariant files pass 93/93. ruff,check_no_tmp_literals,check-windows-footguns --all,check_compat_pointers,git diff --checkandaudit_pr_attributionare all clean.Sibling surfaces checked: the cron outbox (
cron/delivery_queue.claim_next) already movespendingtodeliveringin its claim CAS. The turn-side producer (BasePlatformAdapter._record_delivery_obligation) already marks rows attempting before the send. The runtime sweep already claims intoattempting.Heads-up for #120344: its strict
xfailontest_boot_killed_inside_plain_redelivery_never_double_deliverswill XPASS once this lands, so that marker should be removed.Gap: if a startup claim's adapter disappears between the claim and the send, the row now stays
attemptinguntil the next boot, which redelivers it marked. Before, a claimedfailedrow in that state could also be picked up by the runtime timer. The adapter set is fixed at claim time, so this window is very small.Infographic