# Milestone 37: Upstream failover budget — deadline ownership and honest attribution Make the pool's failover loop own one deadline (admission included), revive the standby under primary saturation, and stop blaming endpoints for budget exhaustion. ## The defect (field-verified on the Pi, 0.0.11) With defaults attempt=2500 ms / total=5000 ms and two upstreams: 1. `Pool.exchangeLoopLen` (pool.zig:277) never computes remaining time. The semaphore wait consumes the total budget invisibly: under a traffic burst the primary's slots saturate, a new request queues behind them while the standby sits idle with free slots, and the outer `raceWithin(total)` cancels whatever finally runs. 155 requests died at the 5 s cap; 706 of 717 SERVFAILs came from one bursty client. On an idle pool a fast standby works today — the queue is the killer, not the timer arithmetic alone. 2. The reporting contradicts itself. `selected` is written before the attempt runs (pool.zig:348), so the query log blamed the standby on 279 rows; cancelled attempts are excluded from health (pool.zig:363), so its counters read 0/0. Nothing records "the request exhausted its own budget." Design reviewed and converged with Codex (thread of 2026-08-27). Defaults do not change. ## Sessions Three, strictly sequential: A (transport vocabulary) → B (pool) → C (handler, forward client, surfaces). --- ## Session A: transport vocabulary ### A.1 `error.BudgetExhausted` and a fourth group (src/upstream/transport.zig) - Add `BudgetExhausted` to `ExchangeError`. - Add `.budget_exhausted` to `Group`; `group()` maps the new error to it. The switch stays exhaustive with no `else` — every switch over `Group` breaks at compile time until it handles the new group, which is the point. The production switches are `pool.zig` (:353), `handler.zig` (:677, :735) and `forward_client.zig` (:124). Session A owns a mechanical placeholder arm at each (pool and forward_client: propagate the error without recording health; handler: `=> ctx.servFail()`), plus any test switches the compiler flags, so A's `zig build test` passes. B and C then own the real behavior at their sites. (health.zig has no `group` call; the doh/dot occurrences are tests, not switches.) ### A.2 Timer-origin race (src/upstream/transport.zig) `raceWithin` collapses "the expiry task won" and "the raced operation itself returned error.Timeout" into one `error.Timeout`. Add: ```zig pub const RaceOutcome = enum { completed, expired }; pub fn raceUntilTagged( io: std.Io, expiry_at: std.Io.Clock.Timestamp, outcome: *RaceOutcome, comptime f: anytype, args: anytype, ) ExchangeError!RacedPayload(f) ``` The expiry parameter is an ABSOLUTE timestamp, not a duration: a duration computed from `deadline.toDurationFromNow()` and then slept re-anchors at "now", drifting past the total deadline and misclassifying a nominally full attempt. The pool computes `expiry_at = min(now + attempt, total_deadline)` and passes the timestamp. On the expiry side the function sets `outcome.* = .expired` and returns `error.Timeout`; on completion it sets `.completed` and returns the raced result (which may itself be `error.Timeout` from the leaf — that is a completed peer timeout, not an expiry). `raceWithin` keeps its public API and behavior but delegates to the same internal harness (compute the absolute deadline, discard the outcome) — one `Select` harness in the file, not two copies. ### A.3 Acceptance - [ ] `zig build test` green. - [ ] Unit tests: tagged race distinguishes leaf `error.Timeout` (completed) from expiry (`expired`); the untagged wrapper is unchanged behavior. --- ## Session B: the pool (src/upstream/pool.zig) ### B.1 One deadline owns the loop - `Pool.exchange` establishes `deadline = std.Io.Timeout{ .duration = self.timeouts.total }.toDeadline(io)` (`.awake` clock, as the loop uses today) and passes it down. The equal-deadline outer `raceWithin` at pool.zig:267 is REMOVED — the loop owns the deadline; two timers aimed at the same instant race each other and let the outer cancellation bypass the loop's classification. No replacement watchdog (the outer race never bounded uncancelable health writes anyway; a later-firing watchdog is scope creep). - Every blocking step consumes the deadline: - Admission: through the ownership-safe protocol of B.2 — never a bare `Semaphore.wait` raced against an expiry. Racing the wait with `Select.cancelDiscard` can leak a permit: the wait may have decremented the count in the same instant the expiry wins, and the discarded success never reaches the caller's release-defer. If no time remains before admission, return `error.BudgetExhausted` without waiting. - Attempt: raced via `raceUntilTagged` against `expiry_at = min(now + timeouts.attempt, total_deadline)`. - `total < attempt` is already rejected by validate.zig; `total == attempt` (and any admission overhead) simply yields truncated attempts, handled by B.3. ### B.2 Admission without head-of-line blocking Zig 0.16's `Semaphore` has neither try-acquire nor a timed wait, so the pool gains a local admission helper mirroring the standard semaphore's own mutex/decrement/condition protocol (never by patching `../zig`, never by spinning): - `tryAcquire()` — take a permit if one is immediately available, else fail without blocking. - `acquireUntil(deadline)` — timed acquisition; returns Acquired (holding a permit), Expired (holding none), or `error.Canceled` (holding none). `std.Io.Condition` has no timed wait in 0.16, so this is a NEW pool-local primitive, specified exactly: state lives under one mutex (permit count + waiter bookkeeping); the wait itself may race `Condition.wait` against a sleep via `Select`, because a permit is only ever taken under the mutex AFTER the race resolves — a discarded wake is a lost notification, not a lost permit. To keep that lost notification from stranding another waiter, an exiting waiter that may have absorbed a signal (expiry or cancellation path) re-signals the condition before returning. Permit conservation is by construction: the decrement and the "did I win" decision happen under the same mutex. Every take is non-blocking (the mutex's `tryLock`): a lock miss reads as "no permit now", and `acquireUntil` re-enters the absolute-deadline race on each miss, so contention on the admission mutex never carries a call past its deadline (review round 2026-08-27). Tests: an expiry/acquisition tie leaves the permit count exact; a cancelled waiter holds nothing and a peer waiter still wakes; a contended admission mutex does not carry `acquireUntil` past its deadline. Loop semantics — priority means ordering among immediately admissible candidates: 1. Pass one, first sweep in priority order: `tryAcquire` on each available entry; the first immediate success is attempted. A saturated entry is skipped while another eligible entry has capacity. 2. If the attempted entry fails (peer fault), the sweep continues from the next entry, still by `tryAcquire`; previously skipped saturated entries are re-tried by `tryAcquire` on each subsequent sweep step (a slot may have freed). 3. Only when no eligible entry has an immediate permit does the loop block: `acquireUntil(remaining deadline)` on the highest-priority eligible entry. Expired → `error.BudgetExhausted`. The no-head-of-line guarantee is deliberately scoped to capacity observed during the sweep: once blocked, a lower-priority slot freeing does not wake this waiter (any-entry wakeups need multi-wait machinery this milestone does not buy). Record this bound in the admission helper's doc comment. 4. Pass two (backoff probing) keeps today's in-order probing but admits through the same helper bounded by the remaining deadline — it deliberately retains blocking, one entry at a time, because probing a backed-off entry is already a last resort. 5. The post-admission health recheck survives the refactor: after acquiring a permit by either path in pass one, re-read health against a fresh `now`; an entry that entered backoff while this task waited is released (permit returned) and the sweep resumes. The existing regression test for this recheck is retained. 6. Before any attempt starts — immediate admission included — the loop re-reads the clock; a deadline already passed returns `error.BudgetExhausted` with the permit returned and no endpoint named, so an instantly-completing leaf can never manufacture evidence after exhaustion (review round 2026-08-27). Test: an exchange whose deadline is already gone starts no attempt. 7. The compiled pool bound (`Pool.max_entries`, 64) is enforced at config validation: more than 64 ENABLED upstreams is `TooManyUpstreams` at path `upstreams`, so a valid config can never trip the pool assert (review round 2026-08-27). Tests: 64 enabled passes, 65 fails, 65 listed with 64 disabled passes. 8. Queue accounting keeps its meaning: `queued_total` and `queued_seconds_total` count only the blocking `acquireUntil` path, recorded whether it ends in acquisition, expiry or cancellation; a `tryAcquire` — hit or miss — never counts as queued. Acceptance test retained. ### B.3 Classification (uses A.2's tagged race) | Attempt outcome | Budget it ran with | Health | `selected` | Loop action | | --- | --- | --- | --- | --- | | success | any | success | this endpoint | return answer | | expiry (`expired`) | full `attempt` | failure | this endpoint | continue failover | | expiry (`expired`) | truncated | untouched | unchanged | return `error.BudgetExhausted` | | completed peer fault (incl. leaf Timeout) | any, even truncated | failure | this endpoint | continue failover | | local_resource | — | untouched | unchanged | return err (as today) | | cancellation | — | untouched | unchanged | return `error.Canceled` | | wait exhausts deadline | — | untouched | unchanged (null if nothing ran) | return `error.BudgetExhausted` | | completed attempt returns `error.BudgetExhausted` (a leaf may emit it once it exists) | any | untouched | unchanged | return it; counter increments once at the outer pool | A truncated expiry is a censored observation: the pool did not grant the configured observation interval, so it is evidence about the pool's budget, never about the peer. No minimum-attempt floor exists. ### B.4 `selected` = last attributable endpoint Assign `selected.*` only in the success row and the two health-recording failure rows above — after the attempt completes, not before it starts. This attribution rule is POOL-SPECIFIC: leaf clients (DoH, DoT, ForwardClient, test fakes) keep their write-before-attempt behavior — they have one endpoint and record no health, so "the endpoint I tried" is honest there. The `Client.exchange` doc comment in transport.zig is amended to state both contracts: implementations may write before each attempt; `Pool` documents its stricter last-attributable rule on `Pool.exchange` itself. The pool-level tests at pool.zig:748-771 (selected-before-attempt) and :910 invert into the new contract's tests; leaf-client tests are untouched. ### B.5 The counter `Pool` gains one pool-level counter, `budget_exhausted_total` (atomic u64, incremented once per exchange that returns `error.BudgetExhausted`, never per endpoint). `Pool.snapshot` returns per-entry rows and cannot carry a pool-wide number without duplicating it — so the counter is exposed through a separate getter, `Pool.budgetExhaustedTotal()`, and B owns the mechanical plumbing at every existing snapshot call site its change touches so B's own gate passes before C. No per-stage split (queue vs attempt) — fixed-cardinality stage labels are deferred until an operator needs them. ### B.6 Acceptance (the decisive regressions first) - [ ] Fast standby at shipped defaults: entry 0 stalls its full attempt budget, entry 1 answers instantly, attempt = total/2 → entry 1 answers. - [ ] Saturated primary, free standby: entry 0's slots all held by stalled exchanges, entry 1 free and fast → entry 1 answers well inside the deadline. This is the Pi reproduction; it must FAIL against the current code and pass after B.2. - [ ] Truncated expiry mutates no health, returns BudgetExhausted, leaves `selected` at the last attributable endpoint. - [ ] Truncated attempt failing with ConnectionRefused mutates health and updates `selected`. - [ ] Queue-only exhaustion → BudgetExhausted with `selected == null`. - [ ] All-backoff pass two runs under the same deadline. - [ ] External cancellation returns Canceled, counts no budget, mutates nothing. - [ ] `budget_exhausted_total` increments once per exhausted exchange. - [ ] Existing invariants hold: cancelled waiter returns its permit; two stalling upstreams cost ≤ total, not one budget each. --- ## Session C: handler, forward client, surfaces ### C.1 Handler (src/server/handler.zig) Both `transport.group` switches (:677, :735) gain `.budget_exhausted => ctx.servFail()` — SERVFAIL on the wire, same as peer faults; no rcode fits better. No per-query warn log (spam); the counter is the record. ### C.2 Forward client (src/local/forward_client.zig) One outer budget for the whole exchange: wrap UDP attempt → truncation fallback → TCP in a single tagged race against `read_timeout` (the UDP receive's internal deadline and the TCP `raceWithin` at :217 collapse into the one outer bound — remove the fresh TCP budget). Every timeout here concerns the single configured resolver, so expiry stays `error.Timeout` (peer evidence), never BudgetExhausted — the budget/peer distinction is pool policy. Test: truncated-UDP-then-stalled-TCP completes or expires within one `read_timeout`, not two. Update the doc comment on `read_timeout_ms` in the config (validate.zig:360 note and the settings description) to say it bounds the whole forward-zone exchange. The key is NOT renamed (anti-requirement). ### C.3 Surfaces - Metrics: `nxdns_upstream_budget_exhausted_total` (pool-wide) wherever `nxdns_upstream_*` counters render, read via `Pool.budgetExhaustedTotal()`. - `/api/health` is NOT changed (decision, not omission): the counter is an operator metric, it never changes health status, and adding it to the API would drag openapi.yaml, admin types, fixtures, contract samples and the admin gates into a milestone that owes them nothing. Metrics only. - docs: `docs/reference/configuration.md` — every statement describing `read_timeout_ms` as a per-read or per-attempt bound is rewritten to the whole-exchange contract; the upstream timeouts section gains one paragraph on budget semantics (deadline, truncation, attribution). The stale contract text in `src/config/model.zig` (the setting's doc comment) and the note at `src/config/validate.zig:360` are updated to match. ### C.4 Acceptance - [ ] `zig build test` and `zig build test -Dintegration` green. - [ ] Metrics test covers the new counter's rendering. - [ ] Forward-client single-budget test per C.2. --- ## File ownership - A: `src/upstream/transport.zig`, plus mechanical placeholder arms at the broken `Group` switches (`src/upstream/pool.zig`, `src/server/handler.zig`, `src/local/forward_client.zig`, test switches the compiler flags) — sequential ownership, A runs alone; C takes forward_client.zig and handler.zig over later in sequence. - B: `src/upstream/pool.zig` (+ the `Client.exchange` doc contract in transport.zig and snapshot-caller plumbing for the new getter — B runs alone after A). - C: `src/server/handler.zig`, `src/local/forward_client.zig`, the metrics rendering files, `src/config/model.zig` + `src/config/validate.zig` doc text, `docs/reference/configuration.md`. - Orchestrator: CHANGELOG.md. ## Acceptance criteria (milestone complete) - [ ] All session criteria; both suites green; `zig fmt --check` clean. - [x] The saturated-primary regression demonstrably fails on pre-milestone code and passes after. Evidence (orchestrator-run mutation check, 2026-08-27): with the pass-one `tryAcquire` sweep disabled in `Pool.admit` — restoring pre-fix head-of-line blocking — the test "a saturated primary defers to a standby that has capacity" fails with `error.BudgetExhausted` out of the blocking admission path, and four queue-accounting/attribution tests fail with it (5 failed, seed 0x91ce5df5). Sweep restored: 30/30 steps, 1892/2067 passed, 0 failed. - [ ] Changelog: failover now works under primary saturation; SERVFAILs from budget exhaustion are counted, not blamed on an upstream; for upstream-pool queries the query-log `upstream` field now names only endpoints whose outcome was recorded (may be null) — forward-zone queries keep naming their single configured resolver as before. ## Anti-requirements - No default timeout changes. - No parallel/racing fan-out to multiple upstreams. Priority failover stays, with priority defined as ordering among immediately admissible candidates (B.2) — a saturated higher-priority entry defers to an admissible lower-priority one; it does not outrank an idle standby by blocking on it. - No `/api/health` or admin changes; the counter surfaces in metrics only. - No patching of the vendored/system Zig stdlib; the admission helper is pool-local. - No rename of `upstream.read_timeout_ms` (doc fix only). - No minimum-attempt floor constant. - No per-stage split of the budget counter; no per-query budget log lines. - No watchdog replacing the removed outer race. - No fake upstream identity (e.g. "budget") in the query log; no new query-log column. - No changes to backoff policy, slot counts, or health scoring beyond the attribution rules above.