Filed off the #2333 post-install measurement. With the plan-fetch walk gone, the per-database log split shows the phases that remain, and one of them does not subside after catch-up: wm, running 1–3 seconds per database per cycle on the use1 primaries box.
wm is not a query against the monitored SQL Server. It is a read against our own store:
SELECT MAX(last_execution_time) FROM query_store_stats WHERE server_id = $1 AND database_name = $2
(DarlingCollectorRunner.GetLastCollectedTimeForDatabaseAsync.) query_store_stats is a hypertable partitioned on collection_time, so a MAX over a different timestamp column with no time predicate has to touch every chunk in retention for that (server, database).
Measured on the live use1 store (106 GB, 5 chunks, 42 primaries)
Same server/database pair, EXPLAIN (ANALYZE, BUFFERS), current form vs. the same query with a collection_time predicate added:
|
current (unbounded) |
bounded to 3h |
| cold (first touch) |
25,766 buffer reads + 195 written (temp spill) |
5 chunks excluded, 29 ms |
| warm (cache primed) |
228 ms |
29.5 ms |
Warm is a 7.7x difference. The cold row is the one that matters for the field: ~200 MB of physical I/O per database per cycle, and the cost is a function of store size and cache residency, not of anything the monitored server is doing. So it degrades exactly where a customer is weakest — a long-lived store, a busier Query Store, slower disks, a box whose cache is not dominated by this one query. Our dogfood store is 5 chunks; a 90-day-retention store is far more.
The bound is provably free
Every consumer of this value ends up at max(stored, now - 1h):
WatermarkPolicy.MaxCatchup is 1 hour, and ClampCatchup floors anything older to now - 1h.
- A null result falls back to the definition's documented first-run window, which is 60 minutes (
QueryStoreCollector.BuildCutoffParameters) — the same instant.
So a row whose last_execution_time is older than an hour cannot change the answer: found or not found, the caller proceeds from now - 1h. Adding a time predicate is therefore behaviour-identical, not an approximation — the first thing to check when this is implemented, because "identical" is the whole justification.
Fix shape
- Bound the read on the partitioning column (
collection_time), which is what enables chunk exclusion — a predicate on last_execution_time alone would not prune. A row's last_execution_time cannot exceed its collection_time (you cannot collect an execution from the future), so collection_time > now() - margin cannot hide a qualifying row. Use a margin comfortably wider than MaxCatchup (3h against a 1h horizon) so clock skew between the monitored server and the store cannot bite.
- Do not hardcode the horizon in the generic helper.
GetLastCollectedTimeForDatabaseAsync serves any definition with a PerDatabaseWatermarkColumn; the 1-hour clamp is query_store's policy. Pass the bound in (nullable — no bound = today's behaviour) so a future collector without a clamp keeps correct semantics rather than silently inheriting a horizon it never agreed to.
- Check the siblings.
GetLastCollectedTimeAsync (server-scoped) and Lite's DuckDB twin have the same unbounded-MAX shape. Lite's store is smaller but the argument is identical, and a fix on one SKU only is the drift this codebase keeps paying for.
- While here:
idx_query_store_stats_server_db_query_plan_time exists but is not serving this MAX. Worth checking whether the bounded form wants a supporting index or whether chunk exclusion alone is enough — the measurement above says exclusion alone gets it to 29 ms, so an index is probably unnecessary write tax.
Why this is worth doing before the release
Erik's framing: "there's no accounting for local factors, e.g. busier query stores, worse hardware." This particular multi-second cost is not the customer's SQL Server at all — it is our store, it scales with how long they have been running, and it is removable by adding a predicate that provably cannot change the answer. That is the cheapest juice left in the collection path.
Filed off the #2333 post-install measurement. With the plan-fetch walk gone, the per-database log split shows the phases that remain, and one of them does not subside after catch-up:
wm, running 1–3 seconds per database per cycle on the use1 primaries box.wmis not a query against the monitored SQL Server. It is a read against our own store:(
DarlingCollectorRunner.GetLastCollectedTimeForDatabaseAsync.)query_store_statsis a hypertable partitioned oncollection_time, so aMAXover a different timestamp column with no time predicate has to touch every chunk in retention for that (server, database).Measured on the live use1 store (106 GB, 5 chunks, 42 primaries)
Same server/database pair,
EXPLAIN (ANALYZE, BUFFERS), current form vs. the same query with acollection_timepredicate added:Warm is a 7.7x difference. The cold row is the one that matters for the field: ~200 MB of physical I/O per database per cycle, and the cost is a function of store size and cache residency, not of anything the monitored server is doing. So it degrades exactly where a customer is weakest — a long-lived store, a busier Query Store, slower disks, a box whose cache is not dominated by this one query. Our dogfood store is 5 chunks; a 90-day-retention store is far more.
The bound is provably free
Every consumer of this value ends up at
max(stored, now - 1h):WatermarkPolicy.MaxCatchupis 1 hour, andClampCatchupfloors anything older tonow - 1h.QueryStoreCollector.BuildCutoffParameters) — the same instant.So a row whose
last_execution_timeis older than an hour cannot change the answer: found or not found, the caller proceeds fromnow - 1h. Adding a time predicate is therefore behaviour-identical, not an approximation — the first thing to check when this is implemented, because "identical" is the whole justification.Fix shape
collection_time), which is what enables chunk exclusion — a predicate onlast_execution_timealone would not prune. A row'slast_execution_timecannot exceed itscollection_time(you cannot collect an execution from the future), socollection_time > now() - margincannot hide a qualifying row. Use a margin comfortably wider thanMaxCatchup(3h against a 1h horizon) so clock skew between the monitored server and the store cannot bite.GetLastCollectedTimeForDatabaseAsyncserves any definition with aPerDatabaseWatermarkColumn; the 1-hour clamp is query_store's policy. Pass the bound in (nullable — no bound = today's behaviour) so a future collector without a clamp keeps correct semantics rather than silently inheriting a horizon it never agreed to.GetLastCollectedTimeAsync(server-scoped) and Lite's DuckDB twin have the same unbounded-MAX shape. Lite's store is smaller but the argument is identical, and a fix on one SKU only is the drift this codebase keeps paying for.idx_query_store_stats_server_db_query_plan_timeexists but is not serving this MAX. Worth checking whether the bounded form wants a supporting index or whether chunk exclusion alone is enough — the measurement above says exclusion alone gets it to 29 ms, so an index is probably unnecessary write tax.Why this is worth doing before the release
Erik's framing: "there's no accounting for local factors, e.g. busier query stores, worse hardware." This particular multi-second cost is not the customer's SQL Server at all — it is our store, it scales with how long they have been running, and it is removable by adding a predicate that provably cannot change the answer. That is the cheapest juice left in the collection path.