Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **Lite's portable ZIP is self-contained, which HALVED it** ([#2501]) - `Publish Lite` is now `-r win-x64 --self-contained` in both `build.yml` and `nightly.yml`, so neither Lite artifact has a .NET prerequisite any more and the failure [#2489] documented stops existing: a tester who unzips onto a stock Windows Server no longer meets the .NET host's bare `You must install .NET to run this application` before a line of our code runs. **The size went the opposite way from what bundling a runtime suggests.** The old publish was RID-agnostic, so it copied every platform its packages ship - **537 MB of `runtimes\` on a 565 MB tree** (osx 130, linux-x64 116, linux-arm64 70, win-arm64 56, then win-x86, musl, loongarch64 and riscv64), of which only the **52 MB `win-x64`** folder could ever load on Windows. `DuckDB.NET.Bindings.Full` is most of it, SkiaSharp and SqlClient behind it. Dropping ~485 MB of unloadable native payload beats the cost of bundling .NET, WPF and ASP.NET Core by roughly two to one: measured on one commit and one SDK, **565 MB tree / 212.7 MB zipped becomes 277 MB / 114.2 MB**. It matters most for the **nightly** ZIP, which is the UAT download and is not offered as a `Setup.exe` at all. **A RID-specific publish needed two more files than the flag.** `Lite/packages.lock.json` had only a `net10.0-windows7.0` target, and a RID restore adds `net10.0-windows7.0/win-x64` to it - after which the `dotnet restore --locked-mode` that BOTH workflows run before the publish fails `NU1004: the project's runtime identifiers have changed`, because locked mode compares the PROJECT's RID set (empty) against the lock file's (win-x64). Reproduced locally; that is a red CI run on every PR, not the future `--no-restore` trap it was filed as. The fix is `<RuntimeIdentifiers>win-x64</RuntimeIdentifiers>` in `PerformanceMonitorLite.csproj`, so the project itself asks for that graph and one committed lock file satisfies the RID-less locked-mode restore and the RID publish alike; `RuntimeIdentifiers` (plural) sets no RID on the build, so a plain `dotnet build` stays RID-agnostic and `Lite.Tests` is untouched. **SignPath needed nothing** - the `Lite` artifact-configuration slug already receives both shapes today, and the signed re-zip reads `signed/Lite/*`, inheriting whatever shape `publish/Lite` has. Auto-update is unaffected; the ZIP is not a Velopack channel. `LiteRuntimePrerequisiteDocsTests` went red on the flag alone (3 of its 7 facts) and was rewritten to state every claim BOTH ways round: [#2499]'s version asserted only that the docs DID name the runtimes, so two of its facts stayed green while the prose went stale. It now also derives the lock file's RID coverage from the `-r` flags in the workflows, and every new assertion was proven red with its fix reverted.

### Fixed
- **A healthy server mid-sweep is no longer painted with the red Offline overlay** ([#2794]) - the display's freshness classifier banded a server `Offline - no recent collection` past a bare 15-minute constant, while the alert engine had deliberately decided collection is not "stopped" until 30 (`DarlingSelfAlertEvaluator.StaleWindow`, configurable as `collectionStaleMinutes`) - one condition, two definitions, and only the alert side configurable. The tighter one false-alarmed by design: the sweep skips relaunch while a server's collection body is still running, so one long `query_store` cycle holds the whole body and NOTHING writes to `collection_log` for the duration - a healthy server legitimately goes 12-19 minutes quiet with zero failures anywhere, and five production shards wore the red overlay at a snapshot while actively mid-cycle, which sent an investigation chasing phantom dark servers. **Measured before changing anything, per the issue's own instruction**: with [#2792] deployed, the worst legitimate inter-collection gap across the whole fleet in 24h is 12m12s and ZERO real servers cross 15 minutes (the sole >15m "server" is `server_id = 0`, the service's own bookkeeping rows, whose gaps end exactly at the deploy restarts) - but issue-day load produced 235 crossings on every server in 12 hours with legitimate stretch reaching 19m18s, so the margin between routine operation and the threshold was load-dependent and paper-thin, while genuinely dark servers run HOURS. `OfflineThreshold` now derives from a shared `CollectionStoppedMinutesDefault = 30` alongside every other spelling of the same claim - the evaluator's `StaleWindow`, `AlertsConfig.CollectionStaleMinutes`'s default, Lite's `AppAlertEngineSettings`, Lite's `AgentStatusRow.StaleWindow` (whose comment already said it "mirrors" the service's window, at a numerically-equal but independently-editable copy, the exact [#1562] drift shape), and `DarlingWorker`'s Postgres long-running-query recency bound, whose own doc comment derives it from the fleet staleness convention and which would otherwise have re-created the alert-blindness it warns about. A server between stretched sweeps now bands `Stale` - amber, "collection has lagged" - which is the honest reading; every surface follows because all four banding reads resolve through the one `ClassifyFreshness` ([#2473]), and `CollectionStoppedThresholdAgreementTests` pins the definitions together BY VALUE on both apps, proven red against the pre-change tree (15m != 30m, and the measured 19m18s stretch banding Offline).
- **Abandon-path forensics no longer record a session id of 0** ([#2884]) - every `ABANDONED` row V109 wrote carried `target_session_id = 0` - a value no real session has - while successful runs recorded real ids, and the abandon path is the ONE place the id is load-bearing: it exists to join a stalled run to `waiting_tasks` / `dmv_blocking_snapshot` / `query_snapshots`. Two defects compounded. The capture sat BEFORE `ExecuteReaderAsync`, and `SqlConnection.ServerProcessId` is not reliably populated until the connection has round-tripped a command, so a connection that had not yet completed that exchange reported the provider's not-populated 0. And the write side guarded every OTHER forensic figure - `>= 0` on the row, byte and last-read counts, rejecting the in-memory -1 sentinel - but passed the session id through bare, so the 0 landed in the column as though it were data. The capture now happens AFTER the open round trip: a drain-stall abandon (open completes in ~100-200ms; the [#2673] budget fires minutes into the read) therefore records its REAL id, which is exactly the case the column was built for, while an abandon that fires INSIDE `ExecuteReaderAsync` leaves it NULL - the declared NOT-RECORDED convention, and the honest answer for a connection that never finished its first exchange. `TryReadTargetSessionId` normalizes non-positive values to null at the source, and the writer grows the same-shaped guard as its sibling columns (`spid > 0`) - belt beside braces, because `DrainForensics` is a public record any caller can construct with a 0 the capture path would never produce. Pinned three ways, each proven red against the unfixed source: the capture must sit AFTER the `ExecuteReaderAsync` it depends on (a source-order pin, because the property is unmockable without a real `SqlConnection` and a refactor hoisting the capture back above the open reintroduces the bug while compiling clean), the helper's normalization, and the writer's guard.
- **Every read in the alert evaluation pass now carries an explicit deadline** ([#2874]) - all forty-five commands across the six alert-pass types set no `CommandTimeout`, so every one inherited Npgsql's undocumented 30s default. On 2026-09-04 the forced-plan read failed five times on the production store, each surfacing as "Exception while reading from stream" - which is how Npgsql renders its OWN deadline, the misdiagnosis [#2826] exists to prevent. Unlike [#2810] and [#2871], which sit under `DarlingWorker.s_analysisTimeout`'s 120s `CancelAfter`, **this pass has no enclosing budget at all**: `EvaluateAlertsAsync` is called with the plain stopping token, so the per-command deadline IS the pass budget multiplied by the number of sequential reads - 45 x 30s of worst-case exposure while the body holds one of only four fleet sweep permits and that server's relaunch is skipped throughout. The value is therefore the first in this family set BELOW what it inherited rather than above: 10s, bounded under by the measured worst case (the shipped queries timed on the production store's three busiest servers put the whole pass's cost in one read - the forced-plan check at 1,744.9ms cold over ~6.0GB, every other read under 3ms) and bounded over by the 30s `s_alertSweepInterval` the pass runs on, so one stalled read still lets it finish inside the interval that restarts it. The asymmetry is what justifies erring short: an exceeded deadline skips one alert check and logs it, retrying 30s later, while a long-running read starves fleet-wide collection. Every observed failure was killed AT the 30s ceiling, so the record is right-censored - 10s is chosen from the measured cost and the cadence, not fitted to a failure distribution the data cannot show. Pinned structurally over both construction shapes: [#2874]'s census counted only `new NpgsqlCommand(`, missing `NpgsqlDataSource.CreateCommand(sql)`, which inherits the same default and accounts for four of this group's own sites - the repo-wide recount is recorded on the issue.
- **A held Ctrl+V during a busy clipboard no longer spawns several 'Pasted Plan' tabs from one keypress** ([#2870]) - [#2837] moved the clipboard can't-open retry off a synchronous `Thread.Sleep` onto an awaited `Task.Delay`, which keeps the UI pump responsive but also yields the thread for the ~175 ms retry window. The old `Thread.Sleep` had incidentally serialized input, so a HELD Ctrl+V (OS key-repeat) could not dispatch a second paste until the first finished; with the async retry, on a transiently busy clipboard several paste KeyDown handlers can sit in their retry loops at once, and when the clipboard frees each succeeds and loads its own tab. Each paste surface now carries a `_pasteInProgress` re-entrancy flag: set synchronously before the awaited read and cleared in a `finally` that runs only after the load finishes, so it spans the clipboard read AND the off-thread plan parse (the loaders return `Task` and the paste handlers `await` them, not fire-and-forget) and a repeat paste arriving during either window is dropped (its key stays claimed via `e.Handled`). The same flag guards both the Ctrl+V handler and the Paste XML button on each surface, across all three front ends (Lite, the Darling viewer, and the deprecated Dashboard). A rare, benign burst that the [#2837] async change had newly made possible; source-pinned so the guard can't be dropped without failing the build.
Expand Down Expand Up @@ -3161,6 +3162,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#2785]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2785
[#2791]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2791
[#2795]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2795
[#2794]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2794
[#2473]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2473
[#2792]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2792
[#1562]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1562
[#2800]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2800
[#2801]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2801
[#2803]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2803
Expand Down
61 changes: 61 additions & 0 deletions Darling/Darling.Tests/CollectionStoppedThresholdAgreementTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright (c) 2026 Erik Darling, Darling Data LLC
*
* This file is part of the SQL Server Performance Monitor.
*
* Licensed under the MIT License. See LICENSE file in the project root for full license information.
*/

using System;
using PerformanceMonitor.Common;
using PerformanceMonitor.Darling.Service;
using Xunit;

namespace Darling.Tests;

/// <summary>
/// #2794: "collection has stopped" is ONE condition and must have ONE definition. The display's Offline band
/// and the alert engine's Collection Stopped window disagreed — 15 minutes against 30 — so a long
/// <c>query_store</c> cycle that stretched a sweep past 15 minutes painted a healthy server with the red
/// Offline overlay while the alert engine (correctly) stayed quiet, and an investigation chased five phantom
/// dark shards. Measured on the production fleet: legitimate sweep stretch reached 19m18s under issue-day
/// load (12m12s post-#2792), while genuine dark events run hours — so the two populations are separable and
/// 30 minutes is the number the alert engine already committed to.
///
/// <para>These pins hold the definitions together BY VALUE across the seams a shared constant cannot reach
/// (a config default is an int, the evaluator's window is a TimeSpan, the band threshold is a TimeSpan) —
/// so any one of them drifting apart goes red here rather than silently re-splitting the condition.
/// Proven red against the pre-#2794 tree, where the first assertion fails 15m != 30m.</para>
/// </summary>
public class CollectionStoppedThresholdAgreementTests
{
[Fact]
public void TheDisplaysOffline_AndTheAlertEnginesStopped_AreTheSameWindow() =>
Assert.Equal(DarlingSelfAlertEvaluator.StaleWindow, ServerHealthThresholds.OfflineThreshold);

[Fact]
public void TheConfigDefault_MatchesTheSharedWindow() =>
Assert.Equal(
TimeSpan.FromMinutes(new DarlingConfig().Alerts.CollectionStaleMinutes),
ServerHealthThresholds.OfflineThreshold);

[Fact]
public void TheSharedConstant_IsTheWindowBothDeriveFrom() =>
Assert.Equal(
TimeSpan.FromMinutes(ServerHealthThresholds.CollectionStoppedMinutesDefault),
ServerHealthThresholds.OfflineThreshold);

/// <summary>
/// The regression itself, as behavior: the worst legitimate sweep stretch the issue measured (19m18s on
/// a healthy production server mid-<c>query_store</c> cycle) bands Stale — visibly lagged, honestly
/// amber — never the red Offline overlay that claims the server is dark.
/// </summary>
[Fact]
public void AMeasuredLegitimateSweepStretch_BandsStale_NotOffline()
{
var now = new DateTime(2026, 9, 2, 20, 49, 0, DateTimeKind.Utc);
var lastCollection = now - new TimeSpan(0, 19, 18);

Assert.Equal(ServerFreshness.Stale, ServerHealthClassifier.ClassifyFreshness(lastCollection, now));
}
}
8 changes: 5 additions & 3 deletions Darling/Darling.Tests/ServerHealthClassifierTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ public void ClassifyFreshness_NoCollection_IsNeverCollected() =>
[InlineData(0, ServerFreshness.Fresh)]
[InlineData(120, ServerFreshness.Fresh)] // exactly 2x cadence — still fresh
[InlineData(121, ServerFreshness.Stale)] // just past 2x cadence
[InlineData(900, ServerFreshness.Stale)] // exactly 15 min — still stale
[InlineData(901, ServerFreshness.Offline)] // just past 15 min
[InlineData(1200, ServerFreshness.Offline)]
[InlineData(900, ServerFreshness.Stale)] // 15 min — the OLD Offline boundary, now mid-band (#2794)
[InlineData(1158, ServerFreshness.Stale)] // 19m18s — the worst MEASURED legitimate sweep stretch (#2794's evidence); must never band Offline
[InlineData(1800, ServerFreshness.Stale)] // exactly 30 min — still stale (strict >)
[InlineData(1801, ServerFreshness.Offline)] // just past the shared collection-stopped window
[InlineData(3600, ServerFreshness.Offline)]
public void ClassifyFreshness_BandsByAge(int ageSeconds, ServerFreshness expected) =>
Assert.Equal(expected, ServerHealthClassifier.ClassifyFreshness(Now.AddSeconds(-ageSeconds), Now));

Expand Down
3 changes: 2 additions & 1 deletion Darling/Darling.Tests/ViewerServerChromeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ public void ApplyFreshness_OldCollection_IsOffline()
var now = DateTime.UtcNow;
var server = Server();

server.ApplyFreshness(now.AddMinutes(-30), now);
/* -31: exactly 30 minutes is the shared collection-stopped boundary and bands Stale (strict >, #2794). */
server.ApplyFreshness(now.AddMinutes(-31), now);

Assert.False(server.IsOnline);
Assert.Equal("Offline", server.DotStatus);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void TheDot_AndTheCard_RenderOneLadder()
{
Now, // Fresh
Now.AddMinutes(-5), // Stale
Now.AddMinutes(-30), // Offline
Now.AddMinutes(-31), // Offline — just past the shared 30-min collection-stopped window (#2794)
null, // NeverCollected — the one that disagreed
};

Expand Down Expand Up @@ -116,7 +116,7 @@ public void TheDotWords_AreTheCardsWords()
{
Assert.Equal("Online", Dot(Now).DotStatus);
Assert.Equal("Warning", Dot(Now.AddMinutes(-5)).DotStatus);
Assert.Equal("Offline", Dot(Now.AddMinutes(-30)).DotStatus);
Assert.Equal("Offline", Dot(Now.AddMinutes(-31)).DotStatus);
Assert.Equal("Awaiting first collection", Dot(null).DotStatus);
Assert.Equal("Unknown", new DarlingServer(1, "SQL2022", "Prod", true, 16).DotStatus);
}
Expand Down
10 changes: 6 additions & 4 deletions Darling/Darling.Tests/ViewerW2aTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,11 @@ public void ClassifyFreshness_NoCollection_IsNeverCollected()
[InlineData(120, ServerFreshness.Fresh)] // exactly 2x cadence — still fresh
[InlineData(121, ServerFreshness.Stale)] // just past 2x cadence
[InlineData(300, ServerFreshness.Stale)] // 5 min — stale
[InlineData(900, ServerFreshness.Stale)] // exactly 15 min — still stale
[InlineData(901, ServerFreshness.Offline)] // just past 15 min
[InlineData(1200, ServerFreshness.Offline)] // 20 min — offline
[InlineData(900, ServerFreshness.Stale)] // 15 min — the OLD Offline boundary, now mid-band (#2794)
[InlineData(1158, ServerFreshness.Stale)] // 19m18s — the worst measured legitimate sweep stretch (#2794)
[InlineData(1800, ServerFreshness.Stale)] // exactly 30 min — still stale (strict >)
[InlineData(1801, ServerFreshness.Offline)] // just past the shared collection-stopped window
[InlineData(3600, ServerFreshness.Offline)] // an hour dark — offline
public void ClassifyFreshness_BandsByAge(int ageSeconds, ServerFreshness expected)
{
var lastCollection = Now.AddSeconds(-ageSeconds);
Expand Down Expand Up @@ -246,7 +248,7 @@ public void ApplyFreshness_Stale_IsWarning()
[Fact]
public void ApplyFreshness_Offline_ShowsOverlay()
{
var offlineByAge = new ServerSummaryItem { LastCollectionTime = Now.AddMinutes(-20) };
var offlineByAge = new ServerSummaryItem { LastCollectionTime = Now.AddMinutes(-31) };
offlineByAge.ApplyFreshness(Now);
Assert.False(offlineByAge.IsOnline);
Assert.True(offlineByAge.IsOffline);
Expand Down
7 changes: 5 additions & 2 deletions Darling/PerformanceMonitor.Darling.Service/DarlingConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using System.Net.Sockets;
using System.Text.Json;
using PerformanceMonitor.Collectors;
using PerformanceMonitor.Common;
using System.Text.Json.Serialization;
using PerformanceMonitor.Notifications;

Expand Down Expand Up @@ -558,9 +559,11 @@ public sealed class AlertsConfig
public int SelfDiskFreeWarnPercent { get; set; } = 10;

/// <summary>#2107: how long collection may go quiet before Collection Stopped / Agent Not
/// Running fire (was a compile-time 30 minutes).</summary>
/// Running fire (was a compile-time 30 minutes). Defaults to the shared constant behind the
/// display's Offline band, so the two definitions of "collection stopped" agree out of the
/// box (#2794); widening it here widens only the ALERT window.</summary>
[JsonPropertyName("collectionStaleMinutes")]
public int CollectionStaleMinutes { get; set; } = 30;
public int CollectionStaleMinutes { get; set; } = ServerHealthThresholds.CollectionStoppedMinutesDefault;

/// <summary>#2107: the Collection Stopped fast path — consecutive failures with zero successes
/// that fire without waiting out the staleness window (was a compile-time 10).</summary>
Expand Down
Loading
Loading