Repository navigation
fix(relay): deliver multi-node fan-out concurrently and never drop silently - #9195
Merged
Merged
Conversation
…lently The Redis fan-out consumer delivered every cross-node event serially, awaiting the access gate for each one. Under load it fell more than 4,096 events behind the broadcast, and every Lagged(n) skipped n events for good: about half of cross-pod events at 10x load and three quarters at 13x in staging load tests. Loadgen did not see it because it only times events that arrive. A dispatcher now reads the broadcast and only routes: it drops local echoes and queues each event on one of 16 workers by (community, topic), so a channel's events keep their order while channels deliver concurrently. Loss is handled, not hidden. When a worker's bounded queue is full, the dispatcher drops that event, resyncs the syncs it would have reached, and closes the connections it would have reached so their clients catch up from their cursor. When the dispatcher itself lags the broadcast it cannot know what it skipped, so it does the same for every subscribed connection. One closer paces closes at 5,000/s per node so a loss cannot become a reconnect storm. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
TheSentinel454
marked this pull request as ready for review
November 6, 2026 16:34
TheSentinel454
had a problem deploying
to
codex-review
November 6, 2026 16:35 — with
GitHub Actions
Error
…lose A connection that lost a live event waited for its paced close while still receiving newer events. Its client resumes from the newest event it received, so a close backlog of more than a few seconds moved that cursor past the lost event for good. The closer now fences each connection when it is queued: the live fan-out send path delivers it nothing from then on. Every event dispatched after the loss is therefore withheld, and the client's cursor stays before the loss. Only the socket close stays paced. The close counter now counts each batch as it closes instead of once the whole backlog drains. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
TheSentinel454
had a problem deploying
to
codex-review
November 6, 2026 16:39 — with
GitHub Actions
Error
A client resumes from the newest event it received minus 5 s of slack. An event waiting behind a worker backlog, or in a stalled delivery, could be withheld far longer while local fan-out and other workers moved that cursor past it, so a reconnect never replayed it. Give every queued event a 2 s deadline. The worker delivers it by the deadline or handles it as lost: resync its syncs, fence its recipients, and queue them for the paced close. Make the fence atomic with the live send path: it is written under the connection map's write guard, and the send path checks it and queues the frame under one read guard. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
TheSentinel454
had a problem deploying
to
codex-review
November 6, 2026 16:49 — with
GitHub Actions
Error
The 2 s delivery deadline started at dispatch, so time an event spent waiting in the local broadcast before dispatch went uncounted. Stamp `ChannelEvent::received_at` in the pub/sub subscriber and run the deadline from there. Document the replay bound: the deadline plus writer clock skew and publish delay must stay under the client's 5 s resume slack, the same limit an ordinary reconnect has. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
TheSentinel454
had a problem deploying
to
codex-review
November 6, 2026 16:53 — with
GitHub Actions
Error
The deadline bounds when an event may still be delivered, not when its fence lands: a dispatcher that falls behind without overflowing the broadcast detects a loss late. Say so next to the other replay limits. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
TheSentinel454
had a problem deploying
to
codex-review
November 6, 2026 16:56 — with
GitHub Actions
Error
The unit lane selects buzz-relay tests by explicit filter, so the new handlers::multinode_fanout::tests ran nowhere in CI. Add them to the test-unit nextest filter and the cargo fallback in run-tests.sh. They are infra-free. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
TheSentinel454
temporarily deployed
to
codex-review
November 6, 2026 17:08 — with
GitHub Actions
Inactive
🔐 Codex Security Review
|
An ephemeral event (presence, typing, observer frames) is never stored, so a reconnect cannot replay it. Fencing and closing its recipients recovered nothing, and a global presence event reaches most of a community's connections, so one stale presence frame closed nearly all of them. A lost ephemeral event is now only counted, with reason="ephemeral"; it is still never delivered past its deadline. The loss tests move to a stored kind. New tests pin that an ephemeral loss neither fences nor closes while a stored loss on the same queue still does, and that the fence also withholds relay-local fan-out. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
TheSentinel454
had a problem deploying
to
codex-review
November 6, 2026 19:05 — with
GitHub Actions
Error
Contributor
Author
|
🤖 Addressed in d1ba7ee.
|
bradseiler
approved these changes
Nov 6, 2026
bradseiler
added a commit
that referenced
this pull request
Nov 6, 2026
* origin/main: fix(relay): route live REQ events by each filter's own channel scope (#8519) fix(relay): deliver multi-node fan-out concurrently and never drop silently (#9195) perf(db): bound since-limited events reads with the month-floor literal (#9197) fix(mobile): match Agents sign-in to Home’s reconnect capsule (#9204) fix(cli): clearer desktop pairing errors, paired default relay, and auth desktop list (#9196) Signed-off-by: Brad Seiler <seiler@squareup.com>
TheSentinel454
added a commit
that referenced
this pull request
Nov 6, 2026
…#9214) ## Summary `buzz-relay` lib tests fail to compile on `main`: `error[E0063]: missing field received_at in initializer of buzz_pubsub::ChannelEvent` at `crates/buzz-relay/src/handlers/system_channel_postgres_tests.rs:1549`. This is a semantic merge conflict. #9195 added `ChannelEvent::received_at`, and #8519 merged a minute later with a test helper that builds a `ChannelEvent` without it. Neither PR's CI saw the other. This sets the field to `tokio::time::Instant::now()`, as every other initializer does. It breaks clippy (`Rust Lint`) and the backend-integration test archive for every open PR, because CI builds the merge with `main`. ### Testing - `cargo fmt --check` and `cargo clippy -p buzz-relay --all-targets -D warnings` on a cloud workstation at this head. ## What got simpler Nothing; a one-field compile fix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
TheSentinel454
added a commit
that referenced
this pull request
Nov 9, 2026
…ind (#9302) ## Summary Cross-node fan-out no longer closes connections when it falls behind. Before this change, an event closed its local recipients when: - it was not delivered within 30 s of this node reading it from Redis, - its worker queue was full, - or the access gate still could not check a recipient by that deadline. Dispatcher lag also closed every subscribed connection. A rate-limited closer spread the closes out, at 1% of a node's connections per second with a floor. The trouble is that a database stall slows every node's deliveries at once. So the closes landed on every node together. Each reconnect then does AUTH, membership reads, NIP-11 and REST catch-up against the database that was already slow. That turned a short stall into a reconnect storm and bursts of 503s. On small nodes the closer's floor dominated, so a node could close all of its connections within tens of seconds. Review-Proven Rule 9 (design every client-driven path for the whole fleet running it at once) is the reason: a server-forced close starts a fleet-wide reconnect. ## What changes Kept from #9195 and #9235: - One dispatcher plus 16 workers, routed by `(community, topic)`. Each channel stays in order and channels deliver concurrently. - The bulk membership read in the access gate, and denying an event for a channel that no longer exists. Removed: - The delivery deadline and `ChannelEvent.received_at`. - The live fence (`fence_live_delivery`, `live_fenced`). - `LagCloser` and its rate limit. - The queue-full closes and the dispatcher-lag closes. - The access-gate retry loop (`buzz_multinode_fanout_gate_retries_total`). Behavior now: - A slow delivery holds only its own worker and arrives late. - A full worker queue or dispatcher lag drops the event on this node. The drop is counted in `buzz_multinode_fanout_dropped_total{reason="queue_full"}` or `buzz_multinode_fanout_lag_total`, and the affected artifact syncs get `CLOSED "resync: live delivery lagged"` (NIP-AR Sync rule 5). This is the same as before the closer existed. - A recipient the access gate cannot check misses that event, and a sync among them resyncs. Same as the local fan-out path. Trade-off: a plain subscription that misses a dropped event is not told. Its client may replay the event on its next reconnect, but not if later deliveries moved its resume cursor past the event by more than its replay slack (5 s for the ACP client). That was the behavior before #9195. Queue-full and lag drops stay visible in the counters, so we can see whether they actually happen before adding any recovery mechanism. ## What got simpler Net −700 lines. - `multinode_fanout.rs` drops from ~940 lines to ~350. - `deliver_pubsub_event` loses its retry loop and goes back to the shared `route_fanout_matches` path. The Redis path and the local path once again apply the access gate the same way. - `ConnectionManager` loses the fence flag and the split send helper. `SubscriptionRegistry` loses `subscribed_connections`. ## Testing - `handlers::multinode_fanout::tests`: - in-order delivery through the workers; - a full queue resyncs only the matched syncs and closes nothing, and a local echo is not counted as a drop; - dispatcher lag resyncs every sync and closes nothing; - a recipient the gate cannot check misses the event with no retry, stays connected, and still gets the next event. - `handlers::event::tests` is unchanged apart from removing `received_at`. - Mutants killed: the queue-full resync, the lag resync and the local-echo skip. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This branch was successfully deployed
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.
Problem
Each relay node runs one consumer for events that other nodes publish to Redis. It delivered them one at a time and awaited the access gate (visibility and membership lookups) for each event. Under load it fell more than 4,096 events behind the pub/sub broadcast. Each
Lagged(n)then skippednevents for good: nothing replayed them, and plain subscribers were never told.In a 15-node load test, about 50% of cross-node events were skipped at 10x load and about 75% at 13x. Throughput at 13x was also lower than at 10x. Loadgen didn't show this, because it only times events that arrive.
Change
(community, topic). Each worker delivers its queue in order, so a channel's events keep their order while different channels deliver concurrently.handlers/multinode_fanout.rsChannelEventnow carriesreceived_at, stamped by the pub/sub subscriber. For a lost event, the relay sendsCLOSED "resync: live delivery lagged"to every sync the event would have reached (NIP-AR Sync rule 5). It also closes every connection the event would have reached, so those clients reconnect and catch up from their cursor. Matching runs before the access gate, so a recipient that would have been filtered out may be closed too. That costs a reconnect, never a leak.send_to_text_bytes, local and cross-node) delivers it nothing from then on. The fence is written under the connection map's write guard, and the send path checks it and queues its frame under one read guard, so no frame slips in between the check and the send.created_atit received, minus 5 s of slack. The desktop app (block/buzz-app) resumes 960 s before its connection last heard the relay, which already covers the full 900 screated_atdrift. While an event waits, local fan-out and other workers keep moving that cursor. The fence stops the cursor, and the deadline means no withheld event is more than 2 s older than the fence, so a reconnect replays it. Without the deadline, an event stuck behind a long backlog could be withheld past the slack and never replayed.fan_out_pubsub_eventis now split intotake_local_echoanddeliver_pubsub_event, so the dispatcher can check echoes before queueing. Its behavior is unchanged.created_atup to 15 min off, so a writer clock more than about 3 s slow can still put an event behind the cursor. An ordinary reconnect has the same limit today; closing it needs a client rewind instruction, which is out of scope here. The desktop app's 960 s resume window has no such gap. A dispatcher that falls behind without overflowing the broadcast still never delivers a late event, but it detects the loss, and fences, late. Events skipped by dispatcher lag are never seen. Neither case is bounded by the deadline, which is a cutoff for attempting delivery, not a guaranteed fence time.buzz_multinode_fanout_dropped_total{reason="stale"},buzz_multinode_fanout_lag_closed_totaland the reconnect REQ rate next to pool wait. On dispatcher lag, the dispatcher scans the subscription registry and fences each subscribed connection synchronously, so also watch how long it stays behind after aLagged.Metrics:
buzz_multinode_fanout_lag_total(dispatcher lag) andbuzz_multinode_fanout_total(delivered) keep their meaning.buzz_multinode_fanout_dropped_total{reason}counts lost events:queue_full, orstalefor one not delivered by its deadline. A lost ephemeral event counts asephemeralinstead and closes nothing.buzz_multinode_fanout_lag_closed_totalcounts close attempts for a loss, one batch at a time.Rollout
dropped_totalfor stored kinds and zerolag_closed_total. Report delivered %,dropped_total{reason},lag_closed_total, reconnect REQ rate and pool wait.lag_closed_totalclimbing steadily, or reconnect REQs pushing pool wait up, at normal load. Rollback is a plain revert and redeploy: there are no schema, config or wire changes.Ordering
Order is kept within a
(community, topic). Events on different topics, for example a channel event and a global event, can now reach one subscriber in either order. Before this change they arrived in Redis arrival order. Nothing in the relay depends on order across topics: the access gate re-reads membership and visibility per event. Clients order bycreated_at.What got simpler / known debt
main.rsshrinks to onespawncall, and the lag handling lives next to the delivery code.artifact_sync::watch_link), but ordinary subscriptions are not.Tests
handlers::multinode_fanout::tests, each checked with a mutant on Blox:ephemeraland neither fences nor closes, while a stored event lost on the same queue still closes🤖 Generated with Claude Code