Skip to content

The store accepts a known plan digest for a row and keeps that plan alive (inert, #5158) - #5174

Merged
erikdarlingdata merged 6 commits into
devfrom
feat/5158-plan-digest-writer
Oct 5, 2026
Merged

erikdarlingdata merged 6 commits into
devfrom
feat/5158-plan-digest-writer

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Part of #5158. This is the second of three PRs, and it changes no behavior: no collector passes a digest yet.

Why

#5158 stops query_stats from re-rendering plans the store already holds. A row whose plan the host has already stored will carry only the plan's content digest. The store has to write that digest straight into the row, with no content, and keep the plan's dimension row from aging out while rows still reference it.

What changes (PerformanceMonitor.Darling.Storage)

  • PgCollectorRowWriter.PayloadOrDigest(content, knownDigest):
    • with a known digest and no content, it writes the digest into the row's digest column, adds the digest to the batch's touch set, and inserts no dimension row;
    • every other call takes today's Value(content) path;
    • a malformed digest throws: it must be hex and exactly PayloadDimensions.DigestLengthBytes (32) bytes. So does a known digest at a payload position with no dimension diversion, which would otherwise write a row with no plan, silently.
    • The digest is PayloadDimensions.Digest's own output, held by the host. Nothing re-derives it.
  • PayloadDimensionBatch keeps the touch set per dimension table, deduplicated and in digest order. IsEmpty counts it.
  • PayloadDimensionWriter.FlushAsync runs one more statement per dimension table, in the batch's transaction (PayloadDimensions.TouchSql):
    • it refreshes last_seen for the touched digests older than the existing 6-hour guard. It uses FOR UPDATE SKIP LOCKED: the upsert and the touch are two locking statements, so no single lock order covers both, and two batches that each upsert one stale digest and touch the other's would otherwise deadlock. Skipping a row is safe, because whoever holds it is refreshing it (an upsert, another touch, or query_store's touch). A holder that rolls back is retouched next cycle. The dimension sweep can't be the holder, because it only reaches rows at least the guard plus a day old;
    • it returns the touched digests that have no dimension row;
    • FlushAsync returns those as a list of upper-case hex digests (compare case-insensitively) and logs their count at Debug. This PR doesn't act on the list; the next PR uses it to forget those digests, so the plan is fetched again. A digest still being inserted by another, uncommitted batch can be reported absent, which costs one extra render.
  • Why the plan stays alive: a referenced digest gets its last_seen refreshed at least every 6 hours, which is the same guard the insert path uses. That keeps it inside the dimension sweep's liveness window, as it is today.

Tests (PayloadDigestPassThroughLiveTests, live store, 8 facts)

  • A digest-only row resolves its plan through v_query_stats the same way a row written with content does: one dimension row, two fact rows on one digest.
  • The 6-hour guard: at 5 h 59 m last_seen stays put; at 6 h 01 m it moves to the batch time.
  • An absent digest is reported back, and its row reads a NULL plan without an error.
  • Content plus a digest stays on the ordinary path: nothing is touched or reported.
  • A malformed digest throws, and so does a known digest at a position with no diversion.
  • A lower-case known digest round-trips, and the absent list reports it upper-cased.
  • Two batches, two connections: each upserts one stale digest and touches the other's. Neither waits, neither deadlocks, and both commit. It uses the real upsert and touch SQL, with lock_timeout at 2 s and no sleeps.
  • Mutation checks, each reverted:
    • writing the digest as NULL fails 2 facts;
    • removing the 6-hour guard fails the guard fact;
    • removing SKIP LOCKED fails the two-batch fact with a lock timeout (55P03).
  • Checked against the collector change: the class also passes on a local merge with query_stats can fetch plan XML in a second target query for only the rows that need it (inert, #5158) #5173. The test writer forwards the row's own known digest.

SECTION: None (inert; the user-facing entry ships with the PR that turns it on)

… it alive (inert)

Adds ICollectorRowWriter.PayloadOrDigest(content, knownDigest), defaulting to Value(content). PgCollectorRowWriter overrides it: a known digest with no content writes the digest alone to the row's digest column, with no plan text and no dim insert, and the digest joins the batch's touch set.

PayloadDimensionWriter.FlushAsync runs one statement per dimension table for the touch set in the batch's own transaction. It refreshes last_seen only past the same 6-hour guard as the insert path, so the dimension GC's liveness invariant is unchanged, and it returns the digests that have no dim row. No collector passes a digest yet, so nothing changes at runtime.
…cannot deadlock

- TouchSql locks with FOR UPDATE SKIP LOCKED: the upsert's conflict arm and the touch have no shared lock order, so a waiting touch could close a cycle and roll back a whole batch. A live fact crosses two connections' upserts and touches under a 2 s lock timeout.
- A known digest at a position with no diverted payload throws instead of writing a NULL plan.
- FlushAsync documents that its absent list is upper-case hex; a fact round-trips a lower-case digest.
- The test wrapper forwards the collector's own digest ahead of its stored one.
- TouchSql's doc notes that a digest another uncommitted batch is inserting can be reported absent.
…writer

# Conflicts:
#	PerformanceMonitor.Collectors/ICollectorRowWriter.cs
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review October 5, 2026 05:25
@erikdarlingdata erikdarlingdata added the merge-train In a merge train: its CI ran on the train PR label Oct 5, 2026
@erikdarlingdata
erikdarlingdata merged commit a923f67 into dev Oct 5, 2026
14 of 18 checks passed
@erikdarlingdata
erikdarlingdata deleted the feat/5158-plan-digest-writer branch October 5, 2026 11:02
erikdarlingdata added a commit that referenced this pull request Oct 5, 2026
…behind queryStatsDeferredPlanFetch (on) (#5158) (#5209)

query_stats fetches plan XML only for plans this host hasn't stored, behind queryStatsDeferredPlanFetch (on) (#5158)

Fixes #5158 for query_stats; procedure_stats follows separately. The third of three PRs: #5173 added the collector's deferred fetch and #5174 the store's digest-only write; this wires them together and turns them on.

- Every query_stats run rendered sys.dm_exec_text_query_plan for its ~200 rows and shipped the XML, though almost all were already stored: 53.75 GB/h rendered and 28.83 GB/h shipped on one large store against 1.14 GB/h distinct, with the busiest server's slow runs more than 90% result drain.
- PlanDigestCache<TKey>, one per server for the service's lifetime, keyed for query_stats on QueryStatsPlanKey (server, database, plan_handle, statement offsets, creation_time, plan_generation_num). An entry is confirmed only after the store commit succeeds, including after a store-write reattempt; digests the store reports missing are evicted ignoring case; entries unseen for an hour are pruned.
- For query_stats with plan capture on, the main query runs without plan columns. On the same target connection and within the same per-item budget, a cache hit carries the stored digest and size (an over-cap hit the size with a NULL plan), and misses go to one BuildPlanFetchQuery call, split above 1,000 keys. A failed second query ships the rows without plans, caches nothing and logs one Warning. No store probe runs first; the host's confirmed commits are the authority. Azure SQL Database keeps the inline capture.
- queryStatsDeferredPlanFetch in darling.json, default true, read at service start; false gives today's inline SQL exactly. Documented in the README. Each run records plans_rendered, plans_rendered_bytes and plan_fetch_ms in its collection log note.
- Accepted differences: a statement recompile between the two queries can store the new plan under the old identity for that one row; a plan the engine updates in place without a recompile keeps its first-rendered XML until the identity changes; after a restart each server renders once in full.
- Tests: PlanDigestCacheTests (identity sensitivity, confirm-after-commit, over-cap, case-insensitive eviction, the split, plan_fetch_ms) and QueryStatsDeferredPlanFetchLiveTests against SQL Server 2022 and the store (run 2 renders no cached plan and resolves through v_query_stats; sp_recompile re-fetches; a plan freed between the queries; an over-cap plan kept by size).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-train In a merge train: its CI ran on the train PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant