diff --git a/CHANGELOG.md b/CHANGELOG.md index b7af898..42ee027 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to nxdns are recorded here. The format follows [Keep a Chang Sections are written by hand. Nothing here is generated from commit messages: the point of the file is to say what changed for an operator, which a commit subject rarely does. +## [0.0.21] - 2026-09-12 + +### Changed + +- **An upstream warning means the pool stopped trusting the endpoint.** A Diagnostics episode opens when an upstream reaches the health failure threshold and closes on the success that clears it, instead of one card per lost exchange. Failures during backoff raise the card's occurrence count. The episode detail and the `nxdns check` report name the concrete cause behind the classification, `SendFailed (cause BrokenPipe)`. + ## [0.0.20] - 2026-09-12 ### Fixed diff --git a/specs/milestone-3.md b/specs/milestone-3.md index 98117e4..c7c777b 100644 --- a/specs/milestone-3.md +++ b/specs/milestone-3.md @@ -810,3 +810,39 @@ The implementation matches the spec with these review-driven refinements (three - `dot_client.zig`: handshake `ReadFailed`/`WriteFailed` unwrap the stream's stored cause through `transport.mapLocal` first, and certificate-bundle `OutOfMemory` is a local resource, not a peer fault. - `transport.zig`: `Endpoint.parse` rejects userinfo/query/fragment delimiters (`@`, `?`, `#`) in the authority, and `?`/`#` in a DoH path. - `dot_client.zig` (follow-up, tls_name commit): `DotClient.init` takes a `tls_name`; it is the SNI and certificate-verification name, while the dial target stays `endpoint.host`. Empty keeps the endpoint host, which is the behavior described above. The same follow-up fixed a send bug this file had from the start, invisible until a DoT handshake first succeeded: `tls.Client.flush` only encrypts into the socket writer's buffer and never flushes it, so the query never left the process and the peer eventually closed the connection (`ReceiveFailed`/`EndOfStream`). `TlsStream.flush` now does both flushes and `exchange` calls it; a hermetic loopback test in `tls_client_integration_test.zig` covers it. + +## Addendum (2026-09-12): an upstream episode follows health, and a peer fault carries its cause + +Observed on the Pi over three days (nxdns 0.0.17 to 0.0.19): 20 `upstream.exchange` warning episodes, 17 with one occurrence, 15 resolved within ten seconds; since the 0.0.20 restart 1229 successes, 0 failures, 401 silent stale-session redials. `Pool.recordFailure` reports an episode on every failed exchange and `recordSuccess` resolves it on the next success, while `health.State` trips backoff only at `failure_threshold` consecutive failures. Two rules over one failure stream, and the Diagnostics page shows the looser one. Second gap: the episode detail is `upstream 'tls://1.1.1.1:853' failed: SendFailed`; the DoT client unwraps the concrete cause into `Transact.Failure.cause` and then returns a bare `PeerFault` error to the pool, so the cause is lost before anything records it. Codex reviewed the design (thread 01a09648) and its eight findings are folded in below. + +### 1. Health owns "failing"; the episode is a projection of its transitions + +- `health.State.recordFailure(at, fault, cfg, rand)` and `recordSuccess(at, cfg)` return an `Effect`: `{ revision: u64, state: union(enum) { clear, tripped: Fault } }`. `revision` is a per-state counter incremented by every mutation. The effect is the complete desired state of the episode after the mutation, never a transition: `tripped` when `consecutive_failures >= cfg.failure_threshold` (the predicate line 142 already uses, exposed as `tripped(cfg)`; not `available()`, which turns true when a backoff expires before recovery is proven), `clear` otherwise. The fault in `tripped` is the effective last fault the state holds after the mutation, not the incoming one: a stale failure (case 1 at `recordFailure`) never displaces a newer cause, so the episode detail cannot regress to an older one. A tagged union, not an action enum with a payload field, so a report without a fault is unrepresentable. +- The pool projects every effect onto the store: `tripped` reports, `clear` resolves. A resolve when nothing is open issues no SQL (`storage/events.zig`), so the steady state costs nothing. Because every effect carries the whole desired state, last-writer-wins by revision is correct: the pool applies effects per entry in revision order under a separate `diagnostics_mutex` with a per-entry `applied_revision`, and drops an effect whose revision is below the applied one. Codex's reordering case (a failure trips health, a newer success clears it and its resolve reaches the store first, then the delayed failure arrives) is dropped instead of opening an episode nothing will resolve; and the converse cases, where a `none` transition would have overtaken a required report or resolve, cannot occur because there is no `none`. Health mutates under `Pool.mutex` as today and the effect is computed there; the store call runs after that mutex is released. Lock order is pool mutex, then nothing; diagnostics mutex, then store mutex. No task holds the pool mutex while it takes either of the other two. +- Projection also reconciles a latched store failure for what an entry owns: a `clear` that failed to write is retried by the next `clear` the entry projects, and a `tripped` by the next failure. What no entry owns is handled by the reconciliation points below. +- A success that leaves the state tripped (a stale success behind a newer failure, health case 1) projects nothing: `recordSuccess` returns `?Effect`, null in that case. The card is already open from the failure that tripped it, a report on a success would count a successful exchange as an occurrence, and a no-op that advanced the revision would reintroduce the dropped-report hole. Such a success does not touch `applied_revision`. +- Reconciliation at the two points revision order cannot reach. (1) At boot, once the first generation is built: `store.resolveExcept(.upstream_exchange, kept = the enabled urls of the pool)` closes persisted episodes for upstreams that are gone, disabled or failed to build, and `Pool.reconcile(io)` projects every entry's current effect, which on a fresh pool resolves the rest. A re-asserted `tripped` must not count as an occurrence: the store gains `ensureOpen`, which opens the episode with the detail when none is active and otherwise leaves `occurrences` and `last_seen` untouched; `reconcile` uses it, a recorded failure keeps using `report`. (2) At every retirement of a displaced generation, both the pinned path (`Owner.release` at `refs == 0`, the point after which no exchange of it can still project) and the idle path (`Owner.replace` handing a zero-ref generation back to the caller), the same two calls run against the current generation. One owner function does the retire-and-reconcile and both paths call it, so a third retirement site cannot forget it. A removed or disabled upstream's episode is closed at the next reconciliation point; if the store had latched a failure at that moment, the next reconciliation point retries it. Before this addendum such an episode was never closed at all. `resolveExcept` is the right call for this code: the pool is the only reporter of `upstream_exchange`, unlike `configuration.load`, whose scoped rule in owner.zig stays. The previous "one exchange of lag" claim is withdrawn; the retirement point is exact. +- Occurrences are the count of applied reports. Under concurrent reordering an older report behind a newer one is dropped, so the count can undercount; the health counters on `/metrics` are exact. Stated, not fixed: exact occurrence counts would need the store to accept out-of-order increments, and nobody reads the count as a metric. +- The store folds a repeated report into the open episode (`occurrences`, `last_seen`, detail), so failures during backoff raise counts on one card. A warning card now means exactly what the pool means: it stopped trusting this endpoint. +- Boundary, stated on purpose: an endpoint failing every other exchange never trips, because every newest success clears the count, so it opens no episode and `/api/health` stays `ok`. The rolling success rate on `/metrics` shows it. Diagnostics is for episodes, not chronic rates; a second predicate for that is not added. +- No new configuration. The threshold that exists is the threshold, and it is at least 1: `health.Config` is validated where it is constructed (a comptime check on the default; if it is user-configurable, the config loader refuses 0), because a tripped state with no recorded failure has no fault to report. + +### 2. A peer fault is a value between the leaf client and the pool + +- `transport.zig` gains `Fault = struct { kind: PeerFault, cause: anyerror }`, `Outcome = union(enum) { reply: []u8, fault: Fault }`, and a leaf interface `Leaf` with `exchangeFn(ptr, io, query, response_buf) (LocalResource || Cancellation)!Outcome`. `kind` is the taxonomy as today; `cause` is the concrete unwrapped error (`BrokenPipe`, `ConnectionResetByPeer`, `TlsAlert`, `HttpConnectionClosing`, ...). No phase field: the taxonomy already names it for every kind that has one, and `TlsFailed` cannot say where it failed. No HTTP status number for `HttpStatus`; the claim is limited to concrete I/O causes. No optional metadata bag. +- `dot_client.zig` and `doh_client.zig` implement `Leaf`; a `Transact.Failure` becomes a returned `Fault` instead of a thrown error. The DoT stale-session redial is unchanged and still counted through `reuse_recoveries`, never as a fault. The retry list is not widened; with the cause recorded, a stale-session cause outside the four lifecycle errors shows in the next episode and the list grows on evidence. +- `Pool` consumes `Leaf` for its slots and keeps implementing `transport.Client` toward the handler, the forward client, and the fakes that sit on that side. The outer error set, `transport.group`, and the handler are untouched; the pool returns `last_fault.kind` as the error it returns today. The race harness stays generic: the pool races the leaf through a function whose error set adds `Timeout`; a leaf never returns `error.Timeout` itself (its own timeouts are faults), so `error.Timeout` out of the race is the harness. With `race == .expired and !truncated` the pool synthesizes `Fault{ .kind = error.Timeout, .cause = error.Timeout }` and records it: an untruncated attempt expiry is peer evidence. With `truncated` it stays `error.BudgetExhausted`, unattributed, exactly as today. +- Health stores the effective fault (kind and cause names, `error_name_capacity` sized for both). Every surface that printed the taxonomy prints one text: ` (cause )`, e.g. `SendFailed (cause BrokenPipe)`. Those surfaces are the episode detail, `upstream 'tls://1.1.1.1:853' failed: SendFailed (cause BrokenPipe)`, `Snapshot.last_error` and its one reader, the `nxdns check` FAIL line, and the pool's debug `AttemptFailure` line. `/api/health`, `/api/upstreams` and `/metrics` carry no fault text today and gain none here. +- `transport.raceUntilTagged` becomes generic over the raced function's own error set: it returns `RacedError(f) || error{ Timeout, SystemResources, Canceled }`, so the pool's attempt wrapper returns `LeafError || error{Timeout}` with no `@errorCast` and no `unreachable`; `error.Timeout` out of the race is the harness by type, which sets `race = .expired` before returning it; the loop's catch asserts that and handles it in the expiry branch only, so a regression of the harness contract traps in ReleaseSafe instead of blaming an endpoint. The compiler enforces at the race boundary that a leaf cannot throw a `PeerFault` or `BudgetExhausted`. A leaf's own timeout is a returned `Fault{ .kind = error.Timeout, .cause = }` with `race == .completed`, and a pool test proves it is recorded with the leaf's cause and never confused with the harness expiry. +- The blocklist fetcher's `last_failure` side field stays as it is; same shape, other subsystem, its own addendum. + +### Tests + +- health: one failure returns `clear`; the second returns `tripped`; a success on a tripped state returns `clear`; a stale success behind a newer failure projects nothing; a stale failure returns the newer effective fault; revisions strictly increase. +- pool: a `tripped` effect delivered after a newer `clear` effect is dropped and no episode is open afterwards; a `clear` delivered after a newer `tripped` is dropped and the episode stays open; an episode open in the store before the pool exists is resolved by boot reconciliation, one for a url the pool does not have and one for a url it has; a retired generation's late `tripped` is corrected by the reconciliation at its last release, and the idle-replace path reconciles too; a reconcile of a tripped entry with an open episode moves neither `occurrences` nor `last_seen`; a stale success while tripped projects nothing and the occurrence count does not move; one failed exchange opens no episode and the second opens one that the next success resolves; a truncated expiry records nothing and returns `BudgetExhausted`; an untruncated expiry records `Timeout (cause Timeout)`; a leaf fault of kind `Timeout` is recorded with the leaf's cause. +- clients: the DoT integration tests assert the returned `Fault` with kind and cause, not the kind alone; a DoH integration test drives `DohClient.exchange` against a loopback port that is bound but never listens and stays bound for the test's duration, so the connect is refused deterministically with no port reuse race, and asserts the cause survives to the returned `Fault`; the stub-connection helper tests keep their causes. +- surfaces: the `nxdns check` test asserts the ` (cause )` text. + +### Out of scope + +An idle timer on the DoT session, a chronic-rate predicate, the fetcher migration, and any change to the DoT retry list. diff --git a/src/app.zig b/src/app.zig index 36e3784..b444125 100644 --- a/src/app.zig +++ b/src/app.zig @@ -557,6 +557,13 @@ fn serve(r: cli.Runner, args: cli.RunArgs) !u8 { var upstreams: upstream_owner.Owner = .init(upstream_generation); defer upstreams.deinit(io); + upstreams.diagnostics = event_store; + // The store outlived the process that wrote it, so every `upstream.exchange` + // episode in it describes a pool that no longer exists. This closes the ones + // no enabled upstream of this boot can justify, and projects the health of + // the ones that remain — a fresh pool is all clear, so a warning that + // survives this boot is one this process opened. + upstreams.reconcileDiagnostics(io, upstream_generation); // ----------------------------------------------------------------------- // per-query state diff --git a/src/cli.zig b/src/cli.zig index 4659edf..7a6214a 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -978,14 +978,14 @@ fn probeUpstreams(r: Runner, cfg: model.Config) !usize { // what keeps the close off `dot` while it is still undefined. var dot_wired = false; defer if (dot_wired) dot.close(r.io); - const client: transport.Client = switch (endpoint.scheme) { + const client: transport.Leaf = switch (endpoint.scheme) { .doh => doh: { doh = doh_client.DohClient.init(&http, endpoint, &request_buf, &transfer_buf) catch { try r.out.print("FAIL upstreams[{d}] {f}: not a usable DoH url\n", .{ i, safe_url.redactQuoted(server.url) }); failures += 1; continue; }; - break :doh doh.client(); + break :doh doh.leaf(); }, .dot => dot: { dot = dot_client.DotClient.init(endpoint, server.tls_name, r.gpa, &bundle, &bundle_lock, null, .{ @@ -995,7 +995,7 @@ fn probeUpstreams(r: Runner, cfg: model.Config) !usize { .stream_write = tls_buffers[3 * chunk ..], }); dot_wired = true; - break :dot dot.client(); + break :dot dot.leaf(); }, }; @@ -1792,8 +1792,8 @@ test "a failed DoT probe reaches close through the per-iteration defer without a try testing.expectEqual(@as(usize, 2), try probeUpstreams(r, cfg)); try testing.expectEqualStrings( - "FAIL upstreams[0] 'tls://dns.example:853': ConnectFailed\n" ++ - "FAIL upstreams[1] 'tls://other.example:853': ConnectFailed\n", + "FAIL upstreams[0] 'tls://dns.example:853': ConnectFailed (cause ConnectFailed)\n" ++ + "FAIL upstreams[1] 'tls://other.example:853': ConnectFailed (cause ConnectFailed)\n", captured.out.written(), ); } diff --git a/src/server/resolver_integration_test.zig b/src/server/resolver_integration_test.zig index 60caf90..5e6403c 100644 --- a/src/server/resolver_integration_test.zig +++ b/src/server/resolver_integration_test.zig @@ -93,15 +93,21 @@ fn queryWithId(buf: *[query_bytes.len]u8, id: u16) []const u8 { /// Echoes the question and appends one A record. This is the smallest thing a /// real upstream could return that the handler forwards unchanged, so the /// assertions below check bytes that travelled the whole path. -fn answerQuery(query: []const u8, response_buf: []u8) transport.ExchangeError![]u8 { - const request = packet.parse(query) catch return error.BadResponse; - const q = packet.firstQuestion(request) orelse return error.BadResponse; +fn answerQuery(query: []const u8, response_buf: []u8) transport.Outcome { + const request = packet.parse(query) catch return peerFault(error.BadResponse); + const q = packet.firstQuestion(request) orelse return peerFault(error.BadResponse); var b = packet.ResponseBuilder.init(response_buf, request.header, q) catch - return error.ResponseTooLarge; + return peerFault(error.ResponseTooLarge); b.addAnswer(q.name, .a, .in, answer_ttl, &answer_rdata) catch - return error.ResponseTooLarge; - return b.finish(); + return peerFault(error.ResponseTooLarge); + return .{ .reply = b.finish() }; +} + +/// A fault whose cause is its own classification: these fixtures fail on +/// purpose and have no concrete cause behind the classification. +fn peerFault(kind: transport.PeerFault) transport.Outcome { + return .{ .fault = .{ .kind = kind, .cause = kind } }; } /// The healthy upstream. `calls` is atomic because the listener tasks run on @@ -114,16 +120,14 @@ const GoodUpstream = struct { io: std.Io, query: []const u8, response_buf: []u8, - selected: *?[]const u8, - ) transport.ExchangeError![]u8 { + ) transport.LeafError!transport.Outcome { _ = io; - selected.* = "fake://good-upstream"; const self: *GoodUpstream = @ptrCast(@alignCast(ptr)); _ = self.calls.fetchAdd(1, .monotonic); return answerQuery(query, response_buf); } - fn client(self: *GoodUpstream) transport.Client { + fn leaf(self: *GoodUpstream) transport.Leaf { return .{ .ptr = self, .exchangeFn = exchangeFn }; } }; @@ -140,17 +144,15 @@ const FaultyUpstream = struct { io: std.Io, query: []const u8, response_buf: []u8, - selected: *?[]const u8, - ) transport.ExchangeError![]u8 { + ) transport.LeafError!transport.Outcome { _ = io; - selected.* = "fake://faulty-upstream"; const self: *FaultyUpstream = @ptrCast(@alignCast(ptr)); const seen = self.calls.fetchAdd(1, .monotonic); - if (seen < self.fail_first) return self.fault; + if (seen < self.fail_first) return peerFault(self.fault); return answerQuery(query, response_buf); } - fn client(self: *FaultyUpstream) transport.Client { + fn leaf(self: *FaultyUpstream) transport.Leaf { return .{ .ptr = self, .exchangeFn = exchangeFn }; } }; @@ -166,10 +168,10 @@ const EntryStorage = struct { fn entry( self: *EntryStorage, url: []const u8, - upstream_client: transport.Client, + upstream_leaf: transport.Leaf, priority: i32, ) pool.Entry { - self.slots[0] = .{ .client = upstream_client }; + self.slots[0] = .{ .client = upstream_leaf }; return .{ .endpoint = transport.Endpoint.parse(url) catch unreachable, .slots = &self.slots, @@ -270,8 +272,8 @@ test "the whole resolver answers over udp and tcp and fails over to a healthy up var bad_storage: EntryStorage = .{}; var good_storage: EntryStorage = .{}; var entries = [_]pool.Entry{ - bad_storage.entry("https://bad.example/dns-query", bad.client(), 10), - good_storage.entry("tls://good.example", good.client(), 20), + bad_storage.entry("https://bad.example/dns-query", bad.leaf(), 10), + good_storage.entry("tls://good.example", good.leaf(), 20), }; var upstreams: pool.Pool = .init(&entries, test_cfg, pool_timeouts, 1); diff --git a/src/storage/events.zig b/src/storage/events.zig index 2617473..a1a9af5 100644 --- a/src/storage/events.zig +++ b/src/storage/events.zig @@ -322,6 +322,74 @@ pub const Store = struct { if (!self.active.put(code, digest, id)) self.untracked_active_count += 1; } + /// Opens the episode of `subject_key` when none is open, and restates the + /// severity and the detail of the one that is. + /// + /// The projection counterpart of `report`. A reconciliation asserts the + /// state an endpoint is in now; it is not a new observation, so it must not + /// raise `occurrences` or move `last_seen`. It does own the text: a retired + /// generation's late report can leave a stale cause on a card the live + /// generation still holds open, and this is what restores the true one. + /// Only a recorded failure calls `report`. + /// + /// The open check is a statement rather than a mirror lookup: the mirror is + /// a hint that can point at a row that is gone or already closed, and + /// `report` recovers from that through the touch it was making anyway. + /// There is no write here to learn it from, so this asks. That costs one + /// SELECT per reconciled subject, on a path that runs at boot and at a + /// generation retirement. + pub fn ensureOpen( + self: *Store, + io: std.Io, + now_s: i64, + code: Code, + subject_key: []const u8, + subject_label: []const u8, + severity: Severity, + detail: []const u8, + ) void { + var key_buf: [max_subject_key_len]u8 = undefined; + const key = canonicalKey(subject_key, &key_buf); + const label = truncate(subject_label, max_subject_label_len); + const text = truncate(detail, max_detail_len); + const digest = digestOf(key); + + self.mutex.lockUncancelable(io); + defer self.mutex.unlock(io); + + self.count(); + const existing = events_repo.selectActiveId(self.database, wire(code), key) catch |err| + return self.recordFailure(err); + + // The read is not what clears the write latch: a `resolveExcept` that + // failed a moment ago is still the last word on whether this store can + // write, and only a write of our own can answer that. + if (existing) |id| { + self.count(); + _ = events_repo.restateActive(self.database, id, severity.text(), text) catch |err| + return self.recordFailure(err); + return self.recordSuccess(); + } + + // Nothing is open, so a mirror entry claiming otherwise is stale and + // would make the insert below look like a collision. + if (self.active.find(code, digest)) |entry| self.active.remove(entry); + + self.count(); + const id = events_repo.insertActive( + self.database, + now_s, + wire(code), + key, + label, + severity.text(), + text, + ) catch |err| return self.recordFailure(err); + self.recordSuccess(); + + if (!self.active.put(code, digest, id)) self.untracked_active_count += 1; + } + /// Records that `subject_key` is working again, closing its episode if one /// is open. /// @@ -930,6 +998,38 @@ test "resolving a subject with nothing open executes no SQL at all" { try testing.expectEqual(after_resolve, store.statements); } +test "ensureOpen opens a missing episode and restates an open one without counting it" { + var fx: Fixture = .{}; + try fx.init(1000); + defer fx.deinit(); + const store = &fx.store; + + // The reconciliation path: it must be able to reassert a subject that is + // still failing without inventing an occurrence no exchange produced. + store.ensureOpen(fx.io, 1000, .upstream_exchange, "https://a.example", "a.example", .warning, "Timeout (cause Timeout)"); + try testing.expectEqual(@as(i64, 1), try fx.count("SELECT count(*) FROM operational_events WHERE resolved_at IS NULL")); + try testing.expectEqual(@as(i64, 1), try fx.count("SELECT occurrences FROM operational_events")); + try testing.expectEqual(@as(i64, 1000), try fx.count("SELECT last_seen FROM operational_events")); + + // The second call is a reconciliation of a card that is already open: the + // text and the severity are the reconciler's to state, and the counters + // belong to the exchanges that actually failed. + store.ensureOpen(fx.io, 1500, .upstream_exchange, "https://a.example", "a.example", .@"error", "SendFailed (cause BrokenPipe)"); + try testing.expectEqual(@as(i64, 1), try fx.count("SELECT count(*) FROM operational_events")); + try testing.expectEqual(@as(i64, 1), try fx.count("SELECT occurrences FROM operational_events")); + try testing.expectEqual(@as(i64, 1000), try fx.count("SELECT first_seen FROM operational_events")); + try testing.expectEqual(@as(i64, 1000), try fx.count("SELECT last_seen FROM operational_events")); + try testing.expectEqualStrings("error", try fx.text("SELECT severity FROM operational_events")); + try testing.expectEqualStrings("SendFailed (cause BrokenPipe)", try fx.text("SELECT detail FROM operational_events")); + + // And once the episode is resolved it opens a second one, like any other + // entry point. + store.resolve(fx.io, 1600, .upstream_exchange, "https://a.example"); + store.ensureOpen(fx.io, 1700, .upstream_exchange, "https://a.example", "a.example", .warning, "Timeout (cause Timeout)"); + try testing.expectEqual(@as(i64, 2), try fx.count("SELECT count(*) FROM operational_events")); + try testing.expectEqual(@as(i64, 1700), try fx.count("SELECT first_seen FROM operational_events WHERE id = 2")); +} + test "a one-shot event inserts already resolved and never enters the mirror" { var fx: Fixture = .{}; try fx.init(1000); diff --git a/src/storage/repositories/events_repo.zig b/src/storage/repositories/events_repo.zig index 3adaef1..4790adb 100644 --- a/src/storage/repositories/events_repo.zig +++ b/src/storage/repositories/events_repo.zig @@ -135,6 +135,35 @@ pub fn touchActive( return database.changes() != 0; } +const restate_sql = + \\UPDATE operational_events + \\ SET detail = ?2, + \\ severity = ?3 + \\ WHERE id = ?1 AND resolved_at IS NULL +; + +/// Restates what an open episode says without claiming it happened again: +/// `occurrences`, `first_seen` and `last_seen` keep the values the real +/// failures wrote. The severity `CASE` of `touch_sql` is deliberately absent — +/// a reconciliation asserts the current state of the subject, so it must be +/// able to lower a severity a retired generation's late report raised. +/// +/// False means the row was not there or was already resolved. +pub fn restateActive( + database: *db.Db, + id: i64, + severity: []const u8, + detail: []const u8, +) db.Error!bool { + var stmt = try database.prepare(restate_sql); + defer stmt.deinit(); + try stmt.bindInt(1, id); + try stmt.bindText(2, detail); + try stmt.bindText(3, severity); + try stmt.exec(); + return database.changes() != 0; +} + const resolve_by_id_sql = "UPDATE operational_events SET resolved_at = ?2 WHERE id = ?1 AND resolved_at IS NULL"; diff --git a/src/tests.zig b/src/tests.zig index b96d3fb..bcb427d 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -21,6 +21,7 @@ comptime { _ = @import("upstream/health.zig"); _ = @import("upstream/doh_client.zig"); _ = @import("upstream/doh_client_live_test.zig"); + _ = @import("upstream/doh_client_integration_test.zig"); _ = @import("upstream/pool.zig"); _ = @import("upstream/owner.zig"); _ = @import("upstream/dot_client.zig"); diff --git a/src/upstream/doh_client.zig b/src/upstream/doh_client.zig index d408457..28cac46 100644 --- a/src/upstream/doh_client.zig +++ b/src/upstream/doh_client.zig @@ -66,7 +66,7 @@ pub const DohClient = struct { }; } - pub fn client(self: *DohClient) transport.Client { + pub fn leaf(self: *DohClient) transport.Leaf { return .{ .ptr = self, .exchangeFn = exchangeFn }; } @@ -75,12 +75,8 @@ pub const DohClient = struct { io: std.Io, query: []const u8, response_buf: []u8, - selected: *?[]const u8, - ) transport.ExchangeError![]u8 { + ) transport.LeafError!transport.Outcome { const self: *DohClient = @ptrCast(@alignCast(ptr)); - // The endpoint outlives the client, so the borrow is safe for the whole - // query. Set before the attempt: a failure names this resolver too. - selected.* = self.endpoint.url; return self.exchange(io, query, response_buf); } @@ -96,7 +92,7 @@ pub const DohClient = struct { io: std.Io, query: []const u8, response_buf: []u8, - ) transport.ExchangeError![]u8 { + ) transport.LeafError!transport.Outcome { // `std.http.Client` carries the `std.Io` it was constructed with and // takes none per request, so the interface's `io` is unused here. It // stays in the signature because DoT and the pool need it. @@ -117,22 +113,22 @@ pub const DohClient = struct { // `Request.Headers` has no `accept` field, so this one goes in by // hand. .extra_headers = &.{.{ .name = "accept", .value = media_type }}, - }) catch |err| return mapError(err, .connect); + }) catch |err| return fault(err, .connect); defer req.deinit(); req.sendBodyComplete(self.request_buf[0..query.len]) catch |err| - return mapError(sendCause(&req, err), .send); + return fault(sendCause(&req, err), .send); // An empty redirect buffer is legal under `.not_allowed`: a redirect // is an error before the location is ever read. - var resp = req.receiveHead(&.{}) catch |err| return mapError(headCause(&req, err), .receive); + var resp = req.receiveHead(&.{}) catch |err| return fault(headCause(&req, err), .receive); - if (resp.head.status != .ok) return error.HttpStatus; + if (resp.head.status != .ok) return peerFault(error.HttpStatus, error.HttpStatus); // `head.content_type` points into memory that `resp.reader` invalidates, // so the check happens before the body stream starts. - if (!contentTypeOk(resp.head.content_type)) return error.HttpContentType; + if (!contentTypeOk(resp.head.content_type)) return peerFault(error.HttpContentType, error.HttpContentType); if (resp.head.content_length) |declared| { - if (declared > response_buf.len) return error.ResponseTooLarge; + if (declared > response_buf.len) return peerFault(error.ResponseTooLarge, error.ResponseTooLarge); } const body = resp.reader(self.transfer_buf); @@ -140,7 +136,7 @@ pub const DohClient = struct { var ended = false; while (len < response_buf.len) { const n = body.readSliceShort(response_buf[len..]) catch |err| - return mapError(bodyCause(&resp, err), .receive); + return fault(bodyCause(&resp, err), .receive); len += n; if (n == 0) { ended = true; @@ -152,12 +148,13 @@ pub const DohClient = struct { // that fits from one that was cut off. var probe: [1]u8 = undefined; const n = body.readSliceShort(&probe) catch |err| - return mapError(bodyCause(&resp, err), .receive); - if (n != 0) return error.ResponseTooLarge; + return fault(bodyCause(&resp, err), .receive); + if (n != 0) return peerFault(error.ResponseTooLarge, error.ResponseTooLarge); } - try transport.validateResponse(query, response_buf[0..len]); - return response_buf[0..len]; + transport.validateResponse(query, response_buf[0..len]) catch |err| + return peerFault(err, err); + return .{ .reply = response_buf[0..len] }; } }; @@ -177,8 +174,18 @@ const Phase = enum { connect, send, receive }; /// `Connection.getReadError` (Client.zig:392), so its record-layer members /// arrive here as themselves. Without them a decode error or a bad record MAC /// would be reported as a plain receive failure. -fn mapError(err: anyerror, phase: Phase) transport.ExchangeError { +fn fault(err: anyerror, phase: Phase) transport.LeafError!transport.Outcome { if (transport.mapLocal(err)) |local| return local; + return peerFault(kindOf(err, phase), err); +} + +/// A peer fault as an `Outcome`. Written out rather than inlined at every call +/// site so the classification and the cause cannot drift apart by a typo. +fn peerFault(kind: transport.PeerFault, cause: anyerror) transport.Outcome { + return .{ .fault = .{ .kind = kind, .cause = cause } }; +} + +fn kindOf(err: anyerror, phase: Phase) transport.PeerFault { switch (err) { error.TlsInitializationFailed, error.CertificateBundleLoadFailure, @@ -209,7 +216,7 @@ fn mapError(err: anyerror, phase: Phase) transport.ExchangeError { comptime { for (@typeInfo(std.crypto.tls.Client.ReadError).error_set.?) |member| { const value: anyerror = @field(std.crypto.tls.Client.ReadError, member.name); - if (mapError(value, .receive) != error.TlsFailed) { + if (kindOf(value, .receive) != error.TlsFailed) { @compileError("unclassified TLS read cause: " ++ member.name); } } @@ -268,7 +275,7 @@ test "init builds a uri from the endpoint url" { try testing.expectEqualStrings("dns.example", doh.endpoint.host); } -test "DohClient satisfies the transport.Client interface" { +test "DohClient satisfies the transport.Leaf interface" { var http: std.http.Client = undefined; var request_buf: [min_request_buf]u8 = undefined; var transfer_buf: [min_transfer_buf]u8 = undefined; @@ -278,7 +285,7 @@ test "DohClient satisfies the transport.Client interface" { // Instantiation is the check: the vtable is built from `exchangeFn`, so a // signature drift is a compile error here. The `std.http.Client` above is // never driven, and no exchange runs. - const c: transport.Client = doh.client(); + const c: transport.Leaf = doh.leaf(); try testing.expectEqual(@as(*anyopaque, @ptrCast(&doh)), c.ptr); try testing.expectEqual( @as(@TypeOf(c.exchangeFn), DohClient.exchangeFn), @@ -318,34 +325,46 @@ test "contentTypeOk rejects anything else" { try testing.expect(!contentTypeOk("application/dns-message-extra")); } -test "mapError maps local errors before phase errors" { - try testing.expectEqual(error.OutOfMemory, mapError(error.OutOfMemory, .connect)); - try testing.expectEqual(error.Canceled, mapError(error.Canceled, .receive)); - try testing.expectEqual(error.Unexpected, mapError(error.Unexpected, .send)); +test "a local cause stays an error instead of becoming a fault" { + try testing.expectError(error.OutOfMemory, fault(error.OutOfMemory, .connect)); + try testing.expectError(error.Canceled, fault(error.Canceled, .receive)); + try testing.expectError(error.Unexpected, fault(error.Unexpected, .send)); } -test "mapError maps the collapsed tls errors regardless of phase" { - try testing.expectEqual(error.TlsFailed, mapError(error.TlsInitializationFailed, .connect)); - try testing.expectEqual(error.TlsFailed, mapError(error.TlsInitializationFailed, .receive)); - try testing.expectEqual(error.TlsFailed, mapError(error.CertificateBundleLoadFailure, .connect)); +test "kindOf maps the collapsed tls errors regardless of phase" { + try testing.expectEqual(error.TlsFailed, kindOf(error.TlsInitializationFailed, .connect)); + try testing.expectEqual(error.TlsFailed, kindOf(error.TlsInitializationFailed, .receive)); + try testing.expectEqual(error.TlsFailed, kindOf(error.CertificateBundleLoadFailure, .connect)); } -test "mapError maps every unwrapped record-layer cause to TlsFailed" { +test "kindOf maps every unwrapped record-layer cause to TlsFailed" { // The set is the one `Connection.getReadError` can hand back, so the loop // fails the day std adds a member the switch does not name. inline for (@typeInfo(std.crypto.tls.Client.ReadError).error_set.?) |member| { try testing.expectEqual( - transport.ExchangeError.TlsFailed, - mapError(@field(std.crypto.tls.Client.ReadError, member.name), .receive), + transport.PeerFault.TlsFailed, + kindOf(@field(std.crypto.tls.Client.ReadError, member.name), .receive), ); } } -test "mapError maps remaining errors by phase" { - try testing.expectEqual(error.ConnectFailed, mapError(error.ConnectionRefused, .connect)); - try testing.expectEqual(error.SendFailed, mapError(error.WriteFailed, .send)); - try testing.expectEqual(error.ReceiveFailed, mapError(error.ReadFailed, .receive)); - try testing.expectEqual(error.ReceiveFailed, mapError(error.HttpHeadersInvalid, .receive)); +test "kindOf maps remaining errors by phase" { + try testing.expectEqual(error.ConnectFailed, kindOf(error.ConnectionRefused, .connect)); + try testing.expectEqual(error.SendFailed, kindOf(error.WriteFailed, .send)); + try testing.expectEqual(error.ReceiveFailed, kindOf(error.ReadFailed, .receive)); + try testing.expectEqual(error.ReceiveFailed, kindOf(error.HttpHeadersInvalid, .receive)); +} + +test "a fault carries the classification and the concrete cause" { + const failed = try faultOf(error.ConnectionRefused, .connect); + try testing.expectEqual(transport.PeerFault.ConnectFailed, failed.kind); + try testing.expectEqual(@as(anyerror, error.ConnectionRefused), failed.cause); + + var buf: [64]u8 = undefined; + try testing.expectEqualStrings( + "ConnectFailed (cause ConnectionRefused)", + try std.fmt.bufPrint(&buf, "{f}", .{failed}), + ); } /// Only the fields the unwrap helpers read are set. The rest of a `Connection` @@ -367,6 +386,15 @@ fn stubConnection( return connection; } +/// The fault half of `fault`, for the unwrap tests. A local cause leaves this +/// as an error, which is what those tests assert instead. +fn faultOf(err: anyerror, phase: Phase) !transport.Fault { + return switch (try fault(err, phase)) { + .reply => error.TestExpectedFault, + .fault => |f| f, + }; +} + fn stubRequest(connection: *Connection, body_err: ?std.http.Reader.BodyError) Request { var req: Request = undefined; req.connection = connection; @@ -377,59 +405,46 @@ fn stubRequest(connection: *Connection, body_err: ?std.http.Reader.BodyError) Re test "the send unwrap keeps a cancelled write out of the peer fault group" { var connection = stubConnection(null, error.Canceled); var req = stubRequest(&connection, null); - const mapped = mapError(sendCause(&req, error.WriteFailed), .send); - try testing.expectEqual(transport.ExchangeError.Canceled, mapped); - try testing.expectEqual(transport.Group.cancellation, transport.group(mapped)); + try testing.expectError(error.Canceled, fault(sendCause(&req, error.WriteFailed), .send)); } test "the send unwrap keeps a local resource write failure out of the peer fault group" { var connection = stubConnection(null, error.SystemResources); var req = stubRequest(&connection, null); - const mapped = mapError(sendCause(&req, error.WriteFailed), .send); - try testing.expectEqual(transport.ExchangeError.SystemResources, mapped); - try testing.expectEqual(transport.Group.local_resource, transport.group(mapped)); + try testing.expectError(error.SystemResources, fault(sendCause(&req, error.WriteFailed), .send)); } test "the send unwrap reports a peer side cause as a send fault" { var connection = stubConnection(null, error.ConnectionResetByPeer); var req = stubRequest(&connection, null); - try testing.expectEqual( - transport.ExchangeError.SendFailed, - mapError(sendCause(&req, error.WriteFailed), .send), - ); + const failed = try faultOf(sendCause(&req, error.WriteFailed), .send); + try testing.expectEqual(transport.PeerFault.SendFailed, failed.kind); + try testing.expectEqual(@as(anyerror, error.ConnectionResetByPeer), failed.cause); } test "the head unwrap keeps a local resource read failure out of the peer fault group" { var connection = stubConnection(error.SystemResources, null); var req = stubRequest(&connection, null); - const mapped = mapError(headCause(&req, error.ReadFailed), .receive); - try testing.expectEqual(transport.ExchangeError.SystemResources, mapped); - try testing.expectEqual(transport.Group.local_resource, transport.group(mapped)); + try testing.expectError(error.SystemResources, fault(headCause(&req, error.ReadFailed), .receive)); var canceled = stubConnection(error.Canceled, null); var canceled_req = stubRequest(&canceled, null); - try testing.expectEqual( - transport.ExchangeError.Canceled, - mapError(headCause(&canceled_req, error.ReadFailed), .receive), - ); + try testing.expectError(error.Canceled, fault(headCause(&canceled_req, error.ReadFailed), .receive)); } test "the head unwrap reports a peer side cause as a receive fault" { var connection = stubConnection(error.ConnectionResetByPeer, null); var req = stubRequest(&connection, null); - try testing.expectEqual( - transport.ExchangeError.ReceiveFailed, - mapError(headCause(&req, error.ReadFailed), .receive), - ); + const failed = try faultOf(headCause(&req, error.ReadFailed), .receive); + try testing.expectEqual(transport.PeerFault.ReceiveFailed, failed.kind); + try testing.expectEqual(@as(anyerror, error.ConnectionResetByPeer), failed.cause); } test "the body unwrap keeps a cancelled read out of the peer fault group" { var connection = stubConnection(error.Canceled, null); var req = stubRequest(&connection, null); const resp: Response = .{ .request = &req, .head = undefined }; - const mapped = mapError(bodyCause(&resp, error.ReadFailed), .receive); - try testing.expectEqual(transport.ExchangeError.Canceled, mapped); - try testing.expectEqual(transport.Group.cancellation, transport.group(mapped)); + try testing.expectError(error.Canceled, fault(bodyCause(&resp, error.ReadFailed), .receive)); } test "the body unwrap prefers an http framing fault over the connection" { @@ -440,10 +455,9 @@ test "the body unwrap prefers an http framing fault over the connection" { var req = stubRequest(&connection, error.HttpChunkTruncated); const resp: Response = .{ .request = &req, .head = undefined }; try testing.expectEqual(error.HttpChunkTruncated, bodyCause(&resp, error.ReadFailed)); - try testing.expectEqual( - transport.ExchangeError.ReceiveFailed, - mapError(bodyCause(&resp, error.ReadFailed), .receive), - ); + const failed = try faultOf(bodyCause(&resp, error.ReadFailed), .receive); + try testing.expectEqual(transport.PeerFault.ReceiveFailed, failed.kind); + try testing.expectEqual(@as(anyerror, error.HttpChunkTruncated), failed.cause); } test "the unwraps report the collapsed error when no cause was stored" { @@ -455,14 +469,13 @@ test "the unwraps report the collapsed error when no cause was stored" { try testing.expectEqual(error.ReadFailed, headCause(&req, error.ReadFailed)); try testing.expectEqual(error.ReadFailed, bodyCause(&resp, error.ReadFailed)); - try testing.expectEqual( - transport.ExchangeError.SendFailed, - mapError(sendCause(&req, error.WriteFailed), .send), - ); - try testing.expectEqual( - transport.ExchangeError.ReceiveFailed, - mapError(bodyCause(&resp, error.ReadFailed), .receive), - ); + const sent = try faultOf(sendCause(&req, error.WriteFailed), .send); + try testing.expectEqual(transport.PeerFault.SendFailed, sent.kind); + try testing.expectEqual(@as(anyerror, error.WriteFailed), sent.cause); + + const received = try faultOf(bodyCause(&resp, error.ReadFailed), .receive); + try testing.expectEqual(transport.PeerFault.ReceiveFailed, received.kind); + try testing.expectEqual(@as(anyerror, error.ReadFailed), received.cause); } test "the unwraps pass a non-collapsed error through untouched" { @@ -475,8 +488,7 @@ test "the unwraps pass a non-collapsed error through untouched" { try testing.expectEqual(error.EndOfStream, sendCause(&req, error.EndOfStream)); try testing.expectEqual(error.HttpHeadersInvalid, headCause(&req, error.HttpHeadersInvalid)); try testing.expectEqual(error.EndOfStream, bodyCause(&resp, error.EndOfStream)); - try testing.expectEqual( - transport.ExchangeError.ReceiveFailed, - mapError(headCause(&req, error.HttpHeadersInvalid), .receive), - ); + const failed = try faultOf(headCause(&req, error.HttpHeadersInvalid), .receive); + try testing.expectEqual(transport.PeerFault.ReceiveFailed, failed.kind); + try testing.expectEqual(@as(anyerror, error.HttpHeadersInvalid), failed.cause); } diff --git a/src/upstream/doh_client_integration_test.zig b/src/upstream/doh_client_integration_test.zig new file mode 100644 index 0000000..6c957f2 --- /dev/null +++ b/src/upstream/doh_client_integration_test.zig @@ -0,0 +1,78 @@ +//! Hermetic loopback test for `doh_client.zig`. +//! +//! It lives in its own file because it needs `@import("build_options")`, which +//! only exists when the compilation is driven by build.zig. `-Dintegration` +//! gates it; nothing here leaves the machine and nothing here resolves a name. +//! +//! What it covers that the stub-connection tests in `doh_client.zig` cannot: +//! those call the unwrap and classification helpers directly, so they prove the +//! helpers and not the path. This drives the whole of `DohClient.exchange` +//! against a peer that is provably not listening, and asserts that the concrete +//! cause survives the classification and reaches the returned `Fault`. Without +//! it, a future `exchange` that dropped the cause on the floor would still pass +//! every other test in this tree. + +const std = @import("std"); +const build_options = @import("build_options"); +const net = std.Io.net; + +const doh_client = @import("doh_client.zig"); +const transport = @import("transport.zig"); + +const testing = std.testing; + +/// A query for example.com A: id 0x1234, RD set, one question. +const query_bytes = + "\x12\x34\x01\x00\x00\x01\x00\x00\x00\x00\x00\x00" ++ + "\x07example\x03com\x00\x00\x01\x00\x01"; + +/// A loopback TCP port that is bound and never listening, held open for the +/// whole test. +/// +/// Bound, because an ephemeral port the kernel hands out is the only port a +/// test can be sure of; a hardcoded one could belong to something. Held rather +/// than closed, because a closed port is free for another process on this +/// machine to take between the close and the connect, and then the refusal this +/// test asserts would be a connection instead. Never listening, because a +/// connect to a bound TCP port with no accept queue is refused, which is the +/// deterministic peer failure the test needs. +fn bindDeadPort(io: std.Io) !net.Socket { + return (net.IpAddress{ .ip4 = .loopback(0) }).bind(io, .{ .mode = .stream }); +} + +test "a refused connection reaches the caller as a ConnectFailed carrying its cause" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + var threaded: std.Io.Threaded = .init(gpa, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var dead = try bindDeadPort(io); + defer dead.close(io); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "https://127.0.0.1:{d}/dns-query", .{dead.address.ip4.port}); + + var http: std.http.Client = .{ .allocator = gpa, .io = io }; + defer http.deinit(); + + var request_buf: [doh_client.min_request_buf]u8 = undefined; + var transfer_buf: [doh_client.min_transfer_buf]u8 = undefined; + var doh = try doh_client.DohClient.init(&http, try .parse(url), &request_buf, &transfer_buf); + + var response_buf: [512]u8 = undefined; + switch (try doh.exchange(io, query_bytes, &response_buf)) { + .reply => return error.TestExpectedFault, + .fault => |fault| { + try testing.expectEqual(transport.PeerFault.ConnectFailed, fault.kind); + try testing.expectEqual(@as(anyerror, error.ConnectionRefused), fault.cause); + + var text: [64]u8 = undefined; + try testing.expectEqualStrings( + "ConnectFailed (cause ConnectionRefused)", + try std.fmt.bufPrint(&text, "{f}", .{fault}), + ); + }, + } +} diff --git a/src/upstream/doh_client_live_test.zig b/src/upstream/doh_client_live_test.zig index 83867a5..2d45976 100644 --- a/src/upstream/doh_client_live_test.zig +++ b/src/upstream/doh_client_live_test.zig @@ -44,10 +44,10 @@ fn runExchange(io: std.Io, params: Params) anyerror!usize { const endpoint = try transport.Endpoint.parse("https://cloudflare-dns.com/dns-query"); var doh = try doh_client.DohClient.init(&http, endpoint, &request_buf, &transfer_buf); - var selected: ?[]const u8 = null; - const reply = try doh.client().exchange(io, query_bytes, params.response_buf, &selected); - std.debug.assert(std.mem.eql(u8, selected.?, endpoint.url)); - return reply.len; + return switch (try doh.leaf().exchange(io, query_bytes, params.response_buf)) { + .reply => |reply| reply.len, + .fault => |failed| failed.kind, + }; } fn expire(io: std.Io, duration: std.Io.Clock.Duration) std.Io.Cancelable!void { diff --git a/src/upstream/dot_client.zig b/src/upstream/dot_client.zig index 81fe4ec..65c817a 100644 --- a/src/upstream/dot_client.zig +++ b/src/upstream/dot_client.zig @@ -186,7 +186,7 @@ pub const DotClient = struct { } } - pub fn client(self: *DotClient) transport.Client { + pub fn leaf(self: *DotClient) transport.Leaf { return .{ .ptr = self, .exchangeFn = exchangeFn }; } @@ -199,12 +199,8 @@ pub const DotClient = struct { io: std.Io, query: []const u8, response_buf: []u8, - selected: *?[]const u8, - ) transport.ExchangeError![]u8 { + ) transport.LeafError!transport.Outcome { const self: *DotClient = @ptrCast(@alignCast(ptr)); - // The endpoint outlives the client, so the borrow is safe for the whole - // query. Set before the attempt: a failure names this resolver too. - selected.* = self.endpoint.url; return self.exchange(io, query, response_buf); } @@ -222,24 +218,24 @@ pub const DotClient = struct { io: std.Io, query: []const u8, response_buf: []u8, - ) transport.ExchangeError![]u8 { + ) transport.LeafError!transport.Outcome { // The length prefix is 16-bit, so a longer query cannot be framed. No // listener in this process can produce one; a caller that does gets a // local error rather than a silently truncated frame. if (query.len > transport.max_message_len) return error.BufferTooSmall; const reused = self.session != null; - if (!reused) try self.dial(io); + if (!reused) if (try self.dial(io)) |dial_fault| return .{ .fault = dial_fault }; const failure = switch (self.transact(query, response_buf)) { - .ok => |reply| return reply, + .ok => |reply| return .{ .reply = reply }, .failed => |failure| failure, }; switch (retryDecision(reused, failure.received_any, failure.cause)) { .final => { self.close(io); - return transport.mapPhase(failure.cause, failure.phase); + return .{ .fault = try transport.faultOrLocal(failure.cause, failure.phase) }; }, .retry => {}, } @@ -249,38 +245,41 @@ pub const DotClient = struct { // `reuse_recoveries` is what makes the churn visible. log.debug("{f}", .{self.diagnose(.{ .stale_session = failure.cause })}); self.close(io); - try self.dial(io); + if (try self.dial(io)) |dial_fault| return .{ .fault = dial_fault }; switch (self.transact(query, response_buf)) { .ok => |reply| { if (self.reuse_recoveries) |counter| _ = counter.fetchAdd(1, .monotonic); - return reply; + return .{ .reply = reply }; }, // The retry's outcome is the exchange's outcome: one redial, never // two. .failed => |retried| { self.close(io); - return transport.mapPhase(retried.cause, retried.phase); + return .{ .fault = try transport.faultOrLocal(retried.cause, retried.phase) }; }, } } /// Opens a session and leaves it in `self.session`, or leaves `self.session` - /// null and returns the classified failure. No partially initialized - /// session ever survives this call. - fn dial(self: *DotClient, io: std.Io) transport.ExchangeError!void { + /// null and returns the classified fault. `null` means a session is open. No + /// partially initialized session ever survives this call. + fn dial(self: *DotClient, io: std.Io) transport.LeafError!?transport.Fault { std.debug.assert(self.session == null); const address = resolveAddress(self.endpoint) catch |err| { + // A host that is not an IP literal is a config error, and + // `resolveAddress` has already folded the parse failure into the + // classification, so the cause it carries is the classification. log.warn("{f}", .{self.diagnose(.not_an_ip_literal)}); - return err; + return .{ .kind = err, .cause = err }; }; - try self.ensureBundle(io); + if (try self.ensureBundle(io)) |bundle_fault| return bundle_fault; const stream = address.connect(io, .{ .mode = .stream }) catch |err| { log.debug("{f}", .{self.diagnose(.{ .connect_failed = err })}); - return transport.mapPhase(err, error.ConnectFailed); + return try transport.faultOrLocal(err, error.ConnectFailed); }; // Emplaced before the handshake, never built beside it and copied in: @@ -314,8 +313,9 @@ pub const DotClient = struct { .verify_name = self.verify_name, .cause = cause, } })}); - return transport.mapPhase(cause, error.TlsFailed); + return try transport.faultOrLocal(cause, error.TlsFailed); }; + return null; } /// One query and one reply on the open session, with enough detail on @@ -380,17 +380,17 @@ pub const DotClient = struct { /// cancellation into `error.CertificateBundleLoadFailure`. That name cannot /// tell an `error.OutOfMemory` from a corrupt PEM file, and the first is a /// local resource failure that must not count against the upstream's - /// health. Scanning here keeps the concrete error for `transport.mapPhase`. - fn ensureBundle(self: *DotClient, io: std.Io) transport.ExchangeError!void { + /// health. Scanning here keeps the concrete error for the fault. + fn ensureBundle(self: *DotClient, io: std.Io) transport.LeafError!?transport.Fault { { try self.bundle_lock.lockShared(io); defer self.bundle_lock.unlockShared(io); - if (self.bundle.map.count() != 0) return; + if (self.bundle.map.count() != 0) return null; } try self.bundle_lock.lock(io); defer self.bundle_lock.unlock(io); - if (self.bundle.map.count() != 0) return; + if (self.bundle.map.count() != 0) return null; // A partial scan leaves entries in `map`, which the check above would // read as "already loaded". Reset so the next exchange scans again. @@ -398,8 +398,9 @@ pub const DotClient = struct { self.bundle.deinit(self.gpa); self.bundle.* = .empty; log.warn("{f}", .{self.diagnose(.{ .bundle_load_failed = err })}); - return transport.mapPhase(err, error.TlsFailed); + return try transport.faultOrLocal(err, error.TlsFailed); }; + return null; } }; @@ -642,40 +643,39 @@ fn stubStream( test "the handshake unwrap keeps a cancelled read out of the peer fault group" { var stream = stubStream(error.Canceled, null, null); - const mapped = transport.mapPhase(concreteHandshake(&stream, error.ReadFailed), error.TlsFailed); - try testing.expectEqual(transport.ExchangeError.Canceled, mapped); - try testing.expectEqual(transport.Group.cancellation, transport.group(mapped)); + try testing.expectError( + error.Canceled, + transport.faultOrLocal(concreteHandshake(&stream, error.ReadFailed), error.TlsFailed), + ); } test "the handshake unwrap keeps a local resource write failure out of the peer fault group" { var stream = stubStream(null, error.SystemResources, null); - const mapped = transport.mapPhase(concreteHandshake(&stream, error.WriteFailed), error.TlsFailed); - try testing.expectEqual(transport.ExchangeError.SystemResources, mapped); - try testing.expectEqual(transport.Group.local_resource, transport.group(mapped)); + try testing.expectError( + error.SystemResources, + transport.faultOrLocal(concreteHandshake(&stream, error.WriteFailed), error.TlsFailed), + ); } test "the handshake unwrap reports a peer side cause as a TLS fault" { var reset = stubStream(error.ConnectionResetByPeer, null, null); - try testing.expectEqual( - transport.ExchangeError.TlsFailed, - transport.mapPhase(concreteHandshake(&reset, error.ReadFailed), error.TlsFailed), - ); + const read_failed = try transport.faultOrLocal(concreteHandshake(&reset, error.ReadFailed), error.TlsFailed); + try testing.expectEqual(transport.PeerFault.TlsFailed, read_failed.kind); + try testing.expectEqual(@as(anyerror, error.ConnectionResetByPeer), read_failed.cause); var refused = stubStream(null, error.ConnectionRefused, null); - try testing.expectEqual( - transport.ExchangeError.TlsFailed, - transport.mapPhase(concreteHandshake(&refused, error.WriteFailed), error.TlsFailed), - ); + const write_failed = try transport.faultOrLocal(concreteHandshake(&refused, error.WriteFailed), error.TlsFailed); + try testing.expectEqual(transport.PeerFault.TlsFailed, write_failed.kind); + try testing.expectEqual(@as(anyerror, error.ConnectionRefused), write_failed.cause); } test "the handshake unwrap reports a TLS fault when no cause was stored" { var stream = stubStream(null, null, null); try testing.expectEqual(error.ReadFailed, concreteHandshake(&stream, error.ReadFailed)); try testing.expectEqual(error.WriteFailed, concreteHandshake(&stream, error.WriteFailed)); - try testing.expectEqual( - transport.ExchangeError.TlsFailed, - transport.mapPhase(concreteHandshake(&stream, error.ReadFailed), error.TlsFailed), - ); + const failed = try transport.faultOrLocal(concreteHandshake(&stream, error.ReadFailed), error.TlsFailed); + try testing.expectEqual(transport.PeerFault.TlsFailed, failed.kind); + try testing.expectEqual(@as(anyerror, error.ReadFailed), failed.cause); } test "the handshake unwrap passes other errors through untouched" { @@ -685,42 +685,52 @@ test "the handshake unwrap passes other errors through untouched" { concreteHandshake(&stream, error.CertificateExpired), ); try testing.expectEqual(error.Canceled, concreteHandshake(&stream, error.Canceled)); - try testing.expectEqual( - transport.ExchangeError.TlsFailed, - transport.mapPhase(concreteHandshake(&stream, error.CertificateExpired), error.TlsFailed), - ); - try testing.expectEqual( - transport.ExchangeError.Canceled, - transport.mapPhase(concreteHandshake(&stream, error.Canceled), error.TlsFailed), + const expired = try transport.faultOrLocal(concreteHandshake(&stream, error.CertificateExpired), error.TlsFailed); + try testing.expectEqual(transport.PeerFault.TlsFailed, expired.kind); + try testing.expectEqual(@as(anyerror, error.CertificateExpired), expired.cause); + try testing.expectError( + error.Canceled, + transport.faultOrLocal(concreteHandshake(&stream, error.Canceled), error.TlsFailed), ); } -/// What `exchange` would return for a failure `transact` reported. -fn mappedFailure(outcome: Transact) transport.ExchangeError { - return transport.mapPhase(outcome.failed.cause, outcome.failed.phase); +/// The fault `exchange` returns for a failure `transact` reported. A local cause leaves this as an +/// error, which is what the tests of those causes assert instead. +fn mappedFailure(outcome: Transact) transport.LeafError!transport.Fault { + return transport.faultOrLocal(outcome.failed.cause, outcome.failed.phase); } test "the send and receive unwraps prefer the stored cause" { var send = stubStream(null, error.Canceled, null); - try testing.expectEqual( - transport.ExchangeError.Canceled, - mappedFailure(sendFailed(&send, error.WriteFailed)), - ); + try testing.expectError(error.Canceled, mappedFailure(sendFailed(&send, error.WriteFailed))); // The TLS client's own error wins over the socket reader's. var receive = stubStream(error.ConnectionResetByPeer, null, error.TlsAlert); - try testing.expectEqual( - transport.ExchangeError.ReceiveFailed, - mappedFailure(receiveFailed(&receive, error.ReadFailed, false)), - ); + const received = try mappedFailure(receiveFailed(&receive, error.ReadFailed, false)); + try testing.expectEqual(transport.PeerFault.ReceiveFailed, received.kind); + try testing.expectEqual(@as(anyerror, error.TlsAlert), received.cause); var socket = stubStream(error.SystemResources, null, null); - try testing.expectEqual( - transport.ExchangeError.SystemResources, + try testing.expectError( + error.SystemResources, mappedFailure(receiveFailed(&socket, error.ReadFailed, true)), ); } +test "a transact failure becomes a fault carrying its concrete cause" { + var stream = stubStream(null, error.ConnectionResetByPeer, null); + + const sent = try mappedFailure(sendFailed(&stream, error.WriteFailed)); + try testing.expectEqual(transport.PeerFault.SendFailed, sent.kind); + try testing.expectEqual(@as(anyerror, error.ConnectionResetByPeer), sent.cause); + + var text: [64]u8 = undefined; + try testing.expectEqualStrings( + "SendFailed (cause ConnectionResetByPeer)", + try std.fmt.bufPrint(&text, "{f}", .{sent}), + ); +} + test "a send failure is always pre-first-byte, and a receive failure reports what it read" { // The retry rule reads `received_any`, so where it comes from is part of the // contract rather than an incidental field: nothing is read before the query @@ -804,14 +814,13 @@ test "a validation failure is final and its phase survives the mapping" { }; for (outcomes) |outcome| { try testing.expectEqual(RetryDecision.final, retryDecision(true, true, outcome.cause)); - try testing.expectEqual( - @as(transport.ExchangeError, outcome.phase), - mappedFailure(.{ .failed = .{ - .cause = outcome.cause, - .phase = outcome.phase, - .received_any = true, - } }), - ); + const failed = try mappedFailure(.{ .failed = .{ + .cause = outcome.cause, + .phase = outcome.phase, + .received_any = true, + } }); + try testing.expectEqual(outcome.phase, failed.kind); + try testing.expectEqual(outcome.cause, failed.cause); } } @@ -830,26 +839,21 @@ test "a CA bundle scan failure keeps local resource errors out of the peer fault transport.Group.local_resource, transport.group(transport.mapPhase(err, error.TlsFailed)), ); + try testing.expectError(err, transport.faultOrLocal(err, error.TlsFailed)); } - try testing.expectEqual( - transport.ExchangeError.Canceled, - transport.mapPhase(error.Canceled, error.TlsFailed), - ); + try testing.expectError(error.Canceled, transport.faultOrLocal(error.Canceled, error.TlsFailed)); // A missing or corrupt bundle is not this process running out of anything, - // so it stays a TLS fault. - try testing.expectEqual( - transport.ExchangeError.TlsFailed, - transport.mapPhase(error.FileNotFound, error.TlsFailed), - ); - try testing.expectEqual( - transport.ExchangeError.TlsFailed, - transport.mapPhase(error.MissingEndCertificateMarker, error.TlsFailed), - ); + // so it stays a TLS fault, and the cause names which of the two it was. + for ([_]anyerror{ error.FileNotFound, error.MissingEndCertificateMarker }) |err| { + const failed = try transport.faultOrLocal(err, error.TlsFailed); + try testing.expectEqual(transport.PeerFault.TlsFailed, failed.kind); + try testing.expectEqual(err, failed.cause); + } } -test "DotClient satisfies the Client interface" { +test "DotClient satisfies the Leaf interface" { const gpa = testing.allocator; const buffer = try gpa.alloc(u8, 4 * tls.Client.min_buffer_len); @@ -878,7 +882,7 @@ test "DotClient satisfies the Client interface" { dot.close(undefined); try testing.expect(dot.session == null); - const iface: transport.Client = dot.client(); + const iface: transport.Leaf = dot.leaf(); try testing.expectEqual(@as(*anyopaque, @ptrCast(&dot)), iface.ptr); } diff --git a/src/upstream/dot_client_integration_test.zig b/src/upstream/dot_client_integration_test.zig index b318d7f..7bbb8ba 100644 --- a/src/upstream/dot_client_integration_test.zig +++ b/src/upstream/dot_client_integration_test.zig @@ -391,12 +391,12 @@ fn twoExchangesOverOneSession(io: std.Io, fixture: *ClientFixture) anyerror!void var buf: [512]u8 = undefined; const first = try fixture.dot.exchange(io, query_bytes, &buf); - try testing.expectEqualSlices(u8, response_bytes, first); + try testing.expectEqualSlices(u8, response_bytes, first.reply); // The point of the milestone: the connection outlives the exchange. try testing.expect(fixture.dot.session != null); const second = try fixture.dot.exchange(io, query_bytes, &buf); - try testing.expectEqualSlices(u8, response_bytes, second); + try testing.expectEqualSlices(u8, response_bytes, second.reply); try testing.expect(fixture.dot.session != null); } @@ -451,7 +451,7 @@ test "a session the upstream closed is recovered by one redial and counted, not // Through a real pool entry, because the claim is about what the pool does // *not* see: an upstream reaping an idle connection must not cost it health // or raise an operational event. - var slots = [_]pool_mod.Slot{.{ .client = fixture.dot.client() }}; + var slots = [_]pool_mod.Slot{.{ .client = fixture.dot.leaf() }}; var entries = [_]pool_mod.Entry{.{ .endpoint = fixture.dot.endpoint, .slots = &slots, @@ -496,7 +496,7 @@ fn exchangeThenReadTruncatedReply(io: std.Io, fixture: *ClientFixture) anyerror! // One prefix byte arrived, so the reply had started: re-sending the query on // a fresh connection would be a second question, not a recovery. - try testing.expectError(error.ReceiveFailed, fixture.dot.exchange(io, query_bytes, &buf)); + try expectFault(error.ReceiveFailed, error.EndOfStream, try fixture.dot.exchange(io, query_bytes, &buf)); try testing.expect(fixture.dot.session == null); } @@ -532,7 +532,7 @@ fn exchangeThenReadWrongId(io: std.Io, fixture: *ClientFixture) anyerror!void { // The read succeeded; only the bytes are wrong. Nothing about that says the // connection is stale, so it is final — but the stream position after a // frame this client will not trust is unknowable, so the session goes. - try testing.expectError(error.ResponseMismatch, fixture.dot.exchange(io, query_bytes, &buf)); + try expectFault(error.ResponseMismatch, error.ResponseMismatch, try fixture.dot.exchange(io, query_bytes, &buf)); try testing.expect(fixture.dot.session == null); } @@ -571,7 +571,7 @@ fn exchangeThenFailTheRetry(io: std.Io, fixture: *ClientFixture) anyerror!void { // taken, and connected. The exchange on that fresh session then fails its // read, and the retry's outcome is the exchange's outcome — no third dial, // because a session this call dialed itself is never retried. - try testing.expectError(error.ReceiveFailed, fixture.dot.exchange(io, query_bytes, &buf)); + try expectFault(error.ReceiveFailed, error.EndOfStream, try fixture.dot.exchange(io, query_bytes, &buf)); try testing.expect(fixture.dot.session == null); // What the `nxdns check` probe loop relies on: closing a client whose @@ -609,7 +609,20 @@ test "a stale session whose retry fails is final and leaves no session behind" { try testing.expectEqual(@as(u64, 0), fixture.recoveries.load(.acquire)); } -fn oneExchange(io: std.Io, fixture: *ClientFixture) transport.ExchangeError!void { +/// A returned fault, whole: the classification the pool counts and the concrete +/// cause it records. Both are asserted, because the cause is what an operator +/// reads and only a real exchange proves it survives the wire. +fn expectFault(kind: transport.PeerFault, cause: anyerror, outcome: transport.Outcome) !void { + switch (outcome) { + .reply => return error.TestExpectedFault, + .fault => |f| { + try testing.expectEqual(kind, f.kind); + try testing.expectEqual(cause, f.cause); + }, + } +} + +fn oneExchange(io: std.Io, fixture: *ClientFixture) transport.LeafError!void { var buf: [512]u8 = undefined; _ = try fixture.dot.exchange(io, query_bytes, &buf); } diff --git a/src/upstream/dot_client_live_test.zig b/src/upstream/dot_client_live_test.zig index d955c59..bf4dafb 100644 --- a/src/upstream/dot_client_live_test.zig +++ b/src/upstream/dot_client_live_test.zig @@ -62,10 +62,10 @@ fn runExchange(io: std.Io, params: Params) anyerror!usize { // so the task that built the client is what has to close it. Without this // the socket and its TLS state outlive the test. defer client.close(io); - var selected: ?[]const u8 = null; - const reply = try client.client().exchange(io, query_bytes, params.response_buf, &selected); - std.debug.assert(std.mem.eql(u8, selected.?, endpoint.url)); - return reply.len; + return switch (try client.leaf().exchange(io, query_bytes, params.response_buf)) { + .reply => |reply| reply.len, + .fault => |failed| failed.kind, + }; } fn expire(io: std.Io, duration: std.Io.Clock.Duration) std.Io.Cancelable!void { diff --git a/src/upstream/health.zig b/src/upstream/health.zig index 9346c15..0cb7ba3 100644 --- a/src/upstream/health.zig +++ b/src/upstream/health.zig @@ -20,7 +20,7 @@ //! * Data is data. `total_successes`, `total_failures`, the window and the //! `@max` of `last_success_at` / `last_error_at` always update, however old //! `at` is. -//! * `consecutive_failures`, `backoff_until` and `last_error_buf` describe +//! * `consecutive_failures`, `backoff_until` and the last fault describe //! the present, so a newer recorded outcome overrules a stale call. //! //! `recordSuccess` clears `consecutive_failures` and `backoff_until` only when @@ -33,25 +33,67 @@ //! `State` carries no lock. The pool owns the mutex. const std = @import("std"); +const transport = @import("transport.zig"); pub const Config = struct { - /// Consecutive peer faults before the endpoint is put in backoff. + /// Consecutive peer faults before the endpoint is put in backoff. At least + /// 1: a state tripped by zero failures has no fault to name, and the + /// episode the pool opens from it could not say what went wrong. Nothing + /// builds this from user input, so the default is checked below and + /// `Pool.init` asserts what a caller passes. failure_threshold: u8 = 2, base_backoff_ms: u32 = 500, max_backoff_ms: u32 = 60_000, }; +comptime { + const default: Config = .{}; + if (default.failure_threshold < 1) @compileError("failure_threshold must be at least 1"); +} + /// Rolling success-rate window, in samples. Equal to the bit width of /// `State.window`. pub const window_len = 32; -/// Bytes kept of an `@errorName`, truncated to fit. -pub const error_name_capacity = 48; +/// Bytes kept of a rendered `transport.Fault`, truncated to fit. Sized so the +/// widest classification and the widest concrete cause name fit whole; the +/// comptime-driven test below recomputes that worst case from the std error +/// sets, so a wider name std adds fails the test run rather than silently +/// truncating an operator's only diagnostic. +pub const error_name_capacity = 64; /// The shift is capped so `base_backoff_ms << shift` cannot run away; by then /// `max_backoff_ms` has clamped the result many doublings ago. const max_shift = 20; +/// The complete desired state of this endpoint's Diagnostics episode after one +/// recorded outcome. This file decides it, because "failing" is this file's +/// predicate and nobody else's; the pool projects it onto the store. +/// +/// A state, never a transition. A transition has to be applied to whatever the +/// store holds, so two effects that reach the pool out of order can leave the +/// wrong one standing: a "nothing changed" overtaking a required report would +/// drop the report for good. A desired state makes last-writer-wins by +/// revision correct: a projection of the whole state is also what repairs a +/// card left over from a latched store write, on the next outcome. What a +/// per-pool revision cannot see — an episode that outlived a restart, or one a +/// retired generation opened — is repaired by `Pool.reconcile` instead, at boot +/// and at every retirement, not by waiting for an outcome. +pub const Effect = struct { + /// Per-state counter, incremented by every mutation. The pool applies + /// effects in this order and drops one that arrives behind a newer sibling. + revision: u64, + state: Episode, + + /// A tagged union rather than an enum beside a fault field, so an episode + /// without the fault that explains it is unrepresentable. Named `Episode` + /// rather than `State` because this file's `State` is the health record. + pub const Episode = union(enum) { + clear, + tripped: transport.Fault, + }; +}; + pub const State = struct { consecutive_failures: u32, total_successes: u64, @@ -62,6 +104,12 @@ pub const State = struct { /// Length of the `@errorName` held in `last_error_buf`, truncated to fit. last_error_len: u8, backoff_until: ?std.Io.Timestamp, + /// The effective last fault, as a value. `last_error_buf` is its rendering; + /// both are kept because the text is what every surface prints and the + /// value is what an `Effect` carries out of the mutex. + last_fault: ?transport.Fault, + /// Incremented by every mutation; see `Effect.revision`. + revision: u64, /// Bitset, 1 = success, LSB = most recent. window: u32, window_filled: u8, @@ -75,11 +123,22 @@ pub const State = struct { .last_error_buf = @splat(0), .last_error_len = 0, .backoff_until = null, + .last_fault = null, + .revision = 0, .window = 0, .window_filled = 0, }; - pub fn recordSuccess(self: *State, at: std.Io.Timestamp) void { + /// A success that is the newest outcome clears the count, and the effect + /// then says `clear`. + /// + /// `null` when the state stays tripped, which is the stale success behind a + /// newer failure (case 1 at `recordFailure`). The card that failure opened + /// is already right, and projecting `tripped` again would count a + /// successful exchange as an occurrence of the episode. A `clear`-shaped + /// no-op is not an option either: it would advance `applied_revision` past + /// a report still in flight and drop it. + pub fn recordSuccess(self: *State, at: std.Io.Timestamp, cfg: Config) ?Effect { if (self.isNewestOutcome(at)) { self.consecutive_failures = 0; self.backoff_until = null; @@ -87,6 +146,32 @@ pub const State = struct { self.total_successes += 1; self.push(1); self.last_success_at = later(self.last_success_at, at); + self.revision += 1; + if (self.tripped(cfg)) return null; + return self.effect(cfg); + } + + /// The pool's definition of "failing this endpoint": the predicate that + /// puts it in backoff. Not `available`, which turns true again the moment a + /// backoff expires, before any recovery is proven. + pub fn tripped(self: *const State, cfg: Config) bool { + return self.consecutive_failures >= cfg.failure_threshold; + } + + /// The desired episode state this endpoint is in right now, without a + /// mutation and without advancing the revision. The pool projects this at + /// the two points a recorded outcome cannot reach: the first generation's + /// boot, and the retirement of a generation whose late outcomes are now + /// impossible. + pub fn currentEffect(self: *const State, cfg: Config) Effect { + return self.effect(cfg); + } + + /// A tripped state has always recorded a failure, because the threshold is + /// at least 1, so the fault is always there to name. + fn effect(self: *const State, cfg: Config) Effect { + if (!self.tripped(cfg)) return .{ .revision = self.revision, .state = .clear }; + return .{ .revision = self.revision, .state = .{ .tripped = self.last_fault.? } }; } /// True when no recorded outcome is newer than `at`. Both timestamp fields @@ -96,9 +181,9 @@ pub const State = struct { return !newerThan(self.last_success_at, at) and !newerThan(self.last_error_at, at); } - /// `err_name` is `@errorName` of a PeerFault member. `rand` supplies - /// jitter; the caller owns the RNG so this stays pure and the test is - /// deterministic. + /// `fault` is the peer fault as a value: `transport.group` has already + /// ruled that this is the peer's doing. `rand` supplies jitter; the caller + /// owns the RNG so this stays pure and the test is deterministic. /// /// A stale failure, that is one whose `at` is older than an outcome already /// recorded, is held to the mirror image of the stale-success rule. Three @@ -119,27 +204,29 @@ pub const State = struct { pub fn recordFailure( self: *State, at: std.Io.Timestamp, - err_name: []const u8, + fault: transport.Fault, cfg: Config, rand: u32, - ) void { + ) Effect { const newer_success = newerThan(self.last_success_at, at); const newer_failure = newerThan(self.last_error_at, at); if (!newer_failure) { - const copied = @min(err_name.len, self.last_error_buf.len); - @memcpy(self.last_error_buf[0..copied], err_name[0..copied]); - self.last_error_len = @intCast(copied); + self.last_fault = fault; + var writer: std.Io.Writer = .fixed(&self.last_error_buf); + writer.print("{f}", .{fault}) catch {}; + self.last_error_len = @intCast(writer.end); } self.total_failures += 1; self.push(0); self.last_error_at = later(self.last_error_at, at); + self.revision += 1; - if (newer_success) return; + if (newer_success) return self.effect(cfg); self.consecutive_failures +|= 1; - if (self.consecutive_failures < cfg.failure_threshold) return; + if (self.consecutive_failures < cfg.failure_threshold) return self.effect(cfg); const delay_ms = backoffDelayMs(self.consecutive_failures, cfg); const half = delay_ms / 2; @@ -148,6 +235,7 @@ pub const State = struct { .nanoseconds = at.nanoseconds + @as(i96, jittered) * std.time.ns_per_ms, }; self.backoff_until = later(self.backoff_until, deadline); + return self.effect(cfg); } pub fn available(self: *const State, now: std.Io.Timestamp) bool { @@ -212,10 +300,16 @@ fn ms(count: i96) i96 { return count * std.time.ns_per_ms; } +/// A fault whose cause is its own classification: the shape an expiry has, and +/// short enough to keep the timestamp tests about timestamps. +fn peerFault(kind: transport.PeerFault) transport.Fault { + return .{ .kind = kind, .cause = kind }; +} + test "a failure below the threshold leaves the endpoint available" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(0), "Timeout", cfg, 0); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, 0); try testing.expectEqual(@as(u32, 1), state.consecutive_failures); try testing.expectEqual(@as(u64, 1), state.total_failures); @@ -226,8 +320,8 @@ test "a failure below the threshold leaves the endpoint available" { test "reaching the threshold puts the endpoint in backoff until the deadline" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(0), "Timeout", cfg, 0); - state.recordFailure(ts(0), "Timeout", cfg, 0); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, 0); // Two failures, threshold 2, shift 0: delay 500 ms, jitter 0 => 250 ms. const until = state.backoff_until.?; @@ -244,7 +338,7 @@ test "consecutive failures grow the delay and saturate at max_backoff_ms" { var previous: i96 = -1; var i: usize = 0; while (i < 40) : (i += 1) { - state.recordFailure(ts(0), "Timeout", cfg, 0); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, 0); if (state.backoff_until) |until| { try testing.expect(until.nanoseconds >= previous); previous = until.nanoseconds; @@ -262,11 +356,11 @@ test "consecutive failures grow the delay and saturate at max_backoff_ms" { test "a success resets the consecutive count, the window and the backoff" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(0), "Timeout", cfg, 0); - state.recordFailure(ts(0), "Timeout", cfg, 0); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, 0); try testing.expect(state.backoff_until != null); - state.recordSuccess(ts(ms(1))); + _ = state.recordSuccess(ts(ms(1)), cfg); try testing.expectEqual(@as(u32, 0), state.consecutive_failures); try testing.expectEqual(@as(?std.Io.Timestamp, null), state.backoff_until); try testing.expectEqual(@as(u64, 1), state.total_successes); @@ -280,8 +374,8 @@ test "jitter stays inside half the delay and the whole delay" { for ([_]u32{ 0, std.math.maxInt(u32), 1, 12345 }) |rand| { var state: State = .init; - state.recordFailure(ts(0), "Timeout", cfg, rand); - state.recordFailure(ts(0), "Timeout", cfg, rand); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, rand); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, rand); const offset = state.backoff_until.?.nanoseconds; try testing.expect(offset >= ms(delay / 2)); try testing.expect(offset <= ms(delay)); @@ -289,20 +383,21 @@ test "jitter stays inside half the delay and the whole delay" { } test "an out-of-order success does not move last_success_at backwards" { + const cfg: Config = .{}; var state: State = .init; - state.recordSuccess(ts(100)); - state.recordSuccess(ts(50)); + _ = state.recordSuccess(ts(100), cfg); + _ = state.recordSuccess(ts(50), cfg); try testing.expectEqual(@as(i96, 100), state.last_success_at.?.nanoseconds); } test "an out-of-order failure does not shorten the backoff" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); const until = state.backoff_until.?.nanoseconds; - state.recordFailure(ts(ms(50)), "Timeout", cfg, 0); + _ = state.recordFailure(ts(ms(50)), peerFault(error.Timeout), cfg, 0); try testing.expect(state.backoff_until.?.nanoseconds >= until); try testing.expectEqual(@as(i96, ms(100)), state.last_error_at.?.nanoseconds); } @@ -310,11 +405,11 @@ test "an out-of-order failure does not shorten the backoff" { test "an out-of-order success does not clear the backoff of a newer failure" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); const until = state.backoff_until.?.nanoseconds; - state.recordSuccess(ts(ms(50))); + _ = state.recordSuccess(ts(ms(50)), cfg); try testing.expectEqual(@as(i96, until), state.backoff_until.?.nanoseconds); try testing.expectEqual(@as(u32, 2), state.consecutive_failures); try testing.expectEqual(@as(u64, 1), state.total_successes); @@ -325,10 +420,10 @@ test "an out-of-order success does not clear the backoff of a newer failure" { test "a stale failure behind a newer success does not raise the consecutive count" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(ms(10)), "Timeout", cfg, 0); - state.recordSuccess(ts(ms(100))); - state.recordFailure(ts(ms(20)), "Timeout", cfg, 0); - state.recordFailure(ts(ms(30)), "Timeout", cfg, 0); + _ = state.recordFailure(ts(ms(10)), peerFault(error.Timeout), cfg, 0); + _ = state.recordSuccess(ts(ms(100)), cfg); + _ = state.recordFailure(ts(ms(20)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(30)), peerFault(error.Timeout), cfg, 0); try testing.expectEqual(@as(u32, 0), state.consecutive_failures); try testing.expectEqual(@as(?std.Io.Timestamp, null), state.backoff_until); @@ -338,21 +433,21 @@ test "a stale failure behind a newer success does not raise the consecutive coun test "a stale failure behind a newer success still counts into the totals" { const cfg: Config = .{}; var state: State = .init; - state.recordSuccess(ts(ms(100))); - state.recordFailure(ts(ms(20)), "Timeout", cfg, 0); + _ = state.recordSuccess(ts(ms(100)), cfg); + _ = state.recordFailure(ts(ms(20)), peerFault(error.Timeout), cfg, 0); try testing.expectEqual(@as(u64, 1), state.total_failures); try testing.expectEqual(@as(u8, 2), state.window_filled); try testing.expectEqual(@as(u32, 0), state.window & 1); try testing.expectEqual(@as(i96, ms(20)), state.last_error_at.?.nanoseconds); - try testing.expectEqualStrings("Timeout", state.lastError()); + try testing.expectEqualStrings("Timeout (cause Timeout)", state.lastError()); } test "a stale failure with no newer success still counts as consecutive" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); - state.recordFailure(ts(ms(50)), "Timeout", cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(50)), peerFault(error.Timeout), cfg, 0); try testing.expectEqual(@as(u32, 2), state.consecutive_failures); try testing.expect(state.backoff_until != null); @@ -362,19 +457,19 @@ test "a stale failure with no newer success still counts as consecutive" { test "a stale failure does not overwrite the error of a newer failure" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); - state.recordFailure(ts(ms(50)), "ConnectFailed", cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(50)), peerFault(error.ConnectFailed), cfg, 0); - try testing.expectEqualStrings("Timeout", state.lastError()); + try testing.expectEqualStrings("Timeout (cause Timeout)", state.lastError()); } test "the newest failure records its own error and extends the backoff" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(ms(50)), "Timeout", cfg, 0); - state.recordFailure(ts(ms(100)), "ConnectFailed", cfg, 0); + _ = state.recordFailure(ts(ms(50)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.ConnectFailed), cfg, 0); - try testing.expectEqualStrings("ConnectFailed", state.lastError()); + try testing.expectEqualStrings("ConnectFailed (cause ConnectFailed)", state.lastError()); try testing.expectEqual(@as(u32, 2), state.consecutive_failures); try testing.expectEqual(ms(100) + ms(250), state.backoff_until.?.nanoseconds); } @@ -382,11 +477,11 @@ test "the newest failure records its own error and extends the backoff" { test "the newest success clears the backoff of an older failure" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); try testing.expect(state.backoff_until != null); - state.recordSuccess(ts(ms(101))); + _ = state.recordSuccess(ts(ms(101)), cfg); try testing.expectEqual(@as(?std.Io.Timestamp, null), state.backoff_until); try testing.expectEqual(@as(u32, 0), state.consecutive_failures); } @@ -394,10 +489,10 @@ test "the newest success clears the backoff of an older failure" { test "a success at the timestamp of the newest failure clears the backoff" { const cfg: Config = .{}; var state: State = .init; - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); - state.recordFailure(ts(ms(100)), "Timeout", cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(100)), peerFault(error.Timeout), cfg, 0); - state.recordSuccess(ts(ms(100))); + _ = state.recordSuccess(ts(ms(100)), cfg); try testing.expectEqual(@as(?std.Io.Timestamp, null), state.backoff_until); try testing.expectEqual(@as(u32, 0), state.consecutive_failures); } @@ -409,37 +504,176 @@ test "successRate over a half-success window is 0.5" { var i: usize = 0; while (i < 4) : (i += 1) { - state.recordSuccess(ts(0)); - state.recordFailure(ts(0), "Timeout", cfg, 0); + _ = state.recordSuccess(ts(0), cfg); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, 0); } try testing.expectEqual(@as(u8, 8), state.window_filled); try testing.expectEqual(@as(f32, 0.5), state.successRate()); } test "successRate counts only the filled part of the window" { + const cfg: Config = .{}; var state: State = .init; - state.recordSuccess(ts(0)); + _ = state.recordSuccess(ts(0), cfg); try testing.expectEqual(@as(f32, 1.0), state.successRate()); var i: usize = 0; - while (i < window_len * 2) : (i += 1) state.recordSuccess(ts(0)); + while (i < window_len * 2) : (i += 1) _ = state.recordSuccess(ts(0), cfg); try testing.expectEqual(@as(u8, window_len), state.window_filled); try testing.expectEqual(@as(f32, 1.0), state.successRate()); } -test "lastError returns the last recorded name, truncated not overflowed" { +test "lastError renders the classification and the concrete cause" { const cfg: Config = .{}; var state: State = .init; try testing.expectEqualStrings("", state.lastError()); - state.recordFailure(ts(0), "ConnectFailed", cfg, 0); - try testing.expectEqualStrings("ConnectFailed", state.lastError()); + _ = state.recordFailure(ts(0), .{ .kind = error.SendFailed, .cause = error.BrokenPipe }, cfg, 0); + try testing.expectEqualStrings("SendFailed (cause BrokenPipe)", state.lastError()); + try testing.expectEqual(transport.PeerFault.SendFailed, state.last_fault.?.kind); - state.recordFailure(ts(0), "Timeout", cfg, 0); - try testing.expectEqualStrings("Timeout", state.lastError()); + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, 0); + try testing.expectEqualStrings("Timeout (cause Timeout)", state.lastError()); +} - const long = "A" ** 200; - state.recordFailure(ts(0), long, cfg, 0); - try testing.expectEqual(@as(usize, 48), state.lastError().len); - try testing.expectEqualStrings(long[0..48], state.lastError()); +test "lastError truncates a cause too wide for the buffer instead of overflowing" { + const cfg: Config = .{}; + var state: State = .init; + const Wide = error{AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA}; + _ = state.recordFailure( + ts(0), + .{ .kind = error.ReceiveFailed, .cause = Wide.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA }, + cfg, + 0, + ); + + const whole = "ReceiveFailed (cause " ++ "A" ** 64 ++ ")"; + try testing.expectEqual(@as(usize, error_name_capacity), state.lastError().len); + try testing.expectEqualStrings(whole[0..error_name_capacity], state.lastError()); +} + +test "error_name_capacity holds the widest classification and cause whole" { + // The widest concrete cause an upstream unwrap can produce. The three sets + // are the ones `transport.zig`'s unwraps return a member of; a wider name + // std adds fails here rather than truncating an operator's diagnostic. + const widest_cause = comptime blk: { + var widest: []const u8 = ""; + for (@typeInfo(std.crypto.tls.Client.ReadError).error_set.?) |member| { + if (member.name.len > widest.len) widest = member.name; + } + for (@typeInfo(std.http.Client.RequestError).error_set.?) |member| { + if (member.name.len > widest.len) widest = member.name; + } + for (@typeInfo(std.http.Reader.BodyError).error_set.?) |member| { + if (member.name.len > widest.len) widest = member.name; + } + break :blk widest; + }; + const widest_kind = comptime blk: { + var widest: []const u8 = ""; + for (@typeInfo(transport.PeerFault).error_set.?) |member| { + if (member.name.len > widest.len) widest = member.name; + } + break :blk widest; + }; + + try testing.expectEqualStrings("DetectingNetworkConfigurationFailed", widest_cause); + try testing.expect(widest_kind.len + " (cause ".len + widest_cause.len + ")".len <= error_name_capacity); +} + +/// The fault of a `tripped` effect, or an error when the effect says `clear`. +fn trippedFault(effect: Effect) !transport.Fault { + return switch (effect.state) { + .clear => error.TestExpectedTripped, + .tripped => |fault| fault, + }; +} + +test "one failure stays clear and the second trips the episode" { + const cfg: Config = .{}; + var state: State = .init; + + const first = state.recordFailure(ts(0), peerFault(error.ConnectFailed), cfg, 0); + try testing.expectEqual(Effect.Episode.clear, first.state); + + const second = state.recordFailure(ts(ms(1)), .{ .kind = error.SendFailed, .cause = error.BrokenPipe }, cfg, 0); + const fault = try trippedFault(second); + try testing.expectEqual(transport.PeerFault.SendFailed, fault.kind); + try testing.expectEqual(@as(anyerror, error.BrokenPipe), fault.cause); + try testing.expect(second.revision > first.revision); +} + +test "a failure while tripped stays tripped" { + const cfg: Config = .{}; + var state: State = .init; + _ = state.recordFailure(ts(0), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(1)), peerFault(error.Timeout), cfg, 0); + + const third = state.recordFailure(ts(ms(2)), peerFault(error.Timeout), cfg, 0); + _ = try trippedFault(third); +} + +test "a success on a tripped state returns clear, and so does one below the threshold" { + const cfg: Config = .{}; + var state: State = .init; + + try testing.expectEqual(Effect.Episode.clear, state.recordSuccess(ts(0), cfg).?.state); + + _ = state.recordFailure(ts(ms(1)), peerFault(error.Timeout), cfg, 0); + try testing.expectEqual(Effect.Episode.clear, state.recordSuccess(ts(ms(2)), cfg).?.state); + + _ = state.recordFailure(ts(ms(3)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(4)), peerFault(error.Timeout), cfg, 0); + try testing.expectEqual(Effect.Episode.clear, state.recordSuccess(ts(ms(5)), cfg).?.state); + try testing.expectEqual(Effect.Episode.clear, state.recordSuccess(ts(ms(6)), cfg).?.state); +} + +test "a stale success behind a newer failure projects nothing" { + const cfg: Config = .{}; + var state: State = .init; + _ = state.recordFailure(ts(ms(10)), peerFault(error.Timeout), cfg, 0); + _ = state.recordFailure(ts(ms(20)), peerFault(error.Timeout), cfg, 0); + + // The endpoint is still failing, so the card the failures opened is + // already right. Reporting it again on a success would count that success + // as an occurrence of the episode. + try testing.expectEqual(@as(?Effect, null), state.recordSuccess(ts(ms(5)), cfg)); + try testing.expect(state.tripped(cfg)); + try testing.expectEqual(@as(u64, 1), state.total_successes); +} + +test "a stale failure carries the newer effective fault, not its own" { + const cfg: Config = .{}; + var state: State = .init; + _ = state.recordFailure(ts(ms(10)), peerFault(error.Timeout), cfg, 0); + const newest = state.recordFailure( + ts(ms(20)), + .{ .kind = error.SendFailed, .cause = error.BrokenPipe }, + cfg, + 0, + ); + _ = try trippedFault(newest); + + const stale = state.recordFailure(ts(ms(15)), .{ .kind = error.ConnectFailed, .cause = error.ConnectionRefused }, cfg, 0); + const fault = try trippedFault(stale); + try testing.expectEqual(transport.PeerFault.SendFailed, fault.kind); + try testing.expectEqual(@as(anyerror, error.BrokenPipe), fault.cause); + try testing.expectEqualStrings("SendFailed (cause BrokenPipe)", state.lastError()); +} + +test "every mutation advances the revision" { + const cfg: Config = .{}; + var state: State = .init; + var previous: u64 = 0; + + // The successes here are all the newest outcome, so none of them is the + // one case that projects nothing. + for (0..8) |i| { + const effect = if (i % 3 == 0) + state.recordSuccess(ts(ms(@intCast(i))), cfg).? + else + state.recordFailure(ts(ms(@intCast(i))), peerFault(error.Timeout), cfg, 0); + try testing.expect(effect.revision > previous); + previous = effect.revision; + } } diff --git a/src/upstream/owner.zig b/src/upstream/owner.zig index b9f303e..75b086e 100644 --- a/src/upstream/owner.zig +++ b/src/upstream/owner.zig @@ -23,6 +23,7 @@ const tls = std.crypto.tls; const doh_client = @import("doh_client.zig"); const dot_client = @import("dot_client.zig"); const events = @import("../storage/events.zig"); +const events_fixture = @import("../storage/events_fixture.zig"); const health = @import("health.zig"); const model = @import("../config/model.zig"); const pool_mod = @import("pool.zig"); @@ -285,6 +286,11 @@ pub const Owner = struct { /// `old.refs == 0` comparison and the branch on its result, and `replace` /// then returns null for every input. See AGENTS.md. published: u64 = 0, + /// The Diagnostics store, for the `upstream.exchange` reconciliation at + /// boot and at every retirement. Defaulted rather than an `init` parameter, + /// for the reason `Pool.diagnostics` is: the composition root wires it + /// after the owner exists, and every unit test here runs without one. + diagnostics: ?*events.Store = null, pub fn init(live: *Generation) Owner { return .{ .live = live }; @@ -323,13 +329,84 @@ pub const Owner = struct { return copies; } + /// Drops one pin, and tears the generation down when it was retired and + /// this was its last reader. pub fn release(self: *Owner, io: std.Io, generation: *Generation) void { + if (self.drop(io, generation)) self.retireDisplaced(io, generation); + } + + /// Drops one pin and says whether that made the generation this caller's to + /// tear down. The mutex is released before the caller acts on the answer, + /// because tearing a generation down closes sockets. + fn drop(self: *Owner, io: std.Io, generation: *Generation) bool { self.mutex.lockUncancelable(io); + defer self.mutex.unlock(io); std.debug.assert(generation.refs > 0); generation.refs -= 1; - const retire_it = generation.retired and generation.refs == 0; - self.mutex.unlock(io); - if (retire_it) generation.retire(io); + return generation.retired and generation.refs == 0; + } + + /// Tears a displaced generation down and reconciles its episodes against + /// whatever is live now. + /// + /// The one retire-and-reconcile, and both paths a displaced generation can + /// leave by call it: `release` when the last reader of a retired generation + /// goes, and the publisher when `replace` found no reader at all and handed + /// the generation back. Two call sites and one rule, so the idle path + /// cannot skip the reconciliation the pinned path does — which is what it + /// did before this. + /// + /// This instant is the whole point: after it, no exchange of `generation` + /// can project anything onto the store, so whatever it left standing is + /// nobody's truth and the live generation's health is the answer. + /// + /// The loop is the pin the reconcile itself takes. Reconciling reads the + /// generation that is live now, which a concurrent `replace` can retire + /// under it, and dropping that pin lands back here. One iteration per + /// concurrent replace, and a replace is a configuration write. + pub fn retireDisplaced(self: *Owner, io: std.Io, generation: *Generation) void { + // Only the publisher's idle path and a reader's last release reach + // here, and both hold a generation the swap already displaced. + std.debug.assert(generation.retired); + std.debug.assert(generation.refs == 0); + + var target = generation; + while (true) { + target.retire(io); + // No store means nothing to reconcile, and then no pin to take. + if (self.diagnostics == null) return; + + const live = self.acquire(io); + self.reconcileDiagnostics(io, live); + if (!self.drop(io, live)) return; + target = live; + } + } + + /// Closes every `upstream.exchange` episode `generation` cannot justify, + /// and projects the health of the endpoints it can. + /// + /// `resolveExcept` rather than the scoped walk `reconcileReport` does, + /// because the two codes have different owners. Every `upstream.exchange` + /// episode in the store was opened by a pool of this process, so an active + /// one outside the kept set names an upstream that was removed, disabled or + /// failed to build, and closing it is right. `configuration.load` is shared + /// with the boot collector, which is why its rule stays scoped. + /// + /// Called at the two points revision order cannot reach: the first + /// generation at boot, and the retirement of a displaced one. + pub fn reconcileDiagnostics(self: *Owner, io: std.Io, generation: *Generation) void { + const store = self.diagnostics orelse return; + const pool = generation.pool orelse return; + + // The store refuses a kept list longer than it can canonicalize, and + // refusing is right there, so the two bounds must agree rather than one + // silently clipping the other. + comptime std.debug.assert(pool_mod.Pool.max_entries <= events.Store.max_kept_keys); + var kept: [pool_mod.Pool.max_entries][]const u8 = undefined; + const count = pool.enabledUrls(&kept); + store.resolveExcept(io, std.Io.Clock.real.now(io).toSeconds(), .upstream_exchange, kept[0..count]); + pool.reconcile(io); } /// Publishes `prepared` and retires the live generation. Infallible and @@ -534,7 +611,7 @@ const Upstreams = struct { self.doh_buf[base + doh_request_buf_len ..][0..doh_transfer_buf_len], ) catch return false; self.doh_used = index + 1; - slot.* = .{ .client = self.doh[index].client() }; + slot.* = .{ .client = self.doh[index].leaf() }; } return true; } @@ -571,7 +648,7 @@ const Upstreams = struct { }, ); self.dot_used = index + 1; - slot.* = .{ .client = self.dot[index].client() }; + slot.* = .{ .client = self.dot[index].leaf() }; } } @@ -679,6 +756,21 @@ fn buildTestGeneration( bundle: *Certificate.Bundle, bundle_lock: *std.Io.RwLock, servers: []const model.UpstreamServer, +) BuildError!*Generation { + return buildTestGenerationWith(io, gpa, http, bundle, bundle_lock, servers, null); +} + +/// The generation a reconciliation test needs: its pool projects onto the same +/// store the owner reconciles against, which is how the composition root wires +/// the two. +fn buildTestGenerationWith( + io: std.Io, + gpa: Allocator, + http: *std.http.Client, + bundle: *Certificate.Bundle, + bundle_lock: *std.Io.RwLock, + servers: []const model.UpstreamServer, + store: ?*events.Store, ) BuildError!*Generation { return build(.{ .gpa = gpa, @@ -689,6 +781,7 @@ fn buildTestGeneration( .bundle_lock = bundle_lock, .timeouts = test_timeouts, .seed = 1, + .diagnostics = store, }); } @@ -743,7 +836,7 @@ test "a replace with no reader holding the live generation retires it through th // Nobody holds G1: `replace` must hand it back, because no release will. const displaced = owner.replace(io, g2) orelse return error.ExpectedIdleGeneration; try testing.expectEqual(g1, displaced); - displaced.retire(io); + owner.retireDisplaced(io, displaced); owner.deinit(io); } @@ -918,9 +1011,177 @@ test "a metrics scrape running against the owner survives a replace under it" { var future = try io.concurrent(Scrape.run, .{ &owner, io, &started, &seen }); started.waitUncancelable(io); - if (owner.replace(io, g2)) |old| old.retire(io); + if (owner.replace(io, g2)) |old| owner.retireDisplaced(io, old); future.await(io); // Every one of the 256 scrapes read at least one intact URL. try testing.expect(seen >= 256); } + +test "boot reconciliation closes the episodes of upstreams this generation does not serve" { + var t: TestIo = .init(testing.allocator); + defer t.deinit(); + const io = t.io(); + + var http: std.http.Client = .{ .allocator = testing.allocator, .io = io }; + defer http.deinit(); + var bundle: Certificate.Bundle = .empty; + defer bundle.deinit(testing.allocator); + var bundle_lock: std.Io.RwLock = .init; + + // Two episodes outlived the process that opened them. One names the + // upstream this generation serves and one does not, and neither has a pool + // that could ever resolve it through an exchange: the first has recorded + // nothing yet, and the second no longer exists in the configuration at all. + var fx: events_fixture.Fixture = .{}; + try fx.init(io, 1000); + defer fx.deinit(); + fx.store.report(io, 1000, .upstream_exchange, "https://kept.example/dns-query", "https://kept.example", .warning, "before the restart"); + fx.store.report(io, 1000, .upstream_exchange, "https://gone.example/dns-query", "https://gone.example", .warning, "before the restart"); + + const generation = try buildTestGenerationWith(io, testing.allocator, &http, &bundle, &bundle_lock, &.{ + .{ .url = "https://kept.example/dns-query" }, + }, &fx.store); + var owner: Owner = .init(generation); + defer owner.deinit(io); + owner.diagnostics = &fx.store; + + owner.reconcileDiagnostics(io, generation); + + try testing.expectEqual( + @as(i64, 0), + try fx.count("SELECT count(*) FROM operational_events WHERE resolved_at IS NULL"), + ); +} + +test "an idle replace that drops an upstream closes its episode through retireDisplaced" { + var t: TestIo = .init(testing.allocator); + defer t.deinit(); + const io = t.io(); + + var http: std.http.Client = .{ .allocator = testing.allocator, .io = io }; + defer http.deinit(); + var bundle: Certificate.Bundle = .empty; + defer bundle.deinit(testing.allocator); + var bundle_lock: std.Io.RwLock = .init; + + var fx: events_fixture.Fixture = .{}; + try fx.init(io, 1000); + defer fx.deinit(); + + const g1 = try buildTestGenerationWith(io, testing.allocator, &http, &bundle, &bundle_lock, &.{ + .{ .url = "https://gone.example/dns-query" }, + }, &fx.store); + const g2 = try buildTestGenerationWith(io, testing.allocator, &http, &bundle, &bundle_lock, &.{ + .{ .url = "https://kept.example/dns-query" }, + }, &fx.store); + + var owner: Owner = .init(g1); + defer owner.deinit(io); + owner.diagnostics = &fx.store; + + fx.store.report(io, 1100, .upstream_exchange, "https://gone.example/dns-query", "https://gone.example", .warning, "before the reload"); + + // No reader holds G1, so `replace` hands it back for the publisher to tear + // down. That teardown is the reconciliation point for the upstream the new + // configuration no longer serves. + const displaced = owner.replace(io, g2) orelse return error.TestUnexpectedResult; + try testing.expectEqual( + @as(i64, 1), + try fx.count("SELECT count(*) FROM operational_events WHERE resolved_at IS NULL"), + ); + + owner.retireDisplaced(io, displaced); + + try testing.expectEqual( + @as(i64, 0), + try fx.count("SELECT count(*) FROM operational_events WHERE resolved_at IS NULL"), + ); +} + +test "an idle replace resolves an episode the live pool never opened" { + var t: TestIo = .init(testing.allocator); + defer t.deinit(); + const io = t.io(); + + var http: std.http.Client = .{ .allocator = testing.allocator, .io = io }; + defer http.deinit(); + var bundle: Certificate.Bundle = .empty; + defer bundle.deinit(testing.allocator); + var bundle_lock: std.Io.RwLock = .init; + + var fx: events_fixture.Fixture = .{}; + try fx.init(io, 1000); + defer fx.deinit(); + + const g1 = try buildTestGenerationWith(io, testing.allocator, &http, &bundle, &bundle_lock, &.{ + .{ .url = "https://one.example/dns-query" }, + }, &fx.store); + const g2 = try buildTestGenerationWith(io, testing.allocator, &http, &bundle, &bundle_lock, &.{ + .{ .url = "https://one.example/dns-query" }, + }, &fx.store); + + var owner: Owner = .init(g1); + defer owner.deinit(io); + owner.diagnostics = &fx.store; + + // The same upstream survives the reload, so `resolveExcept` keeps the card + // and only the fresh pool's own projection can close it. That pool has + // recorded nothing, so it projects clear. + fx.store.report(io, 1100, .upstream_exchange, "https://one.example/dns-query", "https://one.example", .warning, "opened by the old generation"); + + const displaced = owner.replace(io, g2) orelse return error.TestUnexpectedResult; + owner.retireDisplaced(io, displaced); + + try testing.expectEqual( + @as(i64, 0), + try fx.count("SELECT count(*) FROM operational_events WHERE resolved_at IS NULL"), + ); +} + +test "a retired generation's late report is corrected when its last reader leaves" { + var t: TestIo = .init(testing.allocator); + defer t.deinit(); + const io = t.io(); + + var http: std.http.Client = .{ .allocator = testing.allocator, .io = io }; + defer http.deinit(); + var bundle: Certificate.Bundle = .empty; + defer bundle.deinit(testing.allocator); + var bundle_lock: std.Io.RwLock = .init; + + var fx: events_fixture.Fixture = .{}; + try fx.init(io, 1000); + defer fx.deinit(); + + const g1 = try buildTestGenerationWith(io, testing.allocator, &http, &bundle, &bundle_lock, &.{ + .{ .url = "https://one.example/dns-query" }, + }, &fx.store); + const g2 = try buildTestGenerationWith(io, testing.allocator, &http, &bundle, &bundle_lock, &.{ + .{ .url = "https://one.example/dns-query" }, + }, &fx.store); + + var owner: Owner = .init(g1); + defer owner.deinit(io); + owner.diagnostics = &fx.store; + + // A reader pins G1 across the replace, which is what lets G1 project after + // G2 is live. Revisions are per pool, so G2 has no way to know that report + // happened; the release below is the point after which G1 can project no + // more, and reconciling there is what corrects it. + const held = owner.acquire(io); + try testing.expectEqual(@as(?*Generation, null), owner.replace(io, g2)); + + fx.store.report(io, 1100, .upstream_exchange, "https://one.example/dns-query", "https://one.example", .warning, "the retired generation's last word"); + try testing.expectEqual( + @as(i64, 1), + try fx.count("SELECT count(*) FROM operational_events WHERE resolved_at IS NULL"), + ); + + owner.release(io, held); + + try testing.expectEqual( + @as(i64, 0), + try fx.count("SELECT count(*) FROM operational_events WHERE resolved_at IS NULL"), + ); +} diff --git a/src/upstream/pool.zig b/src/upstream/pool.zig index 9dcf71b..4bd79d7 100644 --- a/src/upstream/pool.zig +++ b/src/upstream/pool.zig @@ -32,6 +32,15 @@ //! never held across an exchange, so a slow upstream cannot block a health read //! or an attempt on some other entry. //! +//! `Pool.diagnostics_mutex` guards the Diagnostics store and every entry's +//! `applied_revision`. Health decides the episode's whole desired state after +//! each outcome and returns it as a `health.Effect` computed under +//! `Pool.mutex`; the effect is projected onto the store after that mutex is +//! released, in revision order per entry. +//! Lock order: the pool mutex, then nothing. The diagnostics mutex, then the +//! store's own. No task holds the pool mutex while it takes either of the +//! other two. +//! //! An entry owns one leaf client per slot, each behind its own `Slot.busy` //! mutex, plus an `Entry.admission` carrying one permit per slot. A task takes a //! permit, then the first free slot's mutex, and holds that mutex for a whole @@ -80,13 +89,18 @@ const log = std.log.scoped(.upstream); /// not happen. const AttemptFailure = struct { endpoint: transport.Endpoint, - err: transport.ExchangeError, + fault: transport.Fault, pub fn format(self: AttemptFailure, w: *std.Io.Writer) std.Io.Writer.Error!void { - try w.print("upstream {f} failed: {t}", .{ safe_url.redactQuoted(self.endpoint.url), self.err }); + try w.print("upstream {f} failed: {f}", .{ safe_url.redactQuoted(self.endpoint.url), self.fault }); } }; +/// What an attempt that ran its whole configured interval without an answer +/// says about the peer. Nothing else was observed, so the cause is the +/// classification. +const timeout_fault: transport.Fault = .{ .kind = error.Timeout, .cause = error.Timeout }; + /// How many leaf clients the composition root builds per enabled upstream, and /// therefore how many exchanges one upstream may have in flight at once. /// @@ -204,7 +218,7 @@ pub const Admission = struct { /// One leaf client of an entry, and the lock that keeps one task inside it. pub const Slot = struct { - client: transport.Client, + client: transport.Leaf, /// Held for the whole of one attempt, so `client` is never re-entered while /// it is using its own buffers. Defaulted because `Pool.init` sorts /// `entries` by value, which may only copy unlocked mutexes, and it runs @@ -241,6 +255,11 @@ pub const Entry = struct { /// Incremented by a DoT slot client when a stale reused session was /// recovered by its single redial. Zero forever for DoH entries. reuse_recoveries: *std.atomic.Value(u64), + /// The newest health revision whose `Effect` reached the diagnostics store, + /// guarded by `Pool.diagnostics_mutex`. Effects are computed under the pool + /// mutex and applied after it is released, so two tasks can arrive here out + /// of order; the older one is dropped. + applied_revision: u64 = 0, /// Both counters, on every exit of a blocking wait for a permit. A waiter /// that burned its budget in the queue is the field failure this records, so @@ -263,28 +282,38 @@ pub const Entry = struct { /// One exchange, raced until `expiry_at` and tagged with which side won. /// -/// `race` distinguishes an expiry from the leaf returning `error.Timeout` of -/// its own, which are the same error and opposite evidence: the first is about -/// the clock the caller set, the second is about the peer. -/// -/// The leaf client reports an identity of its own, which the pool discards: the -/// entry's endpoint is the pool's own naming of the same resolver, and it is -/// what the caller was handed. A leaf writing its identity into the caller's -/// slot would let a test fake overwrite the endpoint that actually answered. +/// `error.Timeout` out of this call is always the harness: a leaf reports its +/// own timeout as a `transport.Fault`, never as an error. `race` still tags the +/// two sides, and the tag is what the loop reads. fn attemptUntil( io: std.Io, - entry_client: transport.Client, + leaf: transport.Leaf, query: []const u8, response_buf: []u8, expiry_at: std.Io.Clock.Timestamp, race: *transport.RaceOutcome, -) transport.ExchangeError![]u8 { - var leaf_selected: ?[]const u8 = null; - return transport.raceUntilTagged(io, expiry_at, race, transport.Client.exchange, .{ - entry_client, io, query, response_buf, &leaf_selected, +) AttemptError!transport.Outcome { + return transport.raceUntilTagged(io, expiry_at, race, leafExchange, .{ + leaf, io, query, response_buf, }); } +/// Everything one attempt can fail with, derived from the leaf's own error set +/// rather than declared. `error.Timeout` is the harness's expiry and nothing +/// else, because a leaf reports its own timeout as a `transport.Fault`, and a +/// `PeerFault` or an `error.BudgetExhausted` out of a leaf would not compile. +const AttemptError = transport.RaceError(leafExchange); + +/// The leaf's exchange as a plain function, for the race harness. +fn leafExchange( + leaf: transport.Leaf, + io: std.Io, + query: []const u8, + response_buf: []u8, +) transport.LeafError!transport.Outcome { + return leaf.exchange(io, query, response_buf); +} + /// A copy of one entry's health, taken under the mutex. Feeds `/metrics` and /// the `/api/health` upstream condition. pub const Snapshot = struct { @@ -341,6 +370,11 @@ pub const Pool = struct { /// store at all. Every emit here sits outside `mutex`; see /// `recordDiagnostics`. diagnostics: ?*events.Store = null, + /// Serializes the application of health effects to `diagnostics`, and + /// guards every `Entry.applied_revision`. Separate from `mutex` because the + /// store takes a lock of its own: the lock order is this mutex, then the + /// store's, and no task holds the pool mutex while it takes either. + diagnostics_mutex: std.Io.Mutex = .init, /// Exchanges that ran out of budget, counted once each by `exchange`. A /// pool-wide number rather than a per-entry one on purpose: budget /// exhaustion is this process failing to give any peer its interval, so @@ -355,6 +389,9 @@ pub const Pool = struct { /// this many. pub const max_entries = 64; + /// Which caller an effect came from; see `applyEffect`. + const EffectSource = enum { fresh, reconcile }; + pub fn init( entries: []Entry, cfg: health.Config, @@ -363,6 +400,8 @@ pub const Pool = struct { ) Pool { std.debug.assert(entries.len > 0); std.debug.assert(entries.len <= max_entries); + // A state tripped by zero failures would carry no fault to report. + std.debug.assert(cfg.failure_threshold >= 1); for (entries) |*entry| { std.debug.assert(entry.slots.len >= 1); std.debug.assert(entry.admission.permits == entry.slots.len); @@ -432,6 +471,48 @@ pub const Pool = struct { }; } + /// Projects every entry's current health onto the Diagnostics store, + /// without recording an outcome. + /// + /// Revision order reconciles what the pool itself did; it cannot reconcile + /// what happened before this pool existed or in a pool that no longer + /// does. Two callers, both in `upstream/owner.zig`: the first generation at + /// boot, where every episode in the store outlived a restart, and the + /// retirement of a displaced generation, at the instant after which no + /// exchange of it can still project anything. A fresh pool is all `clear`, + /// so both cases close what the endpoints no longer justify. + /// + /// The urls this pool does not name at all are `resolveExcept`'s business, + /// and the caller makes that call: an entry cannot project a state for an + /// upstream that was removed from the configuration. + pub fn reconcile(self: *Pool, io: std.Io) void { + if (self.diagnostics == null) return; + for (self.entries) |*entry| { + if (!entry.enabled) continue; + const effect = blk: { + self.mutex.lockUncancelable(io); + defer self.mutex.unlock(io); + break :blk entry.health.currentEffect(self.cfg); + }; + self.applyEffect(io, entry, effect, .reconcile); + } + } + + /// The urls of the enabled entries, in pool order, written into `out`; + /// returns the number written. The kept set of the caller's + /// `resolveExcept`: a disabled entry keeps no episode open, because nothing + /// will exchange against it to resolve one. + pub fn enabledUrls(self: *const Pool, out: [][]const u8) usize { + var count: usize = 0; + for (self.entries) |*entry| { + if (!entry.enabled) continue; + if (count == out.len) break; + out[count] = entry.endpoint.url; + count += 1; + } + return count; + } + /// Exchanges that ended in `error.BudgetExhausted`, pool-wide. pub fn budgetExhaustedTotal(self: *const Pool) u64 { return self.budget_exhausted_total.load(.acquire); @@ -456,7 +537,7 @@ pub const Pool = struct { deadline: std.Io.Clock.Timestamp, ) transport.ExchangeError![]u8 { const now = std.Io.Clock.awake.now(io); - var last_fault: ?transport.ExchangeError = null; + var last_fault: ?transport.Fault = null; var attempted = false; var pass: u8 = 0; @@ -520,39 +601,60 @@ pub const Pool = struct { const result = attemptUntil(io, slot.client, query, response_buf, expiry_at, &race); const completed_at = std.Io.Clock.awake.now(io); - const response = result catch |err| { - // A censored observation: the pool cut the attempt short, - // so the expiry is evidence about this budget and about no - // peer. A leaf's own `error.Timeout` is not this case — the - // race tag is what tells the two apart. - if (truncated and race == .expired) return error.BudgetExhausted; - switch (transport.group(err)) { - .peer_fault => { - log.debug("{f}", .{AttemptFailure{ .endpoint = entry.endpoint, .err = err }}); - self.recordFailure(io, entry, completed_at, err); - selected.* = entry.endpoint.url; - last_fault = err; - continue; - }, - // The next upstream would hit the same wall, and this - // says nothing about any peer, so it is never recorded. - .local_resource => return err, - .cancellation => return error.Canceled, - // The caller's budget, not this endpoint's conduct. - .budget_exhausted => return err, - } + const outcome = result catch |err| switch (err) { + // The harness's expiry, and only that: a leaf reports its + // own timeout as a fault, so no other origin exists. + error.Timeout => { + std.debug.assert(race == .expired); + // A censored observation: the pool cut the attempt + // short, so the expiry is evidence about this budget + // and about no peer. + if (truncated) return error.BudgetExhausted; + // An attempt that got its whole configured interval and + // did not answer is evidence about the peer, so it is + // recorded like any other fault. + log.debug("{f}", .{AttemptFailure{ + .endpoint = entry.endpoint, + .fault = timeout_fault, + }}); + self.recordFailure(io, entry, completed_at, timeout_fault); + selected.* = entry.endpoint.url; + last_fault = timeout_fault; + continue; + }, + error.Canceled => return error.Canceled, + // The next upstream would hit the same wall, and this says + // nothing about any peer, so it is never recorded. + error.OutOfMemory, + error.SystemResources, + error.ProcessFdQuotaExceeded, + error.SystemFdQuotaExceeded, + error.BufferTooSmall, + error.Unexpected, + => return err, }; - self.recordSuccess(io, entry, completed_at); - selected.* = entry.endpoint.url; - return response; + switch (outcome) { + .reply => |reply| { + self.recordSuccess(io, entry, completed_at); + selected.* = entry.endpoint.url; + return reply; + }, + .fault => |fault| { + log.debug("{f}", .{AttemptFailure{ .endpoint = entry.endpoint, .fault = fault }}); + self.recordFailure(io, entry, completed_at, fault); + selected.* = entry.endpoint.url; + last_fault = fault; + continue; + }, + } } if (attempted) break; } // Guaranteed non-null whenever any entry is enabled: pass two attempts // every enabled entry regardless of backoff. - if (last_fault) |err| return err; + if (last_fault) |fault| return fault.kind; return error.ConnectFailed; } @@ -659,19 +761,19 @@ pub const Pool = struct { } fn recordSuccess(self: *Pool, io: std.Io, entry: *Entry, at: std.Io.Timestamp) void { - { + const effect = blk: { // Uncancelable: this section takes no Io and never blocks on a // peer. Losing the bookkeeping for a completed exchange to a // cancellation that arrives one instruction later would corrupt // health for good. self.mutex.lockUncancelable(io); defer self.mutex.unlock(io); - entry.health.recordSuccess(at); - } + break :blk entry.health.recordSuccess(at, self.cfg); + }; // The block above closes before this line, and that ordering is the // constraint: the store takes a mutex of its own, and no task may hold // one of the two while it takes the other. - self.recordDiagnostics(io, entry, .success); + if (effect) |projected| self.applyEffect(io, entry, projected, .fresh); } fn recordFailure( @@ -679,40 +781,83 @@ pub const Pool = struct { io: std.Io, entry: *Entry, at: std.Io.Timestamp, - err: transport.ExchangeError, + fault: transport.Fault, ) void { - { + const effect = blk: { self.mutex.lockUncancelable(io); defer self.mutex.unlock(io); - entry.health.recordFailure(at, @errorName(err), self.cfg, self.rng.random().int(u32)); - } + break :blk entry.health.recordFailure(at, fault, self.cfg, self.rng.random().int(u32)); + }; // After the pool mutex is released, for the reason `recordSuccess` // states. - self.recordDiagnostics(io, entry, .{ .failure = @errorName(err) }); + self.applyEffect(io, entry, effect, .fresh); } - const Outcome = union(enum) { success, failure: []const u8 }; - - /// The store takes a mutex of its own, so this runs after the pool's is - /// released. + /// Projects one health `Effect` onto the Diagnostics store, in revision + /// order per entry. + /// + /// The effect is computed under the pool mutex and projected here, after + /// that mutex is released, so two tasks recording outcomes against one + /// entry can reach this function in either order. An effect carries the + /// whole desired state rather than a transition, so dropping the older of + /// two is correct: the newer one already says everything the store needs. + /// + /// Projecting the whole state on every outcome is also what retries a write + /// the store latched and dropped: the next outcome states the card again, + /// one exchange of lag and never a stranded card. The two cases a per-pool + /// revision cannot see at all — an episode that outlived a restart, and one + /// a now-retired generation opened — do not wait for an outcome: `reconcile` + /// settles them at boot and at every retirement. + /// + /// Occurrences count applied reports, so a report dropped behind a newer + /// effect undercounts. The health counters on `/metrics` are the exact + /// numbers; nobody reads the occurrence count as a metric. /// /// A success is the steady state of the whole program, so `resolve` is /// built to issue no SQL when nothing is open (`storage/events.zig`). - fn recordDiagnostics(self: *Pool, io: std.Io, entry: *Entry, outcome: Outcome) void { + /// + /// `source` decides what an effect at the revision already applied means. A + /// recorded outcome carries a revision of its own and a repeat is stale; a + /// reconcile carries the state's current revision and is a re-assertion of + /// the truth, so it projects at equal revision and is dropped only by a + /// strictly newer outcome. + fn applyEffect( + self: *Pool, + io: std.Io, + entry: *Entry, + effect: health.Effect, + source: EffectSource, + ) void { const store = self.diagnostics orelse return; + + self.diagnostics_mutex.lockUncancelable(io); + defer self.diagnostics_mutex.unlock(io); + const stale = switch (source) { + .fresh => effect.revision <= entry.applied_revision, + .reconcile => effect.revision < entry.applied_revision, + }; + if (stale) return; + entry.applied_revision = effect.revision; + const url = entry.endpoint.url; const now_s = std.Io.Clock.real.now(io).toSeconds(); - switch (outcome) { - .success => store.resolve(io, now_s, .upstream_exchange, url), - .failure => |name| { + switch (effect.state) { + .clear => store.resolve(io, now_s, .upstream_exchange, url), + .tripped => |fault| { var label_buf: [events.Store.max_subject_label_len]u8 = undefined; const label = std.fmt.bufPrint(&label_buf, "{f}", .{safe_url.redact(url)}) catch &label_buf; var detail_buf: [events.Store.max_detail_len]u8 = undefined; - const detail = std.fmt.bufPrint(&detail_buf, "upstream {f} failed: {s}", .{ + const detail = std.fmt.bufPrint(&detail_buf, "upstream {f} failed: {f}", .{ safe_url.redactQuoted(url), - name, + fault, }) catch &detail_buf; - store.report(io, now_s, .upstream_exchange, url, label, .warning, detail); + // A recorded failure is an occurrence; a re-assertion is not. + // `ensureOpen` leaves an already-open card's count, last_seen + // and detail exactly as the failures that opened it left them. + switch (source) { + .fresh => store.report(io, now_s, .upstream_exchange, url, label, .warning, detail), + .reconcile => store.ensureOpen(io, now_s, .upstream_exchange, url, label, .warning, detail), + } }, } } @@ -766,13 +911,16 @@ const Fake = struct { const Behavior = union(enum) { /// Copy these bytes into the caller's buffer and return them. reply: []const u8, - /// Fail with this error. - fail: transport.ExchangeError, + /// Report this peer fault. + fail: transport.Fault, + /// Fail with an error of this process's own, which is never a peer + /// fault and never recorded against health. + local: transport.LeafError, /// Sleep, then reply. Used to outrun the pool's attempt budget. slow: struct { duration: std.Io.Clock.Duration, reply: []const u8 }, /// Sleep, then fail. Holds the entry's lock long enough for a second /// task to queue on it before the failure opens a backoff window. - slow_fail: struct { duration: std.Io.Clock.Duration, err: transport.ExchangeError }, + slow_fail: struct { duration: std.Io.Clock.Duration, fault: transport.Fault }, /// Wait for the gate, then reply. A test that needs two calls provably /// in flight together cannot get that from a duration: a loaded /// scheduler can start the second call after the first one's sleep @@ -782,7 +930,7 @@ const Fake = struct { /// Wait for the gate, then fail. The failing half of `hold`, with the /// same constraint: the test releases the call once it has observed /// whatever had to be true while the call was still in flight. - hold_fail: struct { gate: *std.Io.Semaphore, err: transport.ExchangeError }, + hold_fail: struct { gate: *std.Io.Semaphore, fault: transport.Fault }, }; fn exchangeFn( @@ -790,12 +938,8 @@ const Fake = struct { io: std.Io, query: []const u8, response_buf: []u8, - selected: *?[]const u8, - ) transport.ExchangeError![]u8 { + ) transport.LeafError!transport.Outcome { _ = query; - // Deliberately not the endpoint url: the pool must report its own - // entry, so a test can tell the two apart. - selected.* = "fake://leaf"; const self: *Fake = @ptrCast(@alignCast(ptr)); _ = self.calls.fetchAdd(1, .acq_rel); // The behaviour is taken before `in_flight` rises: a test that waits @@ -817,14 +961,15 @@ const Fake = struct { _ = self.peak_in_flight.fetchMax(entrants, .acq_rel); switch (behavior) { .reply => |bytes| return copy(bytes, response_buf), - .fail => |err| return err, + .fail => |fault| return .{ .fault = fault }, + .local => |err| return err, .slow => |slow| { try slow.duration.sleep(io); return copy(slow.reply, response_buf); }, .slow_fail => |slow| { try slow.duration.sleep(io); - return slow.err; + return .{ .fault = slow.fault }; }, .hold => |held| { try held.gate.wait(io); @@ -832,22 +977,30 @@ const Fake = struct { }, .hold_fail => |held| { try held.gate.wait(io); - return held.err; + return .{ .fault = held.fault }; }, } } - fn copy(bytes: []const u8, response_buf: []u8) transport.ExchangeError![]u8 { - if (bytes.len > response_buf.len) return error.ResponseTooLarge; + fn copy(bytes: []const u8, response_buf: []u8) transport.LeafError!transport.Outcome { + if (bytes.len > response_buf.len) { + return .{ .fault = .{ .kind = error.ResponseTooLarge, .cause = error.ResponseTooLarge } }; + } @memcpy(response_buf[0..bytes.len], bytes); - return response_buf[0..bytes.len]; + return .{ .reply = response_buf[0..bytes.len] }; } - fn client(self: *Fake) transport.Client { + fn leaf(self: *Fake) transport.Leaf { return .{ .ptr = self, .exchangeFn = exchangeFn }; } }; +/// A fault whose cause is its own classification: what a test means when it +/// only cares which classification the pool recorded. +fn peerFault(kind: transport.PeerFault) transport.Fault { + return .{ .kind = kind, .cause = kind }; +} + fn testEntry(url: []const u8, fake: *Fake, priority: i32) Entry { return testEntrySlots(url, fake, priority, 1); } @@ -859,7 +1012,7 @@ fn testEntry(url: []const u8, fake: *Fake, priority: i32) Entry { fn testEntrySlots(url: []const u8, fake: *Fake, priority: i32, slot_count: usize) Entry { std.debug.assert(slot_count >= 1 and slot_count <= fake.slots.len); const slots = fake.slots[0..slot_count]; - for (slots) |*slot| slot.* = .{ .client = fake.client() }; + for (slots) |*slot| slot.* = .{ .client = fake.leaf() }; return .{ .endpoint = Endpoint.parse(url) catch unreachable, .slots = slots, @@ -888,11 +1041,11 @@ const test_timeouts: Timeouts = .{ .total = .{ .raw = .fromSeconds(30), .clock = .awake }, }; -fn expectFailureLine(expected: []const u8, url: []const u8, err: transport.ExchangeError) !void { +fn expectFailureLine(expected: []const u8, url: []const u8, fault: transport.Fault) !void { var buf: [8 * safe_url.max_len]u8 = undefined; const line = try std.fmt.bufPrint(&buf, "{f}", .{AttemptFailure{ .endpoint = try .parse(url), - .err = err, + .fault = fault, }}); try testing.expectEqualStrings(expected, line); } @@ -902,21 +1055,21 @@ test "the failed-attempt line names an upstream by a url carrying no credential" // this line ran at `debug` on every peer fault, so a debug-level operator // persisted it to the journal once per failure. try expectFailureLine( - "upstream 'https://dns.nextdns.io' failed: Timeout", + "upstream 'https://dns.nextdns.io' failed: Timeout (cause Timeout)", "https://dns.nextdns.io/abcd12", - error.Timeout, + peerFault(error.Timeout), ); try expectFailureLine( - "upstream 'https://cdn.example:8443' failed: TlsFailed", + "upstream 'https://cdn.example:8443' failed: TlsFailed (cause TlsAlert)", "https://cdn.example:8443/d/hunter2/dns-query", - error.TlsFailed, + .{ .kind = error.TlsFailed, .cause = error.TlsAlert }, ); // What it still says, because an operator reading a failover has to know // which upstream failed: the scheme, the host and the port. try expectFailureLine( - "upstream 'tls://9.9.9.9:853' failed: ConnectFailed", + "upstream 'tls://9.9.9.9:853' failed: ConnectFailed (cause ConnectionRefused)", "tls://9.9.9.9:853", - error.ConnectFailed, + .{ .kind = error.ConnectFailed, .cause = error.ConnectionRefused }, ); } @@ -1046,7 +1199,7 @@ test "a failover reports the endpoint that answered, not the first one tried" { defer threaded.deinit(); const io = threaded.io(); - var bad: Fake = .{ .behavior = .{ .fail = error.Timeout } }; + var bad: Fake = .{ .behavior = .{ .fail = peerFault(error.Timeout) } }; var good: Fake = .{ .behavior = .{ .reply = response_bytes } }; var entries = [_]Entry{ testEntry("https://bad.example/dns-query", &bad, 10), @@ -1065,8 +1218,8 @@ test "an all-failed exchange reports the last endpoint attempted" { defer threaded.deinit(); const io = threaded.io(); - var first: Fake = .{ .behavior = .{ .fail = error.ConnectFailed } }; - var last: Fake = .{ .behavior = .{ .fail = error.Timeout } }; + var first: Fake = .{ .behavior = .{ .fail = peerFault(error.ConnectFailed) } }; + var last: Fake = .{ .behavior = .{ .fail = peerFault(error.Timeout) } }; var entries = [_]Entry{ testEntry("https://first.example/dns-query", &first, 10), testEntry("https://last.example/dns-query", &last, 20), @@ -1125,7 +1278,7 @@ test "a truncated attempt that fails on its own still blames the endpoint" { // Same truncated budget, but the attempt completes with a peer fault of // its own before the deadline. A completed failure is real evidence // whatever budget it ran with, so health moves and `selected` names it. - var refusing: Fake = .{ .behavior = .{ .fail = error.ConnectFailed } }; + var refusing: Fake = .{ .behavior = .{ .fail = peerFault(error.ConnectFailed) } }; var entries = [_]Entry{testEntry("https://refusing.example/dns-query", &refusing, 10)}; var pool: Pool = .init(&entries, test_cfg, .{ .attempt = .{ .raw = .fromMilliseconds(200), .clock = .awake }, @@ -1140,7 +1293,7 @@ test "a truncated attempt that fails on its own still blames the endpoint" { ); try testing.expectEqualStrings("https://refusing.example/dns-query", selected.?); try testing.expectEqual(@as(u64, 1), entries[0].health.total_failures); - try testing.expectEqualStrings("ConnectFailed", entries[0].health.lastError()); + try testing.expectEqualStrings("ConnectFailed (cause ConnectFailed)", entries[0].health.lastError()); try testing.expectEqual(@as(u64, 0), pool.budgetExhaustedTotal()); } @@ -1188,7 +1341,7 @@ test "a peer fault fails over to the next entry and is recorded" { defer threaded.deinit(); const io = threaded.io(); - var bad: Fake = .{ .behavior = .{ .fail = error.Timeout } }; + var bad: Fake = .{ .behavior = .{ .fail = peerFault(error.Timeout) } }; var good: Fake = .{ .behavior = .{ .reply = response_bytes } }; var entries = [_]Entry{ testEntry("https://bad.example/dns-query", &bad, 10), @@ -1203,7 +1356,7 @@ test "a peer fault fails over to the next entry and is recorded" { try testing.expectEqual(@as(u32, 1), entries[0].health.consecutive_failures); try testing.expectEqual(@as(u64, 1), entries[0].health.total_failures); - try testing.expectEqualStrings("Timeout", entries[0].health.lastError()); + try testing.expectEqualStrings("Timeout (cause Timeout)", entries[0].health.lastError()); try testing.expectEqual(@as(u64, 1), entries[1].health.total_successes); } @@ -1212,7 +1365,7 @@ test "an entry in backoff is skipped while another is available" { defer threaded.deinit(); const io = threaded.io(); - var bad: Fake = .{ .behavior = .{ .fail = error.ConnectFailed } }; + var bad: Fake = .{ .behavior = .{ .fail = peerFault(error.ConnectFailed) } }; var good: Fake = .{ .behavior = .{ .reply = response_bytes } }; var entries = [_]Entry{ testEntry("https://bad.example/dns-query", &bad, 10), @@ -1238,8 +1391,8 @@ test "every entry in backoff is still probed" { defer threaded.deinit(); const io = threaded.io(); - var first: Fake = .{ .behavior = .{ .fail = error.ConnectFailed } }; - var second: Fake = .{ .behavior = .{ .fail = error.BadResponse } }; + var first: Fake = .{ .behavior = .{ .fail = peerFault(error.ConnectFailed) } }; + var second: Fake = .{ .behavior = .{ .fail = peerFault(error.BadResponse) } }; var entries = [_]Entry{ testEntry("https://first.example/dns-query", &first, 10), testEntry("https://second.example/dns-query", &second, 20), @@ -1264,7 +1417,7 @@ test "a local resource error short-circuits and records nothing" { defer threaded.deinit(); const io = threaded.io(); - var broke: Fake = .{ .behavior = .{ .fail = error.OutOfMemory } }; + var broke: Fake = .{ .behavior = .{ .local = error.OutOfMemory } }; var good: Fake = .{ .behavior = .{ .reply = response_bytes } }; var entries = [_]Entry{ testEntry("https://broke.example/dns-query", &broke, 10), @@ -1285,7 +1438,7 @@ test "a cancellation short-circuits and records nothing" { defer threaded.deinit(); const io = threaded.io(); - var canceled: Fake = .{ .behavior = .{ .fail = error.Canceled } }; + var canceled: Fake = .{ .behavior = .{ .local = error.Canceled } }; var good: Fake = .{ .behavior = .{ .reply = response_bytes } }; var entries = [_]Entry{ testEntry("https://canceled.example/dns-query", &canceled, 10), @@ -1331,7 +1484,7 @@ test "an attempt that outruns the budget is a recorded Timeout" { try testing.expectEqual(@as(usize, 1), slow.calls.load(.acquire)); try testing.expectEqual(@as(u32, 1), entries[0].health.consecutive_failures); - try testing.expectEqualStrings("Timeout", entries[0].health.lastError()); + try testing.expectEqualStrings("Timeout (cause Timeout)", entries[0].health.lastError()); try testing.expectEqual(@as(u64, 1), entries[1].health.total_successes); } @@ -1366,7 +1519,7 @@ test "a stalled primary fails over to a fast standby inside the total budget" { // The primary got its full configured interval and did not answer, which is // evidence about the primary — so it is recorded, unlike a truncated one. try testing.expectEqual(@as(u32, 1), entries[0].health.consecutive_failures); - try testing.expectEqualStrings("Timeout", entries[0].health.lastError()); + try testing.expectEqualStrings("Timeout (cause Timeout)", entries[0].health.lastError()); try testing.expectEqual(@as(u64, 0), pool.budgetExhaustedTotal()); } @@ -1380,8 +1533,8 @@ test "pass two probes every backed-off entry under the one deadline" { .base_backoff_ms = 60_000, .max_backoff_ms = 60_000, }; - var first: Fake = .{ .behavior = .{ .fail = error.ConnectFailed } }; - var second: Fake = .{ .behavior = .{ .fail = error.ConnectFailed } }; + var first: Fake = .{ .behavior = .{ .fail = peerFault(error.ConnectFailed) } }; + var second: Fake = .{ .behavior = .{ .fail = peerFault(error.ConnectFailed) } }; var entries = [_]Entry{ testEntry("https://first.example/dns-query", &first, 10), testEntry("https://second.example/dns-query", &second, 20), @@ -1481,8 +1634,8 @@ test "pass two probes in priority order rather than stepping over a saturated en .base_backoff_ms = 60_000, .max_backoff_ms = 60_000, }; - var first: Fake = .{ .behavior = .{ .fail = error.ConnectFailed } }; - var second: Fake = .{ .behavior = .{ .fail = error.ConnectFailed } }; + var first: Fake = .{ .behavior = .{ .fail = peerFault(error.ConnectFailed) } }; + var second: Fake = .{ .behavior = .{ .fail = peerFault(error.ConnectFailed) } }; var entries = [_]Entry{ testEntry("https://first.example/dns-query", &first, 10), testEntry("https://second.example/dns-query", &second, 20), @@ -1651,7 +1804,7 @@ test "snapshot reports the counters in pool order" { defer threaded.deinit(); const io = threaded.io(); - var bad: Fake = .{ .behavior = .{ .fail = error.Timeout } }; + var bad: Fake = .{ .behavior = .{ .fail = peerFault(error.Timeout) } }; var good: Fake = .{ .behavior = .{ .reply = response_bytes } }; var entries = [_]Entry{ testEntry("https://bad.example/dns-query", &bad, 10), @@ -1675,7 +1828,7 @@ test "snapshot reports the counters in pool order" { try testing.expectEqual(@as(u64, 2), out[0].total_failures); try testing.expectEqual(@as(u64, 0), out[0].total_successes); try testing.expectEqual(@as(f32, 0.0), out[0].success_rate); - try testing.expectEqualStrings("Timeout", out[0].last_error); + try testing.expectEqualStrings("Timeout (cause Timeout)", out[0].last_error); try testing.expect(out[0].last_error_at != null); try testing.expect(out[0].backoff_until != null); @@ -2055,7 +2208,7 @@ test "overlapping exchanges each report the entry that answered that call" { // under `test_cfg`'s threshold of two, so no backoff steers the failed-over // task away and the two calls end on different entries. var first_entry: Fake = .{ - .behavior = .{ .hold_fail = .{ .gate = &gate_a, .err = error.ConnectFailed } }, + .behavior = .{ .hold_fail = .{ .gate = &gate_a, .fault = peerFault(error.ConnectFailed) } }, .then = .{ .hold = .{ .gate = &gate_b, .reply = alt_response_bytes } }, }; var second_entry: Fake = .{ .behavior = .{ .reply = response_bytes } }; @@ -2134,7 +2287,7 @@ test "an entry that enters backoff while a task waits on it is not attempted" { var slow_bad: Fake = .{ .behavior = .{ .slow_fail = .{ .duration = .{ .raw = .fromMilliseconds(50), .clock = .awake }, - .err = error.ConnectFailed, + .fault = peerFault(error.ConnectFailed), } } }; var good: Fake = .{ .behavior = .{ .reply = response_bytes } }; var entries = [_]Entry{ @@ -2198,15 +2351,18 @@ test "a successful exchange with nothing open costs the store no statement" { try testing.expectEqual(@as(i64, 0), try fx.count("SELECT count(*) FROM operational_events")); } -test "a failing then recovering upstream leaves exactly one resolved episode" { +test "one failed exchange opens no episode and the second opens one" { var threaded: std.Io.Threaded = .init(testing.allocator, .{}); defer threaded.deinit(); const io = threaded.io(); - var flaky: Fake = .{ .behavior = .{ .fail = error.Timeout } }; + // The threshold health trips at is the threshold Diagnostics reports at. + // One lost exchange against an upstream the pool still trusts is not an + // episode; the rolling success rate on `/metrics` is where it shows. + var flaky: Fake = .{ .behavior = .{ .fail = .{ .kind = error.SendFailed, .cause = error.BrokenPipe } } }; var standby: Fake = .{ .behavior = .{ .reply = response_bytes } }; var entries = [_]Entry{ - testEntry("https://flaky.example/dns-query", &flaky, 10), + testEntry("tls://1.1.1.1:853", &flaky, 10), testEntry("https://standby.example/dns-query", &standby, 20), }; var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); @@ -2219,18 +2375,29 @@ test "a failing then recovering upstream leaves exactly one resolved episode" { var buf: [512]u8 = undefined; var selected: ?[]const u8 = null; _ = try pool.exchange(io, query_bytes, &buf, &selected); - // Backoff would park the failing entry, so the second failure is driven - // through `recordFailure` itself rather than through another exchange. - pool.recordFailure(io, &entries[0], std.Io.Clock.awake.now(io), error.ConnectFailed); + try testing.expectEqual(@as(i64, 0), try fx.count("SELECT count(*) FROM operational_events")); + + // Backoff would park the failing entry, so the failures after the first are + // driven through `recordFailure` itself rather than through another + // exchange. + const fault: transport.Fault = .{ .kind = error.SendFailed, .cause = error.BrokenPipe }; + pool.recordFailure(io, &entries[0], std.Io.Clock.awake.now(io), fault); try testing.expectEqual(@as(i64, 1), try fx.count("SELECT count(*) FROM operational_events")); - try testing.expectEqual(@as(i64, 2), try fx.count("SELECT occurrences FROM operational_events")); + try testing.expectEqual(@as(i64, 1), try fx.count("SELECT occurrences FROM operational_events")); try testing.expectEqualStrings("upstream.exchange", try fx.text("SELECT code FROM operational_events")); + try testing.expectEqualStrings("tls://1.1.1.1:853", try fx.text("SELECT subject_key FROM operational_events")); try testing.expectEqualStrings( - "https://flaky.example/dns-query", - try fx.text("SELECT subject_key FROM operational_events"), + "upstream 'tls://1.1.1.1:853' failed: SendFailed (cause BrokenPipe)", + try fx.text("SELECT detail FROM operational_events"), ); + // A failure during backoff raises the count on the card that is already + // open rather than opening a second one. + pool.recordFailure(io, &entries[0], std.Io.Clock.awake.now(io), peerFault(error.Timeout)); + try testing.expectEqual(@as(i64, 1), try fx.count("SELECT count(*) FROM operational_events")); + try testing.expectEqual(@as(i64, 2), try fx.count("SELECT occurrences FROM operational_events")); + flaky.behavior = .{ .reply = response_bytes }; pool.recordSuccess(io, &entries[0], std.Io.Clock.awake.now(io)); @@ -2241,3 +2408,327 @@ test "a failing then recovering upstream leaves exactly one resolved episode" { ); try testing.expectEqual(@as(i64, 2), try fx.count("SELECT occurrences FROM operational_events")); } + +/// A pool over one entry with a diagnostics fixture wired, for the projection +/// tests below. The caller owns every piece; this only names them once. +const ProjectionFixture = struct { + fake: Fake = .{ .behavior = .{ .reply = response_bytes } }, + entries: [1]Entry = undefined, + pool: Pool = undefined, + fx: events_fixture.Fixture = .{}, + + fn init(self: *ProjectionFixture, io: std.Io) !void { + self.entries = .{testEntry("https://a.example/dns-query", &self.fake, 10)}; + self.pool = .init(&self.entries, test_cfg, test_timeouts, 1); + try self.fx.init(io, 1000); + self.pool.diagnostics = &self.fx.store; + } + + fn deinit(self: *ProjectionFixture) void { + self.fx.deinit(); + } + + fn open(self: *ProjectionFixture) !i64 { + return self.fx.count("SELECT count(*) FROM operational_events WHERE resolved_at IS NULL"); + } +}; + +const tripped_fault: transport.Fault = .{ .kind = error.SendFailed, .cause = error.BrokenPipe }; + +test "a tripped effect behind a newer clear is dropped and no episode opens" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // Two tasks recording against one entry compute their effects under the + // pool mutex and project them after it is released, so they can reach + // `applyEffect` in either order. The delayed report below would otherwise + // open an episode that the success before it has already resolved, and + // nothing left running would resolve it again. + var fixture: ProjectionFixture = .{}; + try fixture.init(io); + defer fixture.deinit(); + + fixture.pool.applyEffect(io, &fixture.entries[0], .{ .revision = 5, .state = .clear }, .fresh); + fixture.pool.applyEffect(io, &fixture.entries[0], .{ .revision = 4, .state = .{ .tripped = tripped_fault } }, .fresh); + + try testing.expectEqual(@as(u64, 5), fixture.entries[0].applied_revision); + try testing.expectEqual(@as(i64, 0), try fixture.fx.count("SELECT count(*) FROM operational_events")); +} + +test "a clear effect behind a newer tripped is dropped and the episode stays open" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // The other direction, which a transition-shaped effect got wrong: a stale + // success must not resolve a card the newer failure has just opened. + var fixture: ProjectionFixture = .{}; + try fixture.init(io); + defer fixture.deinit(); + + fixture.pool.applyEffect(io, &fixture.entries[0], .{ .revision = 9, .state = .{ .tripped = tripped_fault } }, .fresh); + fixture.pool.applyEffect(io, &fixture.entries[0], .{ .revision = 8, .state = .clear }, .fresh); + + try testing.expectEqual(@as(u64, 9), fixture.entries[0].applied_revision); + try testing.expectEqual(@as(i64, 1), try fixture.open()); + try testing.expectEqualStrings( + "upstream 'https://a.example' failed: SendFailed (cause BrokenPipe)", + try fixture.fx.text("SELECT detail FROM operational_events"), + ); +} + +test "an episode open before the pool exists is resolved by the first healthy success" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // The restart case. The card outlived the process that opened it, and this + // pool has recorded no failure of its own, so a transition-shaped effect + // would have nothing to resolve and the card would stand forever. Every + // effect carries the whole desired state, so the first success closes it. + var fixture: ProjectionFixture = .{}; + try fixture.init(io); + defer fixture.deinit(); + + fixture.fx.store.report( + io, + 1000, + .upstream_exchange, + "https://a.example/dns-query", + "https://a.example", + .warning, + "upstream 'https://a.example' failed: Timeout (cause Timeout)", + ); + try testing.expectEqual(@as(i64, 1), try fixture.open()); + + var buf: [512]u8 = undefined; + var selected: ?[]const u8 = null; + _ = try fixture.pool.exchange(io, query_bytes, &buf, &selected); + + try testing.expectEqual(@as(i64, 0), try fixture.open()); +} + +test "a stale success while tripped moves no occurrence count" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // The card is already open and already right. Projecting it again on a + // success would count a successful exchange as an occurrence of the + // episode, which is the opposite of what the card says. + var fixture: ProjectionFixture = .{}; + try fixture.init(io); + defer fixture.deinit(); + + const entry = &fixture.entries[0]; + const at = std.Io.Clock.awake.now(io); + fixture.pool.recordFailure(io, entry, at, tripped_fault); + fixture.pool.recordFailure(io, entry, .{ .nanoseconds = at.nanoseconds + 1 }, tripped_fault); + try testing.expectEqual(@as(i64, 1), try fixture.fx.count("SELECT occurrences FROM operational_events")); + + const applied = entry.applied_revision; + fixture.pool.recordSuccess(io, entry, .{ .nanoseconds = at.nanoseconds - 1 }); + + try testing.expectEqual(@as(i64, 1), try fixture.open()); + try testing.expectEqual(@as(i64, 1), try fixture.fx.count("SELECT occurrences FROM operational_events")); + try testing.expectEqual(applied, entry.applied_revision); + try testing.expectEqual(@as(u64, 1), entry.health.total_successes); +} + +test "reconcile resolves an episode this pool cannot justify and keeps the one it can" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // The boot shape, driven at the pool: two episodes are in the store and + // this pool has one entry. The entry is failing, so its card is still the + // truth; the other names an upstream this pool does not have, and only the + // caller's `resolveExcept` can close that one, which is why the assertion + // below is that `reconcile` leaves it alone. + var fixture: ProjectionFixture = .{}; + try fixture.init(io); + defer fixture.deinit(); + + const entry = &fixture.entries[0]; + const at = std.Io.Clock.awake.now(io); + fixture.pool.recordFailure(io, entry, at, tripped_fault); + fixture.pool.recordFailure(io, entry, .{ .nanoseconds = at.nanoseconds + 1 }, tripped_fault); + fixture.fx.store.report(io, 1000, .upstream_exchange, "https://gone.example/dns-query", "https://gone.example", .warning, "stale"); + try testing.expectEqual(@as(i64, 2), try fixture.open()); + + fixture.pool.reconcile(io); + try testing.expectEqual(@as(i64, 2), try fixture.open()); + + // A healthy entry projects `clear`, and reconcile closes its card without + // an exchange having run. + fixture.pool.recordSuccess(io, entry, .{ .nanoseconds = at.nanoseconds + 2 }); + try testing.expectEqual(@as(i64, 1), try fixture.open()); + + var kept: [Pool.max_entries][]const u8 = undefined; + try testing.expectEqual(@as(usize, 1), fixture.pool.enabledUrls(&kept)); + try testing.expectEqualStrings("https://a.example/dns-query", kept[0]); +} + +test "reconcile projects a fresh pool clear over an episode it did not open" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // The restart and the retirement both land here: the pool has recorded + // nothing, so no revision of its own can ever be newer than an episode it + // never opened. Reconcile projects at the revision the entry is at, which + // is what lets a pool that has done nothing yet close a card. + var fixture: ProjectionFixture = .{}; + try fixture.init(io); + defer fixture.deinit(); + + fixture.fx.store.report( + io, + 1000, + .upstream_exchange, + "https://a.example/dns-query", + "https://a.example", + .warning, + "upstream 'https://a.example' failed: Timeout (cause Timeout)", + ); + try testing.expectEqual(@as(i64, 1), try fixture.open()); + + fixture.pool.reconcile(io); + try testing.expectEqual(@as(i64, 0), try fixture.open()); +} + +test "reconcile of a tripped entry with an open episode moves no occurrence count" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // Reconciliation runs at boot and at every retirement, and a card the pool + // still justifies must come out of it exactly as it went in: reasserting an + // open episode is not evidence of another failed exchange. + var fixture: ProjectionFixture = .{}; + try fixture.init(io); + defer fixture.deinit(); + + const entry = &fixture.entries[0]; + const at = std.Io.Clock.awake.now(io); + fixture.pool.recordFailure(io, entry, at, tripped_fault); + fixture.pool.recordFailure(io, entry, .{ .nanoseconds = at.nanoseconds + 1 }, tripped_fault); + try testing.expectEqual(@as(i64, 1), try fixture.open()); + + const occurrences = try fixture.fx.count("SELECT occurrences FROM operational_events"); + const last_seen = try fixture.fx.count("SELECT last_seen FROM operational_events"); + fixture.pool.reconcile(io); + fixture.pool.reconcile(io); + + try testing.expectEqual(@as(i64, 1), try fixture.open()); + try testing.expectEqual(occurrences, try fixture.fx.count("SELECT occurrences FROM operational_events")); + try testing.expectEqual(last_seen, try fixture.fx.count("SELECT last_seen FROM operational_events")); +} + +test "reconcile skips a disabled entry" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // Nothing will exchange against a disabled upstream, so nothing of this + // pool's would ever resolve a card it kept open. Its url stays out of the + // kept set, and the caller's `resolveExcept` is what closes it. + var fixture: ProjectionFixture = .{}; + try fixture.init(io); + defer fixture.deinit(); + fixture.entries[0].enabled = false; + + fixture.fx.store.report(io, 1000, .upstream_exchange, "https://a.example/dns-query", "https://a.example", .warning, "stale"); + fixture.pool.reconcile(io); + + try testing.expectEqual(@as(i64, 1), try fixture.open()); + var kept: [Pool.max_entries][]const u8 = undefined; + try testing.expectEqual(@as(usize, 0), fixture.pool.enabledUrls(&kept)); +} + +test "an untruncated attempt expiry is recorded as a Timeout of the peer" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // The attempt got its whole configured interval and the peer did not + // answer, so the expiry is evidence about the peer and the cause is the + // classification: nothing else was observed. + var stalled: Fake = .{ .behavior = .{ .slow = .{ + .duration = .{ .raw = .fromSeconds(30), .clock = .awake }, + .reply = response_bytes, + } } }; + var entries = [_]Entry{testEntry("tls://9.9.9.9:853", &stalled, 10)}; + var pool: Pool = .init(&entries, test_cfg, .{ + .attempt = .{ .raw = .fromMilliseconds(20), .clock = .awake }, + .total = .{ .raw = .fromSeconds(30), .clock = .awake }, + }, 1); + + var buf: [512]u8 = undefined; + var selected: ?[]const u8 = null; + try testing.expectError(error.Timeout, pool.exchange(io, query_bytes, &buf, &selected)); + + try testing.expectEqualStrings("tls://9.9.9.9:853", selected.?); + try testing.expectEqual(@as(u64, 1), entries[0].health.total_failures); + try testing.expectEqualStrings("Timeout (cause Timeout)", entries[0].health.lastError()); + try testing.expectEqual(@as(u64, 0), pool.budgetExhaustedTotal()); +} + +test "a leaf's own timeout is recorded with the leaf's cause, not as an expiry" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + // A leaf that timed out against its peer returns a fault and completes the + // race. The classification is the same `error.Timeout` the harness's expiry + // produces, and the two are opposite evidence, so the race tag is what + // tells them apart: this one keeps the cause the leaf observed. + var timed_out: Fake = .{ .behavior = .{ + .fail = .{ .kind = error.Timeout, .cause = error.WouldBlock }, + } }; + var entries = [_]Entry{testEntry("tls://9.9.9.9:853", &timed_out, 10)}; + var pool: Pool = .init(&entries, test_cfg, test_timeouts, 1); + + var buf: [512]u8 = undefined; + var selected: ?[]const u8 = null; + try testing.expectError(error.Timeout, pool.exchange(io, query_bytes, &buf, &selected)); + + try testing.expectEqualStrings("tls://9.9.9.9:853", selected.?); + try testing.expectEqual(@as(u64, 1), entries[0].health.total_failures); + try testing.expectEqualStrings("Timeout (cause WouldBlock)", entries[0].health.lastError()); + try testing.expectEqual(@as(u64, 0), pool.budgetExhaustedTotal()); +} + +test "a truncated attempt expiry records nothing and blames the budget" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var stalled: Fake = .{ .behavior = .{ .slow = .{ + .duration = .{ .raw = .fromSeconds(30), .clock = .awake }, + .reply = response_bytes, + } } }; + var entries = [_]Entry{testEntry("tls://9.9.9.9:853", &stalled, 10)}; + // The total budget lands before the attempt budget would, so the peer is + // given less than its configured interval and the expiry says nothing + // about it. + var pool: Pool = .init(&entries, test_cfg, .{ + .attempt = .{ .raw = .fromMilliseconds(200), .clock = .awake }, + .total = .{ .raw = .fromMilliseconds(40), .clock = .awake }, + }, 1); + + var fx: events_fixture.Fixture = .{}; + try fx.init(io, 1000); + defer fx.deinit(); + pool.diagnostics = &fx.store; + + var buf: [512]u8 = undefined; + var selected: ?[]const u8 = null; + try testing.expectError(error.BudgetExhausted, pool.exchange(io, query_bytes, &buf, &selected)); + + try testing.expectEqual(@as(?[]const u8, null), selected); + try testing.expectEqual(@as(u64, 0), entries[0].health.total_failures); + try testing.expectEqual(@as(u64, 1), pool.budgetExhaustedTotal()); + try testing.expectEqual(@as(i64, 0), try fx.count("SELECT count(*) FROM operational_events")); +} diff --git a/src/upstream/transport.zig b/src/upstream/transport.zig index 7d178e2..2f3d2ef 100644 --- a/src/upstream/transport.zig +++ b/src/upstream/transport.zig @@ -190,6 +190,11 @@ pub const LocalResource = error{ pub const Cancellation = error{Canceled}; +/// What a leaf client may fail with. A leaf never *errors* on a peer fault: it +/// returns one as an `Outcome.fault` value, because the pool needs the concrete +/// cause and an error cannot carry one. +pub const LeafError = LocalResource || Cancellation; + /// The caller's own time ran out before any peer could be given the observation /// interval it was configured to get. Evidence about this process's budget, not /// about any endpoint, so it is never recorded against health — that is the @@ -238,7 +243,7 @@ pub fn group(err: ExchangeError) Group { /// This is the only place a foreign error set is folded in. Everywhere else /// the call site names the peer fault it means, because the call site is what /// knows whether it was connecting, sending or receiving. -pub fn mapLocal(err: anyerror) ?ExchangeError { +pub fn mapLocal(err: anyerror) ?LeafError { return switch (err) { error.OutOfMemory => error.OutOfMemory, error.SystemResources => error.SystemResources, @@ -279,21 +284,37 @@ pub fn closeBlocked(io: std.Io, target: anytype) void { } } -/// The payload of `f`'s return type, which the race harness requires to be -/// `ExchangeError!T`. A raced function with any other error set would let a -/// failure reach the pool without passing through `group`. -fn RacedPayload(comptime f: anytype) type { +/// The error union `f` returns, which the race harness requires it to have. +fn racedUnion(comptime f: anytype) std.builtin.Type.ErrorUnion { const info = @typeInfo(@TypeOf(f)); if (info != .@"fn") @compileError("the race harness needs a function, found " ++ @typeName(@TypeOf(f))); const Return = info.@"fn".return_type orelse @compileError("the race harness needs a function with a concrete return type"); - const union_info = switch (@typeInfo(Return)) { + return switch (@typeInfo(Return)) { .error_union => |u| u, - else => @compileError("the race harness needs `ExchangeError!T`, found " ++ @typeName(Return)), + else => @compileError("the race harness needs `E!T`, found " ++ @typeName(Return)), }; - if (union_info.error_set != ExchangeError) - @compileError("the race harness needs `ExchangeError!T`, found " ++ @typeName(Return)); - return union_info.payload; +} + +/// The payload of `f`'s return type. +fn RacedPayload(comptime f: anytype) type { + return racedUnion(f).payload; +} + +/// `f`'s own error set. +fn RacedError(comptime f: anytype) type { + return racedUnion(f).error_set; +} + +/// What racing `f` can fail with: `f`'s own errors, plus the three the harness +/// itself produces — the expiry, a backend that cannot start a second task, and +/// the whole task being torn down. +/// +/// Derived rather than fixed at `ExchangeError`, so a caller's narrow error set +/// survives the race. That is what lets the pool prove at the type level that a +/// leaf cannot hand it a `PeerFault`, instead of asserting it. +pub fn RaceError(comptime f: anytype) type { + return RacedError(f) || error{ Timeout, SystemResources, Canceled }; } /// Runs `f(args...)` raced against `budget`, and cancels the loser. @@ -312,7 +333,7 @@ pub fn raceWithin( budget: std.Io.Clock.Duration, comptime f: anytype, args: anytype, -) ExchangeError!RacedPayload(f) { +) RaceError(f)!RacedPayload(f) { var outcome: RaceOutcome = .completed; return raceUntilTagged(io, .fromNow(io, budget), &outcome, f, args); } @@ -344,9 +365,9 @@ pub fn raceUntilTagged( outcome: *RaceOutcome, comptime f: anytype, args: anytype, -) ExchangeError!RacedPayload(f) { +) RaceError(f)!RacedPayload(f) { const Slot = union(enum) { - raced: ExchangeError!RacedPayload(f), + raced: RacedError(f)!RacedPayload(f), expiry: std.Io.Cancelable!void, }; @@ -378,8 +399,12 @@ fn expire(io: std.Io, expiry_at: std.Io.Clock.Timestamp) std.Io.Cancelable!void return expiry_at.wait(io); } -/// A thing that sends one DNS message and returns one validated DNS message. -/// Implemented by DohClient, DotClient, Pool, and test fakes. +/// A thing that sends one DNS message and returns one validated DNS message, +/// with a peer fault already reduced to an error. +/// +/// Implemented by `Pool`, the forward client, and the fakes that stand in for +/// either. The leaf clients are on the other side of the pool and implement +/// `Leaf` instead, which keeps the fault as a value. pub const Client = struct { ptr: *anyopaque, exchangeFn: *const fn ( @@ -394,7 +419,8 @@ pub const Client = struct { /// passed `validateResponse` against `query`. /// /// `selected` names the resolver the exchange used. A single-endpoint - /// implementation (DoH, DoT, the forward client, test fakes) may write it + /// implementation — the forward client and the test fakes; the DoH and DoT + /// clients are `Leaf`s and a `Pool` carries their answer here — may write it /// *before* each attempt: it has one resolver and records no health, so /// "the one I tried" is an honest answer even for a failure, and a SERVFAIL /// row without its resolver explains nothing. @@ -419,6 +445,70 @@ pub const Client = struct { } }; +/// One peer fault as a value: the taxonomy `PeerFault` names, plus the concrete +/// error that produced it. +/// +/// The classification is what health and backoff count; the cause is what tells +/// an operator which failure it was. `SendFailed` alone cannot separate a peer +/// that reset the connection from one whose TLS record was rejected, and the +/// leaf that unwrapped the cause is the only place that still holds it. +/// +/// There is no phase field: the taxonomy already names the phase for every kind +/// that has one, and `TlsFailed` cannot say where it failed. +pub const Fault = struct { + kind: PeerFault, + cause: anyerror, + + /// The one text every surface prints. ` (cause )`. + pub fn format(self: Fault, w: *std.Io.Writer) std.Io.Writer.Error!void { + try w.print("{t} (cause {s})", .{ self.kind, @errorName(self.cause) }); + } +}; + +/// What one exchange against one endpoint produced. +pub const Outcome = union(enum) { + /// A prefix of the caller's `response_buf`, already validated against the + /// query. + reply: []u8, + fault: Fault, +}; + +/// A client of exactly one endpoint: DoH, DoT, and the pool's test fakes. +/// +/// Separate from `Client` because the two answer different questions. A `Leaf` +/// reports what the peer did, faults included, and leaves every judgement to +/// its caller. A `Client` is the resolver the handler asks for an answer, and a +/// fault has already become an error by the time it is reached. +/// +/// No `selected` out-parameter: the pool discards a leaf's own identity anyway, +/// since the entry's endpoint is the pool's naming of the same resolver. +pub const Leaf = struct { + ptr: *anyopaque, + exchangeFn: *const fn ( + ptr: *anyopaque, + io: std.Io, + query: []const u8, + response_buf: []u8, + ) LeafError!Outcome, + + pub fn exchange( + self: Leaf, + io: std.Io, + query: []const u8, + response_buf: []u8, + ) LeafError!Outcome { + return self.exchangeFn(self.ptr, io, query, response_buf); + } +}; + +/// The fault a call site's phase means, unless `err` is one this process owns. +/// The leaf counterpart of `mapPhase`: same rule, but the peer case comes back +/// as a value carrying the cause instead of as a bare error. +pub fn faultOrLocal(err: anyerror, phase: PeerFault) LeafError!Fault { + if (mapLocal(err)) |local| return local; + return .{ .kind = phase, .cause = err }; +} + /// The unwrap helpers below turn the single collapsed error `std.http.Client` /// reports into the concrete cause it stashed. Every HTTP caller in this tree /// uses them: the DoH client classifies by the unwrapped cause, the blocklist @@ -743,14 +833,16 @@ test "raceWithin passes the raced task's own failure through" { ); } -test "raceUntilTagged tells a leaf Timeout apart from an expiry" { +test "raceUntilTagged tells a raced Timeout apart from an expiry" { var threaded: std.Io.Threaded = .init(testing.allocator, .{}); defer threaded.deinit(); const io = threaded.io(); - // The leaf's own timeout: it returned, so the peer really did time out and - // the outcome is `completed` even though the error is the same one an - // expiry produces. + // A raced function that returned `error.Timeout` of its own: it completed, + // so the tag says `completed` even though the error is the one an expiry + // produces. No leaf does this — a leaf's own timeout is a `Fault` — but the + // harness serves callers with any error set, and the tag is what separates + // the two for every one of them. var outcome: RaceOutcome = .expired; const far: std.Io.Clock.Timestamp = .fromNow(io, .{ .raw = .fromSeconds(30), .clock = .awake }); try testing.expectError( diff --git a/src/web/handlers/apply.zig b/src/web/handlers/apply.zig index ae2aaeb..96e5c63 100644 --- a/src/web/handlers/apply.zig +++ b/src/web/handlers/apply.zig @@ -636,9 +636,16 @@ pub const Plan = struct { ); } // A generation no reader held at the swap has no release left to - // tear it down, so the publisher does — after the reconciliation, - // which reads only the copies taken at prepare. - if (self.retired_upstream) |old| old.retire(io); + // tear it down, so the publisher does — after the reconciliation + // above, which reads only the copies taken at prepare. + // + // `retireDisplaced`, not `retire`: the teardown of a displaced + // generation is also when its `upstream.exchange` episodes are + // reconciled, and this path and a reader's release are the two ways + // a generation reaches it. The `configuration.load` half above + // stays here, because that code is shared with the boot collector + // and its rule is scoped. + if (self.retired_upstream) |old| self.state.upstreams.?.retireDisplaced(io, old); } self.* = undefined; diff --git a/src/web/metrics.zig b/src/web/metrics.zig index 41d9c8f..1cebc5c 100644 --- a/src/web/metrics.zig +++ b/src/web/metrics.zig @@ -1520,18 +1520,16 @@ const AnsweringClient = struct { io: std.Io, query: []const u8, response_buf: []u8, - selected: *?[]const u8, - ) transport.ExchangeError![]u8 { + ) transport.LeafError!transport.Outcome { _ = io; _ = query; const self: *AnsweringClient = @ptrCast(@alignCast(ptr)); _ = self.calls.fetchAdd(1, .acq_rel); - selected.* = "fake://leaf"; @memcpy(response_buf[0..reply.len], reply); - return response_buf[0..reply.len]; + return .{ .reply = response_buf[0..reply.len] }; } - fn client(self: *AnsweringClient) transport.Client { + fn leaf(self: *AnsweringClient) transport.Leaf { return .{ .ptr = self, .exchangeFn = exchangeFn }; } }; @@ -1546,10 +1544,10 @@ test "the queue families carry what a real pool recorded, through the real snaps defer threaded.deinit(); const io = threaded.io(); - var leaf: AnsweringClient = .{}; + var answering: AnsweringClient = .{}; var slots = [_]pool_mod.Slot{ - .{ .client = leaf.client() }, - .{ .client = leaf.client() }, + .{ .client = answering.leaf() }, + .{ .client = answering.leaf() }, }; var recoveries: std.atomic.Value(u64) = .init(0); var entries = [_]pool_mod.Entry{.{ @@ -1569,7 +1567,7 @@ test "the queue families carry what a real pool recorded, through the real snaps var buf: [512]u8 = undefined; var selected: ?[]const u8 = null; _ = try pool.exchange(io, "\x12\x34\x01\x00", &buf, &selected); - try testing.expectEqual(@as(u32, 1), leaf.calls.load(.acquire)); + try testing.expectEqual(@as(u32, 1), answering.calls.load(.acquire)); // The counters a burst would move, written straight into the entry: this // test is about what the snapshot path carries, not about reproducing a