Wire wait stats config as single source of truth (#215 part 2)#300
Merged
erikdarlingdata merged 1 commit intodevfrom May 2, 2026
Merged
Wire wait stats config as single source of truth (#215 part 2)#300erikdarlingdata merged 1 commit intodevfrom
erikdarlingdata merged 1 commit intodevfrom
Conversation
- Trim WaitStats.json from 52 to 44 entries — drop server-level / instance- level waits (THREADPOOL, BACKUPIO, BACKUPBUFFER, HADR_SYNC_COMMIT, DBMIRROR_DBM_EVENT, SQLTRACE_BUFFER_FLUSH) and connection-setup waits (PREEMPTIVE_OS_AUTHENTICATIONOPS, PREEMPTIVE_OS_LOOKUPACCOUNTSID) that don't appear in plan XML's <WaitStats> element. - Populate attributes 6-8 (showWaitCount, showAverageWaitTime, timeCalculationModel) from existing tool behavior. Attributes 9-12 (applicable operators, descriptions, URLs, internal comment) remain null — those need domain expertise. - Add WaitStatsConfig loader (PlanViewer.Core.Services.WaitStatsConfig) that reads the JSON from an embedded resource at first access and caches by name (case-insensitive). Resource lookup is suffix-based so the file works whether embedded under PlanViewer.Core.* or PlanViewer.Web.* manifest prefixes. - BenefitScorer.IsExternalWait now delegates to WaitStatsConfig.IsExternal. Lookup misses fall back to false, preserving prior default for unknown waits. EmitWaitStatWarnings drops the WaitStatsKnowledge dependency entirely. - Delete WaitStatsKnowledge.cs — its ShowEffectiveLatency flag was the only structural data and is now sourced from WaitStatsConfig.ShowAverageWaitTime. Description/HowToFix were intentionally empty since #215 D3, so nothing of substance is lost. - Wire embedded resource into PlanViewer.Core.csproj and link it + WaitStatsConfig.cs into PlanViewer.Web.csproj (which links Core source files rather than referencing the project). All 75 Core tests pass; Core, App, and Web all build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <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
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.
Part 2 of #215. Wires the JSON config from Part 1 (#296) into the analysis pipeline so it actually drives behavior, then collapses the parallel C# data files into the single source of truth.
Changes
WaitStats.json<WaitStats>element. Per @jobbish-sql's feedback on Seed wait stats config file (#215 part 1) #296.showWaitCount: true everywhere (matches currentShowWaitStatsalways rendering "(N waits)")showAverageWaitTime: true forPAGEIOLATCH_SH/EX/UP/DT(the four entriesWaitStatsKnowledge.ShowEffectiveLatency=truewas set on); false elsewheretimeCalculationModel:"cpu time based"forisExternal=true(4 PREEMPTIVE_* + MEMORY_ALLOCATION_EXT);"direct"forRESOURCE_SEMAPHOREandASYNC_NETWORK_IO(Joe's explicit examples);"elapsed time based"for the remaining 37 entries (his stated default)WaitStatsConfig(new)PlanViewer.Core.Services.WaitStatsConfigloads the JSON from an embedded resource on first access, caches by name (case-insensitiveDictionary).EndsWith("Resources.WaitStats.json")) so the file works whether embedded underPlanViewer.Core.*orPlanViewer.Web.*manifest prefixes (Web project compiles linked Core sources into its own assembly).Get(name),IsExternal(name),ShowAverageWaitTime(name). Lookup misses returnnull/false, preserving prior defaults for unknown waits.Code wiring
BenefitScorer.IsExternalWaitnow delegates toWaitStatsConfig.IsExternal.BenefitScorer.EmitWaitStatWarningsdrops theWaitStatsKnowledge.Lookupdependency and readsShowAverageWaitTimefrom the config.WaitStatsKnowledge.csdeleted — its only structural data (ShowEffectiveLatency) is now sourced from the config;DescriptionandHowToFixwere intentionally empty since [FEATURE] Add system to assign a "maximum benefit" for plan analysis rules #215 D3, so nothing of substance is lost.Project files
PlanViewer.Core.csproj: addResources/WaitStats.jsonas<EmbeddedResource>.PlanViewer.Web.csproj: replace the linkedWaitStatsKnowledge.cswithWaitStatsConfig.cs; add a linked<EmbeddedResource>for the JSON so the WebAssembly assembly carries its own copy.Test plan
dotnet buildclean for Core, App, and Web (4 pre-existing AVLN3001 warnings in App, unrelated)dotnet test— 75/75 Core tests pass.sqlplanwithPAGEIOLATCH_SHwaits and verify the warning still includes "Effective latency: ... per wait".sqlplanwithPREEMPTIVE_*orMEMORY_ALLOCATION_EXTwaits and verify the CPU-based external-wait benefit % still calculates🤖 Generated with Claude Code