Skip to content

The CPU alert gate's observation identity is the target's LOCAL sample_time, so the gate is blind for ~1 h after every autumn fall-back on a non-UTC server; it should key on sample_time_utc with a one-time frame re-anchor (Q7 residue) #3744

Closed

Description

@erikdarlingdata

The lie

The CPU alert's persistence gate identifies each ring-buffer sample by its timestamp and counts a sample as fresh only when that timestamp is strictly greater than the last one it saw — PerformanceMonitor.Alerting/AlertEngine.cs:923-931, ObservePersistenceAsync: … || sampleUtc.Value > priorRecord.LastObservedSampleUtc.Value. That rule is what stops the 30 s sweep from advancing the breach streak three times on one 60 s sample (#3282). The timestamp it is fed is cpu_utilization.sample_time, the target's local clock (sys.dm_os_ring_buffers stamps in server-local time): Darling DarlingWorker.LatestCpuSql projects sample_time (Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs:5310, SELECT sqlserver_cpu_utilization, other_process_cpu_utilization, sample_time … ORDER BY collection_time DESC, sample_time DESC LIMIT 1) into AlertServerSnapshot.CpuSampleTimeUtc (:3814); Lite passes summary.CpuSampleTime from its overview read into the same field (Lite/MainWindow.AlertEngine.cs:89). The field's name says UTC; its value is local.

On a server whose zone observes DST, the local clock repeats an hour every autumn. From the fall-back instant, every new sample for the next hour carries a stamp that is less than or equal to the last stamp the gate recorded before the transition, so > says "not fresh": the breach streak does not advance, an open incident does not clear, and CPU alerting is blind for roughly one hour on every non-UTC server, once a year, at the one instant nobody is watching. (Spring-forward skips an hour and is harmless: stamps jump forward, > holds.) #3730 (Q7+Q8, V134) added sample_time_utc beside the local stamp precisely so readers stop guessing frames; the windowed CPU reads prefer it. The gate does not — deliberately, and the reason is written down.

Why it was left alone, verified at source (origin/dev 98d52fe)

DarlingWorker.cs:5298-5309: "Not sample_time_utc, deliberately (V134, #3653 item 13). … switching the projection to the UTC column would change [the frame] exactly once, at the upgrade, so that on every server EAST of UTC the first post-rung sample (UTC) reads OLDER than the last pre-rung one (local) and the gate sees no fresh sample for one offset's worth of hours: CPU alerting frozen, silently, on the half of the world where it is morning. West of UTC the same switch would count one stale sample as fresh." Pinned by Darling.Tests/TimeHonestyRungTests.TheLatestRowReads_KeepTheLocalStamp_AndTheWorkerSaysWhy (:293-306, asserts the projection text). The #3730 lane reported the fall-back blindness as out-of-lane rather than trading a yearly hour for an upgrade-day hour on half the fleet. Both are the same defect: an identity compared by > across two clock frames.

The fix shape

Key the observation identity on sample_time_utc where the row carries it, AND make the frame switch safe:

  1. Project COALESCE(sample_time_utc, sample_time) — no: a single COALESCE still flips frames once per server at the first post-rung row, which is the upgrade-day freeze the worker describes. Instead, carry the frame with the identity: AlertPersistenceRecord gains a frame tag (local / utc, or the V134 rung number), and ObservePersistenceAsync treats a sample whose frame differs from the recorded frame as fresh by definition (a one-time re-anchor), then compares by > within the frame as today. The east-of-UTC freeze and the west-of-UTC stale-count both vanish, because a frame change is never compared across.
  2. The read projects both columns; the snapshot carries CpuSampleTimeUtc (truly UTC when the row has it) and the frame. Lite's overview read (LocalDataService.Overview.cs:37-51) and Darling's LatestCpuSql change together; the TimeHonestyRungTests pin flips from "keeps the local stamp and says why" to "prefers the UTC instant and re-anchors once".
  3. Rename or document CpuSampleTimeUtc: today the name is a lie on every row; after the change it is true where V134 has written the column and documented-local before that.
  4. Fixtures: a fall-back sequence (stamps 01:59 local → 01:00 local, UTC advancing) advances the streak; an upgrade sequence (last local stamp east of UTC, first UTC stamp earlier by the offset) is fresh exactly once; the No alert requires its condition to PERSIST: real fire/resolve pairs 55-87 seconds apart on both engines — and it is not the re-fire class closed seven times #3282 same-sample-on-three-sweeps case still holds the streak.

Descends from #3730 (Q7 of #3653), which found it and stated it. Partial #3653 residue.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions