query_store plan/text fetch goes activity-driven: the store is the watermark (#2312) - #2333
Merged
Merged
Conversation
…termark The invariant 40-110s-per-run bill (#2312) had a named mechanism at last: the #2210 watermark walk's daily expiry was supposed to be replaced by a re-verify cursor that was built, tested, documented, and never wired (Finding 4) - so catalogs whose full walk needs more than a day expired MID-walk, restarted from plan_id 0, and looped the full catalog fetch forever. TouchSql, the liveness refresh the dimension GC depends on, had the same story: designed as 'the whole of the protection', zero callers (Finding 3), latent only because the perpetual walk's re-upserts were accidentally standing in for it. The reshape retires all of it in one shape change: the cycle's collected rows name their plans/texts; one touch-and-probe round trip per database refreshes map/dim liveness AND returns the missing set plus per-cycle in-place-rewrite / Query-Store-reset detection via the live hashes the payload already carries; the fetch selects exactly those ids under the same byte-budget arithmetic. A caught-up database issues NO target query. A reset recovers as the normal path. A dormant plan resuming execution is fetched the cycle it resumes. V77: query_store_plan_map.digest goes nullable (the NULL-XML content-less marker, so unpersistable plans read as known instead of riding every fetch list), query_store_text gains query_hash (the reset detector), and the orphaned planwm:/textwm: state rows are deleted wholesale. Budget- deferred ids carry over in memory so a plan referenced once cannot starve; target-side-gone ids drop only on a provably-uncut pass. Upserts COALESCE digest and hash toward knowledge, never absence. Retired: QueryStorePlanXmlState's watermark half (sizing stays - it caps server-side decompression), QueryStoreTextState wholesale, the cursor members, and the planwm:/textwm: entries in the shared prune set. Tests: QueryStorePlanWatermarkTests is reborn as QueryStorePlanFetchTests (new builder/probe/upsert shape pins + the surviving sizing tables, harness- executed against the built assembly), the V77 rung ceremony lands in ActivityDrivenPlanFetchStoreTests with the V74/V75/V76 files demoted, and QueryStoreFetchProbeLivePostgresTests round-trips the probe verdicts, liveness stamping, hash adoption, and the NULL-digest marker against a real store. Closes #2312 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…metic, and two source pins Both splice edits landed between a member's doc block and its body - the fetch method's doc stranded above the new extractor helpers, and V76's rung doc stranded above the V77 insertion. Third instance of this exact trap; both docs now sit with their members and a local scan mimicking the hygiene test confirms zero stacks. The seed candidate cap pin claimed the issue text's rounded ~118 where ceil(12MB/160KB*1.5) is 116. The text call-site literal QueryStoreTextStoreTests pins is restored to one line, and the runner's comments stop naming the retired prefixes literally so the retirement source pin can hold. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 19, 2026
erikdarlingdata
added a commit
that referenced
this pull request
Aug 19, 2026
With #2333's catalog walk gone, the per-database split named the phase that does not subside after catch-up: wm, 1-3s per database per cycle. It is a read against OUR store, not the monitored server -- an unbounded MAX over a non-partitioning timestamp, so it touches every chunk that database has. Measured on the live use1 store (106 GB, 5 chunks), same server/database pair: unbounded, cold: 25,766 buffer reads + 195 written (temp spill) unbounded, warm: 228 ms bounded to 3h: 29 ms, 5 chunks excluded The unbounded cost is a function of store size and cache residency rather than of the monitored workload, so it degrades precisely where an operator is weakest: a long-lived store, a busier Query Store, slower disks. The bound changes no answer, and that is the whole justification: every consumer ends at max(stored, now - MaxCatchup), because ClampCatchup floors anything older and a null result falls back to query_store's 60-minute first-run window -- the same instant as the floor. So a row below the horizon cannot move the result whether it is found or not, and the unbounded MAX was paying to confirm a value the clamp would have produced anyway. The predicate goes on collection_time (the partitioning column) because a predicate on the watermark column alone prunes nothing, and a row's watermark can never exceed its own collection_time, so nothing qualifying hides behind it. Bounded for query_store ONLY, on both hosts, name-guarded like the other query_store-specific behavior in these runners: a ring-buffer collector whose legitimate catch-up spans days must keep reading its whole history, and the floor would silently truncate it. WatermarkPolicy.ReadFloor carries the reasoning and the measurements; its tests pin that the floor sits strictly older than the clamp horizon and that a buried watermark and a not-found one reach the same instant. Closes #2344 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
erikdarlingdata
added a commit
that referenced
this pull request
Aug 19, 2026
With #2333's catalog walk gone, the per-database split named the phase that does not subside after catch-up: wm, 1-3s per database per cycle. It is a read against OUR store, not the monitored server -- an unbounded MAX over a non-partitioning timestamp, so it touches every chunk that database has. Measured on the live use1 store (106 GB, 5 chunks), same server/database pair: unbounded, cold: 25,766 buffer reads + 195 written (temp spill) unbounded, warm: 228 ms bounded to 3h: 29 ms, 5 chunks excluded The unbounded cost is a function of store size and cache residency rather than of the monitored workload, so it degrades precisely where an operator is weakest: a long-lived store, a busier Query Store, slower disks. The bound changes no answer, and that is the whole justification: every consumer ends at max(stored, now - MaxCatchup), because ClampCatchup floors anything older and a null result falls back to query_store's 60-minute first-run window -- the same instant as the floor. So a row below the horizon cannot move the result whether it is found or not, and the unbounded MAX was paying to confirm a value the clamp would have produced anyway. The predicate goes on collection_time (the partitioning column) because a predicate on the watermark column alone prunes nothing, and a row's watermark can never exceed its own collection_time, so nothing qualifying hides behind it. Bounded for query_store ONLY, on both hosts, name-guarded like the other query_store-specific behavior in these runners: a ring-buffer collector whose legitimate catch-up spans days must keep reading its whole history, and the floor would silently truncate it. WatermarkPolicy.ReadFloor carries the reasoning and the measurements; its tests pin that the floor sits strictly older than the clamp horizon and that a buried watermark and a not-found one reach the same instant. Closes #2344 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
erikdarlingdata
added a commit
that referenced
this pull request
Aug 19, 2026
With #2333's catalog walk gone, the per-database split named the phase that does not subside after catch-up: wm, 1-3s per database per cycle. It is a read against OUR store, not the monitored server -- an unbounded MAX over a non-partitioning timestamp, so it touches every chunk that database has. Measured on the live use1 store (106 GB, 5 chunks), same server/database pair: unbounded, cold: 25,766 buffer reads + 195 written (temp spill) unbounded, warm: 228 ms bounded to 3h: 29 ms, 5 chunks excluded The unbounded cost is a function of store size and cache residency rather than of the monitored workload, so it degrades precisely where an operator is weakest: a long-lived store, a busier Query Store, slower disks. The bound changes no answer, and that is the whole justification: every consumer ends at max(stored, now - MaxCatchup), because ClampCatchup floors anything older and a null result falls back to query_store's 60-minute first-run window -- the same instant as the floor. So a row below the horizon cannot move the result whether it is found or not, and the unbounded MAX was paying to confirm a value the clamp would have produced anyway. The predicate goes on collection_time (the partitioning column) because a predicate on the watermark column alone prunes nothing, and a row's watermark can never exceed its own collection_time, so nothing qualifying hides behind it. Bounded for query_store ONLY, on both hosts, name-guarded like the other query_store-specific behavior in these runners: a ring-buffer collector whose legitimate catch-up spans days must keep reading its whole history, and the floor would silently truncate it. WatermarkPolicy.ReadFloor carries the reasoning and the measurements; its tests pin that the floor sits strictly older than the clamp horizon and that a buried watermark and a not-found one reach the same instant. Closes #2344 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
erikdarlingdata
added a commit
that referenced
this pull request
Aug 19, 2026
…ally use (#2344) (#2346) With #2333's catalog walk gone, the per-database split named the phase that does not subside after catch-up: wm, 1-3s per database per cycle. It is a read against OUR store, not the monitored server -- an unbounded MAX over a non-partitioning timestamp, so it touches every chunk that database has. Measured on the live use1 store (106 GB, 5 chunks), same server/database pair: unbounded, cold: 25,766 buffer reads + 195 written (temp spill) unbounded, warm: 228 ms bounded to 3h: 29 ms, 5 chunks excluded The unbounded cost is a function of store size and cache residency rather than of the monitored workload, so it degrades precisely where an operator is weakest: a long-lived store, a busier Query Store, slower disks. The bound changes no answer, and that is the whole justification: every consumer ends at max(stored, now - MaxCatchup), because ClampCatchup floors anything older and a null result falls back to query_store's 60-minute first-run window -- the same instant as the floor. So a row below the horizon cannot move the result whether it is found or not, and the unbounded MAX was paying to confirm a value the clamp would have produced anyway. The predicate goes on collection_time (the partitioning column) because a predicate on the watermark column alone prunes nothing, and a row's watermark can never exceed its own collection_time, so nothing qualifying hides behind it. Bounded for query_store ONLY, on both hosts, name-guarded like the other query_store-specific behavior in these runners: a ring-buffer collector whose legitimate catch-up spans days must keep reading its whole history, and the floor would silently truncate it. WatermarkPolicy.ReadFloor carries the reasoning and the measurements; its tests pin that the floor sits strictly older than the clamp horizon and that a buried watermark and a not-found one reach the same instant. Closes #2344 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2312. ⚠ HOLD FROM MERGE until 3.5.0 cuts — the release candidate is soaking on the box from dev @ 9f01385, and this must not enter the release build. Review rounds proceed as usual; the merge waits.
The mechanism, finally named (Findings 3 & 4, on the issue)
The #2210 second-generation design had four moving parts. Three were built, tested, documented, and never wired: the adaptive sizing (Finding 1, wired in #2322),
TouchSqlliveness (Finding 3 — 'the whole of the protection' for Query Store digests, zero callers), and the re-verify cursor (Finding 4 — only tests invoke it). What ran in production was the budgeted walk alone, withResolve's daily expiry still armed: catalogs whose full walk needs more than a day (measured 15.9–107.5 hours) expired MID-walk, restarted from plan_id 0, and looped the full catalog fetch forever. That is the 40–110s-per-cycle bill — and the loop's re-upserts were also accidentally substituting for the unwired liveness refresh, which is why killing the walk and wiring the touch must land together.The reshape
TouchSql) refreshes map+dimlast_seenAND returns per-idresolved/hash_stale— the missing set is the fetch list, and the live hashes the payload already carries give per-cycle in-place-rewrite detection (plans) and Query-Store-reset detection (texts), retiring both the cursor and the reset arm.BuildPlanFetchByIdsQuery/BuildTextFetchByIdsQuery), chunked ascending, same running-bytes budget with the oversized-admission arithmetic intact, hash columns riding along (no decompression). A caught-up database issues no target query at all — the 23s-to-discover-nothing becomes nothing.query_store_plan_map.digestnullable (NULL-XML plans land a content-less marker so they read as known instead of riding every fetch list — the posted design's one gap),query_store_text.query_hash(reset detector; legacy rows adopt the live hash on first touch, zero refetches), and wholesale deletion of the orphanedplanwm:/textwm:rows. Full rung ceremony including the viewer probe arm and the V74/V75/V76 pin-file demotions.Honesty notes
CapturePlanXml); the shared prune set shrinks 5 → 3 and Lite's no-op deletes shrink with it.Tests
QueryStorePlanWatermarkTestsreborn asQueryStorePlanFetchTests: by-ids builder shape + guards (both fetches), touch-and-probe SQL pins (hourly guard, hash adoption, NULL-digest-resolves, stale expression, dim-touch NULL filter), upsert COALESCE pins, the surviving sizing decision tables, and the read-loop stages-no-state pin. The builders were additionally executed against the built assembly in a throwaway harness (escaping, id lists, budgets, all four guard throws — pass).ActivityDrivenPlanFetchStoreTests: the V77 rung ceremony (ladder top, three strokes, probe arm, three-places-agree).QueryStoreFetchProbeLivePostgresTests(gated): live round-trip of probe verdicts (missing/marker/current), liveness stamping on map AND dim, hash adoption then stale detection, and the writer's NULL-digest marker.QueryStoreTextWatermarkTestsretired with its subject.🤖 Generated with Claude Code