Bound the ring-buffer watermark read to a recent window, fall back to unbounded only on a miss (#4197)

[erikdarlingdata]

Part of #4197.

Why

The ring-buffer collectors' server-scoped watermark read (SELECT MAX(<col>) FROM <table> WHERE server_id = $1) recomputes the newest stored value every cycle, for every server, with no bound. On TimescaleDB that forces the planner to touch every retained chunk. It decompresses the compressed ones, so the cost grows with retention and fleet size rather than with the work being done. #2344/#2795 bounded this for query_store. They deliberately left the ring-buffer collectors unbounded: default_trace_events, system_health_events, job_history, the deadlock/XE families, cpu_utilization. query_store's bound relies on a catch-up clamp these collectors cannot accept, because clamping them truncates a legitimate multi-day catch-up.

Diagnosis, confirmed on the rig

Seeded a TimescaleDB hypertable with 33 one-day chunks: 32 days of 5-minute rows, one server. Compressed every chunk older than a day, matching TimescaleSupport.ChunkIntervalDays/CompressAfterDays (both 1 day).

• Unbounded MAX(event_time) WHERE server_id = $1 (the shipped, pre-fix shape): the plan visits all 33 chunks. A count of _chunk references in the plan text confirms every child chunk appears, each with its own per-chunk Sort/Scan. Buffers: shared hit=83. Production numbers from the issue, same read, real scale (903k rows/server): 106 ms quiet-minute, mean 344 ms, max 21.4 s, 8.9M blocks read over 5h.
• Bounded ... AND collection_time > now() - 6h: Chunks excluded during startup: 32. One Index Scan on the current chunk only. Buffers: shared hit=4, Execution Time: 1.076 ms.
• Bounded probe over an empty window (no rows within 6h): returns NULL. That correctly signals "the true watermark is older than the window" rather than "first run": that is the case the fallback below exists for. The plain unbounded MAX on the same data still finds the true, older value.

Diagnosis confirmed as stated. The unbounded read's cost is structural: every chunk gets touched. Bounding on collection_time (the partitioning column) lets TimescaleDB's chunk exclusion prune it to the current chunk.

What changes

Rather than persisting watermarks (a store migration), this bounds the existing read and proves the bound changes no answer.

• WatermarkPolicy.RecentWatermarkWindow (PerformanceMonitor.Collectors/WatermarkPolicy.cs, shared with Lite): 6 hours. That sits comfortably inside the 1-day chunk width/compress_after, and far above any collector's actual cadence of 1-15 minutes.
• DarlingCollectorRunner has four server-scoped/per-database watermark readers: GetLastCollectedTimeAsync, GetLastCollectedTimeWithFrameAsync, GetLastCollectedTimeForDatabaseAsync, GetLastCollectedInstanceIdAsync. Every caller except query_store's clamped path is unaffected by that clamp, and for those callers each reader now probes collection_time > now - RecentWatermarkWindow first. When the probe finds no row, it re-runs the true unbounded MAX as a fallback. That covers a gap wider than the window or a genuinely empty table. A probe hit is provably the same value an unbounded scan returns: nothing outside the window can be newer than something inside it. A miss still gets the exact answer through the fallback. The watermark never changes, including right after a restart or after a long outage: that is the case the fallback exists for.
• GetLastCollectedInstanceIdAsync (job_history's numeric instance_id watermark) previously had no bound at all, ever. It now gets the same treatment, bounded on collection_time, the partitioning column, since instance_id itself does not partition the table.
• Two new SQL-builder pins join the existing BuildServerWatermarkSql/BuildServerWatermarkPairSql: BuildServerWatermarkForDatabaseSql and BuildServerWatermarkInstanceIdSql. Every one of the four readers' shipped SQL text is now pinnable, matching the file's existing convention.
• QueryStoreBackfill.GetCandidateDatabasesAsync is the "same class, different read" the issue also flags: a DISTINCT database_name scan, not a watermark. This PR does not touch it. See "Item 2" below for a second lane's diagnosis of why it stays open.

Lite parity (lane S3b)

Lite/Services/RemoteCollectorService.cs has the same three unbounded shapes: GetLastCollectedTimeAsync (~1410), the UTC-twin pair read GetLastCollectedTimeWithFrameAsync (~1452), and the numeric instance_id read GetLastCollectedInstanceIdAsync (~1531). All are SELECT MAX(...) WHERE server_id = $1 with no bound, mirroring Darling's pre-fix shape. (GetLastCollectedTimeForDatabaseAsync, ~1497, already has a bounded/unbounded ternary from #2344.)

#2800 (closed by PR #2879) already measured this exact question and method. Against DuckDB 1.5.5 at 50M rows, bounding GetLastCollectedTimeAsync was a 65% LOSS in the one-server case (0.34 ms unbounded vs 0.56 ms bounded). It was only a 9.8x win in a five-server case that still saved just ~7 ms per cycle. GetLastCollectedTimeAsync's own doc comment records this and names it deliberate.

Lane S3b re-ran the same measurement at the scale of this issue's brief: 30 days, 5 servers, Lite's generated DDL, the SHIPPED idx_<table>_time(server_id, collection_time) index. It ran against the actual ring-buffer tables named in #4197, not just query_store_stats:

• Seeded default_trace_events and job_history: 5 servers x 30 days x 1-minute cadence = 216,005 rows/table, physically interleaved across servers.
• EXPLAIN ANALYZE, unbounded vs bounded on collection_time > now - 6h (WatermarkPolicy.RecentWatermarkWindow):
  • default_trace_events MAX(event_time): unbounded 1.4 ms total time (43,201 rows touched) vs bounded 0.9 ms (360 rows touched).
  • job_history MAX(run_datetime): unbounded 0.9 ms vs bounded 1.0 ms. Bounded was slower.
  • job_history MAX(instance_id), the previously-never-bounded numeric watermark: unbounded 0.8 ms vs bounded 0.9 ms. Bounded was slower again.
  • A 100-iteration warm timing loop on default_trace_events confirms the plan-level numbers: 0.7111 ms/call unbounded vs 0.6878 ms/call bounded, a 3% difference inside run-to-run noise.

At Lite's realistic scale, DuckDB's per-row-group zonemap already answers the unfiltered MAX without a full scan, exactly as #2800 found. Adding the collection_time predicate does not reliably reduce work and twice made the query marginally slower. This holds for both tables tested, including the numeric instance_id case #2800 never covered. GetLastCollectedTimeWithFrameAsync (cpu_utilization only) runs the identical mechanism: two MAX aggregates over one table scan. So the same conclusion applies without a third seeded table.

The bound is not being ported to Lite. No version of this predicate reduced rows scanned, row groups touched, or wall time on the tables #4197 actually names, and it twice cost more. Shipping it trades a clear TimescaleDB win for a DuckDB loss, the opposite of what #4197 asks for.

Item 2: QueryStoreBackfill.GetCandidateDatabasesAsync (lane S3b)

Diagnosed on a rig instead of left untouched on faith. The shipped SQL already carries a bound: collection_time > $2 with CandidateWindow = 7 days, and its inline comment calls that bound inert because CandidateWindow is "wider than raw retention."

That claim resolved exactly: RetentionTierRouter.RawMaxAge (TimescaleSupport.RawRetentionSpan = 4 days, minus a routing margin) is the raw query_store_stats retention once continuous aggregates take over older history. 4 days is narrower than the 7-day CandidateWindow, so on a rollup-enabled store query_store_stats never holds a row older than ~4 days. Every row already satisfies the predicate, and TimescaleDB has nothing to exclude. That matches the issue's own cited production EXPLAIN ("shared hit=176417 across all chunks").

The predicate CAN exclude chunks when raw data outlives the window. Confirmed: seeded 33 one-day chunks (no rollup-driven pruning applied) via PgMigrations plus TimescaleSupport.CreateHypertableSql/EnableCompressionSql, then ran EXPLAIN (ANALYZE, BUFFERS) on the exact shipped SQL. With the 7-day bound, only 8 of 33 chunks appear in the plan. Execution buffers: 901-1030 vs 1633 unbounded. Planning buffers: 74 vs 29,015 for a literal-constant floor.

That setup is the PlainStoreHorizon shape, not the RollupStoreHorizon shape the comment is actually about, so it does not contradict the comment. It just is not production's configuration.

The comment is accurate for the shape that matters (continuous aggregates active), and the existing predicate genuinely cannot help there. Narrowing CandidateWindow further only makes an already-inert bound more inert.

A real fix needs the issue's own suggested shape. Read the candidate list from a maintained per-server set or the latest database_states snapshot instead of DISTINCT over raw. That is a design-level change covering what maintains that set, when it refreshes, and how a database that stops shipping rows ages out. That decision is outside a diagnosis lane. Posted as one comment on #4197 with this evidence. Item 2 stays open there.

Test plan

Rig: local TimescaleDB on port 55963 (original lane) and port 55961 (S3b diagnosis), both deleted/stopped after their runs.

• dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug: 0 Warning(s), 0 Error(s).
• Proved red against origin/dev. Reverted DarlingCollectorRunner.cs and WatermarkPolicy.cs to origin/dev (keeping the new tests) and rebuilt: 8 compile errors. All named the new symbols the tests pin: BuildServerWatermarkForDatabaseSql, BuildServerWatermarkInstanceIdSql, WatermarkPolicy.RecentWatermarkWindow. Restored the fix from a saved patch. Clean rebuild: 0 warnings.
• New pin tests (ServerWatermarkReadFloorTests): exact shipped SQL text for both new builders, plus a check that RecentWatermarkWindow stays under a day.
• New live tests (DarlingCollectorRunnerTests), each seeding real rows on a live Postgres/TimescaleDB rig. Each asserts the read returns the same value an unbounded MAX returns. Each covers an empty table, a gap wider than the window, a normal recent row, and a restart (fresh runner plus fresh data source). All green:
  • GetLastCollectedInstanceIdAsync_MatchesUnboundedMax_OnEmptyGapNormalAndRestart (job_history's numeric watermark, previously unbounded with no bound param at all)
  • GetLastCollectedTimeAsync_RingBufferWatermark_MatchesUnboundedMax_OnEmptyGapNormalAndRestart (default_trace_events, the issue's headline example)
  • GetLastCollectedTimeForDatabaseAsync_FallsBackToUnbounded_WhenProbeWindowIsEmpty (the per-database path deadlocks/blocked_process_report use on Azure SQL DB)
• Two existing live regression suites still pass unchanged: TimeHonestyRungLivePostgresTests for GetLastCollectedTimeWithFrameAsync, and PerDatabaseFaultPathSplitTests. Their normal-path, bounded-hit behavior is unaffected, and the fallback branch is structurally identical to the two methods tested exhaustively above.
• Targeted classes (DarlingCollectorRunnerTests, ServerWatermarkReadFloorTests, ServerWatermarkDispatchGateTests, TimeHonestyRungTests, TimeHonestyRungLivePostgresTests, PerDatabaseFaultPathSplitTests): 43 total, 0 failed, 3 skipped (SQL-Server-gated, expected).
• Full Darling.Tests suite once: 13757 total, 2 failed, 47 skipped, 1 not run, 687 s. Both failures (CaptureDownChunkOrderTests.TheShippedRead_ExecutesOnlyTheNewestChunk_AndTheNewestRunDecides_AgainstDevPostgres, ServerListAndSummaryPlanShapeTests.TheShippedReads_TouchFarFewerChunks_AndReturnTheSameNewestCollection_AgainstDevPostgres) are chunk-plan-shape tests in files this PR does not touch. darlingtest had not been recreated before the full run, so they carried chunk/compression state from earlier iterations. Re-ran both alone on a freshly recreated database: both pass clean, 0 failed. Confirmed environmental, not a regression from this change.
• Lite.Tests parity measurement: covered above. Lane S3b's diagnosis used a scratch xunit fact (seed + EXPLAIN ANALYZE, then a warm timing loop) against Lite.Tests, run and confirmed once, then deleted. No product or test code changed for Lite, so Lite.Tests and Darling.Tests both remain exactly as S3 already built and ran above. A second full-suite run at that cost re-confirms the same numbers and was not repeated.

CHANGELOG entry

SECTION: Fixed
ENTRY:

• Ring-buffer collectors no longer decompress the whole retention window every cycle ([Bound the ring-buffer watermark read to a recent window, fall back to unbounded only on a miss (#4197) #4209]). These collectors used to scan the entire stored history for every server on every cycle. On a large or long-retained store, that meant decompressing chunks unrelated to the current cycle. They now check a recent window first. They fall back to the full history only when the window is empty, such as after a long outage or on a brand-new server. The value returned never changes. Only the frequency of the expensive path changes.
  REF:
  [Bound the ring-buffer watermark read to a recent window, fall back to unbounded only on a miss (#4197) #4209]: Bound the ring-buffer watermark read to a recent window, fall back to unbounded only on a miss (#4197) #4209

Generated with Claude Code

https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3