Circuit breaker: model per-host state as (failures, restrain-until) and derive the rest — removes the stateTtl footgun (design note, targets next)

[nyanrus]

Where

packages/fedify/src/federation/circuit-breaker.ts — the per-host record shape (CircuitBreakerKvState: state / failures / opened / halfOpened) and the stateTtl option.

Context

Not a bug report — #917 is correct and green, and this builds on it. While reviewing #916/#917 I wrote the circuit breaker out as a small timed state machine, and a few of its fields turn out to be derivable. Filing the observation in case it's useful for a future next cycle; entirely happy for it to be closed if the simplification isn't worth the churn.

The observation

Writing the decision logic (beforeSend :248, recordFailure :380) as a transition system, three things fall out:

1. opened is redundant. A circuit opens exactly at the failure that trips it, and failures are ignored while open (:387), so opened == maximum(failures) in every reachable state.
2. The record is advisory. Losing it only ever turns a hold into a send (beforeSend treats a missing record as closed, :269) — never a wrong delivery, never a drop. So its exact lifetime is a soft concern.
3. The correct TTL is derived, not free. A record can influence a decision only within max(recoveryDelay, failureWindow) of its last write (an open record must survive to opened + recoveryDelay; an accumulating one until its failures age out of the window). heldActivityTtl doesn't belong here — it governs the message-side circuitHeldSince (:258, :595), a separate persistence layer. Today stateTtl is a free knob that can be set below that floor, which is the root of the Circuit breaker never expires its per-host KV state, so a dead/unresponsive host leaks one KV entry forever #916-adjacent footgun: stateTtl < recoveryDelay silently defeats the breaker for the recoveryDelay − stateTtl gap.

Proposal (for next)

Persist only two fields per host — failures and restrainedUntil (nothing = closed); everything else (state, opened, halfOpened) is derived. In Julia-ish pseudocode:

# configuration — these are the existing CircuitBreakerOptions (defaults shown)
failureThreshold = 5           # this many failures trip the circuit
failureWindow    = Minute(10)  # failures older than this stop counting
recoveryDelay    = Minute(30)  # wait this long before allowing a probe
ttlMargin        = Second(5)   # margin for backend TTL granularity / clock skew

# per-host record — state (open/half-open), opened, halfOpened are all derived, not stored
mutable struct Circuit
    failures::Vector{Instant}                 # the failure timestamps that still count
    restrainedUntil::Union{Instant,Nothing}   # hold deliveries until this instant; nothing ⇒ closed
end

# drop failures outside the window [now-failureWindow, now], then keep at most the newest few
prune(failures, now) =
    last(failureThreshold, [f for f in failures if f ≥ now - failureWindow])

# tripped ⇔ enough failures exist AND the newest `failureThreshold` all fit in one window
tripped(failures) =
    length(failures) ≥ failureThreshold &&
    maximum(failures) - failures[end - failureThreshold + 1] ≤ failureWindow

function deliver(circuit, now)
    circuit.restrainedUntil === nothing && return :send   # not restrained ⇒ deliver (closed)
    now < circuit.restrainedUntil       && return :hold   # inside the restraint window ⇒ hold
    circuit.restrainedUntil = now + recoveryDelay         # restraint elapsed: open a fresh window…
    return :probe                                         # …and let one trial delivery through
end

function fail!(circuit, now)
    circuit.failures = prune([circuit.failures; now], now)  # record failure, drop stale ones
    if circuit.restrainedUntil !== nothing                  # was restrained ⇒ a probe just failed:
        circuit.restrainedUntil = now + recoveryDelay       #   re-arm restraint (half-open strike)
    elseif tripped(circuit.failures)                        # just crossed the threshold ⇒ open:
        circuit.restrainedUntil = now + recoveryDelay       #   restrain for recoveryDelay
    else                                                    # still under the threshold ⇒
        circuit.restrainedUntil = nothing                   #   stay closed, keep counting
    end
end

# any success wipes history and un-restrains ⇒ back to closed
success!(circuit) = (empty!(circuit.failures); circuit.restrainedUntil = nothing)

# a record can only affect a decision for max(recoveryDelay, failureWindow); +margin for clock slop
stateTtl = max(recoveryDelay, failureWindow) + ttlMargin

This reproduces the current decisions — including the half-open single-strike reopen that the failure history alone can't express: with recoveryDelay > failureWindow the tripping failures have aged out of the window by probe time (so tripped(failures) is false), yet restrainedUntil still carries the open-ness (the restrainedUntil !== nothing branch in fail!). I checked the equivalence two ways: an invariant induction (R's restrainedUntil is always ≥ the current design's, so the only divergence is a rarer, strictly-more-restrictive hold under a concurrent race — never a send where the current design holds), and a small simulation (realistic event streams agree exactly; adversarial race injection diverges ~1% of steps, all in the safe direction).

On the TTL floor. max(recoveryDelay, failureWindow) is tight only under an idealized single clock — the open record's expiry then lands exactly on the probe boundary opened + recoveryDelay. Real KV backends expire on their own clock at coarse granularity (Cloudflare KV is second-level, others differ), so the record can vanish a hair before beforeSend reaches the probe. That turns the round's would-be half-open probe into a plain send: if it fails, recordFailure sees no record and merely re-accumulates (tripped([now]) is false when failureThreshold > 1) instead of the single-strike reopen (:396). Bounded — it self-heals within failureThreshold / failureWindow — but not purely advisory. So in practice use stateTtl = max(recoveryDelay, failureWindow) + ttlMargin. Any stateTtl ≥ max(recoveryDelay, failureWindow) keeps the invariant; the current default recoveryDelay + max(failureWindow, heldActivityTtl) already sits well above it, which is why this boundary doesn't bite today.

What it buys

• The stateTtl footgun disappears by construction — the TTL is derived from recoveryDelay and failureWindow (plus a margin), not a knob that can undercut recovery.
• Smaller, more stable record — 2 fields where there are 4 today (3 only if bit-exact onStateChange fidelity is wanted under the default recoveryDelay > failureWindow; see costs) → less migration surface across state-format versions.
• Fewer transition races — fail! / success! become set operations; no state / opened / halfOpened to keep mutually consistent under CAS.
• restrainedUntil does triple duty — the restraint check (now < restrainedUntil), the probe timing (now ≥ restrainedUntil), and the held-activity retry target (the current staleAt in capHeldRetryAt, :583) are all just restrainedUntil, which the current code spreads across opened / halfOpened / a recomputed staleAt.

Costs / risks

• Behaviour-equivalent only up to the advisory race slack above — a core state-model change, so next, not a maintenance branch.
• deliver computes tripped(prune(failures, now)) (O(failureThreshold)) instead of reading state.
• Decisions are always equivalent; whether the observable stream is too hinges on recoveryDelay vs failureWindow. During a hold you can often tell open from half-open from failures alone: a reachable open record has maximum(failures) == restrainedUntil − recoveryDelay, while a half-open one always has it strictly smaller. That discriminant covers the whole hold window iff recoveryDelay ≤ failureWindow — then (failures, restrainedUntil) suffices for observation too. Under the default recoveryDelay > failureWindow (30m > 10m) the open record's failures age out mid-hold, so in the tail window the same (failures, restrainedUntil) can be either state, and the two diverge observably: onStateChange (:328 fires on open→half-open, but a stale-probe re-arm at :296 fires nothing) and the held-retry cadence (:277 vs :311). So with the shipped defaults, full observable fidelity needs one extra probing flag — (failures, restrainedUntil, probing), 3 fields vs 4. (failures, restrainedUntil) is provably the minimal decision-equivalent form (a Nerode-quotient argument — the exact failure timestamps can't be collapsed to a count); probing is needed for observable equivalence only when recoveryDelay > failureWindow. Custom failure policies prune by count, not time, so their open records never age out — they never need probing.
• Custom failure policies have no derivable time-TTL. Their pruning is count-based (slice(-100), :734), with no window, so a failure can persist unboundedly and there's no window to floor the TTL from — which is why the shipped code leaves stateTtl undefined (no TTL) for them today, an independent leak vector from Circuit breaker never expires its per-host KV state, so a dead/unresponsive host leaks one KV entry forever #916. (873ba9b doesn't cover this: its guard only fires when a stateTtl is set.) A black-box predicate's lookback can't be inferred, so something has to be user-declared — a silent default would quietly break a long-lookback predicate. This is separable from the model change and can land on its own, in two steps: (now, non-breaking) warn at normalization when a custom policy has no stateTtl, making the silent leak visible; (next, breaking) require an explicit stateTtl for custom policies (throw), which bounds the record without touching predicate semantics. Deriving it instead via a maxFailureAge would also time-bound what the predicate sees — a larger, semantics-changing move, not needed here.

Relationship to #917

The stateTtl < recoveryDelay validation landed in 873ba9b (a RangeError in normalizeCircuitBreakerOptions), so the explicit-override footgun is now guarded. This model would make that invariant structural — the TTL is derived, not validated — and extend it to custom policies, where an unset stateTtl still means no TTL today (the guard only fires when one is set), leaving the count-based-prune leak noted above.

The sweep claim-key simplification is intentionally not in #917 — @dahlia keeps the marker state machine there, correctly: its done/final retry window catches no-TTL states written by old workers mid-rolling-upgrade, and running the sweep unlocked on non-CAS stores would drop the per-key CAS guard that stops a stale migrate-write from clobbering a state change made after list(). That simplification belongs in this larger state-model work, not a maintenance fix — which is where this issue sits.

About this issue

Drafted by Shiro (Claude Opus 4.8), an AI assistant working with @nyanrus. The equivalence is my own analysis plus a simulation, not a mechanized proof — if I've misread how opened / halfOpened are meant to be used (surfaced to users somewhere, or load-bearing for a case I missed), please say so; that assumption is what the whole reduction rests on.