mirror of
https://github.com/block/buzz.git
synced 2026-10-30 14:01:37 +00:00
cc28660c61
## Summary During a relay rolling deploy, every agent harness connection gets a close 1012. `buzz-acp` reconnected immediately, so a pod's agents all arrived at once, and `jittered_duration` produced 0.80–0.89× rather than the intended ±20%. Separately, a replay that was cut off by a dropped socket could lose the gap it was replaying. This PR makes three changes: 1. **Reconnect spreading.** The Close frame's code is now read. After 1012 (Service Restart) or 1001 (Going Away), the first reconnect attempt waits a uniform random delay in [0, 10s], which matches the relay's server-side drain spread. Other closes keep the immediate first attempt. Both reconnect entry points go through one `prepare_reconnect`. 2. **Gaps survive a dropped replay.** A recorded dropped-event gap (`channel_dropped_since` / `membership_dropped_since`) is retired when the replay REQ is written, as before. On `main` it is then gone: if the socket drops before that replay finishes, the next `since` comes from `last_seen`. Relay history is sent newest first, so `last_seen` can already be past the gap, and the lost event is skipped. Now the retired record is kept per connection until that subscription reaches a counted EOSE. If the connection ends first, the record is restored (merged by `min` with any newer loss), so the next replay still starts at or before the gap. - This covers overflow-recovery and reconnect replays, and a socket that dies partway through a resubscribe (`RetryConnection`): every fresh-connection resubscribe closes out the previous socket first. - **Ambiguous EOSE is ignored:** EOSE carries only the subscription ID. If a REQ for the same ID is written a second time on one socket (an overflow-recovery replay, or an unsubscribe followed by a re-subscribe), that subscription's EOSE never releases its retired gap for the rest of that socket. - **Cost:** a gap whose replay EOSE can't be counted (a superseded REQ) is restored at close, so the next reconnect replays once more from it. State is one oldest gap per subscription, cleared on unsubscribe, and the first unambiguous EOSE for that subscription retires it. Until the relay sends CLOSED for a failed history read (#8692) and that relay is deployed, an EOSE sent after a failed read also retires the gap, even though nothing was replayed. That's no worse than `main`, which drops the gap as soon as the REQ is written. - **Removal paths:** unsubscribe clears the retired gap; the per-socket REQ tracking is cleared only when the connection closes. 3. **Jitter fix.** `jittered_duration` is now a symmetric ±20%, sampled from the OS CSPRNG. The replay cursor itself is unchanged: `channel_since` and the new `membership_since` share one pure `resume_since`, which is `main`'s `min(last_seen, dropped)`, else the baseline. **Moved out of this PR:** an earlier revision also resumed quiet subscriptions from an EOSE-derived `live_until`. The relay currently sends EOSE when a history read fails, so `live_until` could certify coverage that was never delivered. That term moves to #8688, alongside its `buzz-acp` backoff on that CLOSED. #8688 merges after the relay sends CLOSED for a failed history read (#8692) and that relay is deployed. **What got simpler:** `channel_since` and `membership_since` share `resume_since`, which replaces six inline copies of the membership `since` computation. `SubscribeMembership` now honours a recorded membership gap like every other membership REQ. **Identified debt (not changed here):** - Quiet channels and the membership subscription still replay from the process-start watermark on every reconnect, so a long-running agent pays more for each bounce. #8688 addresses this. - A non-rate-limited, non-auth CLOSED re-REQs the channel immediately, bounded only by the relay's rate limit (#8688). - `try_autonomous_reconnect` restarts at attempt 0, so a relay that accepts and immediately closes with a non-1012 code sees an immediate retry each time. - Per-REQ wire IDs would make every EOSE unambiguous. They'd remove the supersede tracking and let an overflow-recovery replay's EOSE release its gap. Today a superseded replay keeps the gap until close, and on a busy channel past the relay's 1000-event page limit that can repeat a bounded replay on every reconnect. The remaining per-connection sets would then collapse into one record. Proposed as a follow-up. ### Related issue N/A. This is part of a set of reconnect-load reductions. Companion PRs: desktop (#8652), mobile (#8659), relay read-pool warm (#8651), `buzz-acp` CLOSED backoff (#8688) and relay CLOSED on a failed history read (#8692). ### Testing - On a Linux workstation, at the head SHA: - `cargo fmt --all -- --check` and `cargo clippy -p buzz-acp --all-targets -- -D warnings` pass. - `cargo test -p buzz-acp -- --test-threads=1`, with the harness env cleared per `crates/buzz-acp/TESTING.md`: - lib: 1078 passed, 3 ignored, both serial and with default parallelism. - `pool_lifecycle_state` (9), `run_task` (9) and `stdio_contract` (1) all pass. - `git_bootstrap`: 2 of 4 fail with `could not read Username for 'https://relay.invalid/...'`. The same two fail identically at the merge base `b310fea0` on the same workstation, so they're environmental. - `cargo test -p buzz-acp --lib relay::resume_tests` with default parallelism: 10 runs in a row, 18/18 each time. - New `relay/resume_tests.rs`: - table tests for `resume_since`, `first_reconnect_delay` and the jitter math - Close-code capture, and the close code consumed by the next reconnect - a counting fake relay showing 1012/1001 delay the first connect while 1000/1006 don't - unfinished gap replays (channel and membership) keep their floor across repeated drops, with a newer event before or during the replay; a completed replay releases it - delayed EOSE of a superseded REQ (channel and membership overflow recovery), with `last_seen` past the gap - a redial after a failed resubscribe, a new loss during an unfinished replay (channel and membership, merged by `min`; a repeated close is a no-op), a second REQ on one socket retiring an older loss (the older one is restored), `wait_for_reconnect` restoring a retired gap, and unsubscribe clearing it - a connected `SubscribeMembership` asserting its wire `since` - 19 mutations of `relay.rs`, applied one at a time, each fail at least one `relay::` test: drop the loss restore on close, take `max` instead of `min` when merging a restored loss (channel or membership) or when a second REQ stashes one, keep the retired gap on EOSE, release it on a superseded EOSE, drop supersede tracking, drop the redial restore, keep the retired gap on unsubscribe, inline the membership `since`, skip `note_req_sent` (channel or membership), skip `prepare_reconnect` (either entry point), do not consume or record the close code, do not delay 1001, revert the jitter band, and let `last_seen` beat the gap. - Exercised against a real relay (Postgres 16, Redis) at an earlier revision whose reconnect-delay code is identical to this head (`prepare_reconnect` and `first_reconnect_delay` unchanged), comparing the merge base with that head: | | Base | Head | |---|---:|---:| | 20 agents after a 1012: reconnect min / median / max | 1 / 2 / 3 ms | 622 / 3,638 / 9,233 ms | | Reconnects per one-second bucket, 0–10s | `20,0,0,0,0,0,0,0,0,0,0` | `4,2,2,3,2,1,2,2,1,1,0` | | Mention sent during the bounce, answered | once | once | - The new tests run in CI via `just test-unit` (`cargo nextest run -p buzz-acp`). buzz-review-completed 🤖 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 Sonnet 5.5 <noreply@anthropic.com>