perf(stats): answer top domains and top clients from one index scan - #277
Merged
Merged
Conversation
Both lists are always asked for together, by the Statistics page and by every dashboard tick, but each read its own group-first index: (domain, timestamp) and (client_ip, doh_token, timestamp). Because timestamp was the last column, a time window could not narrow either scan. Migration 15 replaces the client index with idx_query_logs_ts_domain_client on (timestamp, domain, client_ip, doh_token). traffic_lists_since groups by (domain, client_ip, doh_token) once and builds both lists in Rust. (domain, timestamp) stays, because the query log's domain prefix search depends on it: without it, searching an unseen prefix reads the whole table (12 170 pages instead of 3). On a 370k-row database, a whole-table Statistics visit drops from 9 758 to 7 937 page misses and a dashboard tick from 4 755 to 3 113. The cost is 8.4 MiB more index; pages written per 500-row batch stay at 52-53. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #277 +/- ##
==========================================
- Coverage 91.27% 91.14% -0.13%
==========================================
Files 31 31
Lines 11342 11401 +59
==========================================
+ Hits 10352 10392 +40
- Misses 990 1009 +19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Summary
The Statistics page and every dashboard tick ask for top domains and top clients together, but each list read its own index:
(domain, timestamp)and(client_ip, doh_token, timestamp). Both indexes put the grouped columns first, so the time window could not narrow either scan. A 24-hour question skip-scanned most of each index anyway.idx_query_logs_client_ts(only top clients used it) and addsidx_query_logs_ts_domain_client ON (timestamp, domain, client_ip, doh_token).traffic_lists_sincegroups by(domain, client_ip, doh_token)once and builds top domains, the distinct-domain count and top clients in Rust.compute_range_stats(Statistics) and the newcompute_top_domains_and_clients(dashboard SSR and thestatssnapshot) call it once.top_clients_sincenow reads from it.idx_query_logs_domain_tsstays. The query log's domain search seeks it by prefix. Without it, searching a prefix nobody has queried reads the whole table:domain_stats_since(domain suggestions, API) keeps its statement. Both spellings now break ties by name, so they return the same rows.Page misses
All figures come from the same 370k-row database (7 days of data, default retention).
Statistics page,
stats_page_miss_bench, 30d (whole table). The previous run onmainread 9 758:Dashboard tick, each statement on a fresh connection. The folded lists match the two statements they replace (diffed):
Tests
stats_db_test::traffic_lists_answer_what_the_separate_lists_did: 7 domains × 5 IPs × with/without a DoH token, with tied counts and a row just before the window. Domains matchdomain_stats_sincerow for row, and clients match counts computed directly from the entries. An empty-window case is included.stats_page_miss_test::a_short_window_reads_a_short_stretch_of_the_traffic_lists: a tenth of the window must read under a quarter of the pages, which holds the index to being timestamp-first.the_page_reads_less_than_its_readings_do_separatelynow also counts the client list.db::tests: the fresh schema has the new index and no client index; a v9 database migrates to the same state; the lists' plan isCOVERING INDEX idx_query_logs_ts_domain_client.cargo nextest run: 680 passed. fmt and clippy clean. E2E was not run locally; CI runs it.🤖 Generated with Claude Code