Skip to content

Detect a server-wide wait-stats clear once per pass (#4428) - #4434

Merged
erikdarlingdata merged 2 commits into
devfrom
fix/4428-waitstats-clear-detection
Sep 26, 2026
Merged

erikdarlingdata merged 2 commits into
devfrom
fix/4428-waitstats-clear-detection

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Refs #4428.

Why

On one production store, every monitored server's wait_stats pass shows 100% of wait types reading LOWER at one instant, many times an hour. A scheduled job on those servers clears wait statistics (DBCC SQLPERF('sys.dm_os_wait_stats', CLEAR)); the busiest run every 5 minutes.

22% of that store's 24-hour wait_stats rows had zero interval. The control store had 8%.

Not service restarts, and not collection gaps: 42,195 of 42,234 zero-interval instants sat next to a normal pass. WaitStatsCollector.WritePayload was reading each wait type's clear-to-zero as an independent counter reset — the honest "unknowable" (0, 0) pair — for every one of the (typically hundreds of) wait types the clear touched, instead of recognizing the pass as one event and crediting each type's current value since the clear.

What changes

  • WaitStatsCollector.ReadAsync now peeks the wait_stats_time family's cached baselines (a read-only peek, no mutation) right after the rows are read and before any subtraction happens — the same "decide before any subtraction" point the existing identity-epoch carrier already uses.

  • WaitStatsCollector.DetectClear (a pure, directly-testable rule) calls the pass a clear only when BOTH hold:

    • among wait types with a cached wait_time_ms baseline > 0, at least 50% read lower now, AND at least 20 such types exist;
    • the pass's summed wait_time_ms across every row is lower than the summed cached baselines.
      The rule fails toward "not a clear" on any ambiguity — a false positive would rebase (and so silently discard real accrual for) hundreds of wait types that never reset.
  • On a detected clear, ICollectorDeltaCalculator gets a new RebaseFamiliesToZero(serverId, collectorNames) operation (default no-op, like the existing ClearServer/ClearGroups/DecideRow members), implemented on the shared CollectorDeltaCalculator both hosts run. It zeroes the cached VALUE for the three wait_stats_* delta families on that server, keeping each key's cached timestamp — so the very next ordinary delta call reports "current value minus zero" as the delta since the clear, over the real (kept) interval, instead of reading a shrink as an unknowable reset.

  • A new PeekBaselines(serverId, collectorName) member (default empty map) lets a caller inspect a family's cached values without updating anything, the same peek-only contract DecideRow already established for Decide a cached plan's restarted counters row-coherently in query_stats (#4428) #4431.

  • The log line is queued through two new members, NoteWaitStatsClear(serverId, serverName, nowUtc) and DrainWaitStatsClearWarnings(serverId), throttled to once per server per UTC calendar day (in-memory, like every other per-server throttle on this type). Both hosts (DarlingWorker.cs, Lite/Services/RemoteCollectorService.cs) drain it right beside the existing Brains-review campaign: deferred structural residue (from #3538 / #3539 / #3540 / #3541) #3653 A5 discontinuity drain, at Information:

    Wait statistics on {server} were cleared between collections, as a DBCC SQLPERF(..., CLEAR) job does. This collection's wait figures cover only the time since the clear. Frequent clears also reset the wait history any other tool reads from this server.

  • Not changed: a single wait type resetting outside a clear (fewer than 20 baselined types, or a lower majority that doesn't clear the bar) still takes the existing independent per-type path. Server epochs (restart/failover detection) are unaffected. No migration — this is purely an in-memory delta-cache behavior change; no new columns or schema.

Documented limitation, stated in the new RebaseFamiliesToZero doc comment: only the slice between the previous pass and the clear is lost — unknowable, since the DMV never reports the pre- and post-clear values in the same row — and the first post-clear pass's per-second rate is therefore slightly understated (its denominator still spans back to the pre-clear baseline's timestamp).

#4428 builds on #4431's row-coherent reset (DecideRow), already on dev; this PR does not touch query_stats or QueryStatsCollector.

Test plan

New Darling.Tests.WaitStatsClearDetectionTests (7 facts; the earlier in-test "mutation" fact was removed in favour of the real code mutation below), run in-process on macOS per the repo's xunit v3 recipe:

  • FieldShape_AllLowerAndTotalLower_EveryRowCreditsCurrentValueOverRealInterval: ~900 baselined types, all lower, total lower — runs the SAME rows end to end through the real CollectorDeltaCalculator (seed baselines → RebaseFamiliesToZero → WaitStatsCollector.WritePayload) and asserts every row's delta_wait_time_ms equals its current value with a real, non-zero sample_interval_seconds. RED on pre-fix code: without the clear detection and rebase, every one of these 900 types independently resets in WritePayload (current value below the stale, un-rebased baseline), so delta_wait_time_ms and sample_interval_seconds are 0 for all 900 rows — the assertions Assert.All(deltaTimes, d => Assert.Equal(500L, d)) and Assert.DoesNotContain(0, intervals) both fail against that shape.
  • SixtyPercentLowerAndTotalLower_IsAClear / FiveOfNineHundredLower_IsNotAClear / FirstPass_NoBaselines_IsNotAClear: boundary shapes for the two-part rule.
  • SixtyPercentLowerButTotalRose_IsNotAClear: the false-positive guard — a majority of types lower but the summed total rose (a few heavy waits absorbing ordinary growth) must NOT be read as a clear.
  • LogLine_OncePerServerPerDay_EvenAcrossTwoClears: two NoteWaitStatsClear calls the same UTC day queue one line, drained once; a call the next day queues a fresh one.
  • FieldShape_ThroughObserveWaitStatsClear_EveryRowCreditsCurrentValue: pins the collector's own read step, not just the delta math. Seeds a real CollectorDeltaCalculator with one ordinary pass's baselines, then calls WaitStatsCollector.ObserveWaitStatsClear(rows, context) — the new internal method ReadAsync extracted its post-read block into, with no behaviour change — and asserts it returns true and that WritePayload on the lower rows afterward credits every row's current value with a real, non-zero interval. There is no manual RebaseFamiliesToZero call in this test, so a ReadAsync that stopped calling the detector would leave the assertion failing.
    • RED on dev (a same-shape scenario run in a detached worktree of origin/dev, which has no ObserveWaitStatsClear/rebase step at all — the scenario just seeds baselines and calls WritePayload with no rebase): Assert.Equal() Failure: Expected: 500 Actual: 0 on all 900 rows, because every wait type reads as its own independent counter reset against the un-rebased baseline.
    • Real code mutation, not a tautology: with the 50% majority bar in DetectClear changed from lowerCount * 2 >= baselinedCount to the unreachable lowerCount * 100 >= baselinedCount * 101, this same pin goes RED: Assert.True() Failure: Expected: True Actual: False (from Assert.True(WaitStatsCollector.ObserveWaitStatsClear(currentRows, context))). Restoring the bar and rebuilding brings it back GREEN (Darling.Tests.WaitStatsClearDetectionTests: 7 passed, 0 failed). The old Mutation_RequireOverHundredPercentLower_FieldShapeNoLongerDetectsAClear fact, which re-evaluated the rule inside the test instead of mutating the shipped code, is removed.

Also run: QueryStatsRowCoherentResetTests, DeltaSeriesAgeTests, DocCommentHygieneTests (required) — all green alongside the new class (101 passed, 0 failed for that combined run).

Lite.Tests.WaitStatsCollectorDefinitionTests was checked by inspection — it pins the query text and payload column order only, neither of which this change touches — and needs no update.

Both Darling.Tests and Lite.Tests build 0 warnings / 0 errors with -p:EnableWindowsTargeting=true. Full suite was not run; DocCommentHygieneTests plus the collector-specific classes above were run explicitly and are green.

CHANGELOG entry

SECTION: Fixed
ENTRY:

WaitStatsCollector.ReadAsync now peeks the wait_time_ms baselines
before any subtraction and calls a pass a clear only when at least
50% of baselined types (with at least 20 baselined) read lower AND
the pass's summed wait_time_ms is lower than the summed baselines.
On a clear, the three wait_stats delta families are rebased to zero
for that server (keeping each baseline's timestamp) through a new
ICollectorDeltaCalculator.RebaseFamiliesToZero operation, and an
Information line is queued once per server per day through
NoteWaitStatsClear/DrainWaitStatsClearWarnings, drained by both
hosts beside the existing #3653 A5 discontinuity drain.

Refs #4428.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant