Skip to content

Query Store backfill: the per-tick database list no longer walks the newest chunk's index (#4662) - #4703

Merged
erikdarlingdata merged 10 commits into
devfrom
fix/4662-candidate-read-cut-chunk
Sep 29, 2026
Merged

erikdarlingdata merged 10 commits into
devfrom
fix/4662-candidate-read-cut-chunk

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Closes #4662.

Why

Every backfill tick lists a server's databases with a read bounded at the backfill floor (collection_time > floor). The floor sits inside one chunk of collect.query_store_stats. Inside that chunk collection_time is the fifth key column of the skip-scan index, so the bound can only filter: the read walked the chunk's index entry by entry for every name with no row after the floor, and the cost grew as the chunk filled. Measured on a store monitoring 43 servers with a 12 h cut chunk, one round of these reads (buffers and ms are median / max / sum over the round):

Floor Read Buffers ms
20 min before chunk end today 11,229 / 24,457 / 495,218 11.6 / 44.3 / 560
same new 40 / 188 / 1,815 0.12 / 0.51 / 5.3
60 min before end today 40 / 1,764 / 3,542 0.11 / 1.5 / 6.2
same new 40 / 188 / 1,815 0.08 / 0.37 / 3.5
mid-chunk today 39 / 188 / 1,805 0.09 / 0.46 / 4.4
same new 40 / 188 / 1,815 0.07 / 0.36 / 3.4

The new read listed 0 extra names at every position in that sweep.

What changes

QueryStoreBackfill.GetCandidateDatabasesAsync (Darling only) picks its statement by store, using the existing _hasContinuousAggregates provider (true exactly when TimescaleDB is present). The union with recorded hole databases, the command timeout and the failure handling (log at Debug, return the hole databases) are unchanged.

TimescaleDB store, two steps. The floor is the same floorLimit as before, formatted yyyy-MM-dd HH:mm:ss.ffffff with the invariant culture. The boundary is read from the catalog at run time and never derived from a constant, because set_chunk_time_interval changes only chunks created after it.

-- CutChunkCatalogSql
SELECT range_start AT TIME ZONE 'UTC' AS cut_start, range_end AT TIME ZONE 'UTC' AS cut_end
FROM timescaledb_information.chunks
WHERE hypertable_schema = 'collect' AND hypertable_name = 'query_store_stats'
  AND range_start <= TIMESTAMP '<floor>' AT TIME ZONE 'UTC' AND range_end > TIMESTAMP '<floor>' AT TIME ZONE 'UTC'
-- CutChunkCandidateSql, when a row came back
SELECT DISTINCT database_name FROM query_store_stats
WHERE server_id = $1 AND collection_time >= TIMESTAMP '<cut_start>' ORDER BY database_name

When no chunk holds the floor, or the catalog read fails, the statement that was there before runs unchanged (CandidateSql, floor as a parameter). A catalog failure falls through to that statement; it does not reach the outer catch, which would drop the whole store list.

Plain PostgreSQL store: a walk of the index with no time bound, one seek per database (WalkCandidateSql).

WITH RECURSIVE walk AS (
  (SELECT s.database_name FROM query_store_stats AS s
   WHERE s.server_id = $1 AND s.database_name IS NOT NULL ORDER BY s.database_name LIMIT 1)
  UNION ALL
  SELECT (SELECT s.database_name FROM query_store_stats AS s
          WHERE s.server_id = $1 AND s.database_name > w.database_name ORDER BY s.database_name LIMIT 1)
  FROM walk AS w WHERE w.database_name IS NOT NULL)
SELECT database_name FROM walk WHERE database_name IS NOT NULL

Lite is unchanged on purpose: DuckDB has no chunks. Its statement is Darling's fallback statement, and a pin now says so.

Test support. The Npgsql command counter that WaitRateTileReadCountLiveTests carried as a private nested class moved, unchanged, into Darling.Tests/NpgsqlCommandCounter.cs, so the new pins and that test share one counter. No product code besides QueryStoreBackfill.cs changes.

Behaviour change

The new read also lists names whose rows are in [cut_start, floor] and none after the floor. Without a Done key such a name reaches the floor read (collection_time <= floor LIMIT 1), which hits, so the name is marked Done at once (one read and one state write, once per database; the MIN read and a slice never run for it; pin 5 measures this). Before this change the name got its Done key on its next active tick anyway, unless its rows left raw retention first; in that case the old code backfilled a first-contact tail when the database came back, and the new read does not. A recorded outage gap is still backfilled, because the hole check runs before the Done check in RunServerSliceAsync. That sentence now rests on pin 6: a database that holds both a done: key and a hole key has the hole's slice attempted, and the floor read never runs for it.

Test plan

  • Darling.Tests builds with 0 warnings.
  • QueryStoreBackfill* classes plus WaitRateTileReadCountLiveTests (it now uses the shared counter) on a TimescaleDB rig (PostgreSQL 18.6, UTC): 25 tests, 0 failed.
  • Full Darling.Tests suite on the rig, after merging dev: Total 16907, Failed 2, Skipped 64 (SQL Server end-to-end tests that need DARLING_TEST_SQL), Not Run 1. The 2 failures are MigrationUpgradeLadderLiveTests.PreviousReleaseStore_ClimbsTheCurrentLadder_ToTheTop for the v3.5.0 and v3.7.0 fixtures, both Npgsql.NpgsqlException: Exception while reading from stream (System.TimeoutException: Timeout during reading attempt) during the 18-minute full run. The class run alone on freshly dropped databases, same build: 5 tests, 0 failed. TrendPayloadBudgetLiveTests.EveryDefaultAnswer_StaysNearTheBudget_AndTheLargestAnswerStaysUnderTheCap passed in this run.
  • Live class QueryStoreBackfillCutChunkLiveTests (its own scratch databases), pins 1 to 4 with fixed floors:
    • Pin 1, exactness, TimescaleDB: a name with rows after the floor, a name with rows only between the chunk start and the floor (one exactly at the start), a name only in the previous chunk, a name on another server, a NULL name, and a hole key for a name with no rows. The list is the first two plus the hole name, in ordinal order.
    • Pin 2, exactness, plain PostgreSQL: every non-NULL name of the server, older ones included, nothing from another server.
    • Pin 3, catalog boundary across an interval change: rows in 1 d chunks, set_chunk_time_interval to 6 h, newer rows. A floor in an old 1 d chunk lists from that chunk's start; a floor in a new 6 h chunk lists from the 6 h start (a 1 d boundary would also list a name from the previous 6 h chunk; a 6 h boundary would miss a name early in the old 1 d chunk).
    • Pin 4, fallback: a floor no chunk holds gives the floor-bound list.
  • Pins 5 to 7, added here. Pins 5 and 6 run RunServerSliceAsync itself, so the floor is the wall clock's (now minus the horizon). They seed relative to it, and first wait until the floor is three minutes clear of a chunk boundary (1 d chunks end at UTC midnight), so a boundary cannot fall between the seed and the tick. Reads are counted in-process from Npgsql's ActivitySource, scoped to the scratch database, and a control call to GetStoredFloorAsync for a name with no rows must count read A once and read B once before any count is trusted. The server's connection string points at a loopback port that refuses, with Connect Timeout=2: a slice throws a connection SqlException (2.16 s on this Windows rig), and "no slice" means the tick returns false without throwing.
    • Pin 5, an extra name costs one read and one Done write, once: the name has rows only in [cut_start, floor) and no Done key. Tick one returns false, runs exactly one read A (SELECT 1 ... collection_time <= $3 LIMIT 1), no read B (SELECT MIN(last_execution_time)), and saves the done: key. Tick two runs no read for it.
    • Pin 6, a recorded hole on a Done database is still dug: the done: key and a hole key are both seeded, the tick throws the connection SqlException from the hole's slice, and neither floor read ran.
    • Pin 7, plan of the plain PostgreSQL walk: on a plain scratch store with the index the worker's start step creates (idx_query_store_stats_server_db_query_plan_time), 30,000 rows (two servers, five names) and ANALYZE, the EXPLAIN (ANALYZE, FORMAT JSON) plan of WalkCandidateSql must have no Seq Scan, and the recursive term must hold a Limit over an Index Scan or Index Only Scan of query_store_stats. The index is the planner's own choice; no enable_seqscan override. The plan is the analyzed one because on PostgreSQL 18.6 plain EXPLAIN leaves the correlated select out of the recursive term's plan.
  • RED for pins 5 to 8, each against a planted break in QueryStoreBackfill.cs. The four plants were built together, one hunk each, and run against the seven tests of QueryStoreBackfillCutChunkLiveTests: exactly pins 4, 5, 6 and 7 failed and the other three stayed green. The file was then restored (git status clean, no plant text left).
    • Pin 5, plant: the Done write for the extra name removed. Fails at "the first tick must save the Done key for the extra name, or every later tick reads for it again".
    • Pin 6, plant: the Done check moved above the hole check. Fails with Assert.Throws() Failure: No exception was thrown, expected SqlException.
    • Pin 7, plant: ORDER BY dropped from the walk's inner select. The planner then picks a Seq Scan on its own, and the pin fails at "the walk must not scan the table".
    • Pin 8 (RED for pin 4, the fallback): plant, ChooseCandidateSqlAsync returns the walk when no chunk holds the floor. Pin 4 fails: collections differ at index 1, expected tail_of_next_chunk, actual old. Before this, pin 4 passed only by construction (the old read is the fallback).
  • Earlier RED against the old read (the store switch forced back to the floor-bound statement, everything else as in this diff): the TimescaleDB exactness pin, the plain PostgreSQL pin and the interval-change pin fail; the fallback pin passes, because the old read is the fallback. It guards against a wrong fallback, not against the old code.
  • SQL-shape pins reworked on purpose: the store-switch statements are pinned (cut-chunk read bound with >= at the catalog value, catalog read qualified to the hypertable with UTC on both sides, walk with no time bound and no NULL names), Lite's twin statement must equal Darling's fallback constant (referenced, not copied), and the plan test also explains the cut-chunk read and requires that it scans no compressed chunk (CandidateRead_BoundAtItsOwnHorizon_ScansNoCompressedChunk stays green).

Also fixed: a failing database no longer blocks the ones after it

The Query Store backfill runs one slice per server on each pass and tries the databases in the same order every time. If one database's slice failed every time, it stayed first in line and the databases after it on that server never got a slice. Darling and Lite now count each database's failed slices in a row (QueryStoreBackfillFailureLedger, shared by both). After 3, that database is skipped while any other database on the server has work, and one Warning is logged when the skipping starts. When no other database has work, one skipped database is retried per pass, the one that failed longest ago first, so several failing databases take turns. A completed slice clears the count. The slice window still narrows per server after failed slices, as before: 60, then 30, then 15 minutes. A timeout usually means the whole server is loaded, so narrowing per database would only add timed-out queries against it. The per-server counter's code is unchanged.

Tests: QueryStoreBackfillSkipFailingDatabaseTests in Darling.Tests (7) and Lite.Tests (6) cover the stall, the retry when nothing else has work, the count reset, several skipped databases taking turns, the Warning logging once, and the window sequence a failing database sees (60, 30, 15, then skipped). With the skip disabled, 5 of 7 (Darling) and 4 of 6 (Lite) fail with "the healthy database was never sliced in 6 ticks; attempts: aaa_failing_db x6". With per-database narrowing put back, the window-sequence test fails in each product. The existing adaptive-shrink tests are unchanged and pass. Full suites on this part before the merge: Darling.Tests 16906 total, 0 failed (64 skipped); Lite.Tests 5561 total, 0 failed. After merging it here, both test projects build with 0 warnings and the QueryStoreBackfill* classes pass (Darling 31, with the 16 live ones skipped without a store; Lite 8).

CHANGELOG

SECTION: Changed
ENTRY:

SECTION: Fixed
ENTRY:

…newest chunk's index (#4662)

On a TimescaleDB store the list starts at the start of the chunk that holds the backfill floor, read from the
catalog at run time, so the skip scan seeks once per database instead of walking that chunk's index. On plain
PostgreSQL the list is a recursive walk of the index, one seek per database. A catalog read that returns no
chunk, or fails, falls back to the floor-bound statement unchanged.
…no longer blocks the databases behind it

A slice runs at most once per server per tick and the candidate list is in the same order every tick, so a database whose slice always threw was first in line forever and no database after it on that server ever got a slice.

Each (server, database) now keeps a consecutive slice-failure count, in memory and cleared by that database's completed slice. It narrows that database's own window (AdaptiveSpan) and, after 3 failures in a row, the loop serves the other databases first. A skipped database is retried on any tick where no other database has work, least recently failed first, so several skipped databases take turns. One Warning is logged when a database is first skipped.

The threshold (SkipAfterConsecutiveSliceFailures) lives in QueryStoreBackfillState, shared with Lite, and is pinned against AdaptiveSpan: the third failure is the first one at the narrowest window.
…longer blocks the databases behind it

The same stall as Darling: the per-server tick ran one slice, the candidate list came back in the same order every tick, so a database whose slice always threw stayed first in line and the databases after it never got a slice.

Lite now keeps the same per-(server, database) consecutive-failure count, shared through QueryStoreBackfillFailureLedger and QueryStoreBackfillState.SkipAfterConsecutiveSliceFailures. It narrows that database's own window, after 3 failures in a row the other databases are served first, and a skipped database is retried, least recently failed first, on a tick where nothing else has work. One Warning is logged when a database is first skipped.
…one database, and the plain-store walk's plan (#4662)

The extra name (rows only between the cut chunk's start and the floor) costs one floor read and one Done write, once. A hole on a Done database is still dug and never reads the floor. The walk's recursive term is a Limit over an index scan, not a Seq Scan.

The Npgsql command counter moves out of the WaitRate read-count test into a shared test class so both use one.
…the per-database count only decides which database to skip

The stall fix replaced the per-server failure count behind the adaptive shrink (#2111) with the per-(server, database) ledger, so the slice window narrowed per database. A command timeout usually means the whole server is loaded, and narrowing per database adds about two more timed-out queries per database against a server that is already struggling.

Both products keep the per-server count exactly as it was for the window: it grows on every failed slice, resets on any completed slice and feeds AdaptiveSpan. The ledger stays, but only for the skip decision: a database that fails 3 slices in a row is served after the databases behind it, is retried only when none of them has work (least recently failed first), and is cleared by a completed slice. The one Warning when a database is first skipped is unchanged.

The skip tests now run with the per-server window. From a fresh count, a failing database first in line is tried at 60, 30 and then 15 minutes and is then skipped. The database behind it runs at the 15 minutes the server has narrowed to, and its completed slice resets the count, so the skipped database's retries start again at 60 minutes.
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 29, 2026 02:50
@erikdarlingdata
erikdarlingdata merged commit 1248a67 into dev Sep 29, 2026
17 of 18 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/4662-candidate-read-cut-chunk branch September 29, 2026 03:03
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