Skip to content

Darling builds a partial index at start so the Query Store legacy-row check reads an index instead of recent raw rows (#4972) - #5021

Merged
erikdarlingdata merged 4 commits into
devfrom
feat/4972-legacy-row-index
Oct 3, 2026
Merged

erikdarlingdata merged 4 commits into
devfrom
feat/4972-legacy-row-index

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Refs #4972. Part of #4952.

Why

Before a Query Store read uses the interval table, QueryStoreIntervalWide.ReadsTableAsync checks that the server's window holds no raw row without an interval start (HasLegacyRowSql, clause 6). Four reads run that check:

  • the Custom Views panels, once per server in scope (DarlingWebEndpoints.RunComposedPanelAsync);
  • get_query_store_top;
  • the viewer's Query Store grid;
  • the viewer's duration trend.

On a field store the answer is almost always no. To say no, the check fetched every raw heap row of the server's window, because the only index it could use is (server_id, collection_time) and the NULL test was a row filter. On a large production store that cost a mean of 4.1 s and 6.8 M blocks per call (#4969). Measured again for this PR, one run each for the busiest server: 614 ms at 24 h and 1,723 ms at 168 h (Production measurement, below). The #4969 figure is a mean over a day of calls, cold and warm.

What changes

  • One start-path index: CREATE INDEX IF NOT EXISTS idx_query_store_stats_server_time_null_start ON collect.query_store_stats (server_id, collection_time) WHERE interval_start_time_utc IS NULL, added to PgTableTuning.Statements, with its name as PgTableTuning.LegacyRowIndexName.
    • It holds only rows with no interval start, so it is empty on a field store. Its write and WAL cost is near zero.
    • It is built where the other query_store_stats indexes are built: at service start, after migrations and before the collectors, through GuardedCreate (its own transaction, a 5 s lock_timeout), and only when the catalog shows it absent. The hourly pass never builds it. It is not a migration, so there is no StorageVersion change.
    • TimescaleDB refuses CREATE INDEX CONCURRENTLY on a hypertable. A per-chunk or background build is unsafe while the service runs, because the Query Store backfill writes backdated rows into older chunks (Darling builds a background index so get_query_store_top no longer reads every entry a server holds (#4952) #4969, QueryStoreBackgroundIndexes).
  • No read's SQL changed. The check is still an exact read: the collector can still store a NULL start when its interval join misses. Only its plan changes.
  • Comments. HasLegacyRowSql's remarks and QueryStoreBackgroundIndexes now describe the index. The new statement's comment carries the measurements below.

How the build behaves on a compressed hypertable

Checked on a local TimescaleDB 2.30.1 rig:

  • CREATE INDEX on the hypertable builds one index per chunk from that chunk's own heap. Each compressed chunk gets an empty 8 KB index, and the compressed relations get none. So the build reads only the uncompressed chunks' heaps, not the compressed data.
  • After the build, the check reads compressed chunks through the columnar scan on their server_id segmentby index, as before (a few thousand buffers per chunk). It reads uncompressed chunks through the new index (1-2 buffers per chunk), where it used to walk the time index with a heap fetch per row.

Production measurement

Read-only, on the largest production store measured, at 15:05-15:26 UTC on 2026-10-03.

The build's read set. collect.query_store_stats had 17 chunks of 6 hours. The 12 compressed chunks had empty heaps. The 5 uncompressed chunks, about the last 27 hours, held 11.2 GB of heap and 19.7 M rows. One serial scan of every chunk's own heap with the index's predicate (FROM ONLY <chunk> WHERE interval_start_time_utc IS NULL, max_parallel_workers_per_gather = 0) is what the build reads:

Execution time 56.1 s
Buffers 99,670 hit, 1,370,459 read
I/O read time 48.3 s
Rows with no interval start 0

Applying the rig's build-to-scan ratio (0.5 with parallel workers to 1.2 with none and a cold cache) puts the first-start build at roughly 30-70 s. That is inferred, not a production build. SetupTimeoutSeconds is 300 s, so the headroom is over 4×.

The check today, before this change. HasLegacyRowSql for the busiest server, with the product's default parallelism, one run each, run right after the scan above:

Window Time Where it went
24 h 614 ms (15,782 hit, 35,007 read) Bitmap Heap Scans of the five uncompressed chunks, 667,802 rows removed by the NULL filter. The index serves this whole window.
168 h 1,723 ms (56,549 hit, 12,179 read) 1.34 s in the twelve compressed chunks' columnar scans (65-153 ms each, mostly disk reads), which this index does not reach; 0.38 s in the uncompressed chunks, warm from the 24 h run.

Limits

  • The first start after the upgrade pays the build once, before collection starts: roughly 30-70 s on the largest store measured (inferred above). A build that runs past SetupTimeoutSeconds (300 s) is rolled back and retried at the next start, and collection waits for it each time. The production scan above is the evidence that this store is well inside that budget.
  • The compressed part of the window still costs what it did. The index does not reach compressed chunks. On the production store that was 1.34 s of the check's 1.72 s at 168 h. On the rig it is the whole remaining cost at 168 h (5.9 ms, warm). Windows inside the last day or so, which is every 24 h read, are served by the index alone.
  • Prepared statements. As a prepared statement, the check's sixth execution switches to a generic plan with no chunk pruning: 57-76 s at 168 h on the rig without the index, 6.2 ms with it. The product does not prepare it today, so this is hardening, not a fix.
  • The trend readers' legacy arms (DarlingTrendReader ~:1176, QueryStoreTrendRouting ~:343, ViewerDataService.QueryTrends ~:271) filter query_store_stats the same way, so the index is eligible for them. Not measured. ViewerDataService.QueryTrends ~:378 reads the interval table, which this index does not touch.
  • PostgreSQL without TimescaleDB: the index is an ordinary partial btree. The plan pin accepts any read of it there.

Test plan

Rig: a local TimescaleDB 2.30.1 (PostgreSQL 18) rig with the real schema. 4 servers × 4 daily chunks × 855,360 rows per server per day (the field rate), 585 B per row, the 2 oldest chunks compressed and the 2 newest uncompressed at 2.0 GB of heap each.

  • The check, before and after. One server, bound parameters, median of 3:

    Window Without the index With it
    24 h 255-359 ms 0.043 ms
    168 h 1.7-3.0 s (126-138 k buffers) 5.9 ms (~7.1 k buffers, all in the compressed chunks' columnar scans)
  • The build: 0.59-0.66 s warm for 4.0 GB of uncompressed heap with the default 2 parallel maintenance workers, and 1.33 s with none. A serial scan of the same heaps takes 1.21-1.33 s, so the build costs about half that scan. With the cache restarted (not the OS page cache), 1.15 s against a 1.43 s scan. NULL-start rows cost about 35 B each in the index.

  • Plan pin (QueryStoreLegacyRowIndexLiveTests.TheShippedCheck_PlansOnThePartialIndex_AndAnswersExactly). It runs in the order of a first start after an upgrade: it seeds two servers' interval-stamped rows over three days in one-day chunks, compresses the older chunks where TimescaleDB is present (asserting at least one compressed and one uncompressed chunk, so the compressed half cannot pass vacuously), and only then runs the start-path tuning pass. From the catalog, each compressed chunk's copy of the index is exactly 8 KB, no compressed relation carries it, and a chunk created afterwards inherits it. It then EXPLAINs the shipped HasLegacyRowSql with its real parameter types. Every uncompressed chunk must be read through the partial index (an index-only, index or bitmap scan of it). No scan may carry the NULL test as a row Filter:. Compressed chunks must use the columnar scan. The check answers false for both servers, then true for one server only once a NULL-start row lands for it.

  • Start-path pin (TheStartPath_BuildsTheIndexOnce_AndTheHourlyPassNeverBuildsIt). After the first start-path pass the index exists with the exact definition and is valid. That a second start issues no CREATE is held by TuningHourlyCreateLiveTests, whose converged-store test now lists this index. With the index dropped, the hourly pass leaves it absent, returns Statements.Count - 1, and logs exactly one warning naming the index and collect.query_store_stats.

  • RED with the statement removed: both new tests failed, PgTableTuningTests failed on its exact-text assert, and TuningIndexReaderPairingTests failed with "Expected 5, Actual 4". Restored, all green.

  • The read's predicate is pinned: a TuningIndexReaderPairingTests fact holds interval_start_time_utc IS NULL exactly in HasLegacyRowSql, beside its server and time bounds, so the index's predicate stays provable from the read. RED with IS NULL mutated out of the read.

  • Pins updated: PgTableTuningTests (15 statements, 5 CREATE INDEX, the exact statement text), TuningIndexReaderPairingTests (the index paired with HasLegacyRowSql on server_id), TuningHourlyCreateLiveTests (the index list).

  • Targeted, census and guard classes with a store: 454 tests. The one failure, TuningDropLockRecoveryLiveTests.BlockedDrop_GivesUpAtItsTimeout…, fails the same way on dev on this machine and passes in CI.

  • Full Darling.Tests suite with a store, on macOS at the head (01a269f59): 21,185 tests, 481 failed, 244 skipped. Every one of the 481 also fails on dev on the same machine and store (Viewer/WPF, install and managed-store classes that need Windows); none fails only on this branch. CI decides those.

  • Builds: Darling.Tests and Lite.Tests with --no-incremental, 0 errors and no new warnings.

  • CI at the head (01a269f59): build, the three PostgreSQL test shards, the four Lite test shards, the whole-tree guards and the Linux build all pass.

CHANGELOG

SECTION: Fixed
ENTRY: - Query Store reads no longer scan the last day's raw rows to rule out legacy rows ([#5021]) - Before a Query Store read uses the interval table (Custom Views panels, get_query_store_top, and the viewer's Query Store grid and duration trend), Darling checks that the server's window holds no raw row without an interval start. That check read every recent raw row in the window: 0.6 s for one busy server at 24 h on a large store. Darling now builds a small partial index on those rows when the service starts, so a 24-hour window's check is answered from the index alone. Older, compressed data is still read as before, so a 168-hour window keeps that part of the cost (1.3 s of 1.7 s). Results are unchanged. The first start after the upgrade builds the index once, before collection begins; on a large store the build's read took about a minute.
REF: [#5021]: #5021

@erikdarlingdata erikdarlingdata changed the title Darling builds a partial index at start so the Query Store legacy-row check stops reading the server's whole raw window (#4972) Darling builds a partial index at start so the Query Store legacy-row check reads an index instead of recent raw rows (#4972) Oct 3, 2026

@erikdarlingdata erikdarlingdata left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude posting for Erik Darling

Verdict: the change is sound and can merge after small fixes. The statement, its start-path-only build and its lock handling are correct. I found no Defects. The main gaps are in the tests and the wording. The plan pin can pass its "over compressed chunks" claim without compressing any chunk. The live tests build the index on an empty hypertable, never on one that already has compressed chunks, which is the case a production upgrade hits. And two comments claim more than the production numbers show.

Claims checked:

  • VERIFIED, start path only: PgTableTuning.cs:418-463. A CREATE statement first runs the indisvalid catalog probe (:131-133). If the index is present, the statement counts and goes on. On the hourly pass a missing index only logs a warning and continues. Only the start path runs GuardedCreate (:111-112, 5 s lock_timeout, 300 s CommandTimeout).
  • VERIFIED by reading the code, not run: "abandoned and retried". This is a plain one-transaction CREATE, so a timeout or cancel rolls it back and leaves no INVALID index. ApplyAsync's catch issues a ROLLBACK. The sibling comment (:286-294) agrees: only the per-chunk form leaves indisvalid = false. An INVALID index of this name could only come from a hand build. The probe would then warn on every start and never rebuild it (:441-449). That is existing behaviour and not new here.
  • Nit on unchanged code: a start-path CREATE that hits its lock_timeout logs "the next pass tries again" (PgTableTuning.cs:474-477). For an index, only the next start tries again.
  • VERIFIED, the second-pass proof: QueryStoreLegacyRowIndexLiveTests.cs:128-147 copies the sibling (TuningHourlyCreateLiveTests.cs:62-78). A CREATE that was issued would wait out GuardedCreate's 5 s SET LOCAL (which overrides the session's 2 s) and fail with 55P03. The LockNotAvailable catch does not count it, so applied == Statements.Count proves that no CREATE was issued, not that one was skipped. The ALTER TABLE SET statements take a lock that ROW EXCLUSIVE does not block. The DROP IF EXISTS of an absent index takes no table lock.
  • VERIFIED, the plan pin is tight: :157-213. I can't name a plan text that passes wrongly on a fresh scratch store:
    • Every chunk-level Scan node must be a ColumnarScan, a read of the partial index, or a Bitmap Heap Scan whose next node is a Bitmap Index Scan on it.
    • A BitmapAnd with a second index fails, because the other Bitmap Index Scan is itself in scans.
    • A DecompressChunk node fails.
    • A heap Filter: on the NULL test fails.
    • The fresh store's compressed relations are _hyper_N_M_chunk_compressed, which :179 excludes.
    • Gaps: an upgraded store's compress_hyper_* child index would fail falsely. Index Scan Backward is rejected. Both are false failures, not false passes.
  • UNVERIFIED (needs a database): which TimescaleDB version CI uses. :205-208 requires the ColumnarScan node name. An older version that names the node DecompressChunk would fail loudly, not pass silently. PlanChunkScans already accepts both names.
  • REFUTED, partly: the comments claim more than was measured (see INLINE QueryStoreIntervalWide.cs:495 and PgTableTuning.cs:327).
  • VERIFIED, the PR title: "…reads an index instead of recent raw rows" holds. The compressed chunks are not "recent raw rows" in the sense the body uses, and the body states the 168 h remainder.
  • VERIFIED, the CHANGELOG ENTRY: it says only the 24-hour window is answered from the index alone, gives "1.3 s of 1.7 s", and calls the minute "the build's read", not the build. That matches the numbers. The 24 h claim also holds by reasoning: with CompressAfterDays = 1 (TimescaleSupport.cs:80), a chunk is compressed only after its whole range is more than a day old.
  • VERIFIED, the PR body: "inferred, not a production build" for 30-70 s; 300/70 is about 4.3, so "over 4x" holds (PgTableTuning.cs:351).
  • UNVERIFIED (needs a database): four TimescaleDB behaviours.
    • A hypertable CREATE INDEX over existing compressed chunks gives each one an empty 8 KB index and reads only the uncompressed heaps.
    • Recompression empties the partial index.
    • A partly compressed chunk (backfill into an old chunk) plans its heap part on this index.
    • Only the new-chunk inheritance is covered by a test (the index exists before the seeded chunks, :63).
  • VERIFIED, other checks:
    • The own-store marker sits on the line directly above the column-0 class (:29-30). There is no [Collection] attribute, the same as the sibling.
    • The source-text pins are one-line literals, so they do not depend on CRLF. The EXPLAIN parse calls ReplaceLineEndings (:159).
    • No CA1068, CA1859 or xUnit2013 bait: CancellationToken is last everywhere, and Assert.Equal(5, created.Count) is not a 0 or 1 count.
    • No host or store names appear.

Counts: 0 Defects, 4 Notes, 3 Nits.

Comment thread Darling/Darling.Tests/QueryStoreLegacyRowIndexLiveTests.cs
Comment thread Darling/Darling.Tests/QueryStoreLegacyRowIndexLiveTests.cs Outdated
Comment thread Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalWide.cs Outdated
Comment thread Darling/Darling.Tests/TuningIndexReaderPairingTests.cs
Comment thread Darling/PerformanceMonitor.Darling.Storage/PgTableTuning.cs Outdated
Comment thread Darling/PerformanceMonitor.Darling.Storage/PgTableTuning.cs
Comment thread Darling/Darling.Tests/QueryStoreLegacyRowIndexLiveTests.cs Outdated
…rect the comments

The plan test now follows the upgrade order (data and compressed chunks first, then the
tuning pass), asserts compression happened, that compressed chunks carry an 8 KB copy and
the compressed relations none, and that a new chunk inherits the index. A unit pin ties
the read's IS NULL predicate to the partial index. The hourly test asserts the single
warning; the duplicate lock block is dropped. Comments state the compressed-chunk read
and the measured figures precisely.

@erikdarlingdata erikdarlingdata left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude posting for Erik Darling

Round 2 at 01a269f59 is clean. All seven round-1 items are answered, there are no new findings, and nothing blocks the merge. I read the one-commit delta (4 files, +94/-31) and did not run it. CI is green at this head.

  • The vacuous compressed half: FIXED. A one-day chunk interval is set before seeding. With TimescaleDB on, the test now asserts that at least one chunk was compressed and at least one was not, so neither half of the plan pin can pass empty.

  • The production order: FIXED. The index is built AFTER the store holds data with compressed chunks, the way a production upgrade meets it. The catalog checks pin three things:

    • every compressed chunk's copy is the 8,192-byte empty index;
    • no *_compressed relation carries it;
    • a chunk created afterwards inherits it.

    The size query matches the index by its own definition (… WHERE (interval_start_time_utc IS NULL)), so another index can't satisfy it.

  • The HasLegacyRowSql remarks: FIXED. They now say the index serves the uncompressed chunks only, and that compressed chunks are still read by the columnar scan.

  • The predicate pin: FIXED. A new fact pins interval_start_time_utc IS NULL exactly, beside the server and time bounds. A rewrite the planner can't prove against the index's predicate now fails a plain build.

  • "(a few buffers)": FIXED. The comment now gives the measured figures.

  • "4.1 s per call": FIXED. The comment now says it is a mean over a day.

  • The duplicated block: FIXED. The ROW EXCLUSIVE block is gone; the sibling TuningHourlyCreateLiveTests still lists this index. The hourly pass now asserts Statements.Count - 1 and exactly one warning that names the index and its table.

On the two long comment lines (~:309-311 and ~:327-328): leave them. A rewrap doesn't justify another push and CI run.

Counts: no Defects, Notes or Nits.

@erikdarlingdata
erikdarlingdata marked this pull request as ready for review October 3, 2026 17:13
@erikdarlingdata
erikdarlingdata merged commit acf6c10 into dev Oct 3, 2026
21 of 22 checks passed
@erikdarlingdata
erikdarlingdata deleted the feat/4972-legacy-row-index branch October 3, 2026 17:20
erikdarlingdata added a commit that referenced this pull request Oct 3, 2026
…ery_stats and procedure_stats read from a small index (#4605) (#5046)

Darling builds two start-path partial indexes on (collection_time) WHERE sample_interval_seconds = 0, one on collect.query_stats and one on collect.procedure_stats, so the restart rows (a first sighting, an uncredited counter reset, or a gap past the policy, where no counter in the row was knowable) read from a small index. Two new readers in IntervalRollupRestartRows read those rows; no routing uses them yet, pending a production measurement after install. The readers carry presence and counts, not CPU.

The large start-path indexes, these two and the legacy-row index from #5021, now share one build per start: once one CREATE INDEX has been issued in a start's tuning pass, the remaining missing ones are deferred to the next start with an Information line naming each. An index that already exists never counts. On a timeout, a dedicated Warning names the index, the table, the elapsed time and that it retries at the next start, pausing collection for up to SetupTimeoutSeconds. The builds run before collectors start and never on the hourly pass. On the largest store measured, each build takes about 0.5-1.5 minutes.

Refs #4605
erikdarlingdata added a commit that referenced this pull request Oct 4, 2026
…rd, plan in the failure message (#5061)

QueryStoreLegacyRowIndexLiveTests' plan check seeds to a fixed time of day, 23:50 UTC on a fixed past date, instead of the wall clock. Chunks align to UTC midnight and the newest chunk stays uncompressed, so a run just after midnight left that chunk holding a page or so of rows. There a Seq Scan costs within a fraction of a point of the partial index, and the planner could skip the index; the test failed three times on CI between 00:08 and 00:30 UTC. The newest chunk now always holds a full day, a guard requires at least 1,000 rows in it, and both plan assertions print the plan on failure. The test still asserts that the shipped check plans on the partial index; no plan is forced.

Follow-up to #5021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant