[TheSentinel454]

Summary

Follow-up to #9195. Under its 2 s deadline, a database stall of a few seconds made every node's cross-node deliveries go stale at the same moment, and each stale delivery fenced and closed its recipients. Their reconnects and catch-up reads hit the same database pool, so still more deliveries went stale. A short burst of HTTP /query traffic, which the relay had absorbed before #9195, became a reconnect storm.

This PR fixes three parts of that loop:

1. Late delivery instead of closing. MAX_QUEUE_DELAY (2 s) becomes MAX_DELIVERY_DELAY (30 s). A slow delivery reaches its recipients late, and only one that misses 30 s is lost (fenced, resynced, closed). The queue-full and dispatcher-lag losses still close as before. A recipient the access gate could not check (a failed visibility or membership read) used to miss the event silently unless it was an artifact sync. It is now re-checked with backoff (250 ms doubling to 2 s) until the 30 s deadline, gets the event if a check passes, and is lost the same way (fenced and closed) only if the gate still cannot check it then. Those reads use the writer pool, which gives up after 3 s, and open-channel visibility is never cached, so without the re-check any stall past 3 s would close the recipients of almost every channel event. Ephemeral events are not re-checked. A channel the database no longer has (ChannelNotFound, e.g. the channel's own deletion event) is an answer, not a failed check: it grants no one access, as on main, and is never re-checked.
2. Close pacing as a share of the node. The closer used to allow 5,000 closes/s per node, which closes any realistic node's connections almost at once. It now closes at most 1% of the node's connections per second, with at least one per 100 ms tick and ±50% jitter on each tick. Each node paces itself, so a loss that reaches every node sends at most that share of the fleet back each second.
3. One membership read per fan-out. For a private channel, check_fanout_access called is_member_cached once per recipient, one after another, and each miss waited on the pool in turn. It now calls MembershipCache::members, which answers fresh users from the cache and loads the rest with one bulk read per 64 users (get_active_membership_ids_bulk_read_at, the same rows as the single-user read with the same per-user cap). The loaded sets are published to the cache under the same gate and invalidation rules as contains. Oversized users share one membership_pairs point read.

Trade-off (fix 1)

A client resumes from the newest created_at it received, minus 5 s. While an event waits, newer events on other channels can move a recipient's cursor past it. If the event then arrives, nothing is lost. If the client disconnects while it waits, or the event misses 30 s, a reconnect replays it only if the cursor moved less than 5 s past it. Before #9195 these events were skipped silently. With #9195 the 2 s deadline closed the recipients, which kept the cursor bound but made a few seconds of stall into a fleet-wide reconnect. The module docs and ARCHITECTURE.md describe the new bound.

Known limits

• A worker delivers its channels in order, so an event that waits up to 30 s holds back the events queued behind it on that worker (one of 16). If a stall outlasts the queue (1,024 events per worker), the overflow is lost through queue_full as before, now with paced closes.
• On the node that accepted the event, the local fan-out path still drops unverified plain subscribers silently. That path has no closer. It is left as identified debt.
• A lost event still closes every recipient fan_out_scoped matched, without the access gate, because the gate needs the database that just failed. Pacing bounds the cost.

Review-Proven Rules

• RPR 9 (whole fleet at once): this is the rule fix(relay): deliver multi-node fan-out concurrently and never drop silently #9195's pacing fell short of. A per-node cap in absolute closes did not scale with node size. The new cap is a share of each node.
• RPR 1 (no silent drops): an unverified recipient used to be dropped silently. It is now re-checked until the deadline, then goes through the same loss path as a stale event. Ephemeral events are still only counted.
• RPR 2 (stale state): bulk loads publish only while they hold the entry's current gate, so an invalidation during the read wins, the same as contains. A user whose gate another load holds is read again here but not published, so a batch never waits behind a single load.
• RPR 4 (bound resources): bulk reads are chunked at 64 users with the per-user cap from the single read.

What got simpler

contains' inline load handling is now Memberships::from_read / contains / publish, which the bulk path shares. The closer's fixed batch constant is gone.

Related issue

Follow-up to #9195. The trigger, every desktop client calling /query in the same second, is not addressed here.

Testing

• New unit tests: pacing at 1% of 2,000 connections over 10 s; an event 10 s late is delivered and closes nothing; an unverified recipient is re-checked until the deadline, then fenced and queued, but an ephemeral event is neither re-checked nor closes anything; a recipient whose check passes on a re-check gets the event late and closes nothing; an event for a missing channel reaches no one and is not re-checked (Postgres); one bulk read per 64 users, published, with oversized users on a point read and a failed read reported as unverified; a batch does not wait for or overwrite a concurrent single load.
• New Postgres test: the bulk read matches the single-user read (removed members and deleted channels excluded, per-user cap applied), and a closed pool reports unverified, not denied.
• Must-fail mutants checked on each new guard (2 s deadline, unverified not lost, no re-check, ephemeral re-checked, missing channel treated as unverified, unpaced closer, bulk read not published, failed read treated as denial).

🤖 Generated with Claude Code

[github-actions]

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

• Exact PR diff: f3b127e3af5c14d2314faa2b63556c3f51ec6743...3d067a8b4dcc55d8726342bceaa250e182b1e247
• Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.

---

Review Summary

Overall Risk: MEDIUM

The longer delivery deadline can cause stored events to be skipped permanently during reconnect recovery.

Findings

[MEDIUM] Keep delayed delivery within the client replay window

• Category: Reliability
• Location: crates/buzz-relay/src/handlers/multinode_fanout.rs:79 (source)
• Description: The deadline increases from 2 seconds to 30 seconds, but existing consumers still resume from their newest received event minus 5 seconds. For example, buzz-acp records a per-channel maximum in record_event and subtracts SINCE_SKEW_SECS in send_subscribe. While an older Redis event waits in its worker, local dispatch can deliver a newer event for the same channel. If the connection drops before the older event arrives, or delivery ultimately expires, reconnect starts after that missing event. The subsequent fence cannot undo the cursor advancement.
• Impact: During delivery stalls, agents and other consumers using the five-second rewind can permanently miss stored messages or control events despite reconnecting successfully.
• Recommendation: Update affected clients' replay windows alongside the longer deadline, including appropriate timing margins, or preserve an explicit replay floor for pending and lost deliveries so newer events cannot advance recovery past them.

Notes

• No additional limitations were reported.

---

_{Generated by Codex Security Review |
Requested by: @TheSentinel454 |
Workflow run}

[TheSentinel454]

🤖 Intentional trade-off for this fix-forward, documented in the multinode_fanout module docs: a late delivery beats closing every recipient during a pool stall. Client replay slack will be widened to cover MAX_DELIVERY_DELAY in a follow-up.

[kalvinnchau]

🤖 Review of 9f98072. One P2 below, inline. CI is green.

The 1%/s close pacing and the bulk membership read address the storm mechanism from the 2026-11-07 incident: what used to be ~2.9k closes fleet-wide in ~10 s is now capped at about 10/s per pod at current pod sizes (~150 conns/pod). The 30 s deadline does less than the description says for private channels, though. See the inline comment.

Scores: minimalness 8/10 (three fixes plus a new unverified→close behavior in one PR), elegance 9/10, correctness 8/10 (inline P2). Not approving until the P2 is resolved or explicitly accepted.

Trade-off to confirm, not a defect: a fenced connection gets no live EVENT frames until the closer reaches it. At 1% per second, a node with 1,000 or more fenced connections takes up to 100 s to close the last one. During that time the client stays connected but receives nothing live.

[kalvinnchau]

🤖 Re-review of ed64e49. The P2 from my previous review is resolved. deliver_or_lose now re-checks unverified recipients with backoff until the 30 s deadline. A stall shorter than the deadline delivers late and closes nothing (recipient_the_gate_checks_on_a_retry_gets_the_event_late). Ephemeral events are not retried. This also covers the case Elrond raised: public-channel visibility is never cached, so it fails on every pass after 3 s.

One non-blocking P3 inline.

Scores: minimalness 9/10 (the three fixes are all parts of one feedback loop; each is justified in the description). Elegance 9/10 (deliver_pubsub_event now repeats route_fanout_matches' signal/divert/resync steps instead of calling it, and the doc comment points this out). Correctness 9/10 (P3 inline).

I'll approve once Rust / Unit Tests passes on this head.

[kalvinnchau]

🤖 [P3, non-blocking] The retry cutoff counts only the backoff, not how long a check takes. Each pass can block for up to the writer pool's 3 s acquire timeout per read: visibility, then membership. Suppose a pass starts within that window of retry_until, for example a 2 s backoff ending at about 28.5 s. Then timeout_at(deadline, ...) in deliver_or_lose can cancel the pass mid-read. The event then goes through lose(.., "stale"), which closes every fan_out_scoped match, including recipients that already received the event on an earlier pass, and it is counted as stale rather than unverified. This is bounded by pacing and rare (it needs a stall of about 27 s or more), so it isn't blocking. A fix is to start the last pass only while now + backoff + acquire timeout < retry_until.

[kalvinnchau]

🤖 Approving 3d067a8. CI is green. The P2 is resolved by the gate retry, and the ChannelNotFound handling is correct: get_channel reads from the writer, so a channel that was just created cannot look missing because of replica lag. The P3 about the retry cutoff not counting how long the check takes is non-blocking.