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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **The dimension GC prunes against the oldest SURVIVING digest-carrying fact row, not an assumed horizon** ([#1813], closes [#1795]) - the proper closure of the #1782 x #1784 interaction those issues shipped with: query_stats and procedure_stats are both coverage-gated tiers AND dim-feeding tables, so on a coverage-lagging store the #1784 clamp held their purges every sweep, the #1782 guard read that as "purge did not complete" and deferred the whole dimension GC - every sweep, until a backfill landed. Proven by execution: a 400-day-old orphan dimension row survived with nothing failed anywhere. The deferral was logically correct (pruning content the held facts reference would dangle digests and the resolving view would serve NULL payload silently) but blunt: it stopped the GC entirely rather than narrowing it.

The true safety boundary is measured, not assumed: content older than the oldest surviving digest-carrying fact row cannot be referenced by anything, whatever the reason those facts are still there. The GC now probes each dim-feeding table's floor - `min(collection_time)` under exactly the predicate of a new V39 partial index per table, an index-edge read once per sweep rather than the oldest-chunk walk the pre-#1767 NULL-digest rows would force, the bounded shape the last_seen watermark design (#1768) demands - takes the minimum across tables (the same reasoning as the widest-retention horizon), and clamps the assumed cutoff to one day before it, the same margin the assumed side carries for the hourly last_seen refresh guard. On a healthy store the measured floor sits inside retention and the assumed horizon rules unchanged; on a held store the GC stays bounded instead of stopped, reclaiming content for queries that stopped running long before while everything the held facts reference survives. The blunt guard is thereby unnecessary rather than dormant - it survives in exactly one honest form: a floor that cannot be MEASURED (the table missing or unreachable) still defers the cycle, because pruning on an unknown boundary is the one way to dangle digests, and bounded growth beats silent corruption. The probe predicate, the V39 index predicate, and the dimension map are pinned to each other three ways, so a new dimension column lands in all of them or none. The viewer's schema ladder gains the matching V39 arm (index-existence sentinel, the V22 idiom) so a fully-migrated store maps to exactly the required version instead of tripping the connect gate.

Chasing the live proof surfaced a REAL test-fixture defect worth its own record: `EnsureContinuousAggregatesAsync` attaches refresh policies whose jobs fire immediately (the #1788 finding), and the class's deep force-refresh collided with them (55P03) while a restore's DROP could collide with a running job's lock and silently strand `query_stats_db_hourly`/`db_daily` in the shared fixture - stranded aggregates whose deep coverage then flipped the #1784 gate for every LATER test, the same manufactured-flake mechanism #1794 documented for connection debris. The tests never needed the scheduler (they refresh manually): the ensure wrapper now removes every rollup's refresh policy immediately, and the one force-refresh that can still catch an already-executing job retries bounded on 55P03 only - the product's own #1788 idiom. Three consecutive full-class live runs leave zero stranded aggregates.

- **Recommendations warm-up now survives the 512 MB archive/reset, and analysis reads the archive tier everywhere** ([#1811], closes [#1809]) - the 24-hour sufficiency check measured MIN..MAX(collection_time) on the RAW hot wait_stats table, while `ArchiveAllAndResetAsync` copies everything to Parquet and empties the hot store. On a multi-server install that trips the size threshold more often than daily, the measured span restarted with every reset and warm-up never completed - even though the archived history was sitting in Parquet and every other tab could read it through the `v_` union views. The check now reads `v_wait_stats`, so archived plus hot history counts, and a genuinely young install still gates (both pinned, watched red by reverting the query to the raw table).

The subtlety that made this a 24-file-line sweep rather than a one-word fix: the broken gate was accidentally SHIELDING a second instance of the same defect. The analysis fact collector read raw tables in 22 more places - the window reads (query stats, snapshots, memory, perfmon, file IO, blocking, deadlocks, storage) and the on-load config snapshots (server/database config, trace flags, server properties), which after a reset are EMPTY until the next app start. Fixing only the gate would have run analysis over a thin post-reset hot window, reintroducing the exact fraction-of-period distortion the 24-hour floor exists to prevent ("5 seconds of THREADPOOL looks alarming in a 16-minute window"). All 23 sites now read their `v_` archive views - column-identical by construction (only `config_alert_log` carries a view-only column, and analysis does not read it), dedup-safe where re-collection could duplicate rows (the QUALIFY views), and a catalog-driven sweep guard holds the whole pipeline there: for every table in `ArchiveService.ArchivableTables`, a raw `FROM` anywhere in Lite/Analysis fails the build's tests naming the file - so a new collector's table is guarded the day it exists.
Expand Down Expand Up @@ -1902,6 +1908,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#1803]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1803
[#1805]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1805
[#1808]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1808
[#1795]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1795
[#1813]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1813
[#1809]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1809
[#1811]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1811
[#1794]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1794
Expand Down
87 changes: 87 additions & 0 deletions Darling/Darling.Tests/DarlingDimensionGcBoundTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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 System.Linq;
using PerformanceMonitor.Darling.Service;
using PerformanceMonitor.Darling.Storage;
using Xunit;

namespace Darling.Tests;

/// <summary>
/// #1795: the dimension GC's cutoff math and the three-way alignment behind its floor probe. The probe's
/// speed contract is that its WHERE clause EXACTLY matches the V39 partial index predicate, and both are
/// derived from <see cref="PayloadDimensions.All"/> — these pins hold the derived strings AND the V39 DDL
/// to each other, so a new dimension column cannot land in the map without landing in the index, and the
/// index predicate cannot drift from the probe's.
/// </summary>
public sealed class DarlingDimensionGcBoundTests
{
private static readonly DateTime Now = new(2026, 7, 28, 12, 0, 0, DateTimeKind.Unspecified);

/* widest = 30 → assumed cutoff = now - (30 + ChunkIntervalDays + 1). */
private static DateTime Assumed => Now.AddDays(-(30 + TimescaleSupport.ChunkIntervalDays + 1));

[Fact]
public void HealthyFloor_LeavesTheAssumedHorizonAlone()
{
/* Floor well inside retention (facts purging normally): the measured bound (floor - 1d) sits
NEWER than the assumed cutoff, and the cutoff must not move forward past the assumed horizon —
the GC never gets MORE aggressive than today. */
var cutoff = DarlingRetention.ComputeDimensionCutoff(Now, 30, Now.AddDays(-4));
Assert.Equal(Assumed, cutoff);
}

[Fact]
public void HeldFloor_ClampsTheCutoffToOneDayBeforeIt()
{
/* The #1795 field state: the clamp holds 45-day-old facts, older than the assumed horizon. The
cutoff follows the MEASURED floor minus the one-day last_seen margin, so content those facts
reference survives while anything older is reclaimed. */
var floor = Now.AddDays(-45);
var cutoff = DarlingRetention.ComputeDimensionCutoff(Now, 30, floor);
Assert.Equal(floor.AddDays(-1), cutoff);
Assert.True(cutoff < Assumed);
}

[Fact]
public void NoDigestFacts_FallBackToTheAssumedHorizon()
{
/* A fresh (or fully-aged) store has no digest-carrying facts at all: nothing can dangle, and
last_seen still bounds what is old enough to take. */
var cutoff = DarlingRetention.ComputeDimensionCutoff(Now, 30, oldestSurvivingDigestFact: null);
Assert.Equal(Assumed, cutoff);
}

[Fact]
public void DigestPredicates_AreExactlyTheDeclaredColumns_InDeclarationOrder()
{
/* The strings themselves, pinned: the probe filters on these, the V39 index is declared with
these, and both derive from PayloadDimensions.All. */
Assert.Equal(
"query_text_digest IS NOT NULL OR query_plan_digest IS NOT NULL",
PayloadDimensions.DigestPredicateByTable["query_stats"]);
Assert.Equal(
"query_plan_digest IS NOT NULL",
PayloadDimensions.DigestPredicateByTable["procedure_stats"]);
Assert.Equal(2, PayloadDimensions.DigestPredicateByTable.Count);
}

[Fact]
public void V39Indexes_UseExactlyTheProbePredicates()
{
var v39 = PgMigrations.Scripts.Single(m => m.Version == 39).Sql;

foreach (var (factTable, predicate) in PayloadDimensions.DigestPredicateByTable)
{
Assert.Contains($"ON {factTable} (collection_time)", v39, StringComparison.Ordinal);
Assert.Contains($"WHERE {predicate}", v39, StringComparison.Ordinal);
}
}
}
4 changes: 2 additions & 2 deletions Darling/Darling.Tests/DarlingObservabilityTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ public void MigrationScripts_AreRegisteredInAscendingOrder_V34AgCollectors_V36Ag
Assert.Equal(33, PgMigrations.Scripts[32].Version);
/* The newest migration is asserted by identity rather than by ordinal: this ladder is walked by every
stacked branch at once, and a positional pin turns each addition into a conflict for the next. */
Assert.Equal(38, PgMigrations.Scripts[^1].Version);
Assert.Equal(38, StorageVersion.SchemaVersion);
Assert.Equal(39, PgMigrations.Scripts[^1].Version);
Assert.Equal(39, StorageVersion.SchemaVersion);

/* V34 (#991) creates the two Availability Group collector tables. Schema-qualified collect.* and
CREATE TABLE IF NOT EXISTS, per the file's additive-create idiom (V29): a no-op on a fresh store
Expand Down
Loading
Loading