-
Notifications
You must be signed in to change notification settings - Fork 30
Release v1.7.0 — wait stats as warnings #245
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; | ||
|
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. The only filter on emitting a warning is Consider a minimum-relevance threshold, e.g. skip emission if Generated by Claude Code |
||
|
|
||
| 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, | ||
| 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.
EmitWaitStatWarningsruns even whenQueryTimeStats == null(line 39'sScoreWaitStatsis skipped in that case). With noWaitBenefitspopulated,benefitByTypeis empty and every wait getsseverity = Inforegardless of how dominant the wait is. That may be intentional (estimated/incomplete plans don't have elapsed to score against) but worth confirming — if it is, a one-line comment here saying "no QueryTimeStats → all severities pinned to Info" would save the next reader a trip through the lookup chain.Also: the two adjacent
if (stmt.WaitStats.Count > 0)blocks can collapse into a single guarded block, slightly cheaper and reads more clearly:Generated by Claude Code