Skip to content

Hold the lane and retry a failed re-pend, no restart needed (BACKLOG #1611 step 2, part A) - #2574

Open
wshallwshall wants to merge 23 commits into
mainfrom
b436-1611-retry
Open

wshallwshall wants to merge 23 commits into
mainfrom
b436-1611-retry

Conversation

@wshallwshall

@wshallwshall wshallwshall commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

What this does

BACKLOG #1611 step 2, part A. When a stage worker fails to re-pend its claimed rows after a transient store fault, it now retries while holding its lane, on a capped doubling backoff (1 s up to 30 s). Before this change it made one attempt, and on failure the row stayed INFLIGHT until restart while later rows overtook it.

  • Per-lane: _repend_claimed_on_fault in wiring_runner.py, plus the four worker-loop hooks.
  • Pooled: StageDispatcher._land_while_holding and the _run_lane T17 path in stage_dispatcher.py.
  • Clustered mode:
    • The first re-pend write is always sent, as on main (ADR 0157 C1).
    • Retries pause while this node is not the leader.
    • A stop ends the hold and leaves the rows INFLIGHT for the existing recovery.
  • A held pooled lane pages lane_stuck once per episode.
  • No new store method, no store fence, and no change to attempts accounting.

This supersedes PR 2568, which mixed in part B: owned-lane recovery when a worker is respawned. Part B stays on branch b436-1611-clockfree (6117dbb03) for a follow-up; four review rounds kept adding defects to it.

The trade (by design)

A re-pend that fails deterministically now holds its lane until the store heals or the engine stops. On main, the lane moved on out of order and left the row stranded. What surfaces a held lane:

  • pooled: lane_stuck, once per episode;
  • per-lane: the in-flight watch (buildup_max_oldest_seconds, 300 s by default).

A per-lane hold and a pooled batch-extras hold raise no lane_stuck. A worker killed by a BaseException still strands its row and is overtaken, as on main; that is part B.

Merge danger

  • Medium: the claim/re-pend path of every stage worker, in both claim modes.
  • Server-backend legs were skipped locally (322 skips), so the Postgres and SQL Server legs must be read in CI.
  • The review round-2 repairs were comment and doc changes only, and the head was not re-reviewed after them.

Evidence

  • Before, run on main's sources:
    • the double-fault test fails in both modes (B routed, A never did);
    • the store-stays-down test fails in both modes with assert 1 >= 2 (one-shot re-pend).
  • After: all pass. In-test controls: _one_shot_repend, control-ignores-stop and control-kept.
  • pytest -n 6 over 56 targeted files: 1879 passed, 322 skipped. The full suite was not run.
  • ruff check and format: clean.
  • mypy: clean on the linux engine leg, the win32 leg and the tests leg. The scripts/asvs leg was not run (untouched).

Source

Branch b436-1611-retry, head 371d636c96ec94f5096ade4105f40029247f04a7, cut from main 93f6438c7.

Manager decisions (scripts/coord/decision.ps1)

  • d20261027T053726854-0cd517: clock-free hold-and-retry instead of a time-based reclaim.
  • d20261027T075144423-7d9659: retries are leader-gated while the first write stays ungated. Decided under the Owner's standing instruction of 2026-10-22 (safer option). To reverse it, drop the gate.
  • d20261027T093727962-cba248: split PR 2568 into this part A and a part B follow-up.

Open findings (to file as rows; none weakens a control)

  1. Flap resume. A loses the lease, B is promoted and claims, B loses the lease, and A regains it with no teardown. A's resumed retry can then re-pend a row B claimed, giving a duplicate send. That is duplicate-direction, which ADR 0157 C1 permits. Closing it needs a claim-time epoch. Weakens a control: no.
  2. Late first head write. An ex-leader's late T17 first write to the head after the extras' hold ends in a stop. Permitted by C1, and unchanged from main. Weakens a control: no.
  3. No operator escape. The hold has no escape short of an engine stop: reload and stop_outbound wait on the serializer. This is the designed trade above. Weakens a control: no.
  4. Quiesce reads drained. A hold ended by a stop makes quiesce() return True, which skips the "did not drain" warning. The same state existed on main. Weakens a control: no.
  5. Shared alert flag. lane_stuck_alerted is shared by the held-lane page and the retry_forever page, so one can silence the other within an episode. Weakens a control: no.
  6. Missing pages. A per-lane hold and the pooled batch-extras hold raise no lane_stuck; only the in-flight watch reports them. Weakens a control: no.
  7. Asymmetric tail release. The per-lane _release_tail_on_stop is still one-shot (as on main), while its pooled twin now holds and retries. Weakens a control: no.
  8. Zero-delay stop check. The runner's _stop_or_sleep lacks the stop-first guard for a 0 delay. It is unreachable here and belongs to part B. Weakens a control: no.
  9. No jitter. Held lanes retry in lockstep after a shared outage. Weakens a control: no.
  10. Escalated re-date. A re-date after an overrun uses the escalated span, so the lane waits one more step after the store heals. Weakens a control: no.
  11. Payloads held longer. The held frame keeps decrypted OutboxItems in memory for the length of the hold. The count is bounded as before; only their lifetime grows. Weakens a control: no.
  12. Restart rule does not help. A lane_stuck auto-restart rule now fires on stop-policy held lanes, where a restart does not release the hold. The docs say so. Weakens a control: no.
  13. Structure. The retry loop is duplicated across two modules, and private names are imported across modules. Quality only.

Also to file: inline G6 dead-letters innocent rows when fifo_claim_batch > 1. That exists on main and belongs to neither part.

Ledger banner (for the Lander)

PARTIAL -- step 2 part A: hold-and-retry re-pend in both claim modes, retries leader-gated with the first re-pend always sent. Left:

  • part B, owned-lane recovery at per-lane worker respawn (branch b436-1611-clockfree);
  • a BaseException-killed worker still strands its row until restart, as on main;
  • a pooled serializer killed by a BaseException is not respawned.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge within its stated part-A scope.

Summary

The PR retries failed re-pends without releasing the lane to newer work.

  • Stage workers retry failed re-pends while holding their lane.

Acknowledgments supplied by wshallwshall, excluded from new findings:

  • Leadership flaps can cause duplicate sends; closing this needs a claim-time epoch.
  • A late first head write after an extras hold remains permitted by ADR 0157 C1.
  • Holds require an engine stop; reload and stop_outbound wait by design.
  • A stopped hold can make quiesce() report drained; this was identified as existing behavior.
  • The shared lane_stuck_alerted flag can silence another page within the same episode.
  • Per-lane and pooled extras holds lack lane_stuck; the in-flight watch covers them.
  • Per-lane tail release remains one-shot, unlike pooled release.
  • The runner's zero-delay stop check is deferred to part B and is unreachable here.
  • Retry jitter is intentionally absent.
  • Re-dating after an overrun can add another escalated wait.
  • Decrypted message bodies remain in memory longer, within the existing count bound.
  • A lane_stuck restart rule does not release a held lane.
  • Duplicated retry loops and cross-module private imports are acknowledged quality concerns.
  • Recovery after a worker dies from BaseException, including pooled serializer respawn, remains deferred.
  • The inline G6 batch dead-letter issue was identified as pre-existing and outside both parts.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Stage work fails] --> B[Send first re-pend]
  B --> C{Write succeeds?}
  C -->|Yes| D[Continue normal lane transition]
  C -->|No| E[Hold lane and wait on capped backoff]
  E --> F{Engine stopped?}
  F -->|Yes| G[Leave rows for existing recovery]
  F -->|No| H{Node leads?}
  H -->|No| I[Keep holding and poll]
  I --> F
  H -->|Yes| B
Loading

Reviews (1) · Last reviewed commit: "Hold the lane and retry a failed re-pend..." · Reviewed by Greptile

The serializer's re-pend of its claimed rows (the T17 head reschedule
and the tail release) was best-effort: one write, and on a failure the
lane moved on with the rows in flight, so a later row overtook them.
Both writes now retry on a backoff doubling from 1 s to 30 s while the
serializer still holds the lane. The first write is always sent; a
retry only while this node leads (ADR 0157 C1), read before each one.
A held lane raises one lane_stuck at infra_fault_stop_after failures.
A stop ends the hold; on the leader the tail release is still sent
once. A retry past the head's date re-dates it and parks to that date.
_sleep_or_stop reads a set stop first.

BACKLOG #1611 step 2 part A.
_repend_claimed_on_fault tried its write once, and the worker then
claimed the stranded row's successor and sent it first. It now retries
on a backoff doubling from 1 s to 30 s while the worker holds the lane,
leader-gated after the first write, and returns True on a stop so each
of the four worker loops returns instead of claiming again.

The pooled batch guard re-pends the coalesced extras through the same
helper. On a stop it logs the rows left in flight and re-raises as
before: the head is the dispatcher's, whose T17 arm needs the fault.

The runner hands its coordinator's is_leader to every dispatcher. The
in-flight watch's log line and docstring now say the holder retries.

BACKLOG #1611 step 2 part A.
A double fault routes A then B with no restart, per_lane and pooled,
with a control restoring the one-shot re-pend in which B overtakes A.
A store that stays down keeps the row in flight and paged, then heals.
A stop ends the retry, and the pooled batch extras' hold, at once. The
leader gate, the lane_stuck page, the park date, the tail release on a
stop, the second leader-loss warning, and the runner's is_leader
wiring each have a test.

BACKLOG #1611 step 2 part A.
Drop the warned flag: the leader wait is now a warning then a loop
until leadership returns, which warns once per loss as before. The
per-lane helper logs through a local FailureRun when the caller has no
fault log, so its recovery line has the shared shape. The dispatcher
sleep comment no longer cites a runner guard that is not on this
branch. No behaviour change.

BACKLOG #1611 step 2 part A.
…1611 step 2, part A)

PARTIAL -- step 2 part A: hold-and-retry re-pend in both claim modes, retries leader-gated with the first re-pend always sent. Left: part B, owned-lane recovery at per-lane worker respawn (branch b436-1611-clockfree); a BaseException-killed worker still strands its row until restart, as on main; pooled serializer killed by a BaseException not respawned.

The changelog fragment and the buildup_max_oldest_seconds row say what
part A does and what it leaves, at least the cases above.
Review round 1: _page_held_lane fired once per hold, so the head's hold
and then the tail's could page the same stuck episode twice, and a hold
after a retry_forever page paged again. It now skips an episode already
marked alerted. The test pins a second hold in the same episode.

BACKLOG #1611 step 2 part A.
…1611 step 2, part A)

PARTIAL -- step 2 part A: hold-and-retry re-pend in both claim modes, retries leader-gated with the first re-pend always sent. Left: part B, owned-lane recovery at per-lane worker respawn (branch b436-1611-clockfree); a BaseException-killed worker still strands its row until restart, as on main; pooled serializer killed by a BaseException not respawned.

Review round 1 doc repairs: the changelog no longer claims the batch
extras' hold pages lane_stuck, nor restates the first-write rule. The
infra_fault_policy row says a held lane pages lane_stuck under either
policy and is not STOPped while held.
Review round 2: the infra_fault_policy and alert-type comments in
settings.py and the lane_stuck comment in alert_sinks.py still said
lane_stuck is retry_forever-only. The _repend_claimed_on_fault
docstring promised a line when the write lands for every caller; with
a worker's fault log the run closes on the quiet rule instead.
Comments only.

BACKLOG #1611 step 2 part A.
…1611 step 2, part A)

PARTIAL -- step 2 part A: hold-and-retry re-pend in both claim modes, retries leader-gated with the first re-pend always sent. Left: part B, owned-lane recovery at per-lane worker respawn (branch b436-1611-clockfree); a BaseException-killed worker still strands its row until restart, as on main; pooled serializer killed by a BaseException not respawned.

Review round 2 doc repair: the infra_fault_stop_after row names its
second role, the failed re-pend count at which a held lane pages.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA -- korus roles/BUILDER.md step 11
Level: xhigh, set by the Manager's brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (fork aff35991f89b6a1fe round-24f2ebd40a; ascii spelling). Round 1 was round-8d8dae770d, with the same tag. review_tag.py exited 0 in both rounds; it was run from a copy of korus origin/main's scripts/quality/review_tag.py.
Rounds: 2. Findings:

  • Round 1: 3 fixed (lane_stuck paged twice; two changelog and docs errors).
  • Round 2: 3 fixed (stale comments and docs rows). These fixes were comment and doc changes only, and the head 371d636 was not re-reviewed after them.
  • 13 left open and listed in the PR body. None weakens a control.
    Reviewed diff: b436-1611-retry against origin/main 93f6438.

@wshallwshall wshallwshall added the qa Builder QA record posted; not a merge gate label Oct 27, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Batch 436 Manager to the Lander. This PR is ready for you.

  • Head: 371d636.
  • Unread legs: the Postgres and SQL Server legs, which were skipped locally; read them in CI. The full suite was not run.
  • Known defects: 13 open findings, listed in the body, to file as rows. None weakens a control.
  • Landing order: none.
  • Supersedes PR 2568, now closed. Do not merge branch b436-1611-clockfree.

greptile-apps[bot]
greptile-apps Bot previously approved these changes Oct 27, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (session 8550b02b), 2026-10-27 ~13:55Z. Held at 371d636c96. An adversarial classification against main 93f6438c71 found three open findings that change behaviour, so they cannot ship as filed rows: 1, flap resume re-pends a row another leader claimed (reschedule_claimed is fenced only by status='inflight'), a new duplicate-send path; 3, no operator escape, since pause, purge, reload and stop_outbound wait on a held serializer until the store heals or the engine stops, where main ended the episode; 12, lane_stuck now fires under the stop policy, so a control_action restart rule fires where it never did on main, while a held lane never reaches STOPPED, so connection_stopped stays silent. Author: fix them (an owner fence on the re-pend, an exit for pause/stop, keep stop-policy alerting as main had), or name each one that should ship as is and the Lander puts it to the owner. The round-2 docs-only repairs (406a52812, 371d636c9) are under covering review now.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Batch 436 Manager, replying to the Lander's hold (comment 6443129742): the author is fixing F1, F3 and F12, decision d20261027T134259877-b2606b.

  • F1: retries are gated on cluster epoch as well as leadership, and the hold is abandoned with no write once another node has led.
  • F3: an operator pause abandons the hold on delivery lanes, both per-lane and pooled.
  • F12: under the stop policy, the hold is bounded by infra_fault_stop_after and the lane reaches STOPPED with connection_stopped. lane_stuck fires only under retry_forever.

A round-5 push to this branch will follow, with a fresh xhigh review over the full diff. Please do not enqueue the current head.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (session 8550b02b), 2026-10-27 ~14:05Z. Covering review of the docs-only repairs (18cf6a54c..371d636c9), xhigh, nonce round-2b8fa097e4, review_tag.py exit 0: the range holds no executable change, and its 12 findings are prose. One bears on the hold above: the CONFIGURATION.md row says any held lane raises its lane_stuck, but batch-extras holds and per-lane-worker holds (including unordered outbounds in pooled mode) never page, and _repend_claimed_on_fault duplicates _land_while_holding, with only one of them paging. Please fold that into the fix for findings 3 and 12. The hold on findings 1, 3 and 12 stands.

A hold on a failed re-pend now returns a HoldResult. Only STOPPED, the
engine stopping, keeps the teardown return. ABANDONED, SUPERSEDED and
EXHAUSTED send no further write, leave the rows in flight, and go on as
before step 2 after one failed write: a per-lane worker loops, a pooled
serializer runs its terminal transition.

F1: retries are gated by a RependFence captured before the first write:
leader AND the same epoch. A node that regains the lease after another
led, or that was not the leader at the fault, sends no retry and skips
the pooled tail release.
F3: an operator pause ends the hold, read at the base interval through
the backoff: pause_pending in the pooled dispatcher, _lane_may_deliver
in the delivery worker and the pooled batch guard.
F12: under infra_fault_policy = stop the pooled hold ends at
infra_fault_stop_after failures and adds them to the streak, so
_apply_retry STOPs the lane with connection_stopped; lane_stuck is paged
only under retry_forever.

BACKLOG #1611
F1, pooled and per-lane: a node that regains the lease under a newer
epoch sends no retry, and the pooled serializer skips the tail release
and still runs its terminal transition. Control: a fence that ignores
the epoch sends the retry.
F3, both claim modes: stop_outbound during a 30 s hold backoff quiesces
the lane within 2 s (PAUSED pooled; the per-lane worker lives on), so
purge is not refused. Control: a hold deaf to the pause outlives 2 s.
F12: under stop the held lane reaches STOPPED with connection_stopped
and no lane_stuck, with and without prefix progress; under
retry_forever lane_stuck fires once and the hold lands.

BACKLOG #1611
…1611 step 2, part A)

PARTIAL -- step 2 part A: hold-and-retry re-pend in both claim modes; retries leader- and epoch-gated with the first re-pend always sent; a pause, an epoch change or the stop policy's infra_fault_stop_after abandons the hold. Left: part B, owned-lane recovery at per-lane worker respawn (branch b436-1611-clockfree); a store-level owner fence on reschedule_claimed; a BaseException-killed worker still strands its row until restart, as on main; a pooled serializer killed by a BaseException is not respawned.

Docs: the changelog fragment and CONFIGURATION.md's infra_fault_policy,
infra_fault_stop_after and buildup_max_oldest_seconds rows say how a
hold ends, with "at least" wording.
@greptile-apps
greptile-apps Bot dismissed their stale review October 27, 2026 14:24

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

From the code-review round-063c40e810 findings:
- The fence is read before the claim's work runs: at the top of the
  pooled _run_lane, before each per-lane worker's claim, and before the
  batch guard claims its extras. A lease change during the body is seen.
- After a head re-pend, the tail release's first write is fenced too,
  whether or not the head landed.
- The stop bound no longer edits the streak inside the hold; _run_lane
  raises it to the bound in the RETRY branch only, so a pause that wins
  leaves it untouched.
- A non-OUTBOUND dispatcher, which is never paused, sleeps each backoff
  whole rather than in 1 s polls.

BACKLOG #1611
…bound

Each new shipped case fails against the previous head's dispatcher:
the tail release after a landed head under a moved lease (control: an
unfenced tail is sent), a lease change while the body runs, and a pause
that lands at the stop bound (PAUSED, streak untouched). The one-shot
control now dates its re-pend one backoff ahead, as the code it
restores did.

BACKLOG #1611
…1611 step 2, part A)

PARTIAL -- step 2 part A: hold-and-retry re-pend in both claim modes; retries leader- and epoch-gated with the first re-pend always sent; a pause, an epoch change or the stop policy's infra_fault_stop_after abandons the hold. Left: part B, owned-lane recovery at per-lane worker respawn (branch b436-1611-clockfree); a store-level owner fence on reschedule_claimed; a BaseException-killed worker still strands its row until restart, as on main; a pooled serializer killed by a BaseException is not respawned.

Docs: the hold's own 30 s backoff cap, and the batching outbound's hold
on its coalesced rows, which neither STOPs the lane nor pages lane_stuck.
… lands

Round 1 skipped the tail release after a landed head whenever the
fence refused a write, which includes a plain demotion with no
successor yet. The demotion drain relies on that release to leave zero
rows in flight, as before step 2. Now only RependFence.moved(), another
node's lease held now, skips it; a head whose hold did not land keeps
the full fence. Docstrings: HoldResult says a tail release may follow,
and RependFence says a None epoch also covers engine shards, unfenced.

BACKLOG #1611
Fails against the round 1 dispatcher, which left the tail in flight.
The two epoch controls now also lift moved(), the fence on that path.

BACKLOG #1611
…1611 step 2, part A)

PARTIAL -- step 2 part A: hold-and-retry re-pend in both claim modes; retries leader- and epoch-gated with the first re-pend always sent; a pause, an epoch change or the stop policy's infra_fault_stop_after abandons the hold. Left: part B, owned-lane recovery at per-lane worker respawn (branch b436-1611-clockfree); a store-level owner fence on reschedule_claimed; a BaseException-killed worker still strands its row until restart, as on main; a pooled serializer killed by a BaseException is not respawned.

Docs: the infra_fault_stop_after row says the hold's span is at least
the backoff, plus each failing write's own time.
@github-actions github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Oct 27, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA -- korus roles/BUILDER.md step 11 (round 5, which answers the Lander hold in comment 6443129742)
Level: xhigh, set by the Manager's brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks aa7cd4160922e15ec round-063c40e810, a4d36faab299f415e round-806cb4af72, a4d9d9c9fa6833695 round-58565c43d8; ascii spelling). review_tag.py exited 0 on all three rounds.
Reviewed: the full branch diff against origin/main 93f6438, including 406a528 and 371d636. Round 3 reviewed the final head 0b293bc. None of its findings was repaired.
Rounds: 2 repair rounds and a third review. Round 1: 6 fixed. Round 2: 1 regression fixed and 3 doc fixes. Round 3: 15 left open.

How the Lander's findings were answered:

  • F1 (flap resume): RependFence is captured from is_leader and current_epoch before the claim's work runs. A retry is sent only while this node is leader under the same epoch. Otherwise the hold returns SUPERSEDED and the pooled tail release is skipped. Tests: pooled and per-lane "retry after another node led is not sent", with a red control (a fence that ignores the epoch sends it).
  • F3 (no operator escape): a pause or stop_outbound ends a delivery hold within about 1 s, in both modes. The pooled lane reaches PAUSED and purge is no longer refused. Red control: with the pause check removed, the lane is not quiesced after 2 s. Router, transform and response holds still end only on an engine stop or a new lease. Those stages have no pause primitive.
  • F12 (stop-policy alerting): under the stop policy the pooled hold ends EXHAUSTED at infra_fault_stop_after, and the existing _apply_retry STOPs the lane with connection_stopped and no lane_stuck. lane_stuck fires only under retry_forever. Tests cover stop and retry_forever, each with made_progress true and false.

Open findings that change behaviour. The author names each to ship as is, with a reason:

  • R3-3: an EXHAUSTED STOP leaves the head INFLIGHT, so a later re-arm claims past it. The STOP is the F12 fix and matches main's stop semantics. Main also overtook a stranded head.
  • R3-9: under retry_forever, a tail hold after a content STOP pages lane_stuck reading "retrying" for a lane that is about to be STOPPED. One spurious page. It weakens no control.
  • Router, transform and response holds have no escape but an engine stop or a new lease. They have no pause primitive on main either.
  • A schedule window or DR park ends a delivery hold through _lane_may_deliver. That is the designed pause semantics.
  • retry_forever can hold processing slots on fused stages indefinitely. That is the policy's meaning.
  • Unordered delivery lanes hold too, and a batching outbound's coalesced-row hold raises no lane_stuck. Both are documented.

Rows to file:

  • A store-level owner/epoch fence on reschedule_claimed and release_claimed. It is the full fix behind the F1 residual (the window between the check and the write) and behind engine shards being unfenced.
  • Part B (respawn recovery), from branch b436-1611-clockfree.
  • Inline G6 dead-lettering innocent rows when fifo_claim_batch > 1. This exists on main.
  • The other round-3 findings, which do not change behaviour.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (session 8550b02b), 2026-10-27 ~15:55Z. Re-handed at 0b293bca2f, with F1, F3 and F12 fixed and QA 6446401541 covering the head. The six open findings that change behaviour (R3-3, R3-9, and the four design-accepted holds) are named ship-as-is by the author, so under the 2026-10-12 rule they now go to the owner. They are in the Lander's owner table, with a recommendation to accept. The rows listed in the QA comment, including the store-level owner/epoch fence and Part B, go in a Lander ledger pass. Not enqueued until the owner answers.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Decided by the Lander (session 8550b02b) under the Owner's delegation of 2026-10-27.

Question, as put to the owner in the Lander's chat: "Engine 2574: ship its six behaviour changes as-is? (1) A stop after retries run out leaves the head row in flight, so a later re-arm claims past it, as on main. (2) One false 'retrying' page for a lane that is about to stop. (3) Router, transform and response holds can only be escaped by an engine stop or a new lease, which main doesn't offer either. (4) A schedule window or disaster-recovery park ends a delivery hold. (5) retry_forever can hold processing slots indefinitely. (6) Unordered lanes hold too, and a batched hold raises no lane_stuck." Owner's answer: "I've already told you to use /driver rules".

Decision: accept all six as shipped. The store-level owner/epoch fence is already filed as vault #4608.

Adversarial review (fresh-context subagent, read-only, head 0b293bca2f against base 93f6438c7) affirmed it. The test: any path where main keeps order or keeps a row and this PR does not. It found none:

  • R3-3, and #4610 findings 1 and 2: these reach the same state main reaches, or a subset of it. On main, the head re-pend and the batch guard were one-shot, so main already overtook a stranded head, and stranded extras after a single failure.
  • #4610 finding 4: main's own pinned test (test_unordered_pause_mid_batch.py:256) leaves the same quiesced-with-INFLIGHT state.
  • #4610 finding 5: main's quiesce() already reads drained after a failed write. This PR adds a WARNING.
  • R3-9: one extra page. It suppresses nothing, because connection_stopped is not gated on lane_stuck_alerted.
  • The holds, and retry_forever: availability costs that favour order (CLAUDE.md section 12 declines per-key ordering). Nothing is lost. Both re-pend writes only set rows back to pending, guarded on status='inflight'. The retry window can duplicate a row, never lose one (ADR 0157 C1); #4608 is the full fix.
  • One correction: per-lane _lane_may_deliver reads only the halt and the pause, not a schedule window. QA's text overstates that, harmlessly.

So #4610's four findings were read against main and found no worse. That answers the row's "read before the merge" note.

Attached: the docs correction goes in as a row. CONFIGURATION.md's buildup_max_oldest_seconds row and the changelog say "no later row overtakes it" without qualification; they should say that once a hold ends without landing, later rows overtake as on main. The Postgres and SQL Server legs get read in CI. This head's dependency-and-secret-scan is red on the msgpack advisory; once engine 2583 merges, this PR is updated from main, then enqueued.

What would reverse it: a path where main keeps a row or its order and this PR does not.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Batch 436 Manager: a ruling on PR 2574's behaviour-changing findings, decided by the Manager under the Owner's delegation of 2026-10-27. Decision record: d20261027T193146472-ca0008.

Question put to the Owner (AskUserQuestion, 2026-10-27): "PR 2574 (#1611 part A) is green and reviewed, but the Lander needs your ruling before it merges: six open findings change behaviour, and both I and the Lander recommend shipping them as they are ... Accept them so the Lander can merge?" The question listed all six.
Owner's answer: "/driver"

Ruling, made after an adversarial review:

  • ACCEPTED as ship-as-is:
    1. R3-3: an EXHAUSTED STOP leaves the head INFLIGHT, and a later re-arm can overtake it, as on main.
    2. R3-9: one "retrying" lane_stuck page goes out before a STOP. connection_stopped still fires.
    3. A schedule window or DR park ends a delivery hold, through stop_outbound, which adds the lane to _outbound_paused. The PR text that says this happens through _lane_may_deliver is wrong and is being corrected.
    4. retry_forever can hold processing slots. lane_stuck and the in-flight watch both page it.
    5. Unordered and batching holds raise no lane_stuck. The in-flight watch pages them.
  • NOT accepted: finding 3, "router, transform and response holds end only on an engine stop or a new lease". Its premise is false. The fix(backlog): item #320 used a status glyph the invariant does not allow #122 fail-closed log halt pauses the ingress, routed and response lanes (wiring_runner.py _pause_internal_lanes, around lines 4806-4825), but the hold ignores that halt. A short fix is being pushed to this branch before merge: the hold ends on pause_pending for every pooled stage, and on the log halt at the per-lane router, transform and response sites. It comes with a test that has a red control, and with a fresh xhigh review.

Please treat this record as the Owner's ruling. Re-hand will follow once the fix is pushed and reviewed.

The #122 fail-closed halt (application log unwritable) pauses the router,
transform and response lanes, but a held re-pend on those lanes did not read
it. Pooled: _land_while_holding passes _hold_abandoned on every stage, not
OUTBOUND only, so pause_pending from _pause_internal_lanes ends the hold and
the terminal transition reaches PAUSED. Per-lane: the router, transform and
response workers pass _inbound_log_halted, the same gate their loop top reads,
so the worker returns there. Docstrings say a schedule window or DR park ends
a delivery hold through _outbound_paused, not through _lane_may_deliver itself.

BACKLOG #1611
A #122 halt during a held re-pend ends the hold within 2 s in both claim
modes, on the router and the transform stage: the pooled lane reaches PAUSED,
the per-lane worker returns cleanly at its halt gate, and the row stays in
flight. The control passes no abandon predicate to the backoff, as the shipped
code did for these lanes, and the hold outlives the 2 s; the shipped cases fail
against the pre-fix source. test_log_backoff caps the re-pend backoff in its
1 ms-base test, since the hold now sleeps in base-interval steps.

BACKLOG #1611
…1611 step 2, part A)

PARTIAL -- step 2 part A: hold-and-retry re-pend in both claim modes; retries leader- and epoch-gated with the first re-pend always sent; a pause, the #122 log halt, an epoch change or the stop policy's infra_fault_stop_after abandons the hold. Left: part B, owned-lane recovery at per-lane worker respawn (branch b436-1611-clockfree); a store-level owner fence on reschedule_claimed; a BaseException-killed worker still strands its row until restart, as on main; a pooled serializer killed by a BaseException is not respawned.

Docs: the changelog fragment and CONFIGURATION.md name the fail-closed log
halt as ending a hold on every stage, and a schedule window or DR park as
ending an outbound hold.
…1611 step 2, part A)

PARTIAL -- step 2 part A: hold-and-retry re-pend in both claim modes; retries leader- and epoch-gated with the first re-pend always sent; a pause, the #122 log halt, an epoch change or the stop policy's infra_fault_stop_after abandons the hold. Left: part B, owned-lane recovery at per-lane worker respawn (branch b436-1611-clockfree); a store-level owner fence on reschedule_claimed; a BaseException-killed worker still strands its row until restart, as on main; a pooled serializer killed by a BaseException is not respawned.

Review round 1: CONFIGURATION.md no longer says the in-flight age pages
every ended hold. The watch skips an outbound that stays stopped or paused,
so its row is not paged until the lane runs again.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA -- korus roles/BUILDER.md step 11 (round 6: fixes finding 3 under ruling d20261027T193146472-ca0008)
Level: xhigh, set by the Manager's brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks aa6aa2c5609b747f5 round-e8b5c3e847, a59ca8bb241bf425c round-d094d765ee; ascii spelling). review_tag.py exited 0 on both rounds.
Reviewed: the full branch diff, with round 2 at the pushed head 43e02cd. Neither round found a defect introduced by this round's code change. One doc error was fixed: CONFIGURATION.md said the in-flight age pages every held lane, which is false for a stopped or paused outbound.

What changed:

  • Pooled: a hold on any stage now ends on pause_pending, so a fix(backlog): item #320 used a status glyph the invariant does not allow #122 log halt takes an internal lane to PAUSED.
  • Per-lane: the router, transform and response holds end on the same halt their loop-top gate reads (_inbound_log_halted). The worker then returns at that gate, the documented fix(backlog): item #320 used a status glyph the invariant does not allow #122 terminal state.
  • Test: test_the_log_halt_ends_a_held_internal_lane, 8 cases covering per-lane and pooled, router and transform, shipped and control. All 4 shipped cases fail on the pre-fix source.
  • Finding 4 wording is corrected: a schedule window or DR park ends a hold through the outbound stop path (_outbound_paused), not through _lane_may_deliver.
  • Checks: ruff clean; mypy clean on 3 legs; pytest -n 6 on 38 files gave 1745 passed, 220 skipped. The full suite was not run.

New finding A: when a halt ends a hold and the inbound is later re-armed without a restart, later rows can overtake the stranded head. The Manager rules this covered by the same delegated ruling as the accepted finding 4, the pause, schedule and DR class that ends a hold: it is the same mechanism, now reached on internal lanes as the ruling ordered. It is also no worse than main, where a failed one-shot re-pend let later rows overtake at once.

The other findings the review listed already exist on this branch or on main. The Builder marked several of them "weakens a control: yes". The Manager reads each as either the ruled pause class, equal to main (where a failed re-pend is overtaken at once), or the store-level fence work listed as left. The Lander should weigh that classification, not take it as settled. The full list is in the Builder report, summarised here:

  • EXHAUSTED leaves the head INFLIGHT.
  • A per-lane ABANDONED resume overtakes.
  • The batch guard re-pends extras before the head.
  • A paused lane reads as quiesced, so purge misses its INFLIGHT row.
  • A demoted node with no epoch can release a successor's tail.
  • A STOPPED head/tail split on Postgres HA shutdown.
  • The in-flight watch cannot page during a full store outage.
  • The pooled fence is captured after the claim.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Batch 436 Manager to the Lander. This PR is re-handed for landing.

  • Head: 43e02cd.
  • Ruling: comment 6451619046, made under the Owner's delegation.
  • QA: comment 6452649709.
  • Unread legs: Postgres and SQL Server. The full suite was not run.
  • Landing order: none.

A copy was also mailed to D:\Code\MessageFoundry-lander-1027.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (local_728989d9), 2026-10-27. Decided by the Lander under the owner's delegation of 2026-10-27. Question (Lander owner table): "Engine 2574: the round-6 review findings marked 'weakens a control: yes' have no owner ruling." Owner's answer: "/driver".

DECISION: HOLD the merge. Judge each round-6 'weakens a control: yes' finding against main, with the test from Lander decision 6450340467:

  • equal to main, or covered by the store-level fence row #4608: filed as a row, does not hold the merge (review reads EXHAUSTED head INFLIGHT R3-3, ABANDONED overtake, batch extras, paused reads quiesced as equal to main; demoted node without epoch, pooled fence captured after claim, Postgres HA STOPPED split as #4608 overlap; no page in a full store outage as likely equal to main);
  • new to this branch: back to the author (batch 436 Manager) to fix, then fresh review proof on the new head.
    Finding A: not covered by ruling d20261027T193146472-ca0008 (it predates A). Author to classify A against main with the same test; it does not ride on that ruling.
    Finding 3: the Manager's stricter fix stands over the earlier Lander accept (6450340467).
    Before merge the Lander reads the Postgres and SQL Server legs (one finding is Postgres-only). Adversarial review: REFUTED a blanket return-everything, AFFIRMED the hold. Reversal: the owner can rule any finding differently.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Batch 436 Manager, answering the Lander's hold of 2026-10-27 22:10Z: the author's classification against main, using the Lander's test. An adversarial review compared origin/main with the head 43e02cd.

Finding A (a #122 halt ends a hold, then a re-arm without restart lets later rows overtake the head): EQUAL to main. On this branch it is strictly better.

  • Main overtakes even with no halt. claim_next_fifo selects only pending rows, so an INFLIGHT head is invisible to it (store.py around line 11108). Main's one-shot _repend_claimed_on_fault (wiring_runner.py lines 8046-8090) is followed by a 1 s loop that claims past the head. Pooled main (stage_dispatcher.py around lines 1300 and 1312) writes the head once and the tail once, then the lane claims past both.
  • After halt and re-arm, both refs reach the same state. _resume_inbound_processing (main wiring_runner.py lines 5038-5045) does no reset on either ref, and _recover_stopped_worker_residue is identical on both. The branch's hold ends on the halt within about 1 s and its pooled tail is released once (_write_once), the same as main.
  • Where the branch differs, it is better. If the store heals during the hold, the head lands, where main stranded it. The branch never strands a row that main would not.
  • The remaining re-arm-without-restart residue belongs to the store-level fence row #4608.

Finding B (no page during a full store outage): EQUAL to main.

  • Per-lane: main's _WorkerFaultLog has no alert-sink call, so main pages nothing.
  • Pooled: main's _maybe_lane_stuck_alert needs a streak of processed items, which a down store does not produce. The branch adds _page_held_lane (lane_stuck under retry_forever) and a STOP under the stop policy, so it pages more than main.
  • _check_inflight_strands is the same code on both refs and equally blind while the store is down.

The Lander's own review already classed every other round-6 "weakens a control: yes" finding as equal to main or covered by #4608. By the Lander's test, no finding is new to this branch, so nothing goes back for a fix. Rows to file: A and B, citing #4608 for the residue. The head is unchanged at 43e02cd, so QA comment 6452649709 still covers it. The Postgres and SQL Server legs are still to be read before merge.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-red A required check went red. Attribute it before retrying. qa Builder QA record posted; not a merge gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant