Bound the Query Store backfill candidate read at its horizon (#4197)

[erikdarlingdata]

Part of #4197.

Why

Lane D3's design (issue comment 5830310090) and the coordinator's ruling (5834871359) measured
that the Query Store backfill's candidate read (SELECT DISTINCT database_name ... collection_time > now - 7 days) and its per-database MIN(last_execution_time) floor lookup both bind at a window
wider than raw retention. Neither predicate excludes any chunk as a result. On the design lane's rig,
the heavy-server candidate read cost 206 ms and 21,411 buffers. Bounded at the backfill's own floor
(floorLimit, from HorizonFor), the design measured 1.7 ms and 792 buffers. This lane implements
the ruling's decisions 1-4 (part a) for both Darling and Lite. Part b (persisting ring-buffer
watermarks) is explicitly out of this lane and waits for a field measurement after #4209.

What changes

• QueryStoreBackfill.RunServerSliceAsync (Darling) now computes floorLimit (HorizonFor)
  before calling GetCandidateDatabasesAsync, not after. It binds floorLimit as the candidate
  read's own collection_time lower bound in place of the old 7-day CandidateWindow (removed).
• GetCandidateDatabasesAsync unions the store's bounded result with every database a hole key
  already names. It does this through a new shared helper, QueryStoreBackfillState.MergeHoleDatabases
  (sorted ordinal, deduped). A database that goes fully quiet mid-hole (no row newer than the horizon)
  is still serviced this way, instead of silently aging out of the scan the moment its recent rows
  expire.
• GetStoredFloorAsync takes its exact bounded form: EXISTS ... collection_time <= floorLimit LIMIT 1 first. A hit means the database is done (see the method's doc comment for why that is
  exact: last_execution_time never exceeds its own row's collection_time). Otherwise it falls
  back to MIN(last_execution_time) bounded at collection_time > floorLimit, which ranges over the
  exact same rows the old unbounded MIN did once the EXISTS branch is ruled out.
• Lite's twin (RemoteCollectorService.QueryStoreBackfill.cs) gets the identical shape: the same
  bound, the same hole-key merge, the same exact floor form, using the shared
  QueryStoreBackfillState.MergeHoleDatabases helper. Lite persists nothing (ruling item 3).
• Stale comments fixed. The "inert" claim on the candidate predicate is corrected in two places:
  the inline comment, and ServiceCommandDeadlines.QueryStoreBackfillReadSeconds's doc comment, which
  also claimed the MIN "cannot be bounded." Both now say what is true after this change. The 120 s
  timeout value itself is unchanged: it is now a wide safety margin rather than a close one.
• CandidateWindow (the unused 7-day constant) is removed.

Test plan

• Darling/Darling.Tests/Darling.Tests.csproj builds with 0 Warning(s), 0 Error(s). (R1, unverified by R2)

• Lite.Tests/Lite.Tests.csproj builds with 0 Warning(s), 0 Error(s). (R1, unverified by R2)

• PerformanceMonitor.Darling.Tests.QueryStoreBackfillTests: 12/12 pass, including 3 new tests
  (the pure MergeHoleDatabases unit test, and the Darling + Lite SQL-shape pins). (R1, unverified by R2)

• Lite.Tests.QueryStoreServerGateTests: 13/13 pass. (R1, unverified by R2)

• Live test on a seeded hypertable with compressed chunks: new file
  Darling/Darling.Tests/QueryStoreBackfillCandidateLiveTests.cs.
  CandidateDatabases_ServicesAQuietHole_DropsAFinishedDatabase_FindsANewDatabase — a hole on a
  quiet database (quiet_hole_db) is still serviced, a finished database (finished_db, no hole
  key, nothing newer than the floor) drops out, a new database (new_db) is found, asserted as one
  exact-list equality so the candidate method's own exception-swallowing cannot pass on a dead
  read. StoredFloor_ARowAtOrBeforeTheFloor_ReturnsTheFloor_ABackfilledRow_ReturnsItsTrueMin —
  GetStoredFloorAsync returns floorLimit itself for quiet_hole_db (EXISTS hit) and the true
  backfilled last_execution_time (2026-05-01, not floorLimit, not collection_time) for
  new_db (EXISTS miss, falls through to the bounded MIN). Compression is bounded with
  older_than so only the old (2026-06-10) chunk compresses, proven against
  timescaledb_information.chunks.is_compressed before either test asserts anything else — the
  bare show_chunks() call the repair test's own pattern uses would have compressed the hot chunk
  too and made the plan check below impossible to pass. (R3a)

• Plan check and EXISTS numbers:
  CandidateRead_BoundAtItsOwnHorizon_ScansNoCompressedChunk EXPLAINs
  QueryStoreBackfill.CandidateSql with LITERAL values substituted for $1/$2 in place of
  parameters (a parameterized EXPLAIN can defer chunk exclusion to executor startup and leave
  never-executed Decompress nodes in the text) — no DecompressChunk, no ColumnarScan, no
  mention of the compressed chunk's name, and at least one (the hot) chunk still scanned.
  StoredFloorExistsMiss_ForANewDatabase_WalksTheCompressedChunk_MeasuredCost runs EXPLAIN (ANALYZE, BUFFERS) on the EXISTS text for new_db (no rows at or before floorLimit, so it has
  to walk the compressed old chunk before concluding no match — a Custom Scan (ColumnarScan) on
  the compressed chunk is expected and pinned here, not a defect). Measured on this test's tiny
  3-row fixture: execution Buffers: shared hit=3, Execution Time: 0.099 ms; planning shared hit=796 read=4, Planning Time: 9.027 ms. Small in absolute terms because the fixture itself is
  tiny — the pinned fact is the SHAPE (the EXISTS miss does reach the compressed chunk), not the
  number, per the ruling not to design against it. (R3a)

• Lite twin DuckDB fixture test: flipped GetBackfillCandidateDatabasesAsync and
  GetMinCollectedTimeForDatabaseAsync from private to internal (visibility only) and added
  Lite.Tests/QueryStoreBackfillCandidateTests.cs with the same three database cases and two floor
  facts as the Darling live test. Lite.Tests.QueryStoreBackfillCandidateTests: 2/2 pass. Verified
  both tests fail against dev's pre-Ring-buffer collectors re-derive their watermark with an unbounded MAX that decompresses every retained chunk each cycle; persist it at write time instead #4197 shape (no floorLimit/state parameters, no hole merge,
  an unconditional MIN with no EXISTS clamp) by temporarily reverting the bound/merge to that
  shape in a local build, confirming both failed, then restoring the fix and confirming green
  again. (R3b)

• git merge origin/dev (Lite side): merged cleanly, no conflicts, pushed. (R3b)

• Full Lite suite once, after the dev merge: Lite.Tests.exe — 5403/5403 pass, 0 errors,
  0 failed, 0 skipped, 343.4s. (R3b)

• git merge origin/dev (Darling side): merged cleanly, no conflicts, pushed. (R3a)

• Full Darling suite once, after the dev merge, on a dropped-and-recreated darlingtest:
  Darling.Tests.exe — 14056 total, 0 errors, 3 failed, 49 skipped (other PG clusters this rig
  does not configure), 1 not run, 757.8s. All 3 failures were pre-existing census/pin gaps the
  Ring-buffer collectors re-derive their watermark with an unbounded MAX that decompresses every retained chunk each cycle; persist it at write time instead #4197 SOURCE change (R1) left stale, not caused by this lane's test-only work:
  CommandPlaneCommandTimeoutTests (x2) — GetStoredFloorAsync now builds two NpgsqlCommands
  (the EXISTS pre-check plus the MIN fallback) instead of one, so the command-plane census went
  from 9 to 10 and the hardcoded ExpectedSiteCount = 9 was never bumped; and
  RepoFileAdoptionTests.ExactlyOneFile_DeclaresTheSharedRepoFileReader (x1) —
  QueryStoreBackfillTests.cs declared its own private ReadRepoFile instead of calling the
  shared RepoFile.ReadRepoFile, tripping the "exactly one declarer" adoption census. Both are
  mechanical and fixed here: bumped ExpectedSiteCount to 10 (plus its doc comment), and replaced
  QueryStoreBackfillTests.cs's two calls with RepoFile.ReadRepoFile and deleted the duplicate
  method and its now-unused usings. Re-verified with a targeted run of the 5 affected classes
  (CommandPlaneCommandTimeoutTests, RepoFileAdoptionTests, QueryStoreBackfillTests,
  QueryStoreBackfillCandidateLiveTests, RepoFileResolutionEquivalenceTests) post-fix: 25/25
  pass, 0 failed. Did not re-run the full 14056-test suite a second time after the fix (context
  budget) — flagging this explicitly so the coordinator can re-run it if a second full pass is
  wanted before merge. (R3a)

• CI on 1e9ff7bc failed one test, DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks: the ExpectedSiteCount bump in CommandPlaneCommandTimeoutTests.cs added a second <summary> block. 3bc7f443 merges the two into one. DocCommentHygieneTests 77/77 and CommandPlaneCommandTimeoutTests 16/16 pass locally. CI decides the rest.

CHANGELOG entry

SECTION: Fixed
ENTRY:

• Query Store backfill's candidate check no longer reads old compressed chunks, and no longer drops a hole on a database that goes quiet ([Bound the Query Store backfill candidate read at its horizon (#4197) #4306]) - The backfill's candidate-database scan read across all of raw retention on every check, which decompressed chunks outside the backfill window: measured at 206 ms and over 21,000 buffers per check on a heavy server. It is now bounded at the backfill's own horizon (1.7 ms and 792 buffers in the design measurement). The per-database floor lookup now stops at the first row at or before that horizon instead of taking the minimum over every row. A database that stopped sending Query Store data while a repair "hole" was still open for it used to age out of the backfill scan and never get serviced. It now stays in scope until the hole is filled or expires. Both Lite and the Darling service get the change.
  REF:
  [Bound the Query Store backfill candidate read at its horizon (#4197) #4306]: Bound the Query Store backfill candidate read at its horizon (#4197) #4306

🤖 Generated with Claude Code

https://claude.ai/code/session_01FVjn4PBJN71NQXdFo6ZxNQ