sp_QuickieStore: @find_high_impact fixes from prod dogfooding - #855
Open
erikdarlingdata wants to merge 3 commits into
Open
sp_QuickieStore: @find_high_impact fixes from prod dogfooding#855erikdarlingdata wants to merge 3 commits into
erikdarlingdata wants to merge 3 commits into
Conversation
Four changes driven by running the mode against all 114 prod databases: - Add the absolute total columns (total_cpu_ms, total_duration_ms, total_physical_reads_mb, total_writes_mb, total_memory_mb, total_tempdb_mb, total_rows, max_dop) to the output. They were already in #hi_output but never selected, so the only sizing available was per-server shares — and a 34% share on a quiet server reads identical to a 34% share on a busy one. Shares answer 'how dominant'; the totals answer 'how much'. (In the fleet run, 39 of 114 databases burned less than 10% of one core-day total.) - Honor the query-hash include/ignore lists: parsed locally in the mode block, gating a new #hi_eligible staging table that filters BEFORE the per-dimension TOP picks, so an ignored hash never consumes a slot. Shares and percentiles still compute over the full workload on purpose: ignoring a query must not change what percent of the server another query consumed. Results are query_hash-grained, so the plan-hash lists still do not apply. Motivation: an index-maintenance query held a top-N slot on 114 of 114 production databases with no way to exclude it. - Floor the spills/spools diagnostic at 1 MB/exec: total_writes_mb > 0 over a huge execution count rendered as 'spills/spools (0.0 MB/exec)', a verdict with no content. - Document that @top is per resource dimension (a default run returns several times @top rows), which surprised this author in the field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Eleven new checks: clean default run, summary + detail shape, the absolute total_* columns present, the zero-value spills diagnostic staying quiet, and bidirectional query-hash include/ignore proof against a surfaced hash VALUE in detail rows (ignore removes it, include keeps only it, a hash nobody has returns zero rows with the summary as positive control). 177 total. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…te window The #query_hash_totals aggregation had no date restriction: it summed every runtime stats row in Query Store history for the hash, and those all-time numbers sat next to window-scoped columns in the same output row. On a fleet with weeks of QS retention that inflated the by_query_hash totals ~7.6x in a 72-hour analysis (live A/B on one tenant: 110,841s unscoped vs 14,627s scoped for the same hash and window), which defeats the parameter's whole purpose of gauging a hash's impact for the period being analyzed. Same disease and same fix as #850/#852: restrict on qsrs.last_execution_time to match the date semantics of the main where clause and the wait sort order joins. Third instance of this pattern. Also documents the columns as window-scoped in the parameter help. 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.
What
Stacked on #854 (base =
fix-ps-mode-dogfood; will retarget todevautomatically when that merges). Four@find_high_impactchanges driven by running the mode against all 114 production databases (72h Query Store windows, 20–65s per run), plus the mode's first test coverage.1. Absolute total columns in the output
total_cpu_ms,total_duration_ms,total_physical_reads_mb,total_writes_mb,total_memory_mb,total_tempdb_mb,total_rows,max_dopwere already computed into#hi_outputbut never selected — the only sizing a consumer got was per-server shares, which invites the biggest-slice-of-a-small-pie trap (39 of the 114 fleet databases burn under 10% of one core-day, where a 34%cpu_sharemeans almost nothing). Shares answer "how dominant"; the totals answer "how much." During the fleet analysis these had to be scraped out of theresource_metricsXML.2. Query-hash include/ignore lists are honored
Parsed locally in the mode block (same pattern as #854's PS-mode change), gating a new
#hi_eligiblestaging table that filters before the per-dimensionTOP (@top)picks — an ignored hash never consumes a slot, and the freed slot pulls in the next eligible hash (verified live: row count stays ~90 with the filter on). Shares and percentiles still compute over the full workload deliberately: ignoring a query must not change what percent of the server another query consumed. Results are query_hash-grained, so the plan-hash lists remain non-applicable (documented). Motivation: an index-maintenance query held a top-N slot on 114 of 114 production databases with no way to exclude it across runs.3. Spills/spools diagnostic floored at 1 MB/exec
total_writes_mb > 0over a large execution count producedspills/spools (0.0 MB/exec)— a verdict with no content. Observed on the very first row of the very first fleet database analyzed.4.
@topis per-dimension — now documentedA default
@top = 15run returns ~80–95 rows (top N per resource dimension, unioned). Correct behavior, surprising output size; one help-text line.Tests
run_tests.py: 166 → 177 passing (local SQL 2022 container). The 11 new assertions are the mode's first coverage: clean run, summary/detail shape,total_*columns present, zero-value spills diagnostic absent, and bidirectional include/ignore proof against a surfaced hash value.Prod verification (limited, 4 instances)
Deployed this build under a throwaway name (
sp_QuickieStore_pstest) to 4 prod instances, ran@find_high_impactagainst dune/GTI/bolt/apex, dropped it after (verified gone):API.GetRegisterAdjustments, 32,221s window CPU at 29.7% share)(0.0 MB/exec)diagnostics (baseline run had them on every database)@ignore_query_hasheswith the maintenance query's hash: gone on both databases where it had been in the top-N, with the freed slots refilled🤖 Generated with Claude Code