-
Notifications
You must be signed in to change notification settings - Fork 30
Wait stats as warnings (#215, Joe feedback a1/b4/b5) #244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,10 +38,73 @@ public static void Score(ParsedPlan plan) | |
|
|
||
| if (stmt.WaitStats.Count > 0 && stmt.QueryTimeStats != null) | ||
| ScoreWaitStats(stmt); | ||
|
|
||
| if (stmt.WaitStats.Count > 0) | ||
| EmitWaitStatWarnings(stmt); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Emits a PlanWarning per wait stat entry, merging the per-wait benefit % | ||
| /// from ScoreWaitStats with the descriptive content from WaitStatsKnowledge. | ||
| /// The existing wait-stats chart/card stays as a complementary view. | ||
| /// </summary> | ||
| private static void EmitWaitStatWarnings(PlanStatement stmt) | ||
| { | ||
| // Lookup benefit % by wait type (populated by ScoreWaitStats) | ||
| var benefitByType = new Dictionary<string, double>(StringComparer.OrdinalIgnoreCase); | ||
| foreach (var wb in stmt.WaitBenefits) | ||
| benefitByType[wb.WaitType] = wb.MaxBenefitPercent; | ||
|
|
||
| foreach (var wait in stmt.WaitStats) | ||
| { | ||
| if (wait.WaitTimeMs <= 0) continue; | ||
|
|
||
| var entry = WaitStatsKnowledge.Lookup(wait.WaitType); | ||
| double? benefitPct = benefitByType.TryGetValue(wait.WaitType, out var b) ? b : null; | ||
|
|
||
| var msg = new System.Text.StringBuilder(); | ||
| msg.Append(wait.WaitType).Append(": ").Append(entry.Description); | ||
| msg.Append(" Observed ").Append(wait.WaitTimeMs.ToString("N0")).Append(" ms"); | ||
| if (wait.WaitCount > 0) | ||
| msg.Append(" across ").Append(wait.WaitCount.ToString("N0")).Append(" wait").Append(wait.WaitCount == 1 ? "" : "s"); | ||
| msg.Append('.'); | ||
|
|
||
| if (entry.ShowEffectiveLatency && wait.WaitCount > 0) | ||
| { | ||
| var effLatency = (double)wait.WaitTimeMs / wait.WaitCount; | ||
| msg.Append(" Effective latency: ") | ||
| .Append(FormatLatency(effLatency)) | ||
| .Append(" per wait."); | ||
| } | ||
|
|
||
| var severity = benefitPct switch | ||
| { | ||
| >= 50 => PlanWarningSeverity.Critical, | ||
| >= 10 => PlanWarningSeverity.Warning, | ||
| _ => PlanWarningSeverity.Info, | ||
| }; | ||
|
|
||
| stmt.PlanWarnings.Add(new PlanWarning | ||
| { | ||
| WarningType = "Wait: " + wait.WaitType, | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Generated by Claude Code |
||
| Message = msg.ToString(), | ||
| Severity = severity, | ||
| MaxBenefitPercent = benefitPct, | ||
| ActionableFix = entry.HowToFix | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| private static string FormatLatency(double ms) | ||
| { | ||
| if (ms >= 1000) return $"{ms / 1000:N2} s"; | ||
| if (ms >= 10) return $"{ms:N0} ms"; | ||
| if (ms >= 1) return $"{ms:N1} ms"; | ||
| return $"{ms * 1000:N0} µs"; | ||
| } | ||
|
|
||
| private static void ScoreStatementWarnings(PlanStatement stmt) | ||
| { | ||
| var elapsedMs = stmt.QueryTimeStats?.ElapsedTimeMs ?? 0; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard asymmetry worth confirming:
ScoreWaitStatson line 40 only runs whenstmt.QueryTimeStats != null, butEmitWaitStatWarningson line 43 runs wheneverWaitStats.Count > 0.When
QueryTimeStatsis null (e.g. estimated-only plans with recorded wait stats),stmt.WaitBenefitsstays empty, sobenefitByType.TryGetValuealways misses and every wait is emitted atInfoseverity withMaxBenefitPercent = null. The plan may have a real 99%-elapsed CXPACKET problem that silently shows up as Info.Intentional degrade? If so, fine — but it's worth a comment at line 42 saying so. Otherwise consider matching the
QueryTimeStats != nullguard.Generated by Claude Code