Repository navigation
The forced-plan-failures alert read gets the covering index the planner will actually take — the composite it needed was already there and priced out, so it walked the whole fleet's two-hour slice per server (#3573) - #3586
Conversation
…er will actually take, instead of walking the whole fleet's two-hour slice per server (#3573) The read's (server_id, collection_time) composite was on the production store all along — V1 generates it — and the planner priced it out: server_id's physical correlation is 0.022, so the cost model charged one random page per row and preferred streaming the entire fleet's two-hour slice through the time index (Rows Removed by Filter: 691,058 to keep 37,878; 57,307 buffers; 422 ms warm, 10.3 s cold against a 10 s deadline). Forced under random_page_cost = 1.1 the same statement took the existing composite at 5,063 buffers, so a second plain composite would have been priced and ignored identically. Covering removes the heap term the model got wrong: (server_id, collection_time DESC) INCLUDE every other column the read touches, so it plans as an Index Only Scan over one server's rows (rig: 50 buffers vs 1,514, Heap Fetches: 0). It lives in PgTableTuning, not the ladder — results-invariant perf, autocommit, failure-isolated — as a plain CREATE INDEX: on TimescaleDB 2.28.1 compressed chunks get an 8 KB shell so only the live chunks carry it; transaction_per_chunk was measured leaving an invalid parent index after a mid-build cancel that IF NOT EXISTS then skips forever; CONCURRENTLY is refused on hypertables. Tests pin the index's column list against the read's own column references, the statement text against that list, the two rejected shapes, and (DARLING_TEST_PG) EXPLAIN the shipped statement on a seeded store for the Index Only Scan. The 1,744.9 ms deadline anchors keep their history and gain the arc; the 10 s deadline does not move.
There was a problem hiding this comment.
LGTM — reviewed the covering-index addition (idx_query_store_stats_server_time_forcing) in PgTableTuning, the doc-only updates to DarlingAlertReadAdapter/ServiceCommandDeadlines, and the new ForcePlanFailuresAccessPathTests. Verified: the INCLUDE column list matches every qs.* column the shipped ForcePlanFailuresSql references (checked both directions), the CREATE INDEX statement is derived from the same shared column list the test pins against (no duplicated literal to drift), the live EXPLAIN test's seed/delta math checks out (pass 1→pass 0 gives failure_delta=1, total_failures=7 matching the assertions), the statement is applied through PgTableTuning's existing per-statement failure-isolated loop (not the migration ladder, consistent with the PR's stated rationale), and it doesn't bump StorageVersion. No Lite/Darling parity gap — Lite's DuckDB schema carries none of the Postgres composer covering indexes, so this is Darling/Postgres-only tuning with no counterpart needed. No SQL injection concerns (index name/columns are compile-time constants). No blocking issues found.
The lie
#3573 reopened #3496's deferred second site with the cleanest evidence the series could have produced: since the v3.8.0 install the largest store's alerting-read failures were exactly three, all on
ForcePlanFailuresSql, newest at 10,322 ms against the 10 s deadline, while the fixed recent-runs site sat at zero. The liveEXPLAIN (ANALYZE, BUFFERS)on the shipped statement named the mechanism:Every server's read walks the entire fleet's two-hour slice of
query_store_statsthrough TimescaleDB's default time index and throws 95% of it away. Forty-three servers deep, every 30 s pass, that is the whole slice re-read forty-three times per cycle: 422 ms warm because this statement is what keeps the slice in cache, and a cold tail past 10 s whenever cache pressure evicts a slice ~4x the size it had when the deadline was derived (the table went from ~6 GB to 23 GB).The issue's diagnosis was that
query_store_stats"carries only a collection_time index" and the fix was the(server_id, collection_time)compositecollection_logalready has. That is where this PR had to stop and check, because V1 of the ladder generates exactly that index for every collector table.The index was there. The planner would not take it.
A read of the production catalog (the same SSM/psql path the issue's EXPLAIN used;
pg_indexeson the hypertable and on the very chunk in the plan) showsidx_query_store_stats_time (server_id, collection_time)present on both, beside the twoPgTableTuningindexes and the Timescale default. So the fix as proposed — a second plain composite, DESC-ordered — would have been a duplicate the planner priced and ignored identically, built across a 23 GB hypertable for nothing.Why it is ignored is in
pg_stats:server_id's physical correlation on the live chunk is 0.022 (forty-three servers interleaved by collection pass),collection_time's is 0.99999. The cost model therefore charges the composite one random page per tuple — ~50K pages atrandom_page_cost = 4— while the perfectly-correlated time index's 54,664-page stream costs 58,677 on paper and wins. The controlled toggle proves it is a pricing problem and not a catalog one: underrandom_page_cost = 1.1the identical statement took the existing composite (Bitmap Index Scan,Index Condon both columns) and touched 5,063 buffers instead of 57,307 —Heap Blocks: exact=5001for 67K rows, because a server's rows land in one contiguous run per collection pass (~13 rows a page) that a single correlation statistic cannot see. The composite was right the whole time, by 11x.The fix: make the choice stop depending on the cost model
A covering index:
(server_id, collection_time DESC) INCLUDE (database_name, query_id, plan_id, force_failure_count, is_forced_plan, plan_forcing_type, last_force_failure_reason)— the read's predicate as the key and every other column it touches as INCLUDE. The plan becomes an Index Only Scan whose cost is the index pages for one server's two hours and nothing else: no heap term to misprice, at anyrandom_page_cost, at any share of the fleet the busiest server grows into. Both uncompressed production chunks readrelallvisible= 100% ofrelpages(the insert-autovacuum overridePgTableTuningalready applies to this table is what keeps them there), so the visibility-map precondition an Index Only Scan needs is already met in the field.On a PG18 / TimescaleDB 2.28.1 rig seeded in the production write pattern (43 servers, per-pass contiguous batches, four compressed and two live day-chunks, the production's four indexes), the shipped statement reproduced the production plan exactly — time index,
Filter: (server_id = 1),Rows Removed by Filter: 18900— and with the covering index becameIndex Only Scan Backward … Heap Fetches: 0, 50 buffers against 1,514, at default costs.This is the same cure
PgTableTuningalready applied to this same table in 2026-07 ("a PLAIN index still paid a Bitmap Heap Scan re-fetching the aggregate columns, so the three composer indexes are COVERING") — the discipline existed one class over and had not reached the alerting pass.It lives in
PgTableTuning, not the ladder — and that is a correction to the brief, not a preferencePgTableTuning's own contract is results-invariant perf indexes: "must NOT bumpStorageVersion— that would gate the Viewer's connect-time schema check on perf indexes it does not need." Three more reasons this one belongs there, each measured:CREATE INDEX … WITH (timescaledb.transaction_per_chunk)is refused inside one ("cannot run inside a transaction block").PgTableTuningruns each statement autocommit.PgTableTuningstatement that does warns, the store keeps working, and the next start retries — the right failure mode for a build on a 23 GB hypertable.PgTableTuningruns after the TimescaleDB block (so the index is created on a hypertable and propagates to chunks) and before collectors start (so the build contends with no live inserts from this service).So there is no V127, no
StorageVersionbump, no viewer-probe sentinel, and noPgSchemaGeneratorchange: a fresh store and an upgraded store both get the index from the same startup statement.What the rig measured about compressed chunks, per-chunk builds, and locks
CREATE INDEXon the hypertable with 4 of 6 chunks compressed succeeded in 273 ms with no notice. Every chunk got a copy; the compressed chunks' copies are 8,192 bytes each — their uncompressed relations are empty shells on 2.28.1 — while the two live chunks got 46 MB and 27 MB. The owner's ruling, "index only the uncompressed/new chunks," needs no mechanism: it is what the engine does. New chunks inherit the index at creation;decompress_chunkfills the shell (56 MB) andcompress_chunkempties it again (8 KB). Both measured.transaction_per_chunkwas measured and rejected. It works autocommit and is idempotent withIF NOT EXISTS, but it takes the sameShareLockon the hypertable root first — no write-concurrency gain — and a cancel mid-build (which is exactly what aCommandTimeoutis) commits the chunks built so far, leaves the parent indexindisvalid = falseand the live chunk unindexed, after which the idempotentCREATE INDEX IF NOT EXISTSreports "already exists, skipping" on every start forever. A one-timeout permanent trap. The plain form is one transaction: a cancel leaves nothing, the next start retries.CREATE INDEX CONCURRENTLYis refused on hypertables ("hypertables do not support concurrent index creation").ShareLockon the hypertable root for its duration: reads proceed (AccessShare is compatible), every INSERT into any chunk queues behind it. Pre-collection, on a store with one writing service, that is nothing; a collision with the compression job on yesterday's chunk makes one wait for the other, and if that puts this build past its 300 s budget the outcome is the cancel-and-retry above.The cost, stated plainly — this is the part that wants the owner's eye
INCLUDE disables btree deduplication, so this index is ~86 bytes/row on the rig where V1's deduplicated composite is ~7. On the largest store that is roughly 0.7–0.9 GB per day-chunk (8–16 M rows/day against a 4.7–9.4 GB heap per chunk), one more btree maintained on every insert into the store's largest table, and a first build that reads both live chunks (~14 GB of heap, 24.6 M rows on the day of the catalog read) inside a 300 s budget with
maintenance_work_memat 1.6 GB and two parallel maintenance workers. The footprint is self-limiting — it is the one or two uncompressed chunks, and the compression policy reduces each to 8 KB a day later — but it is ~15x the plain composite the issue pictured. It is in the same class asidx_query_store_stats_query_hash, which the store already carries at 1,639 MB on today's chunk.Two cheaper shapes were considered and are deliberately not in this PR.
random_page_cost: it flipped the plan, but by 53,095 to 58,677 for a server holding ~8% of the fleet's rows, and that ratio scales with the share, so it flips back on the next-busiest store or this one a month on — and a store-wide planner setting is an owner's ruling for every read at once. A partial indexWHERE is_forced_planwould be a few MB, but the read aggregates unforced rows on purpose (a plan's previous sighting may be its unforced one), so it needs the statement reshaped and its first-sighting semantics changed. That is the cheaper long-run shape and the one to weigh against the footprint above; it is deferred rather than smuggled in.Census: the other reads of
query_store_stats32 sites (
grep -rn "FROM query_store_stats" Darling/ | grep -v Tests): 7 are continuous-aggregate definitions with no server predicate, 3 are ladder view re-expansions, 1 is this read. Of the remaining 21:QueryStoreBackfill's candidate-database scan,SELECT DISTINCT database_name … WHERE server_id = $1 AND collection_time > $2— an Index Only Scan on the new index is available to it.DarlingQueryStoreRegressionReader×2,ViewerDataService.QueryStoreRegressions×2), the top/trend readers (DarlingDataReader.GetQueryStoreTop,DarlingTrendReader×2,ViewerDataService.QueryStore×3,ViewerDataService.QueryTrends×2,QueryStoreTrendRouting×2). All areserver_id = $1 AND collection_time <range>with wide column lists, so the planner faces the identical composite-versus-stream choice for their uncompressed tail. Their windows are hours to days and mostly fall on compressed chunks, wherecompress_segmentby = 'server_id'already supplies the server dimension, so their exposure is the same tax at a different dose. Named here, not fixed here.(server_id, database_name, query_id[, plan_id])takeidx_query_store_stats_server_db_query_plan_time;MIN(collection_time)and theSELECT 1 … LIMIT 1probes are covered by the plain composite as it stands.What this PR does not do
DarlingAlertReadAdapter.AlertPassCommandTimeoutSecondsand bothServiceCommandDeadlinessites keep their history and gain the arc (6 GB → 23 GB, the fleet-width tax, 10.3 s, this index).ForcePlanFailuresSqlis byte-identical; its remarks gain a paragraph naming its access path and the consequence of referencing a column the index does not carry.StorageVersionbump, viewer probe, or generator change (above). Norandom_page_costchange. No partial index. NoCHANGELOG.mdedit (coordinator's).Tests
Darling.Teststargetsnet10.0-windows; this Mac has noMicrosoft.WindowsDesktop.App, so CI is the first execution of the new tests. Both affected builds ran clean here —dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Release(0 warnings, 0 errors) and the Linuxdotnet publishgate — and the pins were verified by construction against the committed source:ForcePlanFailuresAccessPathTests.TheCoveringIndex_CarriesExactlyTheColumnsTheReadReferences— parses everyqs.<column>out ofForcePlanFailuresSqland asserts the set equalsPgTableTuning.ForcePlanFailuresIndexColumnsboth ways (a missing column silently degrades the plan; a surplus one is bytes on every insert), and that the key is(server_id, collection_time).…TheStatement_IsBuiltFromTheColumnList_AndTakesNeitherMeasuredTrap— the literal statement equals the derivation from that list; notCONCURRENTLY, nottransaction_per_chunk, not partial.…TheShippedRead_PlansAsAnIndexOnlyScanOnTheCoveringIndex_AgainstDevPostgres(DARLING_TEST_PG) — builds the store the way the service does (ladder, hypertable conversion where TimescaleDB is present,PgTableTuning.ApplyAsync), checkspg_indexesfor the definition, seeds six servers' rows in per-pass contiguous batches,VACUUM ANALYZE, andEXPLAINs the exact shipped SQL with its real bound parameters: assertsIndex Only Scanon the new index, nocollection_time_idx, noserver_idapplied as a post-scanFilter, and that the adapter still returns the one seeded failure delta through the new path. The production failure was a plan choice with the right index already in the catalog, so apg_indexes-only check would have passed on the broken store; this one asks the planner. Its exact seed and EXPLAIN were run on the 2.28.1/PG18 rig and producedIndex Only Scan Backward using _hyper_1_1_chunk_idx_query_store_stats_server_time_forcingwith both predicates in theIndex Cond.PgTableTuningTests— the new statement's text, 8CREATE INDEX IF NOT EXISTS, 13 statements.Out of lane, observed while here
idx_query_store_stats_query_hash(the composer's covering index, 7 INCLUDE columns) is 1,639 MB on today's chunk withidx_scan = 0since that chunk was created, against 58 MB for the plain composite — that is one day of a Custom Views panel not being opened, not evidence of anything, but it is the footprint class this PR joins and worth knowing when weighing the cost above.