perf(stats): answer top domains and top clients from one index scan

ARCHITECTURE.md

@@ -243,20 +243,22 @@ Everything is in a single SQLite file (`noadd.sqlite3` by default; a legacy `noa
 | Index | Serves |
 | --- | --- |
 | `timestamp` | the time-window filter every stats query starts with |
-| `(domain, timestamp)` | top domains, unique domains |
-| `(client_ip, doh_token, timestamp)` | top clients |
+| `(domain, timestamp)` | the query log's domain search; top and unique domains for callers asking for domains alone |
+| `(timestamp, domain, client_ip, doh_token)` | top domains and top clients together, on the Statistics page and the dashboard |
 | `(timestamp, blocked, cached, response_ms, query_type, has_result)` | timeline, query-type breakdown, latency histogram, outcome breakdown, the dashboard summary |
 | `(timestamp, upstream, response_ms) WHERE upstream IS NOT NULL` | top upstreams |
 
-The first two composites put the grouped columns first and `timestamp` last, which is what makes them covering for a `GROUP BY … WHERE timestamp >= ?` shape — the aggregation reads the index alone instead of scanning the window and building a temp b-tree over it. Top clients went from 143 ms to 20 ms on a 447 k-row database that way.
+`(domain, timestamp)` puts the grouped column first and `timestamp` last, which made it covering for a `GROUP BY … WHERE timestamp >= ?` shape — the aggregation reads the index alone instead of scanning the window and building a temp b-tree over it. The client index that sat beside it, `(client_ip, doh_token, timestamp)`, did the same for top clients and took them from 143 ms to 20 ms on a 447 k-row database. The cost of that order is that the window cannot restrict it: a 24-hour question skip-scans the whole index, one seek per distinct group.
 
-The last one inverts that order because its queries do not group by a column at all; they filter on `timestamp` and then read a few narrow values. Carrying those values in the index avoids a row lookup into a table whose rows average ~84 bytes of strings (`domain`, `client_ip`, `upstream`, `result`) that none of those queries want: timeline 78 → 60 ms, query-type 75 → 62 ms, latency 60 → 45 ms.
+Version 15 replaced the client index with `(timestamp, domain, client_ip, doh_token)`. Both lists are always asked for together, and grouped at `(domain, client_ip, doh_token)` one statement answers them; timestamp-first, it reads only the window. On a 370 k-row database the two lists went from 7 589 pages to 5 769 for a window covering the table, and from 2 389 to 747 for the dashboard's 24 hours, for 8.4 MiB more index and no measurable change in what a logger batch writes. A Statistics visit over the whole table went from 9 758 page misses to 7 937, and a dashboard tick from 4 755 to 3 113. `(domain, timestamp)` stays because the query log's domain search seeks it by prefix: without it, searching a prefix nobody has queried reads the whole table, 12 170 pages against 3. Replacing both indexes measured cheaper still on every other axis and was rejected for that alone.
+
+The metrics index is timestamp-first because its queries do not group by a column at all; they filter on `timestamp` and then read a few narrow values. Carrying those values in the index avoids a row lookup into a table whose rows average ~84 bytes of strings (`domain`, `client_ip`, `upstream`, `result`) that none of those queries want: timeline 78 → 60 ms, query-type 75 → 62 ms, latency 60 → 45 ms.
 
 `has_result` is a VIRTUAL generated column — `result IS NOT NULL AND result != ''` — which occupies no table space and exists so the outcome breakdown can classify a query without reading one. It is the last column of the metrics index, and the query that needs it carries an `INDEXED BY`: with `timestamp` alone also matching the range the planner picks that smaller index and pays a rowid lookup per row, which is the whole table. An index on the bare expression rather than a named column was tried first and the planner would not treat it as covering.
 
 The upstream index is partial because blocked and cached answers never reach an upstream, so more than half the rows have nothing to put in it (56% on a 370 k-row database), and its only query excludes them anyway. It is timestamp-first for the writer's sake rather than the reader's: both orders answer the dashboard's 24-hour top upstreams in about 200 pages against 1 608 through `timestamp` and a rowid lookup per row, but the logger appends at the newest end, and an upstream-first index spreads every batch across one insertion point per upstream — 65 pages written per 500-row batch against 56, where no index at all writes 53. It is 5.7 MiB on that database.
 
-Indexes are not free here. On that same 103 MiB database `dbstat` attributes 20 MiB to `(domain, timestamp)`, 18 MiB to `(client_ip, doh_token, timestamp)`, 9 MiB to the metrics index and 7 MiB to `timestamp` — the two composites added for statistics cost about a quarter of the file. Measuring an index by the file-size delta of `CREATE INDEX` understates it whenever the database is carrying a freelist, since the new pages come out of that first; `dbstat` reports the real figure.
+Indexes are not free here. On that same 103 MiB database `dbstat` attributes 20 MiB to `(domain, timestamp)`, 18 MiB to `(client_ip, doh_token, timestamp)`, 9 MiB to the metrics index and 7 MiB to `timestamp` — the two composites added for statistics cost about a quarter of the file. (Those are the figures from before version 15 replaced the client index.) Measuring an index by the file-size delta of `CREATE INDEX` understates it whenever the database is carrying a freelist, since the new pages come out of that first; `dbstat` reports the real figure.
 
 ### Measuring these queries
 
@@ -268,7 +270,7 @@ That distinction is not hypothetical. `outcome_breakdown_since` was left uncover
 
 ### One scan per index, not one per reading
 
-The Statistics page's five readings were four foldings of the metrics index over one window and two foldings of `(domain, timestamp)` over the same one, asked as six separate statements. Each re-walked an index another had just finished with, and the read pool round-robins them across connections holding 2 MiB of page cache each, so nothing was ever warm for the next. `window_metrics_since` groups at `(blocked, cached, has_result, query_type, response_ms)` — one statement both breakdowns and the latency histogram are derived from — and `domain_stats_since` returns the top list and the distinct count from one materialized CTE. The single-purpose functions `/api/stats/*` calls are folds over the same statements, so there is one SQL spelling per fact. Rendering a 7-day window on that 370 k-row database went from 29 274 page misses (114 MiB) to 9 674 (38 MiB).
+The Statistics page's five readings were four foldings of the metrics index over one window and two foldings of `(domain, timestamp)` over the same one, asked as six separate statements. Each re-walked an index another had just finished with, and the read pool round-robins them across connections holding 2 MiB of page cache each, so nothing was ever warm for the next. `window_metrics_since` groups at `(blocked, cached, has_result, query_type, response_ms)` — one statement both breakdowns and the latency histogram are derived from — and `domain_stats_since` returns the top list and the distinct count from one materialized CTE. The single-purpose functions `/api/stats/*` calls are folds over the same statements, so there is one SQL spelling per fact. (The page's domain list has since moved to `traffic_lists_since`, which answers the client list from the same scan; `domain_stats_since` remains for callers that want domains alone, and breaks ties by name the same way so the two agree.) Rendering a 7-day window on that 370 k-row database went from 29 274 page misses (114 MiB) to 9 674 (38 MiB).
 
 The window's grain has no time bucket in it. Bucketing the shared grain was what the first version did, and it cost a scan: the outcome breakdown sums across every bucket, so the bucket only multiplied the rows the folds read, 68 846 of them against 4 658 at the 7-day range's hourly grain, while the query-type and latency folds still needed a second statement of their own. Collapsing the two into one grain fine enough for all three is 2 157 pages against 4 314.
 

CLAUDE.md

@@ -120,7 +120,7 @@ The query log adds the conventions for **filtering and paging**:
 
 Dashboard adds the conventions for a page that is **all readings and no controls**:
 
-- **The numbers are in the first response.** `dashboard_page` makes the five reads `app.js` used to make on its poll — `compute_summary`, `compute_top_domains` / `_clients` / `_upstreams` in `src/admin/stats.rs`, already shared with `/api/stats/*`. A failed read renders zeroes rather than an error page: a dashboard that says nothing beats one that will not load.
+- **The numbers are in the first response.** `dashboard_page` makes the five reads `app.js` used to make on its poll — `compute_summary`, `compute_top_domains_and_clients` and `compute_top_upstreams` in `src/admin/stats.rs` — the same functions the `stats` snapshot calls every tick, which is why the domain and client lists come from one scan rather than one each. A failed read renders zeroes rather than an error page: a dashboard that says nothing beats one that will not load.
 - **`app.js` re-draws the same markup from a pushed snapshot**, so every shape in the template has a counterpart in `DashboardPage`. The five polls are gone: the numbers arrive as `stats` events on the shared stream (see *The event stream* below), and `_apply` reads the same five response bodies the fetches returned, so every renderer is the one that read them before. Number formatting is duplicated in Rust to match (`format_num_adaptive`, `percent1`, `share_percent`, `format_qps`) — a count that changed its own notation when an update landed would read as a change in the number.
 - **The chart is the documented exception** to no-JS: it is drawn from a timeline series by the client. The card says so rather than sitting empty, and the client replaces that text on connect.
 - ⚠️ **A conditional `style` must be merged into the element's existing one.** Two `style` attributes means the second is dropped — the chart card's `animation-delay` and its `display:none` are one attribute for that reason.
@@ -133,7 +133,7 @@ Statistics adds the conventions for a page whose readings sit in a **chosen wind
 - **The range is in the URL and the switcher is three `<a>`s** (`/stats?range=30d`), because the range picks the *server's* window. `StatsRange::label()` is the one spelling shared by the link, the parse and every card title. An unrecognised range renders the default rather than 400ing — it is a link an operator can edit, and every window on offer is spelled out right above it.
 - **A date the server can only write in UTC ships as an ISO day plus its timestamp** (`data-date-ts`), and `app.js` restates it in the browser's locale — the same division as the query log's relative times, and for the same reason.
 - **This page is measured in page misses, not milliseconds.** Development is on an SSD and the appliance runs off an SD card, so a duration measured here says nothing about a Raspberry Pi; the pages a query fetches from the file are the same on both. `cargo nextest run --release --no-capture --run-ignored only stats_page_miss` with `BENCH_DB` pointed at a copy of a real database reports them. A wall-clock reading is the thing to distrust when the two disagree — it is what left the outcome breakdown scanning the whole table through version 11.
-- **One scan per index, not one per reading.** `stats_scan_since` and `domain_stats_since` (`src/db.rs`) are the page's two scans, and every reading on it — the charts included — is folded out of one of them; `compute_range_stats` (`src/admin/stats.rs`) is what the page calls. `stats_scan_since` streams its rows and folds them in Rust rather than grouping in SQL: a grain carrying both the quarter hour and `response_ms` approaches a group per row, which would be a temp b-tree the size of the window. The single-purpose functions `/api/stats/*` uses are statements of their own — adding a seventh reading to the page means folding it out of one of those two scans, not adding a statement.
+- **One scan per index, not one per reading.** `stats_scan_since` and `traffic_lists_since` (`src/db.rs`) are the page's two scans — the second answering top domains, the distinct-domain count and top clients from one grouping of `(domain, client_ip, doh_token)` — and every reading on it — the charts included — is folded out of one of them; `compute_range_stats` (`src/admin/stats.rs`) is what the page calls. `stats_scan_since` streams its rows and folds them in Rust rather than grouping in SQL: a grain carrying both the quarter hour and `response_ms` approaches a group per row, which would be a temp b-tree the size of the window. The single-purpose functions `/api/stats/*` uses are statements of their own — adding a seventh reading to the page means folding it out of one of those two scans, not adding a statement.
 - **A total nobody can count cheaply is maintained, not counted.** `query_logs`' row count lives in `settings` (`query_log_count`), moved by the insert batch, the prune and Clear All inside their own transactions — `SELECT COUNT(*)` walks an index end to end and the Database Health card asks on every load. A fourth write path to `query_logs` means a fourth `bump_log_count`, not a fourth reader.
 - Four bar lists in one template share **one askama macro** (`templates/_macros.html`); `{% call … %}` needs a matching `{% endcall %}` in askama 0.16, and `{% include %}` cannot see a loop variable at all.
 

src/admin/events.rs

@@ -147,11 +147,10 @@ impl Drop for StatsGuard {
 
 /// Read everything the dashboard shows, in the shapes `app.js` renders.
 pub async fn compute_snapshot(db: &Database, now: i64) -> Result<DashboardSnapshot, DbError> {
-    let (summary, timeline, top_domains, top_clients, top_upstreams) = tokio::try_join!(
+    let (summary, timeline, (top_domains, top_clients), top_upstreams) = tokio::try_join!(
         stats::compute_summary(db, now),
         stats::compute_timeline(db, now, TIMELINE_HOURS),
-        stats::compute_top_domains(db, now, TOP_N),
-        stats::compute_top_clients(db, now, TOP_N),
+        stats::compute_top_domains_and_clients(db, now, TOP_N),
         stats::compute_top_upstreams(db, now, TOP_N),
     )?;
 

src/admin/pages.rs

@@ -1442,12 +1442,10 @@ pub async fn dashboard_page(
         .unwrap_or_default();
     // Ten rows each, which is what the page shows — the API's larger default is
     // for callers who want to do their own slicing.
-    let domains = crate::admin::stats::compute_top_domains(&state.db, now, 10)
-        .await
-        .unwrap_or_default();
-    let clients = crate::admin::stats::compute_top_clients(&state.db, now, 10)
-        .await
-        .unwrap_or_default();
+    let (domains, clients) =
+        crate::admin::stats::compute_top_domains_and_clients(&state.db, now, 10)
+            .await
+            .unwrap_or_default();
     let upstreams = crate::admin::stats::compute_top_upstreams(&state.db, now, 10)
         .await
         .unwrap_or_default();
@@ -1667,11 +1665,10 @@ pub async fn stats_page(
         .and_then(stats::StatsRange::parse)
         .unwrap_or(stats::StatsRange::Days7);
 
-    // Three independent reads; running them together keeps the page at one
-    // round trip to the database rather than three in sequence.
-    let (range_stats, clients, health) = tokio::join!(
+    // Two independent reads; running them together keeps the page at one
+    // round trip to the database rather than two in sequence.
+    let (range_stats, health) = tokio::join!(
         stats::compute_range_stats(&state.db, now, range, STATS_TOP_N),
-        stats::compute_top_clients_ranged(&state.db, now, range, STATS_TOP_N),
         stats::compute_db_health(&state.db, now),
     );
 
@@ -1719,7 +1716,7 @@ pub async fn stats_page(
     )
     .expect("a series of integers always serializes");
 
-    let (query_types, outcomes, top_domains) = range_stats
+    let (query_types, outcomes, top_domains, top_clients) = range_stats
         .map(|s| {
             (
                 bar_rows(s.metrics.query_types),
@@ -1731,24 +1728,26 @@ pub async fn stats_page(
                         .map(|d| (d.domain, d.count))
                         .collect(),
                 ),
+                // A client that came in over `DoH` is named by both, the way the
+                // client draws it — the IP alone would collapse every token
+                // behind one proxy.
+                bar_rows(
+                    s.clients
+                        .into_iter()
+                        .map(|c| {
+                            let label = match c.doh_token {
+                                Some(token) if !token.is_empty() => {
+                                    format!("{} · {token}", c.client_ip)
+                                }
+                                _ => c.client_ip,
+                            };
+                            (label, c.count)
+                        })
+                        .collect(),
+                ),
             )
         })
         .unwrap_or_default();
-    // A client that came in over `DoH` is named by both, the way the client
-    // draws it — the IP alone would collapse every token behind one proxy.
-    let top_clients = bar_rows(
-        clients
-            .unwrap_or_default()
-            .into_iter()
-            .map(|c| {
-                let label = match c.doh_token {
-                    Some(token) if !token.is_empty() => format!("{} · {token}", c.client_ip),
-                    _ => c.client_ip,
-                };
-                (label, c.count)
-            })
-            .collect(),
-    );
 
     let health_cards = health.map(build_health_cards).unwrap_or_default();
 

src/admin/stats.rs

@@ -153,6 +153,18 @@ pub async fn compute_top_clients(
     db.top_clients_since(since, limit).await
 }
 
+/// The dashboard's two 24-hour lists from the one scan that answers both —
+/// what it shows every tick, where asking [`compute_top_domains`] and
+/// [`compute_top_clients`] separately would read two indexes.
+pub async fn compute_top_domains_and_clients(
+    db: &Database,
+    now: i64,
+    limit: i64,
+) -> Result<(Vec<TopDomain>, Vec<TopClient>), DbError> {
+    let lists = db.traffic_lists_since(now - 86400, limit).await?;
+    Ok((lists.domains.top, lists.clients))
+}
+
 pub async fn compute_top_upstreams(
     db: &Database,
     now: i64,
@@ -343,15 +355,17 @@ pub async fn compute_highlights(
 /// a top-domain list — and every one of them re-scanned an index another had
 /// just walked. Three of them share
 /// [`crate::db::Database::stats_scan_since`] and two share
-/// [`crate::db::Database::domain_stats_since`], which is two index scans
+/// [`crate::db::Database::traffic_lists_since`], which is two index scans
 /// instead of six.
 ///
 /// The charts ride the first of those scans as `series`, rather than being
-/// fetched by the browser afterwards at the cost of two more.
+/// fetched by the browser afterwards at the cost of two more, and the top
+/// clients ride the second rather than scanning an index of their own.
 pub struct RangeStats {
     pub metrics: crate::db::WindowMetrics,
     pub series: crate::db::QuarterSeries,
     pub domains: crate::db::DomainStats,
+    pub clients: Vec<TopClient>,
 }
 
 pub async fn compute_range_stats(
@@ -362,14 +376,15 @@ pub async fn compute_range_stats(
 ) -> Result<RangeStats, DbError> {
     let (window_secs, _) = range.window();
     let since = now - window_secs;
-    let (scan, domains) = tokio::try_join!(
+    let (scan, lists) = tokio::try_join!(
         db.stats_scan_since(since, now - HEATMAP_WINDOW_SECS),
-        db.domain_stats_since(since, top_n),
+        db.traffic_lists_since(since, top_n),
     )?;
     Ok(RangeStats {
         metrics: scan.metrics,
         series: scan.series,
-        domains,
+        domains: lists.domains,
+        clients: lists.clients,
     })
 }