Skip to content

The CPU alert gate decides "same sample" by equality, not by order: a fall-back's repeated hour and the V134 local→UTC frame switch are new samples rather than an hour of silence, and both SKUs hand it sample_time_utc where the row has one (#3744) #3774

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.

Sign up for GitHub

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

Merged
erikdarlingdata merged 1 commit into from
Sep 20, 2026
Merged

The CPU alert gate decides "same sample" by equality, not by order: a fall-back's repeated hour and the V134 local→UTC frame switch are new samples rather than an hour of silence, and both SKUs hand it sample_time_utc where the row has one (#3744) #3774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 122 additions & 0 deletions Darling/Darling.Tests/AlertEngineTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,128 @@ latency to it. */
Assert.Single(h.Deliverer.Outcomes);
}

/* ---------------- CPU: the #3744 identity rule — equality, not order ---------------- */

/// <summary>One sweep carrying one sample instant, breaching or not, with the same CPU pair the other CPU
/// fixtures use; the #3744 fixtures hand-pick each instant, so a shared driver would obscure the sequence.</summary>
private static Task ObserveCpuAsync(AlertEngine engine, DateTime sampleInstant, bool breaching) =>
engine.EvaluateServerAsync(Harness.Snapshot(
sqlCpu: breaching ? 70 : 20, totalCpu: breaching ? 95 : 30, cpuSampleTime: sampleInstant));

[Fact]
public async Task Cpu_AFallBack_RunsTheStampBackwards_AndTheStreakStillAdvances()
{
/* THE #3744 defect, first shape. cpu_utilization.sample_time is the monitored server's LOCAL wall
clock, and on a zone that observes DST that clock repeats an hour every autumn: 01:59 is followed
by 01:00. Under the pre-#3744 rule (fresh = strictly GREATER than the last counted instant) every
sample in the repeated hour read as stale, so a breach that began at 01:58 never reached the bar
and an open incident never cleared, for an hour, on every non-UTC server, once a year. The rule is
now equality — a different instant is a different sample whichever clock stamped it — and the two
samples one local hour apart across the fall-back are the DISTINCT identities they always were.

The instants are local-frame stamps on purpose: this is what the gate was fed before V134 and what
it is still fed off a pre-rung row, and the fix must hold there too, not only once the identity has
moved to the UTC twin. Under `>` the third observation below is "not fresh" and the fire never
comes; that is the assertion that reddens if the predicate goes back to order. */
var h = new Harness();
h.Settings.CpuEnabled = true;
var engine = h.Build();

var local = new DateTime(2026, 11, 1, 1, 58, 0); /* 01:58 EDT, Kind=Unspecified as stored */
await ObserveCpuAsync(engine, local, breaching: true); /* 1 */
await ObserveCpuAsync(engine, local.AddMinutes(1), breaching: true); /* 01:59 EDT — 2 */
Assert.Empty(h.Deliverer.Outcomes);

var fallenBack = new DateTime(2026, 11, 1, 1, 0, 0); /* 01:00 EST: one real minute later, 59 local minutes EARLIER */
Assert.True(fallenBack < local, "the fixture must run the stamp backwards, or it tests nothing #3744 changed");
await ObserveCpuAsync(engine, fallenBack, breaching: true); /* 3 — the bar */
Assert.Single(h.Deliverer.Outcomes);

/* The same rule on the falling edge: an incident open across the fall-back clears in the repeated
hour rather than waiting for the local clock to climb past 01:59 again. */
await ObserveCpuAsync(engine, fallenBack.AddMinutes(1), breaching: false);
Assert.Empty(h.Resolutions);
await ObserveCpuAsync(engine, fallenBack.AddMinutes(2), breaching: false);
Assert.Single(h.Resolutions);

/* And a repeat of the LAST counted instant is still the same sample: the streak holds and nothing is
written — equality lost none of #3282. */
var writes = h.StateStore.SavedPersistence.Count;
await ObserveCpuAsync(engine, fallenBack.AddMinutes(2), breaching: false);
Assert.Equal(writes, h.StateStore.SavedPersistence.Count);
}

[Fact]
public async Task Cpu_TheUpgradeFrameSwitch_EastOfUtc_IsFreshExactlyOnce_AndNeverFreezes()
{
/* THE #3744 defect, second shape — the one #3730 saw coming and worked around by leaving the gate's
identity on the local stamp. The identity is now the row's UTC twin where the store has one, so a
record persisted by a pre-#3744 build (a LOCAL instant) meets a UTC instant on the first post-upgrade
sweep. East of UTC the UTC instant reads EARLIER by the offset: at UTC+3, local 12:00 becomes UTC
09:00. Under `>` the gate would have seen no fresh sample until the UTC clock climbed past the last
local stamp — three hours of frozen CPU alerting on every server east of UTC, once, on upgrade day.
Under equality the switch is one new identity: counted ONCE, then the same UTC instant re-read is
the same sample, then `!=` within the UTC frame as the samples advance.

A new engine over the same store IS the restart the upgrade implies, so the seed path is the one
that hands the local record to the UTC-fed sweep. */
var h = new Harness();
h.Settings.CpuEnabled = true;

/* The pre-#3744 build: one breaching sample, identity = the local stamp. */
var local = new DateTime(2026, 9, 21, 12, 0, 0);
await ObserveCpuAsync(h.Build(), local, breaching: true); /* 1 */
Assert.Equal(local, Assert.Single(h.StateStore.Persistence).Value.LastObservedSampleUtc);

/* The upgraded build's first sweep: the SAME sample, now identified by its UTC twin — three hours
earlier as a value. Fresh exactly once (the accepted one-time re-anchor), never frozen. */
var upgraded = h.Build();
var utc = local.AddHours(-3);
Assert.True(utc < local, "east of UTC the twin must read earlier than the local stamp, or this is the west fixture");
await ObserveCpuAsync(upgraded, utc, breaching: true); /* 2 */
Assert.Empty(h.Deliverer.Outcomes);

/* The re-read of that identity on the next sweep is the same sample: NOT counted, NOT written. If the
switch were treated as "fresh by frame" on every sweep rather than once, this is where the bar would
be reached early — the assertion that reddens if a re-anchor is not one-time. */
var writes = h.StateStore.SavedPersistence.Count;
await ObserveCpuAsync(upgraded, utc, breaching: true);
Assert.Empty(h.Deliverer.Outcomes);
Assert.Equal(writes, h.StateStore.SavedPersistence.Count);

/* Within the UTC frame the samples advance and the bar is reached where it should be. */
await ObserveCpuAsync(upgraded, utc.AddMinutes(1), breaching: true); /* 3 — the bar */
Assert.Single(h.Deliverer.Outcomes);
Assert.Equal(utc.AddMinutes(1), Assert.Single(h.StateStore.Persistence).Value.LastObservedSampleUtc);
}

[Fact]
public async Task Cpu_TheUpgradeFrameSwitch_WestOfUtc_CountsTheSameSampleOnceMoreAtMost()
{
/* The other direction of the same switch. West of UTC the twin reads LATER than the local stamp
(UTC−5: local 12:00 is UTC 17:00), so even under `>` the first post-upgrade sweep counted the
sample it had already counted — #3730 named that as the west-of-UTC cost of switching. Equality
has the same cost and the same bound, and this pins the bound: the same physical sample is counted
once more at the switch, the re-read of its UTC identity is held, and the streak then needs the
genuinely new samples it always needed. One extra count of one sample per server, once, is the
whole price of moving the identity to the honest clock. */
var h = new Harness();
h.Settings.CpuEnabled = true;

var local = new DateTime(2026, 9, 21, 12, 0, 0);
await ObserveCpuAsync(h.Build(), local, breaching: true); /* 1 */

var upgraded = h.Build();
var utc = local.AddHours(5);
await ObserveCpuAsync(upgraded, utc, breaching: true); /* 2 — the once-more */
await ObserveCpuAsync(upgraded, utc, breaching: true); /* held */
await ObserveCpuAsync(upgraded, utc, breaching: true); /* held */
Assert.Empty(h.Deliverer.Outcomes);

await ObserveCpuAsync(upgraded, utc.AddMinutes(1), breaching: true); /* 3 — the bar */
Assert.Single(h.Deliverer.Outcomes);
}

[Fact]
public async Task Cpu_TheMeasuredTwoMinuteExcursion_IsNotAnIncident()
{
Expand Down
88 changes: 69 additions & 19 deletions Darling/Darling.Tests/TimeHonestyRungTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ namespace Darling.Tests;
/// Two lies retired: a server-LOCAL sample stamp that every UTC-window reader had to de-skew by a derived
/// offset (an hour wrong across a DST transition, silently), and an OFFSET that cannot say which side of a
/// transition an instant fell on where a ZONE can. The rung, the one passthrough refresh, the viewer probe's top
/// arm, the two windowed CPU readers that now prefer the stored UTC instant, the latest-row reads that
/// deliberately do not, and the <c>get_server_properties</c> payload that publishes the clock pair.
/// arm, the two windowed CPU readers that now prefer the stored UTC instant, the latest-row reads (three that
/// order on the local stamp as a within-batch tiebreak and never name the twin; the CPU alert gate's, which since
/// #3744 projects the twin as the gate's identity beside the local stamp it still orders on), and the
/// <c>get_server_properties</c> payload that publishes the clock pair.
///
/// <para>The "I am the top rung" claims this file carried when it landed (moved here off
/// <c>PgNumbackendsAndSampledMsRungTests</c>, V133) moved on again to <c>LongRunningQueryExclusionKnobRungTests</c>
Expand Down Expand Up @@ -283,35 +285,61 @@ public void BothWindowedCpuReads_PreferTheStoredUtcInstant_AndKeepTheDeSkewAsThe
}

/// <summary>
/// The latest-row reads keep the LOCAL <c>sample_time</c> and never name the twin: <c>DarlingWorker.LatestCpuSql</c>
/// projects it as the CPU alert gate's observation identity, compared only against itself by <c>&gt;</c>, and a
/// one-time frame change there would freeze the gate for one offset's worth of hours on every server east of
/// UTC (#3282's trap); the other three ORDER BY it as a within-batch tiebreak, which carries no frame. The
/// worker's doc says so in the words a reader will look for.
/// The latest-row reads all keep the LOCAL <c>sample_time</c> as their within-batch tiebreak, and the three
/// display/fleet reads never name the twin at all. The CPU alert gate's read is the exception since #3744: it
/// PROJECTS <c>sample_time_utc</c> beside the local stamp (the twin is the gate's observation identity where
/// the row has one, the local stamp where it does not) but still does not ORDER on it, because a
/// cross-batch order on the twin, or on a <c>COALESCE</c> of the two, would compare a pre-rung local stamp
/// against a post-rung UTC one and sort a stale row as newest east of UTC. #3730 pinned this read to the
/// local stamp because the gate compared identities by <c>&gt;</c> and a one-time frame change would have
/// frozen it for one offset's worth of hours; #3744 moved the gate to equality and this pin flipped with it.
/// The worker's doc says all of that in the words a reader will look for.
/// </summary>
[Fact]
public void TheLatestRowReads_KeepTheLocalStamp_AndTheWorkerSaysWhy()
public void TheLatestRowReads_KeepTheLocalTiebreak_TheGatesReadProjectsTheTwin_AndTheWorkerSaysWhy()
{
foreach (var sql in new[]
{
DarlingWorker.LatestCpuSql, DarlingHealthReader.ServerSummaryCpuSql, DarlingFleetReader.FleetCpuSql,
ViewerDataService.ServerSummaryCpuSql,
DarlingHealthReader.ServerSummaryCpuSql, DarlingFleetReader.FleetCpuSql, ViewerDataService.ServerSummaryCpuSql,
})
{
Assert.DoesNotContain("sample_time_utc", sql, StringComparison.Ordinal);
Assert.Contains("ORDER BY", sql, StringComparison.Ordinal);
Assert.Contains("collection_time DESC, sample_time DESC", sql, StringComparison.Ordinal);
}

Assert.Contains("SELECT sqlserver_cpu_utilization, other_process_cpu_utilization, sample_time", DarlingWorker.LatestCpuSql, StringComparison.Ordinal);
/* The gate's read: both stamps projected, local first (ordinal 2) and the twin last (ordinal 3), so the
pre-#3744 ordinals still read what they always read; the twin named exactly once, in the SELECT list
and nowhere else — not in the ORDER BY, not in a predicate. */
var gate = DarlingWorker.LatestCpuSql;
Assert.Contains("SELECT sqlserver_cpu_utilization, other_process_cpu_utilization, sample_time, sample_time_utc", gate, StringComparison.Ordinal);
Assert.Contains("ORDER BY collection_time DESC, sample_time DESC", gate, StringComparison.Ordinal);
Assert.Single(Regex.Matches(gate, "sample_time_utc"));
Assert.DoesNotContain("COALESCE", gate, StringComparison.Ordinal);

var worker = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "DarlingWorker.cs");
var doc = worker[..worker.IndexOf("internal const string LatestCpuSql", StringComparison.Ordinal)];
doc = doc[doc.LastIndexOf("/// <summary>", StringComparison.Ordinal)..];
foreach (var phrase in new[] { "Not <c>sample_time_utc</c>, deliberately", "OBSERVATION IDENTITY", "#3282", "EAST of UTC", "frozen", "TimeHonestyRungTests" })
foreach (var phrase in new[]
{
"the UTC twin is the identity where the row has one (#3744", "OBSERVATION IDENTITY", "#3282", "EAST of UTC",
"frozen", "fall-back", "EQUALITY", "The ORDER BY stays on the local stamp", "TimeHonestyRungTests",
})
{
Assert.Contains(phrase, doc, StringComparison.Ordinal);
}

Assert.DoesNotContain("Not <c>sample_time_utc</c>, deliberately", doc, StringComparison.Ordinal);

/* And the reader folds the pair the documented way — twin first, local as the fallback — rather than
reading only one of them. Source pin because ReadLatestCpuAsync is private and needs a live store. */
var reader = CSharpSourceWalker.StripCommentsAndStrings(worker);
var head = reader.IndexOf("ReadLatestCpuAsync(int serverId", StringComparison.Ordinal);
Assert.True(head >= 0, "DarlingWorker has no ReadLatestCpuAsync");
var body = reader.Substring(head, Math.Min(2500, reader.Length - head));
Assert.Contains("reader.IsDBNull(3) ? (DateTime?)null : reader.GetDateTime(3)", body, StringComparison.Ordinal);
Assert.Contains("reader.IsDBNull(2) ? (DateTime?)null : reader.GetDateTime(2)", body, StringComparison.Ordinal);
Assert.Contains("sampleTime = sampleTimeUtc ?? sampleTimeLocal;", body, StringComparison.Ordinal);
}

/// <summary>The properties read projects the clock pair after the pre-rung columns, the row type carries them
Expand Down Expand Up @@ -496,22 +524,29 @@ SELECT deskewed FROM (
Assert.Equal(newestUtc.AddHours(-1), (DateTime)(await deskewOnly.ExecuteScalarAsync(ct))!);
}

/* The worker's latest-row read keeps the LOCAL stamp: the gate's identity did not change frame.
And it shows the pre-existing fall-back quirk this rung deliberately leaves alone: the tiebreak
orders on the local stamp, so inside the one batch that straddles the fall-back the EDT-side
sample (cpu 31, sixty seconds OLDER) sorts as the newest — a stale-by-a-minute reading once a
year, and an identity that the next batch's EST stamps will not exceed for an hour (reported,
not fixed here: switching the identity to the twin would trade that for a one-time freeze of one
offset's worth of hours east of UTC at the upgrade, #3282's trap). */
/* The worker's latest-row read projects BOTH stamps (#3744): the local stamp at ordinal 2, where
it always was, and the UTC twin at ordinal 3, which is what the gate now takes as this row's
identity. It also still shows the one-batch fall-back quirk the tiebreak leaves: ordering on the
local stamp, inside the single batch that straddles the fall-back the EDT-side sample (cpu 31,
sixty seconds OLDER in UTC) sorts as the newest — a reading stale by one sample, once a year.
Before #3744 that identity was also one the next batch's EST stamps would not EXCEED for an
hour, and the gate compared by >, so it froze; the gate now compares for equality, and the next
batch's twins simply differ from this one. The ORDER BY cannot move to the twin without
comparing a pre-rung local stamp against a post-rung UTC one across batches — the worker's doc
says why — so the one-sample staleness inside the straddling batch is the accepted residue. */
using (var latest = new NpgsqlCommand(DarlingWorker.LatestCpuSql, connection))
{
latest.Parameters.AddWithValue(ServerId);
using var reader = await latest.ExecuteReaderAsync(ct);
Assert.True(await reader.ReadAsync(ct));
Assert.Equal(4, reader.FieldCount);
Assert.Equal(31, reader.GetInt32(0));
Assert.Equal(straddleUtc.AddHours(-4), reader.GetDateTime(2));
Assert.Equal("sample_time_utc", reader.GetName(3));
Assert.Equal(straddleUtc, reader.GetDateTime(3));
}


/* server_properties through ServerPropertiesCollector.WritePayload: a 2022+ row with the zone and
an older-engine row with NULL, the offset on both. */
var older = now.AddMinutes(-10);
Expand Down Expand Up @@ -549,6 +584,21 @@ and the note says what NULL means. */
samples.Select(s => (s.SampleTime, s.SqlServerCpu)).ToArray());
}

/* Last, because it removes the post-rung batch: a store whose NEWEST CPU row predates the rung —
fixture 5 of #3744, today's behaviour. The gate's read returns the pre-rung row with its local
stamp at ordinal 2 and a NULL twin at ordinal 3, which is the reader's cue to fall back to the
local stamp as the identity. */
await DarlingMcpTestData.ExecAsync(connection, ct, "DELETE FROM cpu_utilization_stats WHERE server_id = $1 AND sample_time_utc IS NOT NULL", ServerId);
using (var latest = new NpgsqlCommand(DarlingWorker.LatestCpuSql, connection))
{
latest.Parameters.AddWithValue(ServerId);
using var reader = await latest.ExecuteReaderAsync(ct);
Assert.True(await reader.ReadAsync(ct));
Assert.Equal(22, reader.GetInt32(0));
Assert.Equal(pre2Utc.AddHours(-4), reader.GetDateTime(2));
Assert.True(reader.IsDBNull(3), "a pre-rung row must read a NULL twin, or the fallback arm is never exercised");
}

bodySucceeded = true;
}
finally
Expand Down
Loading
Loading