Repository navigation
Plan regression reads stop re-reading three open days of Query Store intervals, and on-demand calls reuse the candidates for one analysis cadence (#5812) - #5817
Conversation
…ewest days first (#5812) The scheduled pass re-ran the 14-day plan_agg read for every server on every pass. A current pass now reuses the candidate rows of a read of the same shape (window day, built days, scope) for up to six hours; the #5630/#5631 input check, the cut and the fact still run every pass. get_analysis_facts and analyze_server with no as_of reuse them too; a past as_of reads live. The daily builder now plans the newest closed days first, so a store that is behind gets back the days nearest the open end first.
…ewest days first (#5812) The scheduled pass re-ran the 14-day plan_agg read for every server on every pass. A current pass now reuses the candidate rows of a read of the same shape (window day, built days, scope) for up to six hours; the #5630/#5631 input check, the cut and the fact still run every pass. get_analysis_facts and analyze_server with no as_of reuse them too; a past as_of reads live. The daily builder now plans the newest closed days first, so a store that is behind gets back the days nearest the open end first.
…ution watermark (#5812) V184 adds plan_regression_daily_built.built_through and moves the late-row trigger's upper bound to one hour before the current hour. The builder builds through tomorrow; an open day is rebuilt at every hourly tick to a watermark two hours before the current hour by the store's clock. The daily read takes the totals for every valid built day below its watermark and the live interval rows after it, so on a current store the live half reads the last two to three hours of first executions instead of the three open days. Checkpoint: rung pins, the live classes that bind the daily read's parameters, and the new live tests are not updated yet (see the PR body).
…ntal' into fix/5812-plan-regression-incremental # Conflicts: # Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.QueryPerf.cs # Darling/PerformanceMonitor.Darling.Storage/PlanRegressionDaily.cs
…SION candidates for one analysis cadence (#5812) V184 rung pins: every probe-ordinal and mapping pin moves up one, the V183 pins relax to below the top, the rewinds drop the built_through sentinel, and the migration-count pins count V184. The builder tests plan T-14 through T+1, and a new live test pins ruling R2: only a day whose watermark can still advance (today, tomorrow) is due on the hourly tick, and a complete day inside the closed-day lag is due again only after its late_seq moves. The trigger tests follow V184's hour-aligned bound, with an hour guard. The hand-bound daily reads pass the watermark array. Ruling R3: the candidate reuse window is one analysis cadence, set by the worker from the configured interval (default 30 minutes). The scheduled pass never reuses; it always reads and refills the entry, and MCP and viewer calls with no as_of reuse that read within one cadence.
… and move the V184 pins (#5812) The daily read's live half excluded built rows with NOT EXISTS over unnest($3, $6), which the planner runs as a join above the interval scan, so the scan returned the floor day's rows and the join dropped them. It is now a scalar lookup in the scan's filter, as #5448's <> ALL($3) was. The parity live test binds the build's watermark and checks three partial builds against the table read; the inputs live test removes the days it built; the compose-stamp, trigger-text and pruning pins take V184.
…on-incremental-s3
…5812) One case per branch of PlanRegressionLiveFloorWithWatermarks: nothing built, contiguous whole days, partial days, a gap, and the bound below the window floor.
…ad from its executed plan (#5812) Half-stale: a late write marks four closed days stale through the real trigger; the read takes totals for the current days only, reads the stale days' intervals, and equals the table read even with the stale totals tampered. Fourteen days with three partial: the interval scan's index condition starts at the lowest watermark and each half returns exactly its planted rows.
…PLAN_REGRESSION read (#5812) pg_stat_statements' calls for the interval read and the cache's hit counter: no interval read after the scheduled pass, and one fresh read once RefreshAfter has passed.
…ork_mem, and add the no-spill rig (#5812) At the default 4MB the read sorted about 384,000 planted rows to disk; with 128MB set for the snapshot transaction only, the plan hashes in memory. The command-count pin counts the new command.
… every read variant (#5812) The test counted calls of the interval-table read by text. In a combined run the cluster evicts pg_stat_statements entries constantly (pg_stat_statements.max 5000; one combined run made about 150 deallocations), so an evicted entry remade by a live read starts over at one call and the count can stand still while a read ran. The raw read's text did not match the filter at all. The check now reads every PLAN_REGRESSION read (all three end FROM plan_agg), holds the pass's entry to its query id and stats_since, fails when that entry is missing or remade, and also asserts the test's own cache made no miss; a failure message names hits, misses and every entry.
…easured value with headroom (#5812) On one plant of the no-spill rig (15 built days, 360,000 daily rows, one open hour of 24,000 interval rows): 4MB spills (an external merge of about 15MB); 32MB, 64MB and 128MB all hash in memory, with hash aggregate peaks of about 35MB, 37MB and 42MB. 32MB is below its own peak and fits only because a hash may use twice work_mem, so 64MB is kept as the headroom. The rig now measures the weighed bounds (32MB and the 128MB this replaces) on the same plant.
…days without a build, and keep partial builds invisible to pre-V184 code (#5812 fix A) H1: queue 1 (never built or late-moved days, newest first, capped at 120) runs before queue 2 (partial days with rows to add, oldest build first, one per server). Idle partial days advance and tomorrow is stamped by one statement each, outside the cap, on the store clock. M1: V184 (changed in place) adds partial_seq; a partial build stamps partial_seq and leaves built_seq NULL. M3: the trigger's bound is a named constant the SQL interpolates, pinned one hour under the watermark lag, with a live seam test. L4: doc comments.
… the candidate cache, and measure against dev (#5812 fix B) M2: the exact-bound table and raw reads run in a short read-only transaction with the same SET LOCAL work_mem as the daily read. L1+L3: Forget is gone; Store evicts entries older than RefreshAfter and entries of servers outside the published set; every reuse decision is logged at Debug with its reason; the exact-bound shapes carry rawWindowStart. L2: the docs say a persisted on-demand pass may use a read up to RefreshAfter old. L7: the watermark live test asserts rows read from the plan's JSON nodes, not Index Cond text. Rigs: MeasureFallback and CompareWithDev.
…fix B) CompareWithDev times each read with a Stopwatch for its printout only; nothing is asserted on it.
… writer tests' steady rows above the trigger's open-day bound at every hour (#5812 fix A2)
…tial day, so it skips only 02:00-04:00 (#5812 fix A2)
…own clock reads, and skip when either crosses midnight (#5812 fix A2)
… pin the second builder's one stamped day (#5812 fix A2)
…and round 2's Lows Dev now carries #5819's V184 (the investigation tables), so this PR's rung becomes V185: the migration and its registration, StorageVersion.SchemaVersion, the probe sentinel at the next ordinal with its MapProbedSchemaVersion parameter and arm, DropV185SentinelAsync for the rewinds, and every rung test's ordinals, version lists and migrate counts. Round 2's Lows: the daily read's SQL comment and PlanRegressionLiveFloor's doc state the partial-day rule (L1); queue 2's cap is documented as the number of builder servers (L2); PlanRegressionDailyWorkMemSql is PlanRegressionWorkMemSql (L4); queue 1 orders by its oldest build first, so servers whose today goes stale on every apply cannot hold a late-marked closed day out of the cap, with a live test that fails on the old order (L5).
| public void EveryPlanRegressionRead_RunsUnderTheOneWorkMemSetting() | ||
| { | ||
| Assert.Equal("SET LOCAL work_mem = '64MB'", PgFactCollector.PlanRegressionWorkMemSql); | ||
|
|
||
| var fact = FactSource(); | ||
| Assert.Equal(1, Count(fact, "new NpgsqlCommand(PlanRegressionWorkMemSql")); | ||
| Assert.DoesNotContain("work_mem =", fact, StringComparison.Ordinal); | ||
| Assert.Equal(2, Count(fact, "await BoundTheReadAsync();")); | ||
|
|
||
| var fallback = fact.IndexOf("if (snapshot is null)", StringComparison.Ordinal); | ||
| var fallbackBegin = fact.IndexOf("snapshot = await connection.BeginTransactionAsync(context.CancellationToken);", StringComparison.Ordinal); | ||
| var read = fact.IndexOf("readsDays ? PlanRegressionDailySql : readsTable ? PlanRegressionTableSql : PlanRegressionSql, connection, snapshot)", StringComparison.Ordinal); | ||
| Assert.True(fallback > 0 && fallback < fallbackBegin && fallbackBegin < read, | ||
| "the exact-bound read must begin its own transaction, and bound it, before the read command"); | ||
| Assert.True(fact.IndexOf("await BoundTheReadAsync();", fallbackBegin, StringComparison.Ordinal) < read); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void TheCacheHasNoForget_AndTheServicePublishesItsServerSet() | ||
| { | ||
| var root = Root(); | ||
| var cache = File.ReadAllText(Path.Combine(root, "Darling", "PerformanceMonitor.Darling.Analysis", "PgFactCollector.PlanRegressionCache.cs")); | ||
| Assert.DoesNotContain("Forget", cache, StringComparison.Ordinal); | ||
|
|
||
| var worker = File.ReadAllText(Path.Combine(root, "Darling", "PerformanceMonitor.Darling.Service", "DarlingWorker.cs")); | ||
| Assert.Contains("PlanRegressionCandidateCache.Shared.RetainServers(initialServers.Select(s => s.ServerId));", worker, StringComparison.Ordinal); | ||
| Assert.Contains("PlanRegressionCandidateCache.Shared.RetainServers(view.EnabledServers.Select(s => s.ServerId));", worker, StringComparison.Ordinal); | ||
| } |
There was a problem hiding this comment.
Trap .github/REVIEW-TRAPS.md (Tests and censuses) applies here. The ruling reads, in its own words:
Ruling (#5833): tests are load-bearing. The censuses, pins and rosters named in this section are being deleted under #5833. Never add one, and never answer a trap with a test that reads source text or asserts how code is spelled (a signature at a line, a SQL fragment's wording, a comment's text, a trait roster, a coverage percentage). A trap is answered with a behaviour test that fails on the defect and passes on the fix, or with one sentence in the PR body saying why none is needed.
Four new assertions in this file are nothing but spelling:
EveryPlanRegressionRead_RunsUnderTheOneWorkMemSetting(L180-189) counts occurrences of the literals"new NpgsqlCommand(PlanRegressionWorkMemSql"and"await BoundTheReadAsync();"inPgFactCollector.QueryPerf.cs, and asserts the relativeIndexOforder of three more source literals.TheCacheHasNoForget_AndTheServicePublishesItsServerSet(L197-201) asserts the substring"Forget"is absent from one file and two whole C# statements are present verbatim inDarlingWorker.cs.AnExactBoundRead_WithADifferentWindowStart_IsADifferentShape(L167-169) asserts the collector source spellsexactWindowStart: readsDays ? null : rawWindowStart, watermarkParameter: readsDays ? 5 : null, plus"out decision);"and aLogDebugformat string.
Each of these has a behaviour twin already in the diff that fails on the defect: the work-memory bound is measured by PlanRegressionDailyNoSpillRig.MeasureFallback, the rawWindowStart shape by the pure-rule assertions right above at L138-163, and RetainServers by AStore_EvictsTheEntriesOfServersNoLongerMonitored_OnlyOnceASetIsPublished. What is left is brittleness: renaming a local function, moving BoundTheReadAsync above the readsTable branch, or hoisting the second SET LOCAL into a helper all go red while the behaviour is unchanged — and none of them goes red on a store that actually spills.
The PR body applies this same reasoning correctly one section down ("The source-text pin of the old ORDER BY in TheConstantsAndStatements_AreTheDocumentedOnes is deleted, not moved: its rule was spelling, and the live test now pins the order by behavior"). These three members are the same shape and want the same treatment. PlanRegressionDailyRungTests.TheStampAndTheBuiltDaysRead_KeepAPartialBuildInvisibleToPreV185Code is in the same position: every one of its six assertions is a SQL fragment's wording, and PlanRegressionDailyWatermarkLiveTests.APartialDay_IsUnbuiltToTheV183Predicate_AndAV183WholeBuildAfterItReadsWhole_WithNoDoubleCount already proves the behaviour against a real store.
(The probe-ordinal and migration-literal pins in the *RungTests classes are a different matter — CONTRIBUTING's "Adding a Darling Migration Rung" mandates them as the house form, and this PR moves them exactly as it should.)
| FOR EACH ROW | ||
| WHEN (NEW.first_execution_time < date_trunc('hour', now() AT TIME ZONE 'UTC') - interval '{PlanRegressionDaily.LateTriggerLagHours} hour' | ||
| AND NEW.first_execution_time >= date_trunc('day', now() AT TIME ZONE 'UTC') - interval '17 days') | ||
| EXECUTE FUNCTION collect.plan_regression_daily_mark_late();"; |
There was a problem hiding this comment.
The margin between the trigger's bound and a steady 60-minute interval is exactly zero, and the two sides of the comparison are two different machines' clocks.
NEW.first_execution_time is the monitored SQL Server's own reading, taken from sys.query_store_runtime_stats.first_execution_time as a datetimeoffset and normalised to UTC client-side (QueryStoreCollector.cs:964, :1644). now() here is the store's clock. The bound is date_trunc('hour', now()) - 1 hour.
With Query Store's default INTERVAL_LENGTH_MINUTES = 60, intervals are hour-aligned, so for the whole hour after an interval closes the collector keeps re-writing its rows (their last_execution_time advanced on the pass that observed the close) with first_execution_time >= <that hour's start> — which is the bound. The PR body states this as the safety argument:
Steady collection does not fire the trigger. The collector fetches only intervals whose
last_execution_timeadvanced (QueryStoreCollector.WatermarkColumn), and those intervals start at or after the trigger's bound for intervals of 60 minutes or less.
True on one clock, with zero slack. The margin is (monitored clock − store clock) + (first execution − interval start), and for a query that runs continuously the second term is milliseconds. A store whose clock is a second ahead of the monitored server's — ordinary NTP drift, and the very condition PlanRegressionSkewMarginDays exists for ("a monitored server whose clock runs ahead of the collector's", PlanRegressionDaily.cs:40) — puts every row of the just-closed interval below the bound.
Failure scenario: store clock 2 s ahead of a monitored server with the default 60-minute Query Store interval. Every apply during the hour after an interval closes fires plan_regression_daily_mark_late() on the first such row, bumping late_seq for today and tomorrow. Both then fail built_seq = late_seq and partial_seq = late_seq, so BuiltDaysSql stops returning them, PlanRegressionLiveFloorWithWatermarks falls back to "one day below the first unbuilt day" (= today's midnight), and PLAN_REGRESSION reads the open days whole again — exactly the pre-#5812 read the PR exists to remove — while queue 1 rebuilds today on every tick. Nothing fails; the gain just silently does not materialise, for the default configuration.
The day-long-interval case is acknowledged in the body ("A server with day-long intervals fires on every apply, so its open days read live, as today, and pay a trigger call per row"), but that paragraph treats 60-minute intervals as the safe case, and it is safe only to within clock skew.
The two tests that pin this both use SteadyFirst(now) = date_trunc('hour', now) (QueryStoreIntervalLatestWriterTests.cs:610), a full hour above the bound, so neither exercises a row sitting at the bound, let alone a second below it. TheLateBatchTestsRows_HoldAgainstTheShippedTriggerBound_AtEveryHourOfAPinnedClock varies the hour, not the margin.
The knob is the pinned pair: taking LateTriggerLagHours to 2 and PartialWatermarkLagHours to 3 buys an hour of skew tolerance and costs an hour of live-half width (the read covers the last 3-4 hours of first executions instead of 2-3) — still far from the 3 open days today. If the zero margin is instead a deliberate bet that the two clocks agree to within the time between an interval's start and its first execution, that bet belongs in this doc comment and in PartialWatermarkLagHours, since it is the condition under which the whole rung pays off.
| /// <summary> | ||
| /// Darling only (#5812): true when this pass may reuse the PLAN_REGRESSION candidate rows an earlier current pass on | ||
| /// the same server read, while they are fresh (<c>PgFactCollector.PlanRegressionCandidateCache</c>). Set by the | ||
| /// current-time callers (the scheduled pass, <c>analyze_server</c> and <c>get_analysis_facts</c> with no <c>as_of</c>); | ||
| /// false everywhere else, so a past <c>as_of</c> and every other caller read live, as before. | ||
| /// </summary> | ||
| public bool ReusePlanRegressionCandidates { get; set; } |
There was a problem hiding this comment.
This doc names the scheduled pass as one of the callers that set ReusePlanRegressionCandidates, and the code sets it to the exact complement:
ReusePlanRegressionCandidates = asOfUtc is null && !persistCollectionCaveats,
RefreshPlanRegressionCandidates = asOfUtc is null && persistCollectionCaveats,(DarlingAnalysisService.cs:392-393, where persistCollectionCaveats is "True only for the SCHEDULED pass" per that method's own doc at :362.)
The next property's summary (L270) says the opposite — "true for the scheduled pass with no as_of. It never reuses PLAN_REGRESSION candidates" — so the pair reads as a leftover from before ruling R3 changed the window from six hours to one cadence.
This matters more than a stale sentence usually would, because this property is the seam a future scheduled caller would read, and the body's own handoff note points the other way: "R3 uses persistCollectionCaveats as the scheduled-pass marker (#5633 made it the one caller that sets it). A new scheduled caller must set it, or it would reuse." Someone trusting L264 would conclude the scheduled pass already reuses and that RefreshPlanRegressionCandidates is the odd one out.
Suggest dropping "the scheduled pass" from the list, and saying instead that the scheduled pass sets RefreshPlanRegressionCandidates and never this one.
| /// and fits only through that multiplier, so 64MB keeps the headroom. A hash or sort takes memory only as it grows, so | ||
| /// a small server's read uses what it needs (<c>PlanRegressionDailyNoSpillRig</c> measures every bound on one plant). | ||
| /// </summary> | ||
| internal const string PlanRegressionWorkMemSql = "SET LOCAL work_mem = '64MB'"; |
There was a problem hiding this comment.
Non-blocking, and the body already hands this to the coordinator — recording it here so the number sits next to the constant rather than only in the PR description.
SET LOCAL work_mem = '64MB'(bound 2, and since fix B every PLAN_REGRESSION read) is a per-node figure, not a per-read cap. … The daily read has six such nodes … Scheduled reads run inside each server's sweep, under the fleet sweep gate … 4 by default (DarlingConfig.MaxConcurrentSweeps), at most 16 (StoreConfigProvider.MaxConcurrentSweepsLimit). On-demandanalyze_serverandget_analysis_factscalls that miss the cache run outside that gate. Nothing caps the product of nodes, bound and degree.
Confirmed from the tree: StoreConfigProvider.cs:1473 has MaxConcurrentSweepsLimit = 16, and M2 extends the setting to the exact-bound reads, which are the ones a store runs before its days are built — i.e. the first tick after this rung lands, for every server at once, is also the worst case for this bound. The measured peak is reassuring (≈37 MB in each of two hash aggregates on the plant), but it is one plant on one shape, and the ceiling the server will actually honour is 6 nodes × 64 MB (×2 for a hash, via hash_mem_multiplier) × degree.
The comment above the constant explains the floor (why not 32MB) thoroughly and says "a small server's read uses what it needs", which is true per node but does not bound the product. Two sentences recording the per-read and per-fleet ceiling — and whether the operator is expected to size MaxConcurrentSweeps against it — would stop the next person raising this to 128MB "because the rig said it also did not spill".
|
Full review pass on V185 / #5812. Four inline notes posted; the formal verdict follows in a review. |
There was a problem hiding this comment.
Verdict: CHANGES REQUESTED
Claims
- VERIFIED — V185 adds
built_through+partial_seqand recreates the late-row trigger at one hour before the current hour —PgMigrations.cs:1257-1267. - VERIFIED —
SchemaVersion = 185, rung after dev's V184, probe sentinel at the last ordinal withreturn 185abovereturn 184—StorageVersion.cs:19,PgMigrations.cs:1600,ViewerDataService.cs:1251,:1269,:1477-1484. - VERIFIED — a partial build is invisible to pre-V185 code:
built_seqNULL on a partial build,built_seq = late_seqread as the whole day whateverbuilt_throughholds —PlanRegressionDaily.cs:175-180,:255-265. - VERIFIED — the exclusion is a scalar lookup in the scan filter, and the live floor is the lower of the lowest partial watermark in the contiguous run and a day below the first unbuilt day —
QueryPerf.cs:545,:588-605, five cases inPlanRegressionDailyBoundaryTests.TheWatermarkFloor_*. - VERIFIED —
RefreshAfteris one analysis cadence from the clamped interval (5-360, default 30) —DarlingWorker.cs:543-544,:4684. - VERIFIED — the scheduled pass never reuses and always refills; only no-
as_ofanalyze_server/get_analysis_factsreuse —DarlingAnalysisService.cs:392-393against:362, and:836. - VERIFIED — two queues with the documented orders and caps; the two lock-free stamps run before the plan and write only while
late_seqholds —PlanRegressionDaily.cs:287-301,:318-336,:348-372,:631. - VERIFIED — M2: both exact-bound reads take the work memory in a short read-only transaction disposed right after the read —
QueryPerf.cs:839-848,:946-950. Lite unchanged, its own 14-day read atLite/Analysis/DuckDbFactCollector.QueryPerf.cs:523, deferred to #5823. - UNVERIFIED — every rig figure (4MB spills; 32/64/128MB do not; peaks ~35/37/42MB) and
CompareWithDev— needs a live store and the plant, opt-in behindDARLING_NOSPILL_RIG_PLANS(PlanRegressionDailyNoSpillRig.cs:46-49). - UNVERIFIED — every suite result and the 200-server plan times — no store here, and the body leaves CI unchecked.
- UNVERIFIED — "Steady collection does not fire the trigger … intervals of 60 minutes or less" — true with zero slack, across two machines' clocks; needs two real clocks with drift. Inline note on
PgMigrations.cs:1264-1267. - UNVERIFIED — the RED mutations, "each run once and reverted", including M3's
PartialWatermarkLagHours = 0plant. - REFUTED — the title, "once per server per six hours" — the tree reuses for one analysis cadence (30 min default) and the scheduled pass never reuses. R3 supersedes the title; the title did not follow.
- REFUTED —
AnalysisContext.cs:264names "the scheduled pass" among the callers settingReusePlanRegressionCandidates;DarlingAnalysisService.cs:392sets the exact complement and:270says the opposite. Inline note posted.
Riskiest lines
Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs:1265— the only guarantee that a row below a partial day's watermark invalidates that day, comparing the monitored server'sfirst_execution_timeagainst the store'snow()with zero margin at 60-minute intervals — pinned byQueryStoreIntervalLatestWriterTests.TheLateBatchTestsRows_HoldAgainstTheShippedTriggerBound_AtEveryHourOfAPinnedClockfor the hour dimension, UNPINNED for the margin (SteadyFirstsits an hour above the bound,:610).Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.QueryPerf.cs:545— the watermark lookup alone decides whether a row is counted once, twice or not at all — pinned byPlanRegressionDailyWatermarkLiveTests.FourteenDays_ThreePartial_...(RED with the old exclusion: 0 of 692) andPlanRegressionDailyParityLiveTests.Darling/PerformanceMonitor.Darling.Storage/PlanRegressionDaily.cs:175-180— a wrong whole/partial arm would leavebuilt_seq = late_seqon a partial build, so every reader takes a partly-built day as whole and loses its newest rows permanently — pinned byAPartialDay_IsUnbuiltToTheV183Predicate_AndAV183WholeBuildAfterItReadsWhole_WithNoDoubleCount.
Traps near this diff
- "Ruling (#5833): tests are load-bearing. … never answer a trap with a test that reads source text or asserts how code is spelled (a signature at a line, a SQL fragment's wording, a comment's text, a trait roster, a coverage percentage)." — applies — three new members of
PlanRegressionCandidateCacheEvictionTests(:167-169,:175-190,:192-202) andPlanRegressionDailyRungTests.TheStampAndTheBuiltDaysRead_KeepAPartialBuildInvisibleToPreV185Codeassert only spelling, each with a behaviour twin already in this diff. Inline note posted; the*RungTestsprobe-ordinal pins are excluded, CONTRIBUTING mandates them. - "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 — a PostgreSQL-store mechanism with no DuckDB analogue; Lite reads one local store in one process (
Lite/Analysis/DuckDbFactCollector.QueryPerf.cs:523), has noplan_regression_dailytwin, and the two newAnalysisContextflags default false. - "A retention-tiered store answers a 7-day question from a 4-day raw tier and labels the missing days quiet unless the read routes by tier and says which tier served it" — does not apply — every day is covered by exactly one tier out of one
REPEATABLE READ READ ONLYsnapshot (QueryPerf.cs:819-824), the live tier shrinks, and the source is still stamped (:1080,:1042). - "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — does not apply — nothing pins two copies equal; the shape's
readsDays ? 5 : readsTable ? 4 : 3/watermarkParameter: readsDays ? 5 : null(QueryPerf.cs:898-899) are magic ordinals over the bind block above, not a copy of it. - "
.gitattributessaystext=auto eol=crlf… A diff that touches every line of a file is a line-ending conversion until proven otherwise" — does not apply — the body records this biting during the merge, and the fix held:PgMigrations.csis two localised hunks, and no file of the 42 is a whole-file rewrite.
Divergence (blind read vs PR body)
-
The title has not followed R3. It sells "once per server per six hours"; the shipped mechanism is one analysis cadence with the scheduled pass excluded. The body is internally honest, but the title is what the changelog carries.
-
The trigger's safety argument is thinner than stated, and the test does not test the thin part. The body treats day-long intervals as the one degraded case; on my read the default 60-minute case degrades the same way under ordinary clock drift between the monitored server and the store — silently, back to the pre-#5812 read this PR exists to remove, with queue 1 rebuilding today every tick.
SteadyFirstsits an hour above the bound, so nothing exercises a row at it. Inline note with the knob and its cost. -
The #5833 ruling is applied in one section and violated in three others. The merge prep deletes the
ORDER BYpin because "its rule was spelling", while fix lane B adds source-literal counts,IndexOforderings and aDoesNotContain("Forget"), and fix lane A adds a SQL-wording pin the live test already covers. The body lists these as achievements.
Everything else held. The partial-day arithmetic is where I expected a hole and found none: double-counting and dropped rows check out across the whole-day, partial-day, gap, pre-V185-builder, around-midnight and tomorrow-stamped cases, and the three invariants that close it — a row's side of the watermark fixed for its life, the floor never above any partial watermark in the run, and both stamps writing only while late_seq holds — are as the comments claim.
Reviewer checklist
- Measure the trigger's real firing rate on a 60-minute-interval server with the store on another host: if
built_seq/partial_seqdisagree withlate_seqfor today and tomorrow after an hour of steady collection, the open days read live and the rung is inert there. - Watch resident memory on the store through the first tick after the rung lands, when every server runs the exact-bound read at 64MB at once under
MaxConcurrentSweeps— per-node bound, node count and concurrency all peak together, which the rig's single plant does not cover. - Open the viewer against a V184 and a V185 store: the
pg_attributesentinel is a column probe rather than this probe's usualto_regclasstable probe, so theattisdroppedarm and the NULL-attrelidpath each want one live read. - Run a tick with the service clock minutes off the store's:
PlanSqlplans from$1whileSeqSqlstamps fromnow(), so a service running ahead can plan a build that restamps the watermark it already had. - Confirm the write amplification at fleet scale: today is rebuilt from scratch once per tick per server, so late in the day each rebuild re-reads most of a day of interval rows.
…e trigger, spelling pins dropped (#5812 review 1) - LateTriggerLagHours 1 -> 2 and PartialWatermarkLagHours 2 -> 3 in the unreleased V185: the just-ended 60-minute interval no longer sits exactly on the trigger's bound, so a store clock ahead of the monitored server no longer fires it on every steady apply. The bet is stated on both constants and on V185. - Live tests: rows at the old bound and a second below it stay quiet, also with the store's clock past the hour; a row below the new bound fires. SteadyFirst is the just-ended interval's start. - Four source-spelling test members (and two '2 hours' SQL-text lines) deleted; behavior twins cover them. - AnalysisContext: the scheduled pass sets RefreshPlanRegressionCandidates, never ReusePlanRegressionCandidates. - PlanRegressionWorkMemSql: the per-read and per-fleet work_mem ceiling.
…pg_stat_statements (#5812 CI red) The get_analysis_facts reuse test read the pass's pg_stat_statements entry back. That view is cluster-wide, and this own-store class runs in parallel with classes that reset it for the whole cluster or force evictions, so on CI the entry vanished while the cache counted a hit and no miss. The test now deletes the costly plan's rows after the pass, as the class's first test does: the call reports the regression only by reusing the pass's candidates, and the read past RefreshAfter finds none.
…the one full reset runs alone (#5812)
Claude posting for Erik Darling. Round 1's four findings were answered at 78ec763 (lags 2/3 h, four spelling pins deleted, both doc summaries true, the 64MB ceiling note, new title); the CI-red reuse test was fixed at d334868 and 80fca7c (test-only). Rounds 2 and 3 hit the 30-turn cap with no verdict (error_max_turns, 31 turns), so the 2-round cap applies; no round-3 findings exist to file. Own Build run 38712490231 green on 80fca7c.
Claude posting for Erik Darling.
Fixes #5812.
Why
The scheduled analysis pass runs the PLAN_REGRESSION read for every server on every pass. Since #5448, closed days (three or more days old) come from
collect.plan_regression_daily. The three open days still come fromcollect.query_store_interval_lateston every run. The issue measured, on its largest store, one run for the busiest server reading about a million live interval rows and sorting them to disk twice, in about five seconds. The pg_stat_statements counts in the issue (about 26,000 calls in 20 days, about 1,300 a day) fit a 30-minute cadence across a few dozen servers, plus MCP and viewer calls.What changes
First lane (commit a87e4a9, rebased onto dev): a current pass reuses the candidate rows of a fresh read of the same shape (
PgFactCollector.PlanRegressionCache.cs), and the daily builder planned the newest days first (ORDER BY d.day DESC). That order now holds only for days never built: since the merge prep (L5), queue 1 takes the longest-waiting day first.This lane (commit b5fba44):
plan_regression_daily_builtgainsbuilt_throughandpartial_seq. The builder now plans days from T-14 through T+1 (NewestBuiltDayAhead), so it builds the open days too. A build covers the day's rows (day = the date oflast_execution_time, as before) whosefirst_execution_timeis below the watermark. The watermark is the next midnight for a whole day. For an open day it is three hours before the current hour, read from the store's clock inSeqSql(PartialWatermarkLagHours). An open day is built again only when it has rows whose first execution is in [built_through, the new watermark); otherwiseAdvanceIdleSqlmoves its watermark with no build. NULLbuilt_through(a build from before V185) reads as the whole day, so the migration invalidates no built day.first_execution_time, notlast_execution_time.first_execution_timeis part of an interval row's identity, so it never changes when the collector updates the row. A row stays on one side of the watermark for its whole life. Alast_execution_timewatermark would let a running interval cross it, so the row would be counted in both halves. The interval table is also partitioned and indexed onfirst_execution_time, so the live half touches only the rows after the watermark.built_seq = late_seq(a whole build) orpartial_seq = late_seq(a partial build, through itsbuilt_through). V185 recreates the late-row trigger with its upper bound moved from yesterday's midnight to two hours before the current hour, so a write that could change a partial day marks it. The builder's watermark sits one hour below the trigger's bound, so a write whose transaction began up to an hour before a build still marks the day. Steady collection does not fire the trigger. The collector fetches only intervals whoselast_execution_timeadvanced (QueryStoreCollector.WatermarkColumn), and those intervals start at or after the trigger's bound for intervals of 60 minutes or less, with an hour to spare: the trigger compares the monitored server'sfirst_execution_timewith the store'snow(), so the bound sits an hour below the start of the interval that has just ended, and a store clock up to an hour ahead of the monitored server's stays quiet (review round 1; at the old one-hour bound that margin was zero). A bigger skew, or day-long intervals, fires on every apply, so the open days read live, as today, and pay a trigger call per row.PlanRegressionDailySqltakes$6, each built day's watermark. The live half drops a row when its day is built and its first execution is below that day's watermark (a lookup of the row's day in$3and its watermark in$6, in the scan's filter, replacing<> ALL($3); see the fourth lane).PlanRegressionLiveFloorWithWatermarkssets the floor to the lower of: the lowest partial watermark, and one day below the first day that is not built. On a current store every day through tomorrow is built, so the floor is today's watermark and the live half reads the last two to three hours of first executions.PlanRegressionDailyNoSpillRig.Measure(skipped unlessDARLING_NOSPILL_RIG_PLANSis set): one server, 15 built days of totals (360,000 daily rows, today built through 10:00) and one open hour (24,000 interval rows). At the defaultwork_mem(4MB) the read spilled: the outerplan_aggGROUP BY sorted 384,000 rows with an external merge, about 16MB on disk, about 0.7 s. The fix isSET LOCAL work_mem = '64MB'(128MB in 8de5251; the sixth lane lowered it, see below) in the snapshot transaction (PgFactCollector.PlanRegressionWorkMemSql, run right afterSET TRANSACTION READ ONLY), so it ends with that transaction and touches no other statement on the pooled connection. With it the planner hashes both aggregates in memory (peak about 37MB each at 64MB), no node spills, about 0.4 to 0.5 s. A sort or hash takes memory only as it grows, so a small server's read uses what it needs.PlanRegressionTimeoutCaveatTestsnow counts four commands in the fact (pin moved deliberately) andPlanRegressionDailyBoundaryTestspins theSET LOCALinside the snapshot.RefreshAfteris now one analysis cadence, not six hours. The worker sets it on every scheduling tick from the configured interval, clamped to 5-360 minutes (default 30) (DarlingWorker.cs, next toanalysisIntervalMinutes). The scheduled pass never reuses: it always reads and refills the entry (AnalysisContext.RefreshPlanRegressionCandidates, set wherepersistCollectionCaveatsmarks the scheduled pass). Onlyanalyze_serverandget_analysis_factswith noas_ofreuse, and only a read younger than one cadence. The scheduled pass, its findings and its alerts are as fresh as before.built_throughreads as whole). A store that is behind gets its newest days first: the builder's first hourly tick after start plans newest-first, queue 1 (days with no valid build) at mostMaxBuildsPerTickbuilds per tick, never-built days newest first, then the oldest build first; queue 2 (partial days with rows to add) adds at most the number of builder servers.Lite/Analysis/DuckDbFactCollector.QueryPerf.cs:523,TimeRangeStart.AddDays(-14)overv_query_store_stats). It is a local single-user store with no scheduled-pass/MCP split over a shared store. No change in this PR; whether it needs one is Lite's PLAN_REGRESSION read aggregates 14 days of Query Store intervals on every analysis pass #5823.Rulings (decided):
first_execution_time(it never changes and is the partition key), and the live floor keeps its one-day lookback for a day that is not built (an interval can cross midnight). Both stay as built in b5fba44.late_seqmoves. Today and yesterday (until its watermark reaches its end) are partial days: queue 2 builds one only when it has rows to add, andAdvanceIdleSqladvances an idle one with no build. Tomorrow is not an empty build every hour:StampTomorrowSqlstamps it with no totals, and it is built (queue 1) only when a row dated tomorrow starts below the watermark. The test this line once named was deleted by fix lane A;ThePlan_PutsALateClosedDayFirst_..._At200Serversand the merge prep'sThePlan_PutsALateClosedDayInTheCap_WithinTwoTicks_...pin the plan.Third lane (commit 3ee66a6), tests:
ReadCallerRungTests,ReadLatencyCapacityRungTests) relax to "below the top",RungRewindArtifacts.DropV185SentinelAsyncdropsbuilt_through, and every rewind drops it. The migration-count and version-list pins count V185.PlanRegressionDailyBuilderLiveTests: the builder plans T-14 through T+1 (16 days); the two-queue plan tests named under R2.PlanRegressionDailyTriggerLiveTests: the steady rows are now first executions within the last 40 minutes (always above V185's hour-aligned bound). The recent edge is two hours before the current hour (review round 1), and the test now waits out the hour boundary as well as midnight (WaitPastTheHour, unit-tested).PlanRegressionDailyParityLiveTestsandPlanRegressionDailyReadShapeLiveTestsbind$6(each built day's watermark) and the floor fromPlanRegressionLiveFloorWithWatermarks, as the collector does.PlanRegressionDailyRungTestsbinds and pins$5.PlanRegressionCandidateReuseTests: the reuse window is one cadence and the worker sets it; a scheduled (refresh) pass reads live and refills, and the call after it reuses that fresh read.Fourth lane (commit 21bcdfb; dev merged at 8765368):
BuildDaySqltakes five (08P01: bind message supplies 4 parameters, but prepared statement "" requires 5,PlanRegressionDailyParityLiveTests.cs:334). The 12000 for 9000 was inPlanRegressionDailyReadShapeLiveTests.cs:96and:175. The rows: 3 servers, 3,000 rows per server per day, T-14 through T-3 built, so the live floor is T-3 (one day below the first unbuilt day, T-2). The live half's scan reads server A's rows from T-3 up, four days, 12,000 rows. With<> ALL($3)the T-3 rows were dropped in the scan's own filter, so the scan returned 9,000. With b5fba44'sNOT EXISTS (unnest($3, $6))the planner ran the exclusion as a join above the scan: the scan returned 12,000 and the join dropped T-3's 3,000. The read's answer was right, as the parity test shows; only the scan's shape had changed. Rows read were 36,000 both ways.l.first_execution_time >= COALESCE(($6::timestamp[])[array_position($3::date[], l.last_execution_time::date)], '-infinity')(PgFactCollector.QueryPerf.cs:544). A built day keeps only its rows at or after its watermark. A day that is not built finds no watermark and keeps every row. The per-row cost is the same as<> ALL($3)(a search of at most 17 days), and no join strategy rests on the planner's estimate of an array. After the change the scan returns 9,000 on the heap and on day partitions, reads 36,000, and scans 7 of 21 partitions. With theNOT EXISTSput back, both read-shape facts fail with 12000, so they pin it.PlanRegressionDailyParityLiveTests, the test's build now binds$5and stampsbuilt_through, as the builder does. A new step also builds the empty days from the window's floor, so the live floor rises to the lowest watermark, as on a current store. It then builds three partial days. Day 2 is cut at 13:00, between its interval at 12:00 and the late interval at 14:00. Day 9 is cut at 11:00, so its regressed plans' intervals are all live, and only the interval that crossed midnight into it is in its totals. Day 10 is cut at its own midnight, so its totals hold only the interval that crossed into it from 23:40. The daily read equals the table read. Read as whole days, it does not, so the step is not vacuous. Each row is counted once: below its day's watermark in the totals, at or above it in the live half.PlanRegressionInputsLiveTests(intervalTable: True, daily: False). Not a code failure. The class's cleanup did not remove the days its daily case built. A rerun on the same database found them valid under the next run's coverage claim, so the interval-table case took the daily path. It passed on a fresh database and failed on the rerun. The cleanup now deletescollect.plan_regression_dailyandcollect.plan_regression_daily_builtfor its servers, and two runs in a row on one database pass. Dev has the same gap; it showed here because the lanes reran on one database.QueryStoreComposeStampTests: the "behind" and "at this rung" mappings clear the V185 sentinel (ProbeOrdinal + 11).QueryStoreIntervalPartitionRungTests(the V170 upgrade): the trigger text after migrating must be the V170 text with one change, V168's upper bound replaced by V185's, both aspg_get_triggerdefrenders them.QueryStoreIntervalBoundedReadPruningLiveTestswas not on the previous list; it failed with42601: wrong number of parameters for prepared statement. Its daily build case now binds$5(D + 1 day for a whole day) and its daily read case binds$6.Fifth lane (commits c4594df, dbeeb3a, 0169dc4, 8de5251), tests:
pgsql/share/extension/pg_stat_statements.control. On a fresh rig from the zip (port 5571, UTC,timescaledb,pg_stat_statementspreloaded, CI's worker settings),pg_config --sharediris the extractedshare, the control file is under it, andCREATE EXTENSION pg_stat_statementsgives 1.12.PlanRegressionDailyBuilderLiveTests:Total: 12, Failed: 0, includingTheGc_ProbesTheTotalsByIndex.... The fourth lane's failure was its rig, not the zip.PlanRegressionLiveFloorWithWatermarksunit cases (PlanRegressionDailyBoundaryTests, beside the existing live-floor cases, which live there and not inPgFactCollectorTests): nothing built (the window less one day), contiguous whole days (a day below the first unbuilt day; a watermark exactly at the next midnight is whole), partial days (the lowest partial watermark), a gap (the walk stops; a partial day after the gap takes no part), and the bound (built days below the window are ignored; the floor never falls more than one day below the window). The brief said "the lowest partial watermark, less the lookback"; the code takes no lookback below a partial watermark, and that is right: a built day's live rows are exactly those at or above its watermark. RED mutations: taking a day off below a partial watermark (floor = t.AddDays(-1)) fails 3 of the 5 new cases and none of the old; treating a watermark at the next midnight as partial (<=) fails 5; dropping the unbuilt lookback fails 9.PlanRegressionDailyWatermarkLiveTests): a late UPDATE to two closed days goes through the real trigger, which marks each day and the day after it, so four of twelve built days go stale. Their totals are then tampered with (plan 1 made 1000 times dearer). The read takes totals for the 8 current days only, and its rows equal the interval-table read. Handed the stale days as valid, it differs, so the check is not vacuous. FromEXPLAIN (ANALYZE, BUFFERS, FORMAT JSON): the interval scan returns exactly the stale days' 1,600 planted rows, and the daily scan exactly the current days' 3,200 totals rows. RED: withbuilt_seq = late_seqdropped fromBuiltDaysSql, it fails.Index Condisfirst_execution_time >= '<T-2 20:00>'. The scan returns exactly the rows after each day's watermark (692, counted by a separate statement with the watermarks spelled out), and reads no more than the rows at or above the lowest watermark. The daily scan returns exactly the built days' totals. Sequential scans are off for this EXPLAIN, so the CI-sized table shows the index path. RED: with the pre-V185 exclusion (<> ALL($3)), the live half returns 0 of 692.get_analysis_factsat 1 h, live (PlanRegressionCandidateReuseLiveTests):DarlingAnalysisService.AnalyzeAsync(..., 4, persistCollectionCaveats: true)(the worker's call) reads and fills the entry. ThenCollectAndScoreFactsAsync(hoursBack: 1)(the tool's entry point, noas_of) makes no interval read. Proved by both pg_stat_statements (theplan_agginterval-readcallsin this database are unchanged) and the cache'sHitscounter (a test cache set on the service's own collector). WithRefreshAfterset to zero, the next call reads fresh (+1 call, +1 miss). RED: with the facts call'sReusePlanRegressionCandidatesforced false, the call count is 2, not 1. The test skips whenpg_stat_statementsis not preloaded. From 01:00 to 04:00 UTC, the 1-hour window starts a day later than the 4-hour pass's, so the shape differs and the call reads live; the test asserts that branch by the clock.SlowClassCensusTests,LiveClassStoreSkipCensusTests,ScratchDatabaseDisposalCensusTests,LivePostgresCollectionHygieneTests,LiveCleanupConversionRatchetTests,TimingTestCensusTestswith the new class:Total: 50, Failed: 0.Sixth lane (commits 06e1991, 48312da):
PlanRegressionCandidateCache.Shared: ruled out by the code. In the failing commit (0169dc4, line 231) the test already set its own cache instance on the service's collector (EngineFor(null).Collector, the same_sqlServerEnginethatAnalyzeAsyncresolves for this unregistered server), andHitsis that instance's counter. No other class can reach it, andSharedis never read by that collector after the setter. The collector already took its cache through the internal setterPlanRegressionCache, withSharedas the default that only the worker configures, so that part of the brief needed no product change. (b) pg_stat_statements evicted the read's entry: proved live in the combined run. On a fresh rig (pg_stat_statements.max5000), one test alone makes about 900 entries (its scratch database's migrations), and one combined*PlanRegression*run made 151 and 165 deallocations in about 70 s, holding the table near 4,900 entries. Each deallocation drops the least-used 5% (about 250 entries; among one-call entries, the oldest), so an entry with one call lives on the order of ten seconds. The old check summedcallsin a fresh database: 0 before the pass and 1 after it. If the entry was evicted and the call then read live, the new entry started at 1, soafterFacts == afterPasspassed while a read ran. The old text filter also missed the raw read variant entirely (it requiredFROM query_store_interval_latest). Why the call missed the cache in that one run was not reproduced: three combined runs and one solo run with the hardened test were all green, and the rig's server log showed only the errors other tests provoke on purpose, plus one statement timeout in the run that added the command-timeout classes. If it recurs, the failure message now names the hits, the misses and every read entry by variant, which separates a changed shape (a miss plus a daily read), a changed source (a miss plus a table or raw read) and an entry that was never stored.FROM plan_agg), classifies each as daily, table or raw, and holds the pass's entry to its query id andstats_since. The pass's read must be exactly one daily entry of one call. After the 1-hour call the entries must be unchanged, the cache must show 1 hit and 0 misses, and an entry that is missing or remade fails the test instead of passing it. The refresh step must add one call to the same entry. The cache stays the test's own instance.PlanRegressionDailyNoSpillRig(360,000 daily rows plus 24,000 open-hour rows), with every bound run in the same session: 4MB spills (an external merge of about 15MB, about 0.5 to 0.8 s). 32MB, 64MB and 128MB all hash in memory with no spill, in about 0.4 to 0.5 s, with hash aggregate peaks of about 35MB, 37MB and 42MB. The lowest bound that does not spill, 32MB, is below its own peak: it fits only because a hash may use twice work_mem (hash_mem_multiplier2). By the rule (keep the next bound up when the lowest is within 20% of its peak), the bound is 64MB. The rig now runs the weighed bounds (32MB and the 128MB it replaced) on the same plant and asserts only the collector's own value. It was planted twice, once to measure 32MB and 64MB beside 128MB, and once to assert the 64MB constant. Each plant took about 7 s.Test plan
Darling.Testsbuilds, 0 warnings, 0 errors.*RungTests*,*PlanRegression*,ViewerSchemaVersionGateTests,CaggGroupIndexDropLiveTests,ReadLatencyFlushLiveTestsandQueryStoreComposeStampTests:Total: 576, Failed: 42.Total: 578, Failed: 17.AwsPerServerRoleRungTests,PlanRegressionCandidateReuse*,PlanRegressionRowcountRungTests,QueryStoreComposeStampTests,QueryStoreWatermarksRungTests,RunnableQueueSettingsRungTests,PlanRegressionDailyRungTests,PlanRegressionDailyBuilderLiveTests,PlanRegressionDailyTriggerLiveTests,ReadCallerRungTests,ReadLatencyCapacityRungTests,ExternalAlertRungTests):Total: 105, Failed: 2. One isQueryStoreComposeStampTests(see Not done). The other,TheGc_ProbesTheTotalsByIndex..., fails on the rig only:pg_stat_statementswas not available there.timescaledb,pg_stat_statementspreloaded, CI's worker settings):PlanRegressionDailyParityLiveTests,PlanRegressionDailyReadShapeLiveTests,PlanRegressionInputsLiveTests:Total: 11, Failed: 0; the inputs class alone a second time on the same database:Total: 8, Failed: 0.QueryStoreIntervalBoundedReadPruningLiveTestsandQueryStoreComposeStampTests:Total: 22, Failed: 0.*RungTests*,ViewerSchemaVersionGateTests, every*PlanRegression*class,QueryStoreComposeStampTests,QueryStoreIntervalBoundedReadPruningLiveTests,PgFactCollectorTests,QueryStoreBackgroundIndexesTests,CaggGroupIndexDropLiveTests,ReadLatencyFlushLiveTests:Total: 612, Failed: 1, Skipped: 1. The failure wasPlanRegressionDailyBuilderLiveTests.TheGc_ProbesTheTotalsByIndex...with0A000: extension "pg_stat_statements" is not available. That rig's cause is unknown: the runtime zip does have the control file (fifth lane, below), and the test passes on a fresh rig from the zip.Census*andRatchet*class,GuardStageCensusTests,PlanForceNoWritePathTests,DocCommentHygiene*,LivePostgresCollectionHygieneTests:Total: 683, Failed: 0.PlanRegressionDailyBuilderLiveTests:Total: 12, Failed: 0(TheGc_ProbesTheTotalsByIndex...passes).PlanRegressionDailyBoundaryTests:Total: 12, Failed: 0.PlanRegressionDailyWatermarkLiveTests:Total: 2, Failed: 0.PlanRegressionCandidateReuse*alone:Total: 8, Failed: 0. Each RED mutation above was run once and reverted.PlanRegressionDailyNoSpillRig.Measure, run on the rig: before, spill yes (sort, external merge, about 16MB); after, spill no (about 0.4 s). The rig was planted twice (once before the fix, once for before and after in one plant), about 8 s each.SET LOCALcommit (Total: 172, Failed: 1, Skipped: 2, the failure beingGetAnalysisFactsAtOneHour...atcache.Hits) is superseded by the sixth lane's runs below.timescaledb,pg_stat_statementspreloaded, CI's worker settings,pg_stat_statements1.12,pg_stat_statements.max5000).GetAnalysisFactsAtOneHour...alone:Total: 1, Failed: 0.*PlanRegression*class,PgFactCollectorTests,QueryStoreComposeStampTests,QueryStoreIntervalBoundedReadPruningLiveTests) twice in a row with the hardened test:Total: 172, Failed: 0, Skipped: 2both times (the two manual rigs). pg_stat_statements deallocations during the runs: 151 and 165.PlanRegressionDailyNoSpillRig.Measure(DARLING_NOSPILL_RIG_PLANS=24000): 4MB spills; 32MB, 64MB and 128MB do not; the 64MB constant passes the rig's assertion.*PlanRegression*class,PgFactCollectorTests,QueryStoreComposeStampTests,QueryStoreIntervalBoundedReadPruningLiveTests,StorageCommandTimeoutTests,StartupCommandTimeoutTests:Total: 211, Failed: 0, Skipped: 2(PlanRegressionDailyTriggerCostRig.MeasureandPlanRegressionDailyNoSpillRig.Measure, manual rigs).PlanRegressionTimeoutCaveatTestsis in the*PlanRegression*set.*Census*and*Ratchet*class (includingSlowClassCensusTests,LiveClassStoreSkipCensusTests,ScratchDatabaseDisposalCensusTests),GuardStageCensusTests,PlanForceNoWritePathTests,DocCommentHygiene*,LivePostgresCollectionHygieneTests:Total: 683, Failed: 0.Fix lane B (review round 1)
Commits 463fc69 and c93e69c (M2, L1, L2, L3, L6, L7, L8), rebased onto fix A (1da1a92). The rig numbers below were measured before that rebase, at 48312da plus this lane; they were not run again after it. Rig: runtime zip, UTC, port 5689,
timescaledb,pg_stat_statementspreloaded, CI's worker settings, serverwork_mem4MB.M2, the fallback reads now run under the same work memory. The table read (no day built yet, or the built-days read failed) and the raw read (a server without coverage) used to run on an ordinary statement at 4MB. They now run in a short read-only transaction of their own, and a local function in the fact method (
BoundTheReadAsync) runsSET TRANSACTION READ ONLYandPlanRegressionWorkMemSqlon whichever transaction holds the read. The constant is still the one place that sets the value (renamed fromPlanRegressionDailyWorkMemSqlin the merge prep, L4). The transaction is disposed right after the read, as the daily snapshot is, so the plan reads and the next user of the pooled connection keep the server's value. Rig check (PlanRegressionDailyNoSpillRig.MeasureFallback,DARLING_NOSPILL_RIG_PLANS=24000: the S4 size, 15 days of 24,000 plans as interval rows plus the open hour, about 384,000 rows, no built day),PlanRegressionTableSqlunderEXPLAIN (ANALYZE, BUFFERS): before (4MB, no transaction) aSort Method: external merge, about 5,000 temp blocks written and 5,000 read, about 0.9 s; after (the collector's transaction andSET LOCAL) no node spills, 0 temp blocks, about 0.6 s.L1 and L3, the cache.
Forgetand its doc claim are gone. The doc now says a failed read stores nothing, so the older entry stays and serves untilRefreshAfterfrom its own read.Storeevicts every entry readRefreshAfteror more before the new one, and every entry of a server outside the set the service last published (RetainServers, called next to both_registryState.Publishcalls inDarlingWorker, at start and on every reload). An entry is removed only if it is still the one judged (TryRemove(KeyValuePair)), so a concurrent store for that server is not lost. A test's own cache never publishes a set, so it evicts by age only. Every decision is logged at Debug with its reason (ReuseDecision: hit; no entry, not current, stale, watermark moved, shape differs), and the scheduled pass logs its refresh. The daily shape puts$6(the watermarks) last, after its own separator, so a builder tick that moves a watermark is reported asWatermarkMoved. The table and raw shapes now carry the exactrawWindowStart, so a 1-hour call never takes a 4-hour pass's exact-bound read. One consequence:rawWindowStartmoves with the clock, so an exact-bound read is in practice never reused. Reuse is now a daily-read matter, which is the case the issue measured.L2. The reuse stays for an on-demand
analyze_serverwithoutas_of. The cache doc's "What an operator sees" and the comment aboveif (context.PersistFindings)inDarlingAnalysisServiceboth say that such a pass saves findings that may come from a read up toRefreshAfterold: the same band as a scheduled pass's own findings when the next call reads them.L7.
PlanRegressionDailyWatermarkLiveTestsno longer assertsIndex Condtext. It asserts rows from the plan's JSON nodes (returned plus removed by filter or recheck, times loops, exact with parallelism off): the interval scans read no more rows than start at or above the lowest watermark, they return exactly the rows after each day's watermark, and the daily scan returns exactly the built days' totals. No other executed-plan assertion in this PR readsIndex Cond.L6 and L8, the body. The memory figure is restated per node under "For the coordinator to double-check". The field figures are gone from the ENTRY, and "Why" cites the issue's figures as the issue's. "For built days" replaces "after the watermark". Item 6 names Lite's PLAN_REGRESSION read aggregates 14 days of Query Store intervals on every analysis pass #5823, and the ENTRY's work memory claim now covers both reads.
The issue's before and after (
PlanRegressionDailyNoSpillRig.CompareWithDev). This is the S4 plant: 15 days of totals for 24,000 plans, today built through 10:00, and the open hour. On top of that, T-2, T-1 and today up to 10:00 carry an interval every four hours, about 360,000 more rows, so dev's read has its three open days to read. Dev's statement is theorigin/devprefix copied verbatim into the rig, plus the shared suffix, which this PR does not change. It gets dev's inputs: the 12 closed days and dev's live floor. All runs are on one plant, three runs each, rounded:Client wall time was within 0.3 s of execution time on every run. The partial days cut the read's buffers by about 60%. On this plant they do not cut its time: the 15 days of totals the branch groups (360,000 rows) cost about what dev's 12 days of totals plus three days of intervals cost. Against dev as it ships, the time falls because the read no longer spills. On a laptop rig the spread between runs is as wide as the difference between the two 64MB reads.
Tests (this lane).
Darling.Testsbuilds with 0 warnings and 0 errors.PlanRegressionCandidateCacheEvictionTests(new: eviction by age and by server set, the entry just stored survives, every decision's reason, the moved watermark, therawWindowStartmiss, the work memory pin for both reads, noForgetand the worker's twoRetainServerscalls),PlanRegressionCandidateReuse*,PlanRegressionDailyWatermarkLiveTestsandPlanRegressionDailyBoundaryTests:Total: 30, Failed: 0. The rigsMeasureFallbackandCompareWithDevpass. Gate run before the rebase, on one database, every*PlanRegression*,*QueryStore*,*Census*and*Ratchet*class plusGuardStageCensusTests,PlanForceNoWritePathTests,DocCommentHygiene*,PgFactCollectorTests,StartupCommandTimeoutTests,StorageCommandTimeoutTestsandLivePostgresCollectionHygieneTests:Total: 1996, Failed: 4, Skipped: 5. (1)TimingTestCensusTestswas this lane's: the rig'sStopwatch. The rig now has an allow-list entry with its reason (it prints wall time and never asserts on it), and the class passes. (2)QueryStoreWatermarkReaderLiveTests(hypertable: True) timed out under load and passes alone (Total: 22, Failed: 1for it and the writer class together, the one failure being (3)). (3)QueryStoreIntervalLatestWriterTests.APlanRegressionLateBatch_ThroughTheRealWritePath_MarksItsDays_AndSteadyBatchesLeaveTheBuiltTableEmptyfails alone too: steady batches marked today and tomorrow ([2026-10-14] = 1, [2026-10-15] = 1), in a run just after midnight UTC. (4)QueryStoreBackgroundIndexesLiveTests.ThePerDayBuildAndTheServerFloor_UseTheLatestBtree_OnceItIsBuilt:08P01: bind message supplies 4 parameters, but prepared statement "" requires 5, so it bindsPlanRegressionDaily.BuildDaySqlwith dev's four parameters. Neither class (3) nor (4) is in this PR's diff, and both read code V185 changed (the late-row trigger's bound andBuildDaySql's fifth parameter), so they look like this PR's fallout, not flakes. They are left to fix A or the coordinator: both sit on the builder and trigger, which are fix A's. After the rebase onto fix A:Darling.Testsbuilds with 0 warnings; the touched classes plusTimingTestCensusTestsandPlanRegressionTimeoutCaveatTests:Total: 40, Failed: 0, Skipped: 2(two of fix A's new watermark tests, which skip themselves).Leftovers. None from this lane's items. Fix A owns H1, M1, M3, L4 and L5. For the coordinator: two failures in classes this PR does not touch (see the gate run above) look like V185 fallout and need an owner before CI.
Not done
None.
For the coordinator to double-check
persistCollectionCaveatsas the scheduled-pass marker ([BUG] MCP tool audit on 3.9.0-nightly.20261006.515: 23 defects (blocking, spills, FinOps, store, tool payloads) #5633 made it the one caller that sets it). A new scheduled caller must set it, or it would reuse.RefreshAfteris process-wide (PlanRegressionCandidateCache.Shared). The worker sets it on every server's scheduling tick, from the live config value.SeqSql), whilePlanSqlplans from the service's clock. A small skew between them can rebuild a day one extra time; it cannot leave a day stale.$3and$6by position (array_position). Both arrays are built from the oneBuiltDaysSqllist (PgFactCollector.QueryPerf.cs, thebuiltDaysbinds), and a day appears in it once (the built table's key is server and day).SET LOCAL work_mem = '64MB'(bound 2, and since fix B every PLAN_REGRESSION read) is a per-node figure, not a per-read cap. Each sort or hash node of the read may take up to 64MB, and a hash node up to 64MB xhash_mem_multiplier(2) = 128MB. The daily read has six such nodes: the inner live HashAggregate, the outerplan_aggaggregate,plan_dedup(array_agg ... ORDER BY), two window sorts and the final sort. The measured plant used about 37MB in each of its two hash aggregates. Scheduled reads run inside each server's sweep, under the fleet sweep gate (DarlingWorker.ProcessServerSweepAsync, which holds a permit throughRunScheduledAnalysisAsync), so as many run at once asMaxConcurrentSweepsallows: 4 by default (DarlingConfig.MaxConcurrentSweeps), at most 16 (StoreConfigProvider.MaxConcurrentSweepsLimit). On-demandanalyze_serverandget_analysis_factscalls that miss the cache run outside that gate. Nothing caps the product of nodes, bound and degree.## Merge prep (round 2 Lows, V185).Fix lane A (review round 1)
Commit 1da1a92 (one commit; pushed to the issue branch). Fix B's files (
PgFactCollector.QueryPerf.cs,PgFactCollector.PlanRegressionCache.cs, the reuse tests) and the ENTRY are untouched.Why two stamps exist that are not builds (approved by the coordinator, outside the cap): the live floor (
PlanRegressionLiveFloorWithWatermarks) is the lower of the lowest partial watermark in the contiguous built run and the first unbuilt day less one day. So (a) a tomorrow that is never built puts the floor at today's midnight (the live half reads all of today, not the last 2-3 hours), and (b) a partial day of a server that goes idle in the evening never gets a row past its watermark, stays partial for good, and holds the floor there for 14 days (the #5812 cost again).PlanRegressionDaily.PlanSqlnow plans in two queues. Queue 1: days with no valid build (never built, orlate_seqmoved), any day of the series,day DESC, server_id, at mostMaxBuildsPerTick(120). Tomorrow is in it only when a row dated tomorrow starts below the watermark. Queue 2: valid partial days whose watermark can advance AND that have a row of the day withfirst_execution_timein [built_through, new watermark), orderedbuilt_at(oldest first), at most the number of builder servers. Both underMaxTickDuration. Before the plan, the tick runsAdvanceIdleSql(a valid partial day with nothing in that span gets the new watermark, or a whole stamp at its end) andStampTomorrowSql(tomorrow with no valid build, no totals and no row dated it below the watermark is stamped with no totals). Both read the store's clock asSeqSqldoes, and both write only whilelate_seqhas not moved (the advance'spartial_seq = late_seqis rechecked on the locked row; the stamp takes the snapshot'slate_seqand its conflict arm writes only while the row still holds it and holds no valid build).TickResultgainsAdvancedandStamped.PlanRegressionDailyBuilderLiveTests.ThePlan_PutsALateClosedDayFirst_ThenEveryServersTodayOldestBuildFirst_AndNoEmptyTomorrow_At200Servers(replaces the R2 test): 200 servers, one late closed day on server 150; the plan is that day, then all 200 todays inbuilt_atorder, no tomorrow; then a row dated tomorrow below the watermark on server 7 puts (7, tomorrow) first in queue 1.PlanRegressionDailyWatermarkLiveTests.ATick_AdvancesAnIdleDayWithoutABuild_StampsTomorrow_AndTheFloorIsTodaysWatermark: idle server A's today carries the new watermark with no plan slot; A's T-2 (partial at 22:00) is stamped whole and its read equalsPlanRegressionTableSql; tomorrow is stamped partial with no totals; A's floor equals today's watermark; the plan is exactly server B's today.partial_seqwithbuilt_through.MarkBuiltSql: whole build stampsbuilt_seq, clearspartial_seqandbuilt_through; partial build stampspartial_seqandbuilt_through,built_seqNULL.BuiltDaysSql: whole whenbuilt_seq = late_seq(watermark NULL, whateverbuilt_throughholds), partial atbuilt_throughwhenpartial_seq = late_seq.RungRewindArtifacts.DropV185SentinelAsyncdrops both columns; comment corrected. The probe sentinel staysbuilt_through.PlanRegressionDailyRungTests.V185_AddsBothPartialDayColumns_...andTheStampAndTheBuiltDaysRead_KeepAPartialBuildInvisibleToPreV185Code.PlanRegressionDailyWatermarkLiveTests.APartialDay_IsUnbuiltToTheV183Predicate_AndAV183WholeBuildAfterItReadsWhole_WithNoDoubleCount.PartialAwareStamp).PlanRegressionDaily.LateTriggerLagHours = 1; V185's trigger SQL interpolates it (V185Sql is nowstatic readonly, same rendered text). Pinned pair:PartialWatermarkLagHours >= LateTriggerLagHours + 1. Live:ABuildOnTheStoreClock_StampsTwoHoursBeforeTheHour_AndAWriteBelowAnAdvancedWatermarkMarksTheDay: a realBuildDayAsyncstampsdate_trunc('hour', now() AT TIME ZONE 'UTC') - 2h; set an hour back, the idle advance restamps it (coordinator condition 2); a row at W - 30 min through the real trigger moveslate_seq; the daily read equalsPlanRegressionTableSql. The stamp tests skip before 04:00 UTC and wait out an hour boundary under 3 minutes off.ClosedDayLagDays(now "a test pin only"),BuildDaySql,MaxBuildsPerTick,MarkBuiltSql,BuiltDaysSql,PlanSql,RunTickAsync, V185's doc.Tests (rig: runtime zip, UTC, port 5647,
timescaledb,pg_stat_statementspreloaded, CI's worker settings):Darling.Testsbuilds, 0 warnings.*PlanRegressionDaily*,*PlanRegressionBuiltDays*,*PlanRegressionInputs*,*RungTests*,*Census*,*Ratchet*,PlanForceNoWritePathTests,*DocCommentHygiene*,QueryStoreComposeStampTests,ViewerSchemaVersionGateTests,CaggGroupIndexDropLiveTests:Total: 1147, Failed: 2, Skipped: 2. The two (TimingTestCensusTests: a Stopwatch in the 200-server test;DocCommentHygieneTests: a displaced summary) were fixed; rerun of those two plus the three touched PlanRegressionDaily classes:Total: 78, Failed: 0. The stamp tests ran (store clock 23:xx UTC) and passed.Leftovers (handoff; the watchdog's 200k mark was reached):
PartialWatermarkLagHours = 0) was not run. Expected red line: thebuilt_throughassertion inABuildOnTheStoreClock_...(PlanRegressionDailyWatermarkLiveTests.cs, the firstAssert.EqualafterBuildDayAsync).TimingTestCensusTests).PlanRegressionCandidateReuseTestsone-hour test: capture the pass's window end, compute both floors withPlanRegressionWindowFloor, skip when they cross; band 01:00-04:00 UTC) not done.SlowClassCensusTests,LiveClassStoreSkipCensusTests,ScratchDatabaseDisposalCensusTestsran inside the*Census*filter above (green); no new class was added.PlanRegressionCandidateReuse*,PgFactCollectorTests,QueryStoreIntervalBoundedReadPruningLiveTests(fix B's area;BuiltDaysSqlchanged its second column, so they read it).Fix lane A2 (review round 1)
Commits 88baf9f, 0a726fc, 5894832, bcbea42 (pushed to the issue branch; fix B's files and the ENTRY untouched). Rig: runtime zip, UTC, port 5648,
timescaledb,pg_stat_statementspreloaded, CI's worker settings. The store clock read 00:09-00:40 UTC during the run.PlanRegressionCandidateReuse*,PgFactCollectorTests,QueryStoreIntervalBoundedReadPruningLiveTests:Total: 28, Failed: 0(no pin moved;PgFactCollectorTestsdoes not nameBuiltDaysSql). The two the coordinator added (88baf9f):QueryStoreBackgroundIndexesLiveTests.ThePerDayBuildAndTheServerFloor_...boundBuildDaySqlwith 4 parameters (08P01: bind message supplies 4 parameters, but prepared statement "" requires 5). It now binds V185's watermark, D + 1 day (a closed day, a whole build).QueryStoreIntervalLatestWriterTests.APlanRegressionLateBatch_...: the test was wrong, not the trigger. Its "steady" row started atnow - 3h, which is below V185's bound (date_trunc('hour', now()) - 1h) at every hour, not only after midnight: V185 moved the bound from yesterday's midnight to the open days on purpose, and a row that old changes a partial day's totals, so marking it is required. The steady rows now start at the current hour's start (SteadyFirst), which stays at or above the bound even if the hour rolls over once mid-test; the fault-arm test's steady rows (now - 1h, a seconds-wide race at the hour) take the same shape. NewTheLateBatchTestsRows_HoldAgainstTheShippedTriggerBound_AtEveryHourOfAPinnedClockreads the trigger's WHEN back from the migrated store (pg_get_triggerdef), pinsnow()to every hour of a day at :00:00 and :59:59, and asserts steady rows never fire it (also one hour later), the late batch always does, and anow - 3hrow always does. Both classes:Total: 17, Failed: 0at 00:2x UTC.PartialWatermarkLagHours = 0(reverted):PlanRegressionDailyWatermarkLiveTests.cs(370)(thebuilt_throughassertion afterBuildDayAsync)Assert.Equal() Failure: Strings differ, Expected: "2026-10-14 22:00:00", Actual: ""(lag 0 at 00:xx makes the build whole, sobuilt_throughis NULL; after 04:00 the actual is the current hour).EXPLAIN (ANALYZE, BUFFERS)in a rolled-back transaction, 3 runs, a temporary probe not committed):PlanSqlabout 12 ms (planning under 1 ms, 201 rows),AdvanceIdleSqlabout 5 ms,StampTomorrowSqlabout 2 ms warm (10 ms cold). Far under 2 s; no finding.PlanRegressionDailyWatermarkLiveTests.TheTicksStampsDuringAHeldPartialBuild_LeaveTheDayConsistent_AndTomorrowWithoutTotals: a partial day set an hour back; a builder holds server A's lock and has readlate_seqand the watermark (BuildDayAsync'safterSeqReadhook), and a second connection runs the tick's stamps (StampWithoutBuildAsync); the idle advance restamps the day (advanced = 1); the build finishes. Asserted:built_through= the watermark,partial_seq = late_seq,built_seqNULL; the daily read (totals plus live rows after the watermark) equalsPlanRegressionTableSql; no totals for any day after today.ASecondBuilder_...'s loosened assertion (built_seq IS NOT NULL AND day <= 03-20is 0) is replaced by the exact one: the one stamped day is tomorrow (03-21) withsource_rows0 (and the totals count above is 0). Race argument: neither stamp changeslate_seqor any totals, so the held build's stamp (partial_seq= thelate_seqit read) stays valid and its totals cover exactly the rows below its own watermark; a stamp that lands after the build commits re-checkspartial_seq = late_seqand the no-rows-in-span test against the committed row, and a row written into the span after the stamp's snapshot sits below the trigger's bound, so itslate_seqbump makes the day stale rather than silently advanced. Not RED-planted (not asked); the interleaving tested is the one the hook offers (stamps between the seq read and the aggregate), not stamps blocked on the build's row lock afterMarkBuiltSql.PlanRegressionWindowFloorat both ends, skips when either pair crosses a midnight, and compares the two floors. The comment's band is 01:00-04:00 UTC (00:00-01:00 both windows start on the day before). Ran at 00:28 UTC (same-day branch):Total: 8, Failed: 0. The PR body's older text "outside 00:00-04:00 UTC" sits under 'For the coordinator to double-check' (line 113 of the body); the band there should read 01:00-04:00 UTC.Tests:
Darling.Testsbuilds, 0 warnings.*PlanRegression*,*RungTests*,*Census*,*Ratchet*,GuardStageCensusTests,PlanForceNoWritePathTests,*DocCommentHygiene*,QueryStoreIntervalLatestWriterTests,QueryStoreBackgroundIndexesLiveTests,PgFactCollectorTests,QueryStoreIntervalBoundedReadPruningLiveTests,LivePostgresCollectionHygieneTests,ViewerSchemaVersionGateTests:Total: 1270, Failed: 0, Skipped: 5(the three no-spill rig measures and the trigger-cost rig, opt-in;ATick_AdvancesAnIdleDayWithoutABuild_..., which skips before 04:00 UTC).Merge prep (round 2 Lows, V185)
Merged
origin/dev(ae0fca7, which carries #5819's V184) with a merge, not a rebase. 18 files conflicted, all at the top of the ladder; each kept both sides.PgMigrations: dev'sV184Sql(investigations) stays V184; this PR's isV185Sql,new Migration(185, "plan-regression-partial-days", ...)after it.StorageVersion.SchemaVersion = 185.ViewerDataService: thebuilt_throughprobe line follows the investigation line,GetBoolean(160)added,MapProbedSchemaVersion(..., hasInvestigations, hasPlanRegressionPartialDays), thereturn 185arm abovereturn 184.PlanRegressionDaily.csdoc mentions say V185.RungRewindArtifacts:DropV184SentinelAsyncis dev's (investigations);DropV185SentinelAsyncdropsbuilt_throughandpartial_seq, and every rewind that drops V184 drops V185 first (CaggGroupIndexDropLiveTestsgains asimulatedVersion < 185step). Rung pins moved in their house form: everyProbeOrdinal + Nlast-ordinal pin gains one ordinal;parameters[^N]lists gainhasPlanRegressionPartialDaysat[^1]; the "absent" cascades gain awithoutV185step; the version lists and migrate counts count V185 (QueryStoreIntervalPartitionRungTests15 rungs,Scripts[^14]/[^15];QueryStoreWatermarksRungTests178-185). Dev's ownInvestigationRungTestsmoved too: V184 is now just below the top (Scripts[^2],parameters[^2]), its live upgrade drops V185's sentinel and expects both rungs to apply. Test renames:V185_AddsBothPartialDayColumns_...,..._InvisibleToPreV185Code.PgFactCollector.QueryPerf.cs) now states the rule: a day not built is all live, a built day keeps only its rows at or above its watermark ($6) live. This changes the statement text, so its pg_stat_statements query id.PlanRegressionLiveFloor's doc says it is the whole-day case ofPlanRegressionLiveFloorWithWatermarksand that only tests call it.MaxBuildsPerTick,PlanSql,PlanBuildsAsync): the queue's total is capped at the number of builder servers, not one per server. The SQL is unchanged.built_seq = late_seqORpartial_seq = late_seq, the caps as they are, the R2 line and the deleted test's name, the rung line, and the ENTRY ("adds the partial-build columns"). Every V184 that named this PR's rung now says V185.PlanRegressionDailyWorkMemSqlisPlanRegressionWorkMemSql, in the production file and its three test files.built_at ASC NULLS FIRST, day DESC, server_id: a never-built day first (so a store that is behind still gets its newest days first), then the oldest build. A server whose today goes stale on every apply rebuilds it every tick, so itsbuilt_atkeeps moving, and a late-marked closed day passes it. New live testThePlan_PutsALateClosedDayInTheCap_WithinTwoTicks_When130ServersHaveTodayLateEveryTick: 130 servers with today late-marked (built 10:00-10:59), one closed day late-marked on server 131 (built 11:00); tick 1 fills its 120 slots with todays, those are built and late-marked again, and tick 2 has the closed day at slot 11. RED: with the oldday DESC, server_idorder, tick 2 does not contain it (Assert.Contains() Failure: Item not found). The source-text pin of the old ORDER BY inTheConstantsAndStatements_AreTheDocumentedOnesis deleted, not moved: its rule was spelling, and the live test now pins the order by behavior.Results (this branch after the merge, local rig on PostgreSQL with TimescaleDB, UTC):
*RungTests,ViewerSchemaVersionGateTests,*PlanRegression*,CaggGroupIndexDropLiveTests,*Investigation*,QueryStoreBackgroundIndexesLiveTests,QueryStoreIntervalLatestWriterTests,QueryStoreComposeStampTests,*Census*,*Ratchet*,GuardStageCensusTests,PlanForceNoWritePathTests,*DocCommentHygiene*,*MigrationDataMoving*:Darling.Tests Total: 1385, Errors: 0, Failed: 0, Skipped: 4(the four skips are the opt-in measuring rigs).PlanRegressionCandidateReuseLiveTests.GetAnalysisFactsAtOneHour_...failed once in a run of the plan regression classes ("the pass's pg_stat_statements entry is gone or was remade") and passed in the run above; its own message says the check could not speak, so I read it as load, not code. CI decides.[guard-darling] Darling.Tests Total: 4610, Errors: 0, Failed: 0, Skipped: 0[guard-lite] Lite.Tests Total: 1281, Errors: 0, Failed: 0, Skipped: 0classes: darling 282, lite 31: darlingTotal: 1507, Failed: 0andTotal: 1825, Failed: 0; liteTotal: 550, Failed: 0.GATE GREEN.OversizedPlanBacklogPins.V121_...: the merge resolution had leftPgMigrations.cswith LF line endings in the working copy, so a verbatim SQL string held LF where the pin expects the checkout's CRLF. Git stores both the same (the commit carries no line-ending change); restoring CRLF in the working copy made it green.Fix lane (review round 1)
All four findings of review 5528556062 and the title divergence, in commit 78ec763 (one push). Rig: PostgreSQL from
pg-runtime.zip, UTC, CI's preload and worker settings.PlanRegressionCandidateCacheEvictionTests.EveryPlanRegressionRead_RunsUnderTheOneWorkMemSetting(twin:PlanRegressionDailyNoSpillRig.MeasureFallback),TheCacheHasNoForget_AndTheServicePublishesItsServerSet(twin:AStore_EvictsTheEntriesOfServersNoLongerMonitored_OnlyOnceASetIsPublished), andPlanRegressionDailyRungTests.TheStampAndTheBuiltDaysRead_KeepAPartialBuildInvisibleToPreV185Code(twin:PlanRegressionDailyWatermarkLiveTests.APartialDay_IsUnbuiltToTheV183Predicate_AndAV183WholeBuildAfterItReadsWhole_WithNoDoubleCount). ForAnExactBoundRead_WithADifferentWindowStart_IsADifferentShapeI deleted only its three source-text lines (theFactSource()asserts), because the twin the finding names is that member's own pure-rule assertions, which stay. TheCount,FactSourceandRoothelpers and two usings went with them. The lag change also broke two'2 hours'SQL-text lines inPlanRegressionDailyBuilderLiveTests' statement pin; they were spelling too, so I deleted them (the watermark is pinned by behavior inABuildOnTheStoreClock_StampsThreeHoursBeforeTheHour_...). The probe-ordinal and migration-literal rung pins stay.LateTriggerLagHours1 -> 2 andPartialWatermarkLagHours2 -> 3, changed in place in the unreleased V185. Both constants and V185's doc comment now state the bet: the trigger compares the monitored server'sfirst_execution_timewith the store'snow(). A store clock up to an hour ahead stays quiet with 60-minute intervals. A bigger skew, or longer intervals, puts the open days back on the live read: slower, with the same numbers. The cost is an hour of live-half width (the last 3-4 hours of first executions instead of 2-3). New live testQueryStoreIntervalLatestWriterTests.TheJustEndedIntervalsRows_AtTheOldBoundAndASecondBelow_StayQuietUnderAStoreClockAhead_AndARowBelowTheBoundFiresruns on a pinned clock at every hour against the shipped trigger. A row at the just-ended interval's start (the old bound) and one a second below it stay quiet, including with the store's clock 2 s into the next hour. A row at the new bound stays quiet, and one a second below it fires.SteadyFirstis now the just-ended interval's start (the old bound), so the existing pinned-clock test's "hour rolls over" case sits on it. Moved with the lag:PlanRegressionDailyTriggerLiveTests' recent edge (-2 h),QueryStoreIntervalPartitionRungTests' V185 bound literal ('02:00:00', a rung pin), the rung's lag pins (2, 3),PlanRegressionDailyWatermarkLiveTests(the watermark now reads the constant, its skip hours moved to 03:00-05:00 and< lag + 2), and the 200-server plan test (fixture an hour earlier). Reverted to 1/2, three tests fail: the new test,TheLateBatchTestsRows_HoldAgainstTheShippedTriggerBound_AtEveryHourOfAPinnedClock, andPlanRegressionDailyTriggerLiveTests.AFirstBeforeTheDayAlignedSeventeenDayEdge_MarksNothing_AndTheEdgesAreExact(Total: 21, Failed: 3). With the fix:Failed: 0.AnalysisContextdoc (finding 4279341518).ReusePlanRegressionCandidatesnow names onlyanalyze_serverandget_analysis_factswith noas_of, and says the scheduled pass never sets it and setsRefreshPlanRegressionCandidatesinstead. Doc only; no test needed.PlanRegressionWorkMemSql: the figure is per node, so one read can take 6 x 64MB, doubled for a hash throughhash_mem_multiplier(768MB). The fleet ceiling multiplies that byMaxConcurrentSweeps(4 by default, at most 16). On-demand reads that miss the cache run outside the sweep gate. SizeMaxConcurrentSweepsbefore raising the bound. Doc only.Results. Gate (handoffs
pre-push.sh; the branch has no.github/scripts/pre-push.sh):GATE GREEN.[guard-darling] Total: 4610, Errors: 0, Failed: 0, Skipped: 0;[guard-lite] Total: 1281, Errors: 0, Failed: 0, Skipped: 0;classes: darling 287, lite 33; classesTotal: 1495, Failed: 0, Skipped: 29,Total: 1837, Failed: 0, Skipped: 31, LiteTotal: 557, Failed: 0, all with the rig. The first gate run was RED on oneScratchDatabaseSweepLiveTests.ADatabaseNobodyDisposed_IsDroppedByTheDrain_AndForgottenAfterwards(a file this PR does not touch). Run alone it passed (Total: 4, Failed: 0), and the full rerun was green. The brief's classes outside the gate's selection (AnalysisContextSecondaryScopeSourceScanTests,DarlingAnalysisServiceEngineRoutingTests,PlanRegressionDailyTriggerCostRig,PlanRegressionInputsAdviceTests,PlanRegressionIntervalTableEquivalenceTests,PlanRegressionReadShapeTests,QsCaptureModeRungTests): DarlingTotal: 51, Failed: 0, Skipped: 1.QsCaptureModeRungTests(not in the gate's selection; this PR changes nothing Lite reads).- 2hand the 02:00-04:00 skip) describe the one-hour lag; this section supersedes them.CI red (reuse test)
Cause: the test, not the product (case a). CI run 38702741139 failed
PlanRegressionCandidateReuseLiveTests.GetAnalysisFactsAtOneHour_AfterTheScheduledPass_ReusesItsCandidates_AndReadsFreshOncePastRefreshAfteron its own pg_stat_statements check. The line of evidence is in its message:hits 1, misses 0; entries after the pass [... Calls = 1 ...], after the call []. The product's cache counted a hit and no miss, so it reused. The entry list after the call is empty, not remade (a re-read after a reset would have left a fresh entry with one call). Something removed the entry, and the product made no read after that. pg_stat_statements is cluster-wide. This class is#1776 own-storeand has no collection, so it runs in parallel with the rest of the shard.PgStatementsDeallocLiveTests(also parallel) runs more distinct statements thanpg_stat_statements.maxto force eviction, and every parallel scratch database that runs the migrations adds its own entries, because the key includes the database. Eviction drops the least-used entries first, and the pass's read has one call. (ComposeStoreRolesLiveTestsresets the whole cluster too, but its collection runs alone, so it is not the cause here.)Fix (commit d334868, test only). The test now proves the reuse the way the class's first test does, through what the product reports. After the scheduled pass, it deletes the costly plan's rows from
query_store_interval_latestandplan_regression_daily. On the same-day shape, the 1-hourget_analysis_factscall must still report PLAN_REGRESSION (it can do that only from the pass's candidates), with the test's own cache at hits 1, misses 0. On the cross-day shape, it must miss, read live and find none. PastRefreshAfter(set to zero), the call reads fresh: one more miss, and no PLAN_REGRESSION. The pg_stat_statements read-back, its helper record and theshared_preload_librariesskip are gone, so the test now also runs on a server without pg_stat_statements. No product change.Verified on a UTC rig (port 5681, CI's preload and worker settings):
*PlanRegressionCandidateReuse*5 runs in a row:Total: 8, Errors: 0, Failed: 0, Skipped: 0each time (18.7 to 21.1 s). The runs fell between 08:00 and 10:00 UTC, so they exercised the same-day reuse branch.Total: 8, Failed: 0. The OLD test, rebuilt with the same loop running:GetAnalysisFactsAtOneHour_... [FAIL],Total: 8, Failed: 1. That reproduces CI's failure and shows the fix holds under the same condition.bash .github/scripts/pre-push.sh:GATE GREEN. guard-darlingTotal: 4613, Failed: 0; guard-liteTotal: 1281, Failed: 0;classes: darling 282, lite 32; classes-darlingTotal: 1479, Failed: 0, Skipped: 29andTotal: 1821, Failed: 0, Skipped: 31; classes-liteTotal: 522, Failed: 0.*PlanRegression*class:Total: 147, Errors: 0, Failed: 0, Skipped: 4(the 4 skips are the opt-in measurement rigsPlanRegressionDailyNoSpillRigandPlanRegressionDailyTriggerCostRig).For the coordinator: the test's new comment names two ways the entry vanishes: classes that call
pg_stat_statements_reset()for the whole cluster, and classes that fill the hash table until it evicts. Only the second can overlap this class: the one full reset is inComposeStoreRolesLiveTests, whose collection runs alone. The brief allowed one push, so that half-sentence is still in the comment. It is the next lane's one-line fix if you want it.SECTION: Fixed
ENTRY: - Plan regression reads no longer re-read three open days of Query Store intervals, and on-demand calls reuse the candidates for one analysis cadence ([#5817]) - The PLAN_REGRESSION read took closed days from stored daily totals but read every interval row of the last three days again for every server on every pass. The hourly builder now also builds the open days as partial days, up to a watermark three hours before the current hour. For every built day the read takes the stored totals and reads only the interval rows after that day's watermark. An analyze_server or get_analysis_facts call with no as_of reuses the scheduled pass's plan regression candidates for up to one analysis cadence (30 minutes by default), and an analyze_server call saves the findings it builds from them. The scheduled pass always reads fresh. Every plan regression read, including the full-window read a server uses before its days are built, raises its work memory to 64MB for its own transaction, so neither read sorted to disk on the planted busy server. Migration V185 adds the partial-build columns and moves the late-row trigger's bound to two hours before the current hour.