Repository navigation
feat(relay): recover a rate-limited subscription and pace reconnect replay - #129
Merged
Merged
Conversation
…y hint Groundwork for recovering a rate-limited subscription instead of dropping it. `RelayRateLimitGate` records that a *connection* is over budget rather than leaving each caller to rediscover it: one refusal slows every request behind it. Windows extend and never shrink, so a second refusal carrying a shorter hint (or none) cannot undercut the first and release everybody early. Clock and sleep are injected, on `ReconnectPolicy`'s principle. `OKReason` gains `retryAfterSeconds`, parsing the relay's canonical `retry in Ns`, and stops treating two `error:` messages as retryable: `too many subscriptions` and `mixed search` are the relay saying the request itself is the problem, and retrying either on a timer is an infinite loop that also keeps the subscription budget full. Ports block/buzz#3053; nothing consumes the gate yet.
A retryable `CLOSED` no longer drops the subscription. It stays registered and re-REQs itself under `ReconnectPolicy` backoff, and only when the retries are spent does the sink hear about it. That matters because surfacing it immediately is what made one refused channel cost a full directory refetch plus a re-REQ of every other channel — recovery that is itself a burst. A `rate-limited:` refusal also opens the session gate, so the rest of a reconnect replay waits behind the same window instead of each request rediscovering the budget. The retry delay is the longer of that window and the subscription's own backoff. Reconnect replay now goes out in batches of 8 with a 50ms pause, consulting the gate before each batch rather than once at the top — a refusal during the replay has to slow the batches still to come. Retry state is cleared by a clean EOSE, by a new socket, and on disconnect; the gate is reset on disconnect, since the budget belonged to the connection that refused. `sendRequest` is the single choke point that honours the gate, and it re-checks subscription identity and epoch after the wait, which a long window can stretch to minutes. Deviation from block/buzz#3053: upstream retries indefinitely. Hive stops at `maxClosedRetries` and surfaces, because unlike upstream it has a backstop to fall back to — the engine's directory-refresh recovery. Retrying forever would keep that from ever running.
…tion path Ten cases: gate windows (default, hint, clamp, extend, never-shrink, reset releasing waiters), hint parsing including a digit-bearing message that is not a rate limit, the two `error:` messages that are now terminal, a rate-limited CLOSED re-REQing rather than surfacing, and a subscription surfacing once its retry budget is spent. Both subscription cases were checked against the negative: with the `.retryable` branch disabled they fail on the missing re-REQ. They wait on a bounded `holds(within:)` rather than the suite's `waitUntil`, which spins forever — `.timeLimit` cannot interrupt a `Task.yield()` loop that never checks cancellation, so a regression there hangs the run instead of failing it. The first negative-control attempt hung for exactly that reason.
`sendRequest` is the one choke point every REQ passes through, which made it the obvious place to honour the gate — and the wrong one. `register(filters:sink:)` awaits it, and the engine registers channels in a serial loop, so a 300-second window would suspend registration for its full length once per channel. Subscribing to a channel would have looked like a hang. The gate stays on the paths that are already background work and already batched: the reconnect replay, per batch, and a refused subscription's retry. A newly registered subscription now sends immediately, and if the relay refuses it for budget, `handleClosed` schedules the retry behind the gate — the same outcome, without stalling the caller to get it. Caught self-reviewing the diff before the device build.
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.
What
A subscription the relay refuses with a retryable
CLOSEDno longer gets dropped. It stays registered and re-REQs itself under backoff; only once the retries are spent does the sink hear about it.That matters because surfacing it immediately is what made one refused channel cost a full directory refetch plus a re-
REQof every other channel — a recovery that is itself a burst.Ports the recovery half of block/buzz#3053. The visible-channel-first half of that PR already shipped here as #127.
Pieces
RelayRateLimitGate— session-owned window. 10s when the relay gives no hint, 300s cap, honours the relay'sretry in Ns. Windows extend but never shrink, so a second refusal carrying a shorter hint cannot release every waiter early, straight back into the budget that just refused them. Clock and sleep injected, onReconnectPolicy's principle.handleClosedacts on.retryable— per-subscription bounded backoff via the existingReconnectPolicy; arate-limited:refusal also opens the gate, and the delay is the longer of the window and this subscription's own backoff.REQs in batches of 8 with a 50ms pause, consulting the gate before each batch, so a refusal partway through slows the batches still to come.EOSE, on a new socket, and on disconnect; the gate resets on disconnect, since the budget belonged to the connection that refused.A real bug found while mapping this
OKReasonclassified everyerror:as retryable, includingerror: too many subscriptions. Retrying that on a timer is an infinite loop that also keeps the subscription budget full — precisely the failure this change exists to prevent. Now terminal, alongsideerror: mixed search.Deliberate deviations from upstream
auth-required:staysreauthThenRetry. Upstream calls it terminal because their session handles auth elsewhere; Hive already re-authenticates and retries once. Following upstream literally would be a regression.sendRequestis the one choke point everyREQpasses through and was the obvious place for it — butregisterawaits that method and the engine registers channels in a serial loop, so an open 300-second window would have suspended registration for its full length once per channel. Subscribing to a channel would have looked like a hang. The gate lives on the paths that are already background and already batched.Validation
swift test -c release)swift test -c release).retryablebranch disabled they fail on the missing re-REQ.New tests wait on a bounded helper rather than the suite's
waitUntil, which spins forever —.timeLimitcannot interrupt aTask.yield()loop that never checks cancellation, so a regression there hangs the run instead of failing it. The first negative-control attempt hung for exactly that reason.