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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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
- **The `tempdb Space` alert measured distance to the next autogrow, not distance to the ceiling** ([#2515]) - `TempDbSpaceInfo.UsedPercent` was `total_reserved / (total_reserved + unallocated)`, and both halves come from `tempdb.sys.dm_db_file_space_usage`, which reports the data files **as currently allocated**. On a pre-sized on-prem box that reads like real headroom only because such a tempdb has already grown to its cap. On **Azure SQL Database** it is not: the platform creates tempdb small and grows it toward the tier limit, so on `GP_S_Gen5_2` - four data files of 16 MB, **62.44 MB allocated**, each with `max_size` 2,097,152 pages for a **65,536 MB** ROWS ceiling - one ordinary ~57 MB `#temp` table read **95.7% full** against the allocation and **0.09%** against the cap. At the shipped 80% default that is a page on the first busy minute of every Azure target, which is what made enabling Azure tempdb collection ([#2512]) unsafe to ship. **The ceiling is discoverable, so this was a denominator bug rather than a policy call**: `tempdb_stats` now collects `max_size_mb` = `SUM(max_size)` over tempdb's **ROWS** files (`dm_db_file_space_usage` reports DATA allocation, so folding the log's cap in would understate usage everywhere), and both SKUs divide by it. Where any data file is unlimited (`max_size` -1) there is no ceiling to measure against and the current allocation stays the denominator - today's behaviour exactly, so **unlimited-growth on-prem and RDS targets do not move**. **What DOES move, deliberately:** any target whose tempdb has a fixed `max_size` it has not yet grown into now reports a **LOWER** percentage than before - 800 MB reserved inside a 1,000 MB allocation that may grow to 4,000 MB reads 20%, not 80% - and climbs back to the alert as the files actually approach the cap. That is a correction, not a regression, but it will move live numbers on existing servers. Deliberately **not** a size floor, the other candidate: a floor suppresses the alert on a genuinely full LARGE tempdb at the moment it starts growing, still fires at the autogrow boundary once cleared, and any value silently redefines "small" for every on-prem and RDS target already depending on today's behaviour. The `analyze_server` `TEMPDB_USAGE` fact carried its own copy of the same denominator and was fixed with it, so the pager and the analysis pass cannot describe one server two ways; the alert detail gained a **Max Size** field naming which denominator produced the percentage (a cap in MB, `Unlimited`, or `Unknown` for a snapshot taken before the ceiling was collected). `FactScorer.ScoreTempDbVersionStore`'s 1/2/5 GB bars are unchanged and needed no change - they are absolute MB and a denominator cannot move their reachability. Darling schema **v81**, Lite schema **v56**; the column is nullable with no backfill, and a historical row's NULL reads as "no ceiling measured", which reports the percentage it always did.
- **Nineteen Darling reads (eighteen on Lite) told an Azure SQL Database user to go and start a capture that cannot exist there** ([#2511]) - `sys.dm_xe_sessions` does not exist on Azure SQL Database (verified live on `EngineEdition` 5, General Purpose and Hyperscale), so the `system_health` ring buffer can never be read on that engine. `SystemHealthEventsCollector` knows this and gates itself off, correctly and permanently. **The reads did not**: all nine `get_health_parser_*` tools answered an Azure caller with *"check that collection is running for this server and that its system_health session is started"* - collection **was** running, and the session cannot be started on that engine. Thirteen collectors gate off on Azure SQL Database and no read anywhere branched on engine edition, so the same confident, specific, wrong instruction was waiting behind `get_server_config`, `get_trace_flags`, `get_default_trace_events`, `get_cpu_scheduler_pressure`, `get_memory_pressure_events`, `get_tempdb_trend`, `get_running_jobs` and the config-change reads too. Every one of those now answers **`not_collected`** - the miss vocabulary's existing word for *"the input names something this server does not collect"* - naming the engine, the collector and the capture path, and saying plainly that the gap is permanent. `unavailable` is kept for a server whose engine **does** support the path and simply has no rows, which is the case that is still worth chasing. **The capability answer is derived from the collectors' own `AppliesTo` gates**, never from a list: a collector is reported as a permanent engine gap only when *no* target of that engine edition runs it, swept across every version, msdb-access and RDS combination - so a gate that reads a **fixable** fact (msdb access, a version floor) is deliberately *not* reported as an engine gap, and a gate that opens up silently stops the claim rather than leaving a stale one standing.
- **Lite's .NET runtime prerequisites were documented against the wrong artifact** ([#2489]) - the README hung "requires .NET 10 Desktop Runtime" on the **`Setup.exe`** download line. That is the one artifact with no prerequisite at all: it is packed by `vpk` from a `--self-contained -r win-x64` publish, and its `runtimeconfig.json` carries `includedFrameworks`, not `frameworks`. Meanwhile the portable **ZIP** - a framework-dependent publish whose `runtimeconfig.json` names **three** shared frameworks - had its prerequisites written down nowhere, and `Lite/README.md` had no prerequisites section at all. So a tester who read the one runtime sentence we published installed a runtime they did not need, while a tester who unzipped onto a stock Windows Server got the .NET host's bare `You must install .NET to run this application` with nothing of ours on it. Each artifact now carries its OWN prerequisites, in the root README and in a new **Prerequisites** section in `Lite/README.md`. **ASP.NET Core Runtime 10 is required unconditionally** for the ZIP, and that is the part nobody expects: `ModelContextProtocol.AspNetCore` brings the framework reference in transitively, so it is named at build time and leaving MCP switched off does not remove it - and because the host reports only the FIRST framework it cannot find, installing just the Desktop Runtime buys a second identical failure. Lite cannot gate on this the way [#2481] did for Darling, because Lite has no install script and the host error precedes every line of our code; what ships instead is `READ-ME-FIRST.txt` beside `PerformanceMonitorLite.exe`, the one surface of ours a stranded reader can still reach. `LiteRuntimePrerequisiteDocsTests` keeps all of it honest by DERIVING the claims - the required runtimes from the built `runtimeconfig.json`, and which artifact is self-contained from the `dotnet publish` lines in `build.yml` and `nightly.yml` - so dropping the MCP package, making the ZIP self-contained, or moving to a new .NET major turns the docs red instead of stale. `README.md` and `Lite/README.md` are named in build.yml's `lite` path filter for the same reason: markdown is carved out of every area filter and a docs-only PR skips .NET setup entirely, so the guard would otherwise be unrunnable on precisely the change it exists to catch.

Expand Down Expand Up @@ -2862,6 +2863,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#2495]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2495
[#2506]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2506
[#2511]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2511
[#2512]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2512
[#2515]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2515
[#2480]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2480
[#2489]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2489
[#2481]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2481
Expand Down
36 changes: 36 additions & 0 deletions Darling/Darling.Tests/AlertEngineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,12 @@ public async Task TempDb_FiresAtThreshold_AndResolutionCarriesTheCurrentPercent(
h.Settings.TempDbSpaceEnabled = true;
var engine = h.Build();

/* #2515: 1000 MB total and NO MaxSizeMb, which is deliberate and stays that way. The fixture predates
the ceiling, so it describes a tempdb whose cap was never measured — and that is exactly the case
where the denominator remains the allocation. It does NOT imply a 1000 MB cap; if it did, this pin
would have to move, and the fact that it does not is the guarantee that no existing on-prem or RDS
target with an unlimited (or uncollected) tempdb sees its number change. The capped case gets its
own test below rather than being folded in here. */
h.Adapter.TempDb = new TempDbSpaceInfo { TotalReservedMb = 800, UnallocatedMb = 200 }; /* 80% used */
await engine.EvaluateServerAsync(Harness.Snapshot());
var fired = Assert.Single(h.Deliverer.Outcomes);
Expand All @@ -1066,6 +1072,36 @@ public async Task TempDb_FiresAtThreshold_AndResolutionCarriesTheCurrentPercent(
Assert.Equal("SRV-A: tempdb usage back to 20%", resolution.Message); /* :461,:464 */
}

/// <summary>
/// #2515, through the ENGINE rather than the arithmetic: the Azure shape from the issue must not fire, and
/// the same allocation without a ceiling must. Same 59.75 MB of reserved tempdb in both, same 80% default —
/// the only difference is whether the collector could see how far the files are allowed to grow.
///
/// <para>This is the assertion the whole change exists for. <see cref="TempDbCeilingStoreTests"/> proves
/// the arithmetic and the store round-trip; this proves the alert engine's decision follows it, which is
/// what actually pages someone.</para>
/// </summary>
[Fact]
public async Task TempDb_TheAzureCeiling_SuppressesTheAlert_ThatTheAllocationWouldFire()
{
var h = new Harness();
h.Settings.TempDbSpaceEnabled = true;
Assert.Equal(80, h.Settings.TempDbSpaceThresholdPercent);
var engine = h.Build();

/* GP_S_Gen5_2 with one ~57 MB #temp table: 62.44 MB allocated, 65,536 MB of headroom behind it. */
h.Adapter.TempDb = new TempDbSpaceInfo { TotalReservedMb = 59.75, UnallocatedMb = 2.69, MaxSizeMb = 65_536 };
await engine.EvaluateServerAsync(Harness.Snapshot());
Assert.Empty(h.Deliverer.Outcomes);

/* The identical snapshot with the ceiling unmeasured is the pre-#2515 reading, and it pages. */
h.Adapter.TempDb = new TempDbSpaceInfo { TotalReservedMb = 59.75, UnallocatedMb = 2.69 };
await engine.EvaluateServerAsync(Harness.Snapshot());
var fired = Assert.Single(h.Deliverer.Outcomes);
Assert.Equal("tempdb Space", fired.MetricName);
Assert.Equal("96% used (60 MB)", fired.CurrentValue);
}

/* ---------------- low disk ---------------- */

[Fact]
Expand Down
37 changes: 18 additions & 19 deletions Darling/Darling.Tests/CollectionLogFanoutRollupStoreTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
namespace Darling.Tests;

/// <summary>
/// The V80 rung (#2472) — the per-database fan-out rollup on <c>collection_log</c>, and the top of the ladder.
/// The V80 rung (#2472) — the per-database fan-out rollup on <c>collection_log</c>. No longer the top of the
/// ladder: V81 (#2515) took that, and the assertions that belong to whichever rung is newest moved with it to
/// <see cref="TempDbCeilingStoreTests"/>. What stays here is everything that is true of THIS rung forever.
///
/// <para><b>What it is for.</b> Five collectors run once per DATABASE and the run writes ONE row whose
/// <c>duration_ms</c> is the sum across all of them. So 8 databases at 10.1s and one at 62s beside seven at
Expand All @@ -32,14 +34,16 @@ namespace Darling.Tests;
public class CollectionLogFanoutRollupStoreTests
{
[Fact]
public void TheRungIsRegisteredAndIsTheTopOfADenseLadder()
public void TheRungIsRegisteredInADenseLadder()
{
var versions = PgMigrations.Scripts.Select(s => s.Version).ToList();

Assert.Equal("collection-log-fanout-rollup", PgMigrations.Scripts.Single(s => s.Version == 80).Name);
Assert.Equal(80, versions.Max());
Assert.Equal(80, PgMigrations.Scripts[^1].Version);
Assert.Equal(80, StorageVersion.SchemaVersion);

/* Demoted at V81 (#2515): "80 is the maximum" was true of the LADDER while this was its top rung,
not of this rung, and leaving it here is how a demotion turns into a red build on the next one.
The density and ordering checks below stay — those are properties of the whole ladder that every
rung's test may assert. */
Assert.Equal(StorageVersion.SchemaVersion, versions.Max());

Assert.Equal(versions.Distinct().OrderBy(v => v), versions);
Expand Down Expand Up @@ -90,28 +94,22 @@ public void TheRungRefreshesThePassthroughView()
}

[Fact]
public void TheProbeAsksForTheColumn_AndTheThreePlacesAgree()
public void TheProbeAsksForTheColumn_AndTheReaderStillFetchesItsOrdinal()
{
Assert.Contains(
"table_name = 'collection_log' AND column_name = 'slowest_item_ms'",
ViewerDataService.StoreSchemaProbeSql, StringComparison.Ordinal);

var mapParameters = typeof(ViewerDataService)
.GetMethod("MapProbedSchemaVersion", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!
.GetParameters().Length;

var viewerSource = ReadViewerSource();

/* Ordinals are positional: the sentinel was APPENDED, so the reader must ask for exactly one more
than it did and no more than that. */
Assert.Contains($"reader.GetBoolean({mapParameters - 1})", viewerSource, StringComparison.Ordinal);
Assert.DoesNotContain($"reader.GetBoolean({mapParameters})", viewerSource, StringComparison.Ordinal);
/* Demoted at V81 (#2515): this used to read `mapParameters - 1`, which is the NEWEST sentinel's
ordinal, so once a rung was appended it silently started testing that rung's wiring instead of
this one's. Pinned at 55 — this rung's own ordinal — which cannot slide. The "and no more than
that" half is the top rung's to assert, and it moved to TempDbCeilingStoreTests with it. */
Assert.Contains("reader.GetBoolean(55)", ReadViewerSource(), StringComparison.Ordinal);
}

[Fact]
public void TheProbeMapsAFullyMigratedStoreTo80()
public void TheProbeMapsAStoreAtExactly80To80()
{
Assert.Equal(80, StorageVersion.SchemaVersion);
Assert.Equal(StorageVersion.SchemaVersion, ViewerDataService.RequiredStoreSchemaVersion);

var method = typeof(ViewerDataService)
Expand All @@ -121,7 +119,8 @@ public void TheProbeMapsAFullyMigratedStoreTo80()
/* 55 positional sentinels, then this rung's own, then FALSE for anything a later rung appends. The
leading count is FIXED at this rung's ordinal deliberately — see the note on V79's twin: deriving
it from arity reads identically while this is the top rung, then slides one place right per new
rung, and the assertion keeps passing while quietly testing a newer arm. */
rung, and the assertion keeps passing while quietly testing a newer arm. V81 has since been
appended and this test needed no edit, which is the fixed count doing exactly what it is for. */
var all = Enumerable.Repeat(true, 55).Cast<object>().ToArray();
object[] Args(bool ownFlag) => all
.Concat(new object[] { ownFlag })
Expand Down
16 changes: 7 additions & 9 deletions Darling/Darling.Tests/FileGrowthAlertStoreTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,18 @@ public void TheRungAddsTheKnobs_Idempotently_AndTheAlertShipsOff()
}

[Fact]
public void TheProbeAsksForTheColumn_AndTheThreePlacesAgree()
public void TheProbeAsksForTheColumn_AndTheReaderStillFetchesItsOrdinal()
{
Assert.Contains(
"table_name = 'config_alert_settings' AND column_name = 'file_growth_enabled'",
ViewerDataService.StoreSchemaProbeSql, StringComparison.Ordinal);

var mapParameters = typeof(ViewerDataService)
.GetMethod("MapProbedSchemaVersion", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!
.GetParameters().Length;

var viewerSource = ReadViewerSource();

Assert.Contains($"reader.GetBoolean({mapParameters - 1})", viewerSource, StringComparison.Ordinal);
Assert.DoesNotContain($"reader.GetBoolean({mapParameters})", viewerSource, StringComparison.Ordinal);
/* Demoted at V81 (#2515), and it should have been at V80. This read `mapParameters - 1`, which is
the NEWEST sentinel's ordinal — correct only while V79 was the top rung, and from V80 onwards it
was quietly asserting a later rung's wiring while reading as though it still guarded this one.
Pinned at 54, this rung's own ordinal, which cannot slide. The "and no more than that" half
belongs to whichever rung is newest and lives there. */
Assert.Contains("reader.GetBoolean(54)", ReadViewerSource(), StringComparison.Ordinal);
}

[Fact]
Expand Down
Loading
Loading