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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **Compression Job Stuck no longer false-alarms on a job it caught mid-run** - measured live on TimescaleDB 2.x: from the moment the scheduler picks up a due job until its run completes, `job_stats.next_start` reads `-infinity` with `job_status = 'Running'`, and the real next start is only computed at completion. The detector's first arm treated `-infinity` unconditionally as "the scheduler will never run it again", so any healthy compression run the self-alert check happened to sample got flagged as stuck, alerted, and "self-healed" with a pointless re-arm - the transient stuck-then-self-healed alert pairs the field has been shrugging off were this false positive, and the stuck-detector live test's CI flake was the same race (it re-arms with `next_start => now()` and then read a single snapshot while the run it had just triggered was still executing). A RUNNING job's `-infinity` now defers to the elapsed-bound arm, which is what actually distinguishes a hung run from a healthy one - a genuinely dead job (`-infinity`, not running) still alerts exactly as before, and a hung run still trips the bound.
- **The Job History tab speaks display names** ([#2126], asked by ghauan) - both the Server filter dropdown and the Server column showed the raw collected server name while every other tab shows the operator's alias, so a fleet navigated by aliases turned into a memory quiz on exactly the tab an operator visits during an incident. Both readers (job history and the Agent status header) now resolve through the servers registry - the alias when one exists, the raw name otherwise - so the filter, the column, the per-column filter popup, and the CSV export all speak the same names as the rest of the viewer, and the Agent roll-up sorts by them. Lite's Job History tab had the same gap through a different mechanism (review catch): Lite's display-name concept lives at the CONFIG layer, not in DuckDB (the stored servers.display_name column is unpopulated by design), so the shell now passes a server_id-to-alias snapshot into the tab Overview-style and rows swap in the alias on every refresh - a server no longer in config keeps its raw collected name, the durable-record case.
- **The long-query completion XE session actually gets created now** ([#2129], from ghauan's field report on #2061 - they enabled the collector on two servers and the Long Queries tab stayed empty forever) - the session DDL SET a customizable attribute `collect_object_name` on `sqlserver.rpc_completed`, and no such attribute exists on that event on ANY version (it belongs to `sp_statement_completed`) - `object_name` is one of rpc_completed's DEFAULT data fields, collected with no SET at all. So the CREATE failed on every server, the session never existed, and the reconcile's follow-up START surfaced as the confusing second error ('Cannot alter the event session... does not exist'). Never caught in dogfood because the collector ships OFF by design, and the DDL test pin asserted the wrong claim, so CI enforced the bug. The SET is gone (the reader already shreds the default field generically - no reader or table change), and the pin now asserts the attribute is ABSENT, with the story attached. Anyone who flipped the collector on before this fix: it starts working on the next reconcile tick after upgrading, no re-toggle needed.
- **`--collapse-legacy-slices` narrows its slice instead of dying when a day does not fit the statement timeout** ([#2105] round three, ghauan once more - with the decompression rail lifted, the run made it ~15 minutes in and died at the NEW wall: a day-wide stage aggregation on a store carrying 60k split intervals blows through the 15-minute per-statement timeout, and the operator got the same bare stream exception) - the verb's fixed day-per-slice loop now runs the same adaptive schedule the Query Store backfill worker shipped this week (`AdaptiveSpan`, 24h base): a failed slice halves the window and retries the SAME start (announced with a [RETRY] line naming the error, so narrowing reads as progress rather than a hang), a completed slice resets to full width, and only a slice that fails at the ~22-minute floor gives up to the existing idempotent re-run message. Healthy stores still repair in a handful of day-wide slices - the narrowing costs nothing until a slice actually fails.

## [3.4.0] - 2026-08-06

Expand Down
34 changes: 34 additions & 0 deletions Darling/Darling.Tests/DarlingCliCommandsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,40 @@ public void DescribeEngineEdition_MapsKnownEditions()
Assert.Equal("Azure SQL Managed Instance", DarlingServerConnector.DescribeEngineEdition(8));
Assert.Contains("Unknown", DarlingServerConnector.DescribeEngineEdition(999), StringComparison.Ordinal);
}

/* ---- the collapse verb's adaptive narrowing decision (#2105 round three) — pure pins ---- */

private static readonly TimeSpan Day = TimeSpan.FromDays(1);

[Fact]
public void NextNarrowingFailureCount_FullWidthSlice_TakesTheFirstHalvingStep()
{
/* A failed 24h slice narrows to 12h — one more failure than before. */
Assert.Equal(1, DarlingCliCommands.NextNarrowingFailureCount(Day, 0, Day));
/* And a 12h slice that fails again narrows to 6h. */
Assert.Equal(2, DarlingCliCommands.NextNarrowingFailureCount(Day, 1, TimeSpan.FromHours(12)));
}

[Fact]
public void NextNarrowingFailureCount_ClampedTail_SkipsStepsThatWouldRerunTheSameWindow()
{
/* The review catch: a clamped 30-minute final slice is already narrower than the 12h/6h/3h/1.5h/45m
nominal steps — re-running any of them is the identical window. The first step that actually
narrows 30m is the 22.5m floor (failure count 6). */
Assert.Equal(6, DarlingCliCommands.NextNarrowingFailureCount(Day, 0, TimeSpan.FromMinutes(30)));
}

[Fact]
public void NextNarrowingFailureCount_AtOrBelowTheFloor_ReturnsNull_TheSameWidthRetryTakesOver()
{
/* The 24h schedule floors at 22.5m (6 halvings). A slice at or under that width cannot be
narrowed — the caller's one fresh-connection same-width retry is the only move left, and it
must NOT be skipped just because narrowing is impossible (the run's usual last slice is a
partial-day clamp of arbitrary width). */
Assert.Null(DarlingCliCommands.NextNarrowingFailureCount(Day, 0, TimeSpan.FromMinutes(22.5)));
Assert.Null(DarlingCliCommands.NextNarrowingFailureCount(Day, 0, TimeSpan.FromMinutes(5)));
Assert.Null(DarlingCliCommands.NextNarrowingFailureCount(Day, 6, TimeSpan.FromMinutes(22.5)));
}
}

/// <summary>
Expand Down
135 changes: 117 additions & 18 deletions Darling/PerformanceMonitor.Darling.Service/DarlingCliCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using System.Threading;
using System.Threading.Tasks;
using Npgsql;
using PerformanceMonitor.Collectors;
using PerformanceMonitor.Darling.Service.Hosting;
using PerformanceMonitor.Darling.Service.Mcp;
using PerformanceMonitor.Darling.Storage;
Expand Down Expand Up @@ -3000,42 +3001,109 @@ public static async Task<int> CollapseLegacySlicesAsync(
return 0;
}

/* SLICED PER DAY, not one call over the whole span. CollapseSliceAsync runs each slice in ONE
/* SLICED, not one call over the whole span. CollapseSliceAsync runs each slice in ONE
transaction, and that transaction takes locks on the raw chunks it touches — which the compression
policy also wants. Handing it the entire survey span would make one long transaction sitting across
however much history the store keeps, which is exactly the lock-duration family that has bitten this
repo before (#1564/#1567). On a default 4-day raw tier this is a handful of slices; on a store with
a widened retention it is the protection the method's own doc promises.
repo before (#1564/#1567).

Slice width is ADAPTIVE (#2105 round three): a day is the fast default, but on the field store
that motivated this the FIRST day-wide stage aggregation blew through the 15-minute statement
timeout — the operator watched it die at minute ~15 with the bare stream exception, three walls
deep. A failed slice now halves the window and retries the SAME start (the shared
QueryStoreBackfillState.AdaptiveSpan schedule the backfill worker uses, 24h base → 22.5m floor),
a completed slice resets to full width, and only a slice that fails AT the floor gives up to the
existing re-run message. Narrowing is announced so the operator sees progress, not a hang.

The half-open upper bound includes the newest collapsed row — the survey reports that instant
itself, not a bound past it — hence the final slice's one-second nudge. */
long removed = 0;
var sliceStart = survey.OldestUtc!.Value.Date;
var collapseEnd = survey.NewestUtc!.Value.AddSeconds(1);
var fullWidth = TimeSpan.FromDays(1);
var consecutiveFailures = 0;
var retriedAtWidth = false;

try
while (sliceStart < collapseEnd)
{
while (sliceStart < collapseEnd)
var span = QueryStoreBackfillState.AdaptiveSpan(fullWidth, consecutiveFailures);
var sliceEnd = sliceStart + span;
if (sliceEnd > collapseEnd)
{
var sliceEnd = sliceStart.AddDays(1);
if (sliceEnd > collapseEnd)
{
sliceEnd = collapseEnd;
}
sliceEnd = collapseEnd;
}

/* The width the slice ACTUALLY covers — the final slice clamps to the range end, so the
nominal AdaptiveSpan width can overstate it, and both the retry decision and the operator
messages must speak in real terms (review catch). */
var actualWidth = sliceEnd - sliceStart;

try
{
removed += await QueryStoreSliceRepair.CollapseSliceAsync(
connection, sliceStart, sliceEnd, cancellationToken);

consecutiveFailures = 0;
retriedAtWidth = false;
sliceStart = sliceEnd;
}
}
catch (Exception ex) when (ex is not OperationCanceledException)
{
/* Each slice is its own transaction, so earlier slices are already committed and are not lost —
and the collapse is idempotent, so re-running picks up where this stopped. */
error.WriteLine($" The collapse failed after {removed:N0} row(s); the failing slice was rolled back: {ex.Message}");
error.WriteLine(" Slices already committed are safe. Re-run to continue — the repair is idempotent.");
return 1;
catch (Exception ex) when (ex is not OperationCanceledException)
Comment on lines +3041 to +3050

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retry reuses a connection that may already be dead.

On failure the loop continues and calls CollapseSliceAsync again on the same connection (line 3037-3038). But the failure mode this PR exists to survive is SliceStatementTimeoutSeconds (900s) expiring — and per the doc comment on that constant a few lines up in QueryStoreSliceRepair.cs ("The failure read as 'Exception while reading from stream' ... which is how an Npgsql command timeout surfaces"), that's a bare stream exception, not a clean 57014 canceling statement due to user request. That's the signature of Npgsql's cancel-on-timeout failing to land and force-closing/breaking the connection rather than cleanly cancelling server-side.

If the connection is actually broken at that point, the next iteration's BeginTransactionAsync fails immediately — not because the narrower window still doesn't fit, but because there's no usable connection. The loop would then burn through all six halving steps almost instantly on local connection errors and give up, without the narrower windows ever really being tried against Postgres — quietly defeating the fix in exactly the scenario (a real statement timeout) it's meant to handle.

The PR description notes this exact retry path "can't be provoked deterministically in a live test," so it looks untested end-to-end. Worth checking connection.FullState/State and reopening (or just opening a fresh connection per slice, like QueryStoreBackfill.cs does per-attempt) before retrying, and verifying against a real 900s timeout before shipping — this repo already treats "a dead connection poisons every collector" as a known failure class elsewhere (DarlingWorker.cs).

{
var next = NextNarrowingFailureCount(fullWidth, consecutiveFailures, actualWidth);

/* A slice already at/below the adaptive floor (usually the clamped final tail — nothing
says the leftover is ≥ the floor) can't be narrowed, but its likeliest failure is the
transient/connection kind the fresh-connection retry exists for — so it earns ONE
same-width retry before the give-up (review catch: giving up on the tail's first
failure silently exempted the run's usual last slice from the retry mechanism). */
var sameWidthRetry = next is null && !retriedAtWidth;

if (next is int || sameWidthRetry)
{
/* The statement-timeout failure this loop exists to survive surfaces as a broken
STREAM, not a clean server-side cancel — the connection underneath is very likely
dead, and retrying on it would fail instantly through every halving step (review
catch). Cycle it: close is safe on a broken connection, and reopen draws a fresh
physical connection. Session state doesn't matter — the slice's SET LOCAL and
per-command timeouts are transaction/command scoped. A failed REOPEN degrades to
the same clean idempotent-rerun message as every other failure here, never an
unhandled crash (review catch — this verb has no caller safety net). */
try
{
await connection.CloseAsync();
await connection.OpenAsync(cancellationToken);
}
catch (Exception reopenEx) when (reopenEx is not OperationCanceledException)
{
error.WriteLine($" The collapse failed after {removed:N0} row(s); the slice at {sliceStart:yyyy-MM-dd HH:mm} failed ({FirstLineOf(ex.Message)}) and the store connection could not be reopened: {FirstLineOf(reopenEx.Message)}");
error.WriteLine(" Slices already committed are safe. Re-run to continue — the repair is idempotent.");
return 1;
}

if (next is int narrowerFailures)
{
consecutiveFailures = narrowerFailures;
retriedAtWidth = false;
var narrower = QueryStoreBackfillState.AdaptiveSpan(fullWidth, narrowerFailures);
output.WriteLine($" [RETRY] slice {sliceStart:yyyy-MM-dd HH:mm} +{actualWidth.TotalMinutes:F0}m failed ({FirstLineOf(ex.Message)}); narrowing to {narrower.TotalMinutes:F0}m and retrying.");
}
else
{
retriedAtWidth = true;
output.WriteLine($" [RETRY] slice {sliceStart:yyyy-MM-dd HH:mm} +{actualWidth.TotalMinutes:F0}m failed ({FirstLineOf(ex.Message)}); already at the narrowest width — retrying once on a fresh connection.");
}

continue;
}

/* Narrowing exhausted AND the same-width retry spent — this range cannot be repaired
unattended. Each slice is its own transaction, so earlier slices are already committed
and are not lost — and the collapse is idempotent, so re-running picks up where this
stopped. */
error.WriteLine($" The collapse failed after {removed:N0} row(s); the failing {actualWidth.TotalMinutes:F0}m slice at {sliceStart:yyyy-MM-dd HH:mm} was rolled back: {ex.Message}");
error.WriteLine(" Slices already committed are safe. Re-run to continue — the repair is idempotent.");
return 1;
}
}

output.WriteLine($" Collapsed. Rows removed: {removed:N0}");
Expand Down Expand Up @@ -3083,6 +3151,37 @@ widened to whole buckets so a partially-covered bucket is recomputed rather than
private static DateTime Floor(DateTime value, TimeSpan bucket)
=> bucket <= TimeSpan.Zero ? value : new DateTime(value.Ticks - (value.Ticks % bucket.Ticks), value.Kind);

/// <summary>
/// The collapse loop's narrowing decision, pure so it pins without a live timeout: the smallest
/// failure count whose <see cref="QueryStoreBackfillState.AdaptiveSpan"/> width actually narrows a
/// slice that COVERED <paramref name="actualWidth"/> (a clamped final slice can be narrower than
/// several nominal halving steps, and re-running an identical window just re-hits the same wall), or
/// null when no step can — the slice already sits at/below the adaptive floor, where the caller's
/// one same-width fresh-connection retry is the only move left.
/// </summary>
internal static int? NextNarrowingFailureCount(TimeSpan fullWidth, int consecutiveFailures, TimeSpan actualWidth)
{
var next = consecutiveFailures + 1;
var narrower = QueryStoreBackfillState.AdaptiveSpan(fullWidth, next);
while (narrower >= actualWidth)
{
var evenNarrower = QueryStoreBackfillState.AdaptiveSpan(fullWidth, next + 1);
if (evenNarrower >= narrower)
{
return null;
}

next++;
narrower = evenNarrower;
}

return next;
}

/// <summary>An exception message's first line, CR-trimmed — one-line operator output must stay one line.</summary>
private static string FirstLineOf(string message)
=> message.Split('\n')[0].TrimEnd('\r');

/// <summary>How <c>--recompress-plan-dim</c> handles the closing VACUUM FULL (#2076).</summary>
public enum RecompressVacuumMode
{
Expand Down
Loading