Darling: the raw procedure duration trend reads per-collection totals, written with each procedure_stats batch (#5638)

[erikdarlingdata]

Claude posting for Erik Darling.

Fixes #5638. Two lanes, one PR: Lane A built the write side (table, writer, retention, MCP clock), Lane B the read side (the raw trend reads the totals; see ## Lane B below).

Why

The raw procedure duration trend sums every row of every collection from procedure_stats (about 560 to 1,060 rows per collection per server since #5463), so a first 24 h read touches about 60 K heap pages. Erik's plan (issue comment, 2026-10-09): the collector writes per-collection totals in the same transaction as the procedure rows, and the trend reads them. The same plan fixes the MCP duration-trend tools' tier decision, which read a second clock a few milliseconds after the window was computed.

What changes

Table and key (V176, procedure-stats-totals). collect.procedure_stats_totals, grain (server_id, collection_time, database_name), same column types as procedure_stats, no unit conversion:

column	type	value
server_id	integer NOT NULL
collection_time	timestamp NOT NULL	naive UTC like procedure_stats
database_name	text	nullable, as in procedure_stats
total_elapsed_time	bigint	SUM(delta_elapsed_time), NULL when every row is NULL
total_execution_count	bigint	SUM(delta_execution_count), NULL when every row is NULL
row_count	bigint NOT NULL	COUNT(*)
max_sample_interval_seconds	integer	MAX(sample_interval_seconds), NULL when every row is NULL

Unique index ux_procedure_stats_totals (server_id, collection_time, database_name) NULLS NOT DISTINCT, plus ix_procedure_stats_totals_time (collection_time) for the retention slice. No backfill: the rung is CREATE ... IF NOT EXISTS only and moves no row.

NULL database_name. procedure_stats.database_name is a nullable text, so the key is NULLS NOT DISTINCT (V170 already uses the form, so PostgreSQL 15 is the floor). The NULL database is one group, as GROUP BY makes it, and the upsert's conflict target matches that index. The totals keep the NULL group exactly as the raw rows have it.

Write path. ProcedureStatsTotalsWriter.UpsertBatchAsync (new, Storage), called from DarlingCollectorRunner.CopyBatchOnceAsync for procedure_stats batches only (chosen by definition.TargetTable, like the ledger), on the COPY's own transaction, after the COPY and the dimension flush, before the commit, no savepoint and no catch around the upsert itself (the gap fill ahead of it is the exception, see "Review round 1 follow-up (seat)"). A fault in the upsert throws out of the batch, the transaction rolls back, and the raw rows are not stored, so rows and totals commit together or not at all (live-tested). The statement reads back the rows the store holds for (server_id, collection_time) and upserts one row per database, setting every measure from EXCLUDED, so it recomputes and never adds: a second batch at the same collection_time, or a re-attempt, is exact. A batch with zero rows writes no totals row and costs no round trip (the read keeps its collection_log axis for an idle run). procedure_stats has exactly one writer, the runner's COPY (pinned by a source test on the other service files). EXPLAIN on the local test store shows the read-back using an index (functional check only; no performance numbers are claimed).

Registration, both store kinds. A plain table, not a hypertable, not a collector (not in CollectorCatalog.All): about one row per database per collection against a thousand raw rows, the same class as the V164 ledger and the V170 rollup. It sits in collect, so the blanket SELECT and owner default privileges cover it; no provisioning change. Chunk interval and compression do not apply. Retention (DarlingRetention) is a time-sliced DELETE at procedure_stats' own effective days (EffectivePurgeRetentionDays, so a per-collector override moves both) on both store kinds. ProcedureStatsTotals.PrunePredicate additionally keeps every totals row at or after the oldest procedure_stats row, because on TimescaleDB the raw chunks leave through the gated purge, which can hold them past the horizon: a totals row is never purged before the raw rows of its age. Version bump chain done: StorageVersion 176, the Viewer probe sentinel plus MapProbedSchemaVersion arm and GetBoolean call, and the older-rung tests (probe ordinals, rung ladder, CaggGroupIndexDropLiveTests artifact drop).

MCP clock fix. get_query_duration_trend and get_procedure_duration_trend now pass nowUtc: TierClock(as_of, now) as well as windowEndUtc. as_of does NOT choose the tier: with as_of the tier is still judged against the wall clock (retention runs on the wall clock, so the raw tier is whatever the store still holds now), using the single DateTime.UtcNow the tool already took. Without as_of, the window end is used as the tier clock instead of a second UtcNow read, so a 95 h window (start exactly RawRetentionSpan 96 h less RawTierMargin 1 h before its end) always routes Raw (ShouldUseRawTier, #2353). The wallClock parameter is the test seam.

Test plan

Local rig: PostgreSQL 18 + TimescaleDB, DARLING_TEST_PG set, built with 0 Warning(s).

• ProcedureStatsTotalsTests + ProcedureStatsTotalsWriterLiveTests: 16 passed, 0 failed. Live facts: totals equal a GROUP BY over the rows for a 2-database batch and for a restart collection (every interval 0); a second batch at the same collection_time recomputes; a re-run of the upsert changes nothing; NULL-interval rows stay NULL and a NULL database_name is one group; a refused totals write rolls back the COPY; the read-back plans an index scan; the retention statement keeps totals at or after the oldest raw row and deletes older ones. Clock: 95 h window routes Raw at 0, 1, 7 and 250 ms (and Hourly with the old late clock, for contrast).
• Census and ratchet sweep (*Census*, *Ratchet*, *Pins, *RungTests, *ViewerSchemaVersionGate*, *StoreSelfMetrics*, *DarlingRetentionTests, *Hygiene*, *CommandTimeout*, *MigrationLadder*, *QueryStatsHourLedger*): 1,464 tests, 17 failed on the first run. All 17 were this change's version bump (probe ordinals in five rung tests, the V170-rung ladder, one regex pin for the transaction term); fixed. Re-run of the fixed classes plus CaggGroupIndexDrop*, ProcedureStatsTotals*, DarlingMcpTrendToolsTests: 83 passed, 0 failed after one more fix (PlanRegressionRowcountRungTests, 72 passed, 0 failed on the second re-run with ViewerSchemaVersionGate*, MigrationDataMovingRungCensusPins, MigrationLadderPins). I did not re-run the whole 1,464-test sweep after the fixes.
• Full suite: not run on this laptop; CI decides.
• Revert-proof of the clock test against the old call shape: not run; the contrast case in A95HourWindowWithNoAsOf_RoutesRawEveryTime asserts the old late clock gives Hourly.
• No Lite parity: Lite's store is DuckDB with its own procedure trend and no rollup tier; the table is Darling-only.
• Merged origin/dev (one commit, Role provisioning stops before it sets a password when it cannot change the roles; store connection strings are reported without their values #5697) into the branch; no conflicts.

Lane B: the raw trend reads the totals

The statement. DurationTrendRouting.ProcedureCollectionsCte (the per-collection CTE under BuildBucketedRawTrendSql("procedure_stats", ...), so the MCP get_procedure_duration_trend, the Viewer chart and the web chart all get it with no caller change) now stitches stored at the per-server floor:

• totals_floor = SELECT COALESCE(MIN(collection_time), 'infinity'::timestamp) FROM collect.procedure_stats_totals WHERE server_id = $1. A server with no totals has the floor infinity, so every collection reads the rows, as before.
• stored = procedure_stats grouped by collection_time with collection_time < (SELECT floor_time FROM totals_floor), UNION ALL the totals table grouped by collection_time (every totals row is at or after the floor by definition). Each arm keeps the $2 - 1 hour to $3 bounds. No collection is read from both.
• Measures, per collection: unfiltered SUM(total_elapsed_time) / 1000.0, SUM(total_execution_count), MAX(max_sample_interval_seconds); filtered COALESCE(SUM(.) FILTER (WHERE match), 0) for the two sums and COALESCE(SUM(row_count) FILTER (WHERE match), 0)::bigint for matched_rows (the cast gives the bigint COUNT(*) FILTER gives). match is the unchanged $4::text[] IS NULL OR database_name = ANY($4), so a NULL database never matches on either source. NULL sums and NULL intervals stay NULL (SUM and MAX over NULLs), so the three-state interval contract (Measurement-layer campaign: the delta honesty contract (11 findings, one keystone) #3540/procedure_stats keeps only the top 150 procedures by lifetime elapsed time, and on busy primaries it misses about 16% of procedure CPU #5449) reads the same: a restart collection (every interval 0) stays unrated, a NULL interval stays NULL.
• runs, events, sequenced, points, gapped, raw and the collection_log idle-run axis are byte-for-byte unchanged. The output columns of stored are unchanged, and the test compares every column's name and Postgres type between the old and new statement.

Why the COALESCE sits inside the floor CTE. A scalar aggregate always returns one row, so COALESCE(MIN(...), 'infinity') inside totals_floor makes the floor never NULL and the rows arm carries a plain collection_time < (SELECT floor_time FROM totals_floor). I tried the first draft (the COALESCE in the rows arm's WHERE) too; the plans for the windows below came out the same either way, so this is the simpler text, not a plan fix.

Store below V176. The brief for Lane A said a store below the rung falls back to the rows. One static statement cannot do that (it names the table), and the seat asked for one statement. The Viewer's connect gate (RequiredStoreSchemaVersion, the probe already carries the procedure_stats_totals sentinel) blocks a store below the rung, and the service migrates before it serves a read, so the table exists wherever the statement runs. The remaining hole is the Viewer's fail-open probe (a probe that errors lets a skewed store through); the statement then fails with 42P01, which the executor already translates. Coordinator: say if you want a separate rows-only statement chosen by GetStoreSchemaVersionAsync; it would need a caller change in each of the three readers.

Plan check (functional only; no performance claim). Throwaway scratch store, PostgreSQL 18.6, also with TimescaleDB 2.30.1 and procedure_stats converted to a 1-day-chunk hypertable. 72 h of 5-minute collections for server 1 (500 rows each, 4 databases, 432 K rows), totals for the last 48 h (floor = now minus 48 h), plus totals for 39 other servers so the floor lookup is not the only server in the index. Filtered statement, 60-minute buckets, EXPLAIN (ANALYZE, BUFFERS):

window	procedure_stats arm	totals arm	floor lookup
24 h after the floor	rows=0 (plain: one idx_procedure_stats_time range, 6 buffers; TimescaleDB: index range in each of the 2 day chunks the window touches, 0 rows, 7 buffers; no chunk is excluded at run time here, the Merge Append of per-chunk index scans carries collection_time < InitPlan as an Index Cond)	the window's 288 collections x 4 databases (about 1,150 rows) from the totals table	Index Only Scan on ux_procedure_stats_totals, rows=1
straddling the floor (61 h to 36 h ago)	78,000 rows (the 13 h below the floor only, index range on idx_procedure_stats_time; TimescaleDB ChunkAppend: "Chunks excluded during runtime: 1")	576 rows through ux_procedure_stats_totals	same
entirely before the floor (70 h to 52 h ago)	all rows of the window, index range (TimescaleDB: 2 chunks, 95,500 and 18,500 rows)	rows=0, ix_procedure_stats_totals_time	same
server with no totals (99)	rows=0 (no rows either), index range	rows=0	Index Only Scan, rows=0

With ONE server in the totals table the planner looked up the floor through ix_procedure_stats_totals_time with a server filter; with 40 servers it chose ux_procedure_stats_totals, which is the one that matters for a real store. Indexes: rows arm idx_procedure_stats_time (server_id, collection_time) on both store kinds; totals arm ux_procedure_stats_totals (and the time index for a window with no server key match); collection_log unchanged (idx_collection_log_watermark). For a window after the floor the rows arm reads no row and no heap page; on TimescaleDB it still probes the index of each chunk the window touches (the table above), it is not excluded at run time there. I did not restructure further because those probes read 0 rows and 7 buffers in total.

Tests.

• ProcedureTrendTotalsStitchLiveTests.TheStitchedStatementReturnsTheBucketsTheRowsOnlyStatementReturns (live, own scratch store): server 7 has 24 five-minute collections; the first 12 are rows only (a store upgraded mid-stream), the rest go through the product's WriteBatchAsync, so their totals are the real writer's. They include a restart collection before and after the floor (interval 0, deltas 0), an all-NULL-interval collection before and after, a collection with a NULL database_name (one totals group), a collection whose deltas are all NULL, a collection that stored one database only, and two collections that stored nothing (idle runs in collection_log, one each side of the floor), plus a FAILED run. Server 8 holds the same rows and no totals at all; server 99 holds nothing. The oracle is the statement as it ran before (the old stored block kept in the test and spliced into the new text), run on the same store. 3 servers x 4 windows (after the floor, straddling it, before it, everything) x 5 filters (unfiltered, filtered with a NULL list, database A, B and C, a database with no rows) x bucket widths 1 and 60 minutes = 120 comparisons, 64 of them non-empty: every column name and Postgres type, and every row's every value, equal. A setup assert checks the floor is the first writer collection, nothing is totalled below it, the idle collection has no totals row and the NULL database is one group. Revert-proof: changing the rows arm to collection_time <= floor (double-counts the floor collection) fails it on the straddling windows.
• ProcedureTrendTotalsStitchLiveTests.TheStatementReadsTheTotalsAtOrAfterTheFloorAndTheRowsBelowIt: text pin for the totals arm, the floor CTE, the rows arm's < floor, the ::bigint matched_rows sum, and that the oracle differs from the new text.
• Pins re-pinned to the new text, none loosened: in DarlingMcpTrendToolsSurfaceAndSqlTests.ProcedureDurationTrendSql_PrefersTheStoredInterval_NeverFabricatesZero_AndMirrorsTheViewer the collection_time >= $2 - INTERVAL '3600 seconds' count 2 to 3 and the collection_time <= $3 count 2 to 3 (one more arm each), the viewer-vs-MCP CTE comparison now also treats the total_elapsed_time / total_execution_count lines as the filtered-sum lines, and its FILTER count in stored 3 to 6; in DurationTrendDatabaseFilterLiveTests.TheFilteredStatements_CarryTheListPredicate_OnEveryArm the procedure statement's FILTER count 3 to 6 (three per arm). The other pins compare the Viewer/MCP constants to the builder's output, so they moved with it.
• Sweeps: the 25 test classes that read the trend statement or the totals (DarlingMcpTrendTools*, DurationTrendDatabaseFilter*, ProcedureTrend*, ProcedureStats*, McpZeroIsAMeasurement, DeltaFamilyIntervalCompletion*, the Viewer trend bucketing/routing classes, DarlingTrendReaderStitch, WebReadReach*, McpToolGuideHeadsSqlTail*, ...): 674 tests, the one failure of the first run was the DurationTrendDatabaseFilterLiveTests pin above, fixed. Census / ratchet / hygiene / command-timeout / collector-coverage / schema-gate / pin classes: 1,243 tests, 0 failed, 2 skipped. Build 0 Warning(s). The whole Darling.Tests suite is not run on this laptop; CI decides.
• Lite parity: none (DuckDB store, no totals table).

For the coordinator. (1) The "store below 176" paragraph above. (2) The test takes about 35 s because it loads rows one at a time; I left it so the data stays readable. (3) My PostgreSQL rig (port 55731, Lane A's data directory under the session scratchpad) is stopped. (4) Nothing in Lane A's write path, migration or retention was changed.

Review round 1

Four findings from the first review, all fixed in this PR.

1. Medium, retention guard scanned all of procedure_stats three times per DELETE. ProcedureStatsTotals.PrunePredicate is now collection_time < COALESCE($2::timestamp, $1). DarlingRetention measures the raw floor once before the DELETE (ProcedureStatsTotals.MeasureRawFloorAsync): on TimescaleDB the earliest range_start of procedure_stats' chunks from timescaledb_information.chunks (the The dimension GC floor probe times out against compressed history: v39 indexes cannot serve compressed chunks #1815 way, immune to compression), on plain PostgreSQL (or a hypertable with no chunks) one SELECT min(collection_time). It binds the floor as $2 through a new optional secondParameter of PurgeOneAsync. A chunk floor is never later than the true minimum, so it can only keep more totals. An empty procedure_stats gives a NULL floor and the cutoff alone decides, as before. A floor that cannot be measured skips this table's purge for the cycle (counted as failed, warned), because the guard cannot be evaluated.
2. Medium, Azure SQL Database read-back grew with the square of the database count. UpsertSql now takes $3 (the batch's non-NULL database names, NULL array = whole collection) and $4 (whether the batch holds the NULL database): AND ($3::text[] IS NULL OR database_name = ANY($3::text[]) OR ($4::boolean AND database_name IS NULL)). UpsertBatchAsync gets a databaseNames argument; the runner's writer call passes the distinct DatabaseNames of the batch's ProcedureStatsCollector.Rows (null for any other row type, which reads the whole collection). Totals are per database and a batch holds whole databases, and the read-back still counts rows of the same database from earlier batches, so the result stays exact.
3. Low, collections written by an older binary above the floor vanished from the trend. ProcedureStatsTotals.GapFillSql runs first in the writer, in the same transaction: it totals every collection of the server that has procedure_stats rows strictly after the server's newest totals row before this batch and strictly before this batch. A server with no totals row needs nothing (the raw arm reads it). EXPLAIN (ANALYZE, BUFFERS) on the rig for the normal, empty case: an Index Only Scan Backward using ux_procedure_stats_totals for the newest earlier totals row (2 buffers, 1 search), then Index Scan using idx_procedure_stats_time on procedure_stats with Index Cond: server_id = ... AND collection_time < $2 AND collection_time > (InitPlan), 0 rows (3 buffers, 1 search); total execution 0.135 ms, no Seq Scan.
4. Low, no retention test at scale. OnTimescaleDb_WithACompressedOldestChunk_... builds the store's hypertables, compresses the oldest procedure_stats chunk, and checks the floor equals the chunk catalog's earliest range_start (strictly earlier than the oldest row's own time, so it did not come from min()), that the sweep's DELETE with a far-future cutoff removes nothing while that chunk is held, that after the chunk is dropped only the totals older than the new floor go, and that with no raw chunk left the cutoff alone decides. The existing plain-PostgreSQL retention test now binds the floor the same way and adds the exact-min floor and the empty-table case.

Tests added: PerDatabaseBatchesAtOneCollectionTime_TotalOnlyTheirOwnDatabases_AndStayExact (three per-database batches at one collection_time, a sentinel in DbA's totals that DbC's batch must not touch, a later DbA batch recomputing from both of its batches, the NULL database through $4, and the whole-collection null-names path), ACollectionAnOlderBinaryWroteAboveTheFloor_IsTotalledByTheNextBatch_AndTheTrendEqualsTheOldStatement (totals at t1, rows without totals at t2, batch at t3; the stitched trend equals the old-SQL oracle for unfiltered and filtered at widths 1 and 60, and it differs from the oracle before the t3 batch, so the test is not vacuous; plus the plan assertion), AServerWithNoTotalsYet_IsNotBackfilledByTheFirstBatch_..., and two text pins in ProcedureStatsTotalsTests (gap fill shape, retention predicate has no procedure_stats scan and the floor is measured before the DELETE). I did not run a revert of each fix to watch its test fail; the sentinel and the not-vacuous comparison are the guards.

Verified on a throwaway PostgreSQL 18 with TimescaleDB (port 5872, stopped): the Writer, Totals, Stitch and DarlingRetention classes (72 tests, 0 failed) and every Census and Ratchet class plus the command-timeout, doc-comment, live-hygiene, store-upgrade, migration and duration-trend classes (923 tests, 0 failed, 13 skipped). Not run here: the rest of the suite, which CI runs.

Review round 1 follow-up (seat)

Claude posting for Erik Darling.

Defect. The gap fill (ProcedureStatsTotals.GapFillSql) ran first in the procedure_stats COPY transaction with no savepoint, so its failure failed the batch. After a rollback to a pre-V176 binary that ran for days the gap can hold millions of rows. If the statement timed out or failed, the batch rolled back and no totals row was written at that batch's time. The next batch saw the same anchor and the same gap and failed again, so procedure_stats collection stopped for that server for good.

Fix (ProcedureStatsTotalsWriter). The gap fill now runs behind a savepoint (pst_gap, the #3953 shape) and under a server-side statement_timeout. On any non-cancellation fault the writer runs ROLLBACK TO SAVEPOINT, logs a warning naming the server and the gap range (after <newest earlier totals row> and before <batch time>), and goes on to the batch's own upsert. The batch commits, its totals row becomes the new anchor and the gap is attempted exactly once; those collections stay out of the trend, which is the behavior before this PR, not a regression. The batch's own upsert is unchanged: still no savepoint, still fatal to the batch.

Timeout choice: the server-side bound, not the #3953 pattern alone. The #3953 pattern (SET LOCAL lock_timeout, then a savepoint) only bounds lock waits; its statements rely on Npgsql's CommandTimeout, which on expiry sends a cancel request, and a failed cancel breaks the connection (the savepoint rollback and the upsert would then fail too). So the gap runs under set_config('statement_timeout', '<half of CommandTimeout, min 1 s>ms', true) set inside the savepoint, while the client's own deadline stays at the full command timeout, so the server cancels first (57014) and the connection stays healthy. On success the previous value (read in the same round trip that reads the gap's start) is put back, because a SET LOCAL outlives a released savepoint and would otherwise bound the batch's own upsert; on failure the rollback to the savepoint undoes it.

Read-back limit type check. The call site was (object)rows is List<ProcedureStatsCollector.Row>. Traced: every procedure_stats write, the server path, the Azure per-database fan-out and an import, goes through CopyBatchOnceAsync<TRow>(... List<TRow> rows ...), and the catalog's only definition for the table is a CollectorDefinitionBase<Row>, so TRow is Row and the check matched on every shipped path. It was still fragile (any other collection shape, or an import definition with another row type, silently read the whole collection). The check is now ProcedureStatsTotalsWriter.DatabaseNamesOf(rows), which accepts any IEnumerable<ProcedureStatsCollector.Row> (list, array, read-only collection) and returns null only for other row types.

Tests.

• ProcedureStatsTotalsWriterLiveTests (PostgreSQL, scratch database): AFailedGapFill_DoesNotFailTheBatch_AndTheNextBatchDoesNotRetryIt (a trigger refusing the gap database's totals row stands in for the fault; through the product's own WriteBatchAsync the batch's rows and totals commit, the gap stays untotalled, and after the fault clears the next batch still does not retry it); AGapFillThatTimesOut_IsCancelledByTheServer_LogsTheRange_AndTheBatchStillCommits (a trigger sleeping 30 s under a 300 ms bound; the transaction's statement_timeout is back to its original value after the rollback, the warning carries the server name and both ends of the range, the commit lands); TheGapFill_TotalsARealGap_LogsNothing_AndRestoresTheStatementTimeout (the normal path: a real gap is totalled, nothing is logged, the timeout is restored, and the next empty-gap batch is normal); TheReadBackLimit_ReachesTheWriter_OnTheRunnersOwnPath (a second batch holding only DbA does not recreate the deleted DbB totals row).
• ProcedureStatsTotalsTests: TheReadBackLimit_MatchesEveryRowCollectionShape_AndEveryProcedureStatsPathCarriesRows (List, array, IEnumerable, read-only collection, other types, plus the call-site trace and a catalog census of the table's definitions) and TheGapFill_RunsBehindASavepointAndAServerSideTimeout_AndNeverFailsTheBatch (statement order pin).
• Proven against the old shape: with the catch arm made to rethrow, both fault tests fail.
• Ran on a throwaway PG rig (timescaledb, UTC): ProcedureStatsTotalsTests 16/16, ProcedureStatsTotalsWriterLiveTests 10/10 (incl. the 4 new), ProcedureTrendTotalsStitchLiveTests 4/4, plus the census/ratchet/command-timeout classes (DocCommentHygiene, Storage/Startup/CollectionSweep CommandTimeout, LivePostgresCollectionHygiene, LiveCleanupConversionRatchet, ScratchDatabaseDisposal, StoreApplicationName, LiveClock, TimingTest, SlowClass, PgClusterObject, QueryStatsWriterCensusPins, LivePlanShapeIsolation, StoreSessionTimeZonePin, UnpooledDropConnection): 271 tests, 0 failures after removing a Stopwatch my first draft used (TimingTestCensus). The full Darling.Tests suite was not run on this laptop; CI runs the rest.
• Files touched: ProcedureStatsTotalsWriter.cs, the writer call (and its comment) in CopyBatchOnceAsync, and the two test files. ProcedureStatsTotals.cs unchanged.

Review round 2 fixes

Claude posting for Erik Darling. Round 2 findings (helper-7-5706-r2.md) and the CI failure, fixed on commit 131858d.

• M1 (a failed gap was retried by every Azure per-database batch of one cycle): the anchor is now max(totals) <= batch time in both the probe and the gap statement, so batch 2 at the same collection_time sees batch 1's committed row, its range is empty and the gap statement never runs. s.collection_time < $2 is kept. A batch that rolled back leaves no anchor and still retries. The "attempted exactly once" class doc and the "not retried" warning now say what is true (once per committed batch, never once per batch of a cycle). Live test TwoBatchesAtOneCollectionTime_TryAFailingGapOnce_...: a trigger bumps a sequence per gap attempt (rolled-back batch = 1, committed batch 1 = 2, batch 2 and the next cycle stay 2) and one warning is logged. With < put back, the test fails (Expected 2, Actual 3).
• L1 (six round trips per empty-gap batch): ProcedureStatsTotals.GapProbeSql reads the current statement_timeout, the anchor and an EXISTS gap probe in one statement before the savepoint. With no anchor or no gap the method returns there: no savepoint, no set_config, no gap statement. Pinned live through pg_stat_statements (2 probes for 2 batches, 0 pst_gap, 0 set_config, 0 gap statements) and by a source pin (one ExecuteReaderAsync, probe then early return then savepoint).
• L2 (gap scan not chunk-excludable): the anchor is $3 in GapFillSql, not a subquery. The existing TimescaleDB compressed-chunk live test now EXPLAINs the gap statement with anchor day 9 and batch day 11: only the chunk overlapping (anchor, batch) appears, the other two chunks (including the compressed oldest) are absent, and there is no DecompressChunk.
• L3 (plain floor was a full scan, doc claimed an edge read): PlainRawFloorSql takes the minimum over the servers that have totals rows (a loose index scan of the totals key) of each server's own ORDER BY collection_time LIMIT 1 through idx_procedure_stats_time (a LATERAL). Doc corrected. EXPLAIN pin (ThePlainFloor_IsAnIndexProbePerServerWithTotals_..., four servers, one without totals): Index Only Scan using idx_procedure_stats_time on procedure_stats p, Index Cond: (server_id = servers.server_id), loops=3, no seq scan; the server without totals does not move the floor.
• L4 (floor probe ran before the The first retention drain after an upgrade makes collection fall behind on a large store #5592 gate): PurgeOneAsync takes a secondParameterFactory that runs on the purge's own connection after the SkipTableOnBudget gate; PurgeAsync no longer opens a connection for the floor. Source pin: gate, then OpenConnectionAsync, then the factory call.
• L5 (retention wiring pinned by source text only): three live tests call the real DarlingRetention.PurgeAsync: plain store (the two expired totals go, the recent one stays), TimescaleDB store (raw chunks held, so all three totals stay; after the gated drop of the oldest chunk the next sweep purges only the totals below the new floor), and the floor-failure branch (procedure_stats renamed away: the totals are untouched, a warning names collect.procedure_stats_totals, and the next sweep with the table back catches up). The text pin of secondParameter: new NpgsqlParameter is replaced by pins of the factory and the gate order.
• CI pin (QueryStoreComposeStampTests.TheProbe_HasThisRungsSentinelAndMapsItsArmToV173, shard 1): V176's sentinel is probe ordinal 151. The pin now reads V176 as the newest (ProbeOrdinal + 3 present, + 4 absent, arity - 1 == ProbeOrdinal + 3), and the "behind" and "at this rung" fixtures clear the extra ordinal. I grepped every other ProbeOrdinal/arity - 1/GetBoolean( DoesNotContain pin in Darling.Tests: the other rung tests (ServiceVersion, PlanRegressionRowcount, Aws, LegacyPinCandidate, PlanRegressionDaily) assert ordinals that remain absent or present at V176, and the run below passes them. RunnableQueueSettingsRungTests does not exist on this branch (PR Alerts: a Runnable Queue alert on runnable tasks per scheduler, for Lite and Darling (#5641) #5709).
• Census catch: LiveClockCensusTests flagged the new live tests (they EXPLAIN and read the clock); they seed from LiveClock.Now().
• One-writer pin scans every Darling source file, not two named files (claude[bot] review 5495745797).

Shards: at 131858d's parent, gh pr checks 5706 showed "Darling PG tests (0)", (2), (3), (4), (5) passed and only (1) failed (the pin above); "Darling PostgreSQL tests" is the aggregate of (1). No unrelated failure to report.

Local run (rig on its own port, UTC, shared_preload_libraries = timescaledb,pg_stat_statements): every *Census*, *Ratchet*, *QueryStoreComposeStamp*, *ProcedureStatsTotals*, *ProcedureTrend*, *Retention*, *SchemaProbe*, *Rung*, *ViewerSchemaVersion*, *DocCommentHygiene*, StartupCommandTimeout and StorageCommandTimeout classes: 1441 tests, 1 failure (the LiveClock census above, fixed); rerun of the touched classes plus the census: 179 tests, 0 failed, 0 skipped. Build: 0 warnings, 0 errors. The full CI suite is not run here.

Behavior change for the changelog entry: none user-visible (the gap and retention paths only get cheaper and stop repeating work), so the proposed entry below stands.

Proposed changelog entry

SECTION: Fixed
ENTRY:
- **The raw procedure duration trend reads one small totals row per database per collection instead of every procedure row, and the duration trend tools pick one data source for a 95 hour window** ([#5706]) - the collector now writes per-collection procedure totals (store schema 176) in the same transaction as each procedure_stats batch, and the raw procedure duration trend (the MCP get_procedure_duration_trend tool and the Viewer and web charts) reads them for every collection at or after the first one written, so a 24 hour read no longer sums the roughly thousand rows each collection stores (about 60 K heap pages on a 43 server store). Collections written before the upgrade keep reading their rows, and so do collections an older version writes after a downgrade, which the next collection totals. The buckets, rates, restart and unknown-interval handling are the same as before. The totals follow procedure_stats retention, and the retention sweep reads the oldest procedure_stats chunk from the TimescaleDB catalog instead of scanning the table. The query and procedure duration trend MCP tools now decide raw versus hourly against the instant their window was computed, so a 95 hour window always reads from the same source.
REF:
[#5706]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/5706

Notes for the coordinator

• Lane A's throwaway PostgreSQL (port 55731) was restarted for Lane B's checks and is stopped again.
• The version bump touched five older rung tests and one regex pin; they are mechanical. Please check the next dev merge for another V176 taker.

Seat decision (pm-worker-helper-7)

No rows-only fallback for a store below V176. The service migrates before it reads, and the Viewer connect gate (the V176 sentinel in this PR) blocks an older store, the same as every other read of a newer table. A probe that errors and fails open makes this read fail with 42P01, as the other newer-table reads already would.

[claude]

The Viewer's chart keeps the exact bug this fixes. TierClock pins the clock for the two MCP tools, but the Viewer's procedure/query duration charts resolve their tier through a second, independent DateTime.UtcNow and were not touched:

• ViewerServerTab.TimeRange.cs:130 — GetWindowUtc() calls ViewerTimeRangeWindow.Window(RangePicker.Value, DateTime.UtcNow, …), and a live relative range returns EndUtc = nowUtc, StartUtc = nowUtc - span (ViewerTimeRangeWindow.cs:33-43).
• ViewerServerTab.Queries.cs:136 → LoadPerformanceTrendsAsync(startUtc, endUtc) → ViewerServerTab.QueryTrends.cs:83-84 call GetQueryDurationTrendAsync / GetProcedureDurationTrendAsync with no nowUtc.
• ViewerDataService.QueryTrends.cs:517-519 then does await GetRollupAvailabilityAsync(…) and only afterwards DurationTrendRouting.ResolveTier(startUtc, nowUtc ?? DateTime.UtcNow, …) — a clock read after an awaited store round-trip, i.e. the same "a few milliseconds LATER" this doc comment describes, and with a wider gap than the MCP path had.

So for a live window of exactly 95 h (TimescaleSupport.RawRetentionSpan - DurationTrendRouting.RawTierMargin), ShouldUseRawTier(startUtc, nowUtc) at DurationTrendRouting.cs:78-79 evaluates now₁ - 95h >= now₂ - 95h with now₂ > now₁ → false → the Viewer serves the hourly tier where get_procedure_duration_trend now serves raw. The range is reachable: TimeRangeParser.cs:123 accepts h/hr/hour(s) and TimeRangePresets.FromHours accepts any positive value, so "past 95 hours" is a live spec.

That makes the two surfaces answer the same question from different tiers — the #3684 shape ViewerTrendRoutingPortTests exists to prevent ("Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted"). The viewer side is unpinned: A95HourWindowWithNoAsOf_RoutesRawEveryTime only exercises DarlingTrendReader.Resolve*DurationTrendRoute.

The fix is the same one-liner on the viewer side — pass the window's own end as the clock for a live range (and keep the wall clock for a fixed/finished one, where IsLive is false and the end really is an anchor in the past, matching TierClock's as_of arm). Please either do that, or say in the PR body that the Viewer keeps the old clock deliberately so the divergence is on the record.

[claude]

Non-blocking. The whole stitch rests on "every collection at or after the floor has a totals row", and the only thing standing behind it is this two-file roster — exactly the #3699 shape in .github/REVIEW-TRAPS.md: "A roster-based census does not see the file it does not roster … An anti-pattern census sweeps the class (every tool body); rosters are for the positive contract only." A third service file that acquires an INSERT INTO collect.procedure_stats (or a BeginBinaryImportAsync naming it) passes here, and the consequence is not a loud failure: a collection at or after the floor with rows but no totals row drops out of stored, the collection_log axis re-adds it through the points idle arm, and it plots as a measured 0 — the fabricated-zero shape the three-state contract exists to prevent.

I checked by hand and the claim holds today: the only other BeginBinaryImportAsync call sites are Targets/RdsDeadlockIngestor.cs:424, RdsLogEventIngestor.cs:303, RdsPlanIngestor.cs:323 and RdsCpuIngestor.cs:582, all on pg_* tables, and CopyBatchOnceAsync is the single procedure_stats writer. Worth sweeping the directory rather than naming two files, so the next writer fails here instead of in a chart.

[claude]

Two smaller notes from the blind read, neither blocking.

1. StoreSelfMetricsTests.PlainCollectTableCensus_EveryTableIsNamedOrDeliberatelyInOther cannot see this table. That census builds its population by regexing CREATE TABLE IF NOT EXISTS collect\.([a-z0-9_]+) out of the string literal bodies of PgMigrations.cs (StoreSelfMetricsTests.cs:363-371). V176's DDL is not a literal in that file — PgMigrations.cs:1006 is private const string V176Sql = ProcedureStatsTotals.CreateSql;, and the text lives in ProcedureStatsTotals.cs:56-68. So collect.procedure_stats_totals is in none of the census's three lists, the census stays green, and the table is sized into StoreSelfMetrics.OtherObjectKind's catch-all with no declared reason — which is the one thing that census says is "the census's whole point" to prevent.

This is a pre-existing gap, not one the PR opens: V164/V165/V167/V170 use the same VnSql = SomeClass.CreateSql indirection. But V164's author added collect.query_stats_hour_ledger and collect.query_stats_hour_ledger_state to deliberatelyOther anyway (StoreSelfMetricsTests.cs:395-396) even though the census could not have forced it, and V170's collect.pg_io_stats_hourly is missing for the same reason. A one-line entry here ("the per-collection totals of procedure_stats, about one row per database per collection, pruned with the raw collector, V176 doc") keeps the inventory honest; widening the census to read the resolved PgMigrations.Scripts strings — the way MigrationDataMovingRungCensusPins already does, which is why that census does see V176 and correctly has nothing to declare — would fix the class of it.

2. ResolveQueryDurationTrendRoute's own doc is now false for the path the PR changed. DarlingTrendReader.cs:1066-1068 still says nowUtc "is the WALL CLOCK, never the window's end — see ShouldUseRawTier — and defaults to the real clock, the same way GetQueryHistoryAsync takes it: the tool passes only its anchored window". After this change the no-as_of path passes exactly the window's end, deliberately, and the tool now passes both. The substance is fine (with no as_of the window's end is a wall-clock reading, just the first one rather than the second), but the sentence a future reader will hit first now contradicts the two call sites at DarlingMcpTrendTools.cs:569 and :656. ViewerDataService.QueryTrends.cs:473-477 carries the same sentence and, per the inline comment, is also still on the old clock.

Things I could not check from a read-only tree, for whoever runs the rig: every live pin here is Assert.SkipWhen(string.IsNullOrEmpty(DARLING_TEST_PG)), so the whole equivalence argument — the 120-comparison old-vs-new oracle in ProcedureTrendTotalsStitchLiveTests, the rollback-on-fault pin, the read-back index plan, the retention ordering — is unexercised without a store. The EXPLAIN table in the body (including "Chunks excluded during runtime: 1" on the straddling window, and the claim that a window after the floor still probes each chunk's index for 0 rows) is a live measurement; the doc at DurationTrendRouting.cs:~400 states the stronger "TimescaleDB excludes chunks past it", which the body's own table contradicts for the after-the-floor case. Worth reconciling the prose with the measurement.

I also traced the retention ordering by hand and it holds in the safe direction: TimeSlicedDeleteSql (DarlingRetention.cs:1293-1301) splices ProcedureStatsTotals.PrunePredicate into the main WHERE and both min() slice anchors, so the slice anchor is the oldest deletable row and the loop cannot spin on a slice it is forbidden to delete. One consequence worth knowing: min(collection_time) FROM collect.procedure_stats is therefore evaluated three times per slice statement, against the store's second-largest hypertable.

[claude]

Verdict: CHANGES REQUESTED

Reasoning for both findings is in the inline comments and the top-level note; this is the ledger.

Claims

• VERIFIED — grain, column types, NULLS NOT DISTINCT key, time index — ProcedureStatsTotals.cs:56-68
• VERIFIED — no backfill, three idempotent DDL statements — same hunk; MigrationDataMovingRungCensusPins reads resolved Scripts, so it does see V176 and has nothing to declare
• VERIFIED — upsert on the COPY's transaction, after the COPY/dimension flush, before the commit, no savepoint, no catch, chosen by TargetTable — DarlingCollectorRunner.cs:4455-4458, :4468, :4482-4487
• VERIFIED — recomputes never adds; zero-row batch writes nothing — ProcedureStatsTotals.cs:86-94, ProcedureStatsTotalsWriter.cs:44-47
• VERIFIED — one writer for procedure_stats — the other four BeginBinaryImportAsync sites are RDS ingestors on pg_*; fan-out :2833 and re-attempt :4417 both reach CopyBatchOnceAsync
• VERIFIED — retention at procedure_stats' effective days, never before raw of that age — DarlingRetention.cs:1007-1019; TimeSlicedDeleteSql:1293-1301 splices PrunePredicate into the WHERE and both min() anchors, so the anchor is the oldest deletable row and the loop cannot spin
• VERIFIED — version bump chain — StorageVersion.cs:19, ViewerDataService.cs:1185, :1888, :1909-1912
• VERIFIED — no GRANT needed — DarlingManagedRoles.cs:1203; BYO ALTER DEFAULT PRIVILEGES for admin/viewer/mcp at Darling/tools/provision-roles.sql:1164-1180
• VERIFIED — no collection read from both arms — floor is MIN(collection_time) for $1 so all totals are at or after it; rows arm strictly <; COALESCE(…,'infinity') sends a totals-less server wholly to the rows
• VERIFIED — measures and NULL semantics match the rows arm — SUM/MAX ignore NULLs; the four UNION ALL columns share types and order (::bigint matching COUNT(*) FILTER), names come from arm 1, and a NULL database_name fails = ANY($4) on both, so the three-state contract and matched_rows survive
• VERIFIED — runs/events/sequenced/points/gapped/raw and the collection_log axis untouched; all readers alias one builder (ViewerDataService.QueryTrends.cs:162, DarlingTrendReader.cs:1173)
• VERIFIED — TierClock is one clock per call, the window end with no as_of, routing 95 h Raw; no Lite twin — DarlingMcpTrendTools.cs:108 / :569 / :656 vs DurationTrendRouting.cs:78-79; Lite is DuckDB with no tier (LocalDataService.TrendBuckets.cs:352)
• UNVERIFIED — "560-1,060 rows per collection, ~60 K heap pages per 24 h read" — live measurement
• UNVERIFIED — the EXPLAIN table, incl. runtime chunk exclusion and the floor index at 1 vs 40 servers — needs the PG 18.6 / TimescaleDB 2.30.1 fixture
• UNVERIFIED — "the read-back uses an index" — asserted but behind Assert.SkipWhen(DARLING_TEST_PG empty)
• UNVERIFIED — every test count (16, 83, 674, 1,243, the 1,464 sweep) and 0 Warning(s); the author states the full suite and full post-fix sweep were not re-run
• UNVERIFIED — the 120-comparison oracle; its arithmetic self-asserts but it is live-gated, so the equivalence argument is unexercised here
• UNVERIFIED — "revert-proof: <= floor fails the straddling windows" — needs the rig
• REFUTED — one writer "pinned by a source test on the other service files": the fact is true, the pin is not. ProcedureStatsTotalsTests.cs:105-109 reads two named files for one literal; no third file, no BeginBinaryImportAsync, no UPDATE

Riskiest lines

1. DurationTrendRouting.cs, the rows arm's AND collection_time < (SELECT floor_time FROM totals_floor) — a collection at or after the floor with rows but no totals row leaves stored, returns via the collection_log axis as an idle point and plots a measured 0, silently. I could not reach it (one writer, one storedCollectionTime per batch at :4530/:4592, sequential batches, retention ordered safely) — pinned by ProcedureTrendTotalsStitchLiveTests.TheStitchedStatementReturnsTheBucketsTheRowsOnlyStatementReturns, live-gated
2. DarlingCollectorRunner.cs:4482-4487 — no savepoint/catch, so a fault costs the batch's sample that StoreWriteReattempt declines to re-run — pinned by AFailedTotalsUpsert_AbortsTheBatch_… (live-gated); position only by TheRunner_WritesTheTotalsOnTheCopysTransaction_…
3. Mcp/DarlingMcpTrendTools.cs:108 — fixes the MCP surface only — pinned by A95HourWindowWithNoAsOf_RoutesRawEveryTime; the Viewer twin at ViewerDataService.QueryTrends.cs:517-519 is UNPINNED and unfixed

Traps near this diff

• "A NEW delta column needs an interval beside it and IS DISTINCT FROM 0 in the aggregates that roll it up." — applies, satisfied (ProcedureStatsTotals.cs:62; the read keeps CASE WHEN is_stored AND max_interval_seconds = 0 THEN NULL)
• "CASE WHEN interval > 0 THEN delta / interval ELSE 0 END spells 'unknowable' as 0 … End at END (NULL)." — does not apply (DurationTrendRouting.cs:600-605, outside the diff, still ends at END)
• "Any a / b over integer columns is suspect until a 1.0 * or a CAST is found." — does not apply: SUM(total_elapsed_time) / 1000.0 is numeric/numeric
• "A total_* assigned from a .Count is a count of the page unless the set is provably whole." — does not apply: row_count is SQL COUNT(*) over the whole row-group (ProcedureStatsTotals.cs:85)
• "hours_back, limit, top, days_back are refused when out of range, never clamped." — does not apply: the clock moved, not the validator (McpHelpers.cs:241-252)
• "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies: the SQL is aliased, the tier clock is not (ViewerDataService.QueryTrends.cs:517-519)
• "Lite and Darling ship in parity … a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit." — does not apply: PostgreSQL-only storage change, identical payload, Lite has no tier (LocalDataService.TrendBuckets.cs:352)
• "A roster-based census does not see the file it does not roster … rosters are for the positive contract only." — applies twice: the one-writer pin names two files (ProcedureStatsTotalsTests.cs:105), and StoreSelfMetricsTests.PlainCollectTableCensus_…:363-371 regexes PgMigrations.cs literals only, so it is blind to V176's DDL at PgMigrations.cs:1006; the second is pre-existing (V164, V170)

Divergence (blind read vs PR body)

The body states the clock defect surface-neutrally — "the MCP duration-trend tools' tier decision, which read a second clock a few milliseconds after the window was computed" — then fixes the MCP tools alone. The Viewer's duration charts have the identical two-clock shape with a wider gap (ViewerServerTab.TimeRange.cs:130 reads DateTime.UtcNow; ViewerDataService.QueryTrends.cs:517-519 reads it again after await GetRollupAvailabilityAsync(…)), and a live 95-hour range is reachable through the picker (TimeRangeParser.cs:123, TimeRangePresets.FromHours). The Viewer's clock appears nowhere in the body — not fixed, not deferred, not out of scope — while "Lane B" is explicit that the Viewer and web charts share the statement with no caller change. That is the blocking item: the two surfaces now answer the same 95-hour question from different tiers.

Smaller: the one-writer claim cites a pin that does not assert it (REFUTED above), and the doc added to DurationTrendRouting.cs says the floor predicate means "TimescaleDB excludes chunks past it", which the body's own EXPLAIN table contradicts for the after-the-floor window. Everything else matched the diff.

Reviewer checklist

• With DARLING_TEST_PG set, run ProcedureTrendTotalsStitchLiveTests, ProcedureStatsTotalsWriterLiveTests, DurationTrendDatabaseFilterLiveTests — the equivalence, atomicity, index and retention-ordering argument is entirely live-gated
• EXPLAIN (ANALYZE, BUFFERS) the rows arm on a real TimescaleDB store for a window after the floor; confirm the per-chunk probes stay cheap rather than growing with the store, then reconcile the "excludes chunks past it" sentence
• Time a retention sweep on a large store: PrunePredicate runs min(collection_time) FROM collect.procedure_stats three times per slice statement
• Open the Viewer on a V176 store at a live "past 95 hours" and compare the Procedure Duration chart's tier against get_procedure_duration_trend(hours_back: 95)
• Watch a real cycle on an Azure SQL DB target (ProcedureStatsCollector.cs:691): each database's batch re-reads the collection's growing row set
• Confirm a fresh install and a V175 to V176 upgrade leave viewer/MCP roles able to SELECT collect.procedure_stats_totals — this read fails hard (42P01/42501), not gracefully

[claude]

Verdict: LGTM

Claims

• VERIFIED — PrunePredicate is now collection_time < COALESCE($2::timestamp, $1), the floor passed as a parameter instead of an inline subquery — ProcedureStatsTotals.cs:197
• VERIFIED — MeasureRawFloorAsync reads the earliest chunk's range_start from timescaledb_information.chunks when Timescale is available, else one min(collection_time) — ProcedureStatsTotals.cs:207-225; pinned by OnTimescaleDb_WithACompressedOldestChunk_... and the plain-PG retention test
• VERIFIED — the floor is threaded through a new secondParameter on PurgeOneAsync and bound to the procedure_stats_totals DELETE, with the purge skipped (and counted failed) when the floor can't be measured — DarlingRetention.cs:1018-1053
• VERIFIED — UpsertSql gained $3/$4 to limit the read-back to the batch's own databases (database_name = ANY($3) OR ($4 AND database_name IS NULL)) — ProcedureStatsTotals.cs:105; exercised live by PerDatabaseBatchesAtOneCollectionTime_TotalOnlyTheirOwnDatabases_AndStayExact
• VERIFIED — GapFillSql totals every collection strictly after the server's newest earlier totals row and strictly before the batch, recomputing via the same upsert target — ProcedureStatsTotals.cs:118-148; pinned by the stitch-trend live tests and the text pin in ProcedureStatsTotalsTests
• VERIFIED — the gap fill now runs behind a savepoint (pst_gap) and a server-side statement_timeout (half the command timeout, floor 1s), and a fault rolls back to the savepoint, logs a warning, and lets the batch's own upsert (still bare, still fatal) proceed — ProcedureStatsTotalsWriter.cs:112-180; pinned by AFailedGapFill_DoesNotFailTheBatch_AndTheNextBatchDoesNotRetryIt and AGapFillThatTimesOut_IsCancelledByTheServer_...
• VERIFIED — on success the prior statement_timeout is restored (read in the same round trip as the anchor) before the savepoint is released, since RELEASE SAVEPOINT does not itself undo a SET LOCAL/set_config(..., true) made inside it — matches Postgres subtransaction semantics; pinned by TheGapFill_TotalsARealGap_LogsNothing_AndRestoresTheStatementTimeout
• VERIFIED — the read-back limit's type check was replaced by ProcedureStatsTotalsWriter.DatabaseNamesOf, which accepts any IEnumerable<ProcedureStatsCollector.Row> rather than only List<Row> — ProcedureStatsTotalsWriter.cs:53-61, DarlingCollectorRunner.cs:4698-4703; pinned by TheReadBackLimit_MatchesEveryRowCollectionShape_... and TheReadBackLimit_ReachesTheWriter_OnTheRunnersOwnPath
• UNVERIFIED — the EXPLAIN (ANALYZE, BUFFERS) numbers quoted for the normal gap-fill probe (2 buffers / 0.135 ms, no Seq Scan) — a live measurement, not checkable from a read-only tree
• UNVERIFIED — the reported local test run counts (271 tests, 0 failures) and "proven against the old shape: with the catch arm made to rethrow, both fault tests fail" — both are claims about runs performed by the author, not reproducible statically

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Storage/ProcedureStatsTotalsWriter.cs (TryGapFillAsync catch block, rollback→release→log sequence) — this is the load-bearing "can never fail the batch" guarantee; a slip here (e.g. not rolling back before the batch's own upsert runs) would reintroduce the original defect — pinned by AFailedGapFill_DoesNotFailTheBatch_AndTheNextBatchDoesNotRetryIt
2. Darling/PerformanceMonitor.Darling.Storage/ProcedureStatsTotalsWriter.cs (SetStatementTimeoutAsync restore-then-release ordering around the gap command) — wrong ordering would leak the shortened timeout into the batch's own upsert — pinned by AGapFillThatTimesOut_IsCancelledByTheServer_LogsTheRange_AndTheBatchStillCommits (asserts statement_timeout is back to the original inside the same transaction after a rollback) and TheGapFill_TotalsARealGap_LogsNothing_AndRestoresTheStatementTimeout (success path)
3. Darling/PerformanceMonitor.Darling.Storage/ProcedureStatsTotals.cs (GapFillSql's correlated anchor subquery, NULL-anchor boundary) — a server with no totals row at all must total nothing (the raw trend arm covers it instead); getting this backwards would either backfill history that belongs to the raw arm or silently drop the server's first batch — pinned by AServerWithNoTotalsYet_IsNotBackfilledByTheFirstBatch_AndTheTrendStillEqualsTheOldStatement and ACollectionAnOlderBinaryWroteAboveTheFloor_IsTotalledByTheNextBatch_AndTheTrendEqualsTheOldStatement

Traps near this diff

• chunk_compression_stats returns ZERO rows for a hypertable whose compression is off, so a chunk count over it reads 0 (#3610) — does not apply: MeasureRawFloorAsync reads timescaledb_information.chunks, not chunk_compression_stats, and the live test exercises exactly the compressed-oldest-chunk case (OnTimescaleDb_WithACompressedOldestChunk_...) — ProcedureStatsTotals.cs:214
• Lite and Darling ship in parity: a fix to one SKU's reader/writer without the counterpart is a finding (#3594) — does not apply: procedure_stats_totals and its gap fill are Darling-Postgres-only constructs with no DuckDB analogue in Lite (confirmed by grep -rl ProcedureStatsTotals Lite returning nothing), and the PR body states this explicitly; not a parity gap introduced by this diff

Divergence (blind read vs PR body)

None — the body's "Review round 1" and "Review round 1 follow-up (seat)" sections both match what the diff does: the floor-as-parameter change, the per-database read-back limit (narrowed first to a List<Row> type check in 4e60c5be2, then widened to DatabaseNamesOf in ff0bd67d3, exactly as the follow-up section describes), the gap fill's savepoint/timeout wrapping, and the test additions all line up with the stated mechanisms.

Reviewer checklist

• Run the live Postgres/TimescaleDB test rig (DARLING_TEST_PG) for ProcedureStatsTotalsWriterLiveTests and ProcedureTrendTotalsStitchLiveTests, including the two new fault-injection tests (refusing trigger, sleeping trigger) and the compressed-chunk retention test, since none of this can be executed from a read-only tree

Claude posting for Erik Darling. All four findings are answered on later commits. (1) Rows-arm zero for a collection with rows but no totals: the gap fill (4e60c5b, ff0bd67) totals every collection between the newest totals row and the batch, verified in the LGTM review at ff0bd67. (2) Bare upsert with no savepoint: intended, so totals and raw rows commit or fail together; the gap fill, the part that can be slow, runs behind savepoint pst_gap with a statement_timeout and never fails the batch. (3) Viewer tier-clock twin: a pre-existing millisecond skew this PR does not introduce, filed as #5725. (4) REFUTED one-writer pin: fixed in 061989f, which scans every Darling source file and was shown to fail on a planted writer.