perf(stats): answer the Statistics page in three index scans, not six - #269
Merged
Merged
Conversation
The page took 29 340 page misses to render a 7-day window on a 370 k-row database — 115 MiB read off the disk for one screen. Two causes, both invisible in wall-clock terms on an SSD and both expensive on the SD card an appliance actually runs from: `outcome_breakdown_since` classified a query by testing `result` for emptiness, which no index carried, so it walked the timestamp index and then looked up every matching row: the entire 10 784-page table, 12 173 misses on its own. It now rides a VIRTUAL generated column in `idx_query_logs_ts_metrics`, which costs no table space and 65 pages of index, and answers from the index alone at 2 157. Version 11's comment called this approach slower; it was measured in milliseconds on an SSD, where the page cache hides the reads. The other four readings re-walked indexes each other had just finished with. The timeline, both breakdowns and the latency histogram are four foldings of one index over one window, and the top-domain list and the unique-domain count are two foldings of another; the read pool spread them over connections with 2 MiB of cache each, so nothing stayed warm. `range_metrics_since` and `domain_stats_since` group at a grain fine enough to derive each set, and the single-purpose functions the `/api` endpoints call are now folds over the same two statements. First response: 29 340 -> 13 291 page misses, 115 MiB -> 52 MiB. `add_column_if_missing` had to move to `pragma_table_xinfo`: the plain `table_info` omits generated columns, so a fresh database would have been told `has_result` was missing and failed the migration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d in Version 11 left the outcome breakdown scanning the whole log table on the strength of a millisecond reading taken on an SSD. ARCHITECTURE.md now says why that reading was the wrong one, names the instrumentation, and documents the shared-scan shape the page reads through; CLAUDE.md points at the benchmark that produces the figure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #269 +/- ##
==========================================
+ Coverage 91.03% 91.16% +0.12%
==========================================
Files 31 31
Lines 10986 11168 +182
==========================================
+ Hits 10001 10181 +180
- Misses 985 987 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Sep 12, 2026
…ked on `NextStepBanner` takes the notice out of the DOM the moment the form is submitted and posts the dismissal in the background, so the operator never waits on it. Without `keepalive` that post is an ordinary fetch tied to the document: navigate in the same breath — click a nav link, reload — and the browser cancels it. The setting is never written and the notice is back on the page they land on. Latent on main, which wins the race often enough for the e2e scenario to pass; the query restructuring in this branch shifted the timing enough to lose it every run. `The next-step banner can be dismissed and stays dismissed` failed on the same tree without this flag and passes with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rendering a 7-day window of the Statistics page cost 29 274 page misses — 114 MiB read off the disk for one screen — on a 370 k-row, 147 MiB database. It now costs 13 287 (52 MiB).
The unit is deliberate. These queries were previously tuned against wall-clock readings taken on an SSD, where the OS page cache hides how much of the file a query actually touches. The appliance runs off an SD card, where it does not. A page count is the same number on both machines; a duration is not.
What was expensive
The outcome breakdown was scanning the whole log table — 12 173 misses, 41% of the page. Classifying a query needs
resulttested for emptiness, and no index carried that, so the query walkedidx_query_logs_timestampand then did a rowid lookup per matching row: the entire 10 784-page table.The emptiness test now rides
idx_query_logs_ts_metricsas a VIRTUAL generated column (has_result), which occupies no table space and added 65 pages (3%) to the index. Same answer, 2 157 misses.Version 11's migration comment ruled this out — "indexing the emptiness expression measured slower than the plain table lookup". That was true in milliseconds on an SSD and wrong about the cost. Two details from re-doing it: an index on the bare expression is not treated as covering by the planner, but a named generated column is; and the query needs
INDEXED BY, because withidx_query_logs_timestampalso matching the range the planner picks the smaller index and pays the lookups anyway.The other readings re-walked indexes each other had just finished with. The timeline, both breakdowns and the latency histogram are four foldings of one index over one window; the top-domain list and the unique-domain count are two foldings of another. Asked as six separate statements — and round-robined by the read pool onto connections holding 2 MiB of page cache each — nothing was ever warm for the next one.
range_metrics_sincegroups at(bucket, blocked, cached, has_result)and(query_type, response_ms);domain_stats_sincereturns the top list and the distinct count from one materialized CTE. The single-purpose functions/api/stats/*calls are now folds over the same statements, so there is one SQL spelling per fact.Measured
Same harness, same database, 7-day window:
range_stats(all of the above)Tests
tests/stats_page_miss_bench.rs— reports the figure against a real database (BENCH_DB=… cargo nextest run --release --no-capture --run-ignored only stats_page_miss_bench).tests/stats_page_miss_test.rs— three guards.the_outcome_breakdown_never_reads_the_log_tablewas observed failing with theINDEXED BYremoved (read 1470 of the database's 1892 pages) and passing with it restored.stats_db_test.rsgains five cases pinning the folded results to what the separate queries returned, including empty-resultclassification and outcome precedence.migration_v12_puts_the_outcome_flag_in_the_metrics_indexcovers both arrival paths — fresh database and one migrated from v11.cargo nextest run665/665,cargo fmt --check,cargo clippy --all-targets -- -D warningsandcargo deny checkall clean.Incidental fix
add_column_if_missingprobedpragma_table_info, which omits generated columns. A fresh database — whoseCREATE TABLEalready declareshas_result— would have been told the column was missing and failed the migration onduplicate column name. It usespragma_table_xinfonow.A latent dismissal bug this exposed
e2efailed onThe next-step banner can be dismissed and stays dismissed, reproducibly, and bisecting it across four throwaway branches put the blame on the domain CTE — which on an empty database produces byte-identical responses to the query it replaced, and measures the same to a tenth of a millisecond. It was not the cause; it was the perturbation.NextStepBannertakes the notice out of the DOM the moment the form is submitted and posts the dismissal in the background, so the operator never waits on it. Withoutkeepalivethat post is an ordinary fetch tied to the document: navigate in the same breath — click a nav link, reload — and the browser cancels it. The setting is never written and the notice is back on the page they land on.mainwins that race often enough to stay green; this branch's timing lost it every run.Confirmed by experiment rather than inference: the same tree with
keepalive: trueadded and nothing else changed goes green.The dismissal is also a real form post without JavaScript, and that path was never affected — verified against the running binary.
Notes
ANALYZE: 0.3 s on an SSD, a one-time cost of perhaps ten seconds on an SD card at first boot after upgrading.(domain, timestamp)cannot be restricted by a timestamp range. A(timestamp, domain)index would only read the window whenever retention exceeds the range on screen — at the cost of another index on the insert path.🤖 Generated with Claude Code