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 @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- **Two guards that turn structural properties into checked ones** ([#1796]) - review closers on [#1789] / [#1793], tests only. **The raw-tier retention policies are now pinned as DERIVED from `TimescaleSupport.RawTierCoverage`**, not merely equal to it. That map is what makes the tiered retention POLICY and the catalog SWEEP unable to judge the same drop differently, and until now the derivation was structural only: re-hardcoding the three rows beside the map produces an identical policy set and passes every behavioural test, right up until the map gains a fourth tier or one of the three changes its covering aggregate — at which point the two paths silently disagree about the exact thing the map exists to keep aligned. No behavioural test can see that, because a duplicated list behaves identically; only reading the construction can. Watched red by re-hardcoding the three rows exactly as a future edit would.

**And the coverage-skip log line is pinned in FULL rather than by prefix**, matching its dimension-GC sibling. Its TAIL is the actionable half — *resumes by itself once a backfill extends coverage* is what tells an operator the state is self-correcting rather than a fault to chase — and a prefix pin covers only the part naming the table. That is not hypothetical: the prefix-pin class already let a wrong line ship earlier in this work, where every assertion passed while the message named a cause the guard never detected. Watched red by deleting just the tail, and the failure output shows the prefix still intact — which is precisely the mutation the old pin would have called green.

- **The catalog retention sweep no longer deletes history the rollups never captured** ([#1793], closes [#1784]) - two purges drop the same raw chunks, and only one of them honored the #1680 coverage gate. The tiered 4-day policy is created paused and armed only once the hourly aggregate covers everything the table holds; `DarlingRetention.PurgeAsync` then dropped those very chunks at the per-collector 30-day horizon with **no coverage check at all**. On a store where the gate is deliberately holding the policy - because the rollup does not reach back over raw's history - the sweep destroyed exactly the uncovered history the gate exists to protect. The gate was not bypassed by a bug in the gate; it was bypassed by an older path that never learned the invariant. Field fuse on a production instance: raw from ~Jul 9, coverage from ~Jul 24, so the uncovered slice began dying ~**Aug 8** unless an operator ran a backfill in time - and the product must not depend on that. Both purges now consult the SAME predicate, reached through one shared map of raw tier to covering aggregate, so they cannot judge the same drop differently.

**It is a binary judgement, not a clamped cutoff, and that is forced by the mechanism.** `drop_chunks` can only remove the OLDEST chunks, and when coverage lags it is exactly the oldest chunks that are uncovered - so no cutoff value drops the covered tail while sparing the uncovered head. Working the intuitive `min(catalog horizon, coverage floor)` clamp against the field numbers shows it: at Aug 20 it yields Jul 21, which deletes Jul 9-21, every row of it uncovered. In the healthy case it fails the other way, clamping to the coverage floor and never dropping anything at all. Tiered tables are deliberately NOT excluded outright either - the moment coverage reaches back the normal horizon applies again, so a never-armed policy cannot mean unbounded growth.
Expand Down Expand Up @@ -1843,3 +1847,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#1789]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1789
[#1792]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1792
[#1793]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1793
[#1796]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1796
14 changes: 12 additions & 2 deletions Darling/Darling.Tests/PayloadDimensionLiveTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,16 @@ namespace Darling.Tests;
[Collection("live-postgres")]
public sealed class PayloadDimensionLiveTests
{
/// <summary>
/// The exact line the catalog sweep emits when the #1784 coverage gate holds a tiered drop, pinned in FULL
/// for the same reason its dimension-GC sibling is: it is a field signature the client's operator reads,
/// and its TAIL is the actionable half — "resumes by itself once a backfill extends coverage" is what tells
/// them this is self-correcting rather than a fault to chase. A prefix pin covers only the part that names
/// the table, which is exactly the shape that let a wrong line ship earlier in this work.
/// </summary>
private const string CoverageSkipSignature =
"Retention purge SKIPPED for query_stats: its rollup does not yet cover the oldest rows, so dropping would delete history no aggregate holds. Resumes by itself once a backfill extends coverage.";

/// <summary>
/// The exact line the dimension GC emits when it stands down, pinned in FULL rather than by prefix: it is
/// a field signature an operator greps for, and a partial pin would let the wording drift to name a cause
Expand Down Expand Up @@ -921,7 +931,7 @@ await TimescaleSupport.IsRawTierDropSafeAsync(connection, "query_stats", ct),
/* THE property: uncovered history survives the sweep. Asserted before the log line so a mutation
lands on the data loss, not on missing text. */
Assert.Equal(1L, await AncientRowCountAsync(connection, serverId, ct));
Assert.Contains("Retention purge SKIPPED for query_stats", skipLog.Joined, StringComparison.Ordinal);
Assert.Contains(CoverageSkipSignature, skipLog.Joined, StringComparison.Ordinal);

/* Now let coverage reach back over the ancient row. force => recompute the older buckets. */
using (var refresh = new NpgsqlCommand(
Expand All @@ -939,7 +949,7 @@ await TimescaleSupport.IsRawTierDropSafeAsync(connection, "query_stats", ct),
await DarlingRetention.PurgeAsync(postgres, timescaleAvailable: true, dropLog, ct, OnlyPurge("query_stats", 40));

Assert.Equal(0L, await AncientRowCountAsync(connection, serverId, ct));
Assert.DoesNotContain("Retention purge SKIPPED for query_stats", dropLog.Joined, StringComparison.Ordinal);
Assert.DoesNotContain(CoverageSkipSignature, dropLog.Joined, StringComparison.Ordinal);
}
finally
{
Expand Down
81 changes: 81 additions & 0 deletions Darling/Darling.Tests/PayloadDimensionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,87 @@ not a three-quote opener. Checking `verbatim` first is what keeps those apart. *
return literals;
}


/// <summary>
/// The raw-tier retention policies must be DERIVED from <see cref="TimescaleSupport.RawTierCoverage"/>,
/// not written out again beside it (#1784).
///
/// <para>That map is the single source of truth for which tables are coverage-gated, and it is what makes
/// the tiered POLICY and the catalog SWEEP unable to judge the same drop differently. Today that property
/// is only structural: someone re-hardcoding the three rows in the policy list gets an identical policy set
/// and stays green — right up until the map gains a fourth tier, or one of the three changes its covering
/// aggregate, and the two paths silently disagree about the very thing the map exists to keep aligned.
/// A behavioural test cannot see that, because a duplicated list produces the same policies; only reading
/// the construction can.</para>
///
/// <para>So this asserts the method BUILDS from the map and names none of the gated relations as literals.
/// It is the "enforce it, don't document it" shape: the failure it prevents is a green test suite over a
/// broken invariant.</para>
/// </summary>
[Fact]
public void RawTierRetentionPolicies_AreDerivedFromTheCoverageMap_NotRehardcodedBesideIt()
{
var root = FindRepoRoot();
Assert.True(root is not null, RepoRootNotFound);

var source = File.ReadAllText(Path.Combine(
root!, "Darling", "PerformanceMonitor.Darling.Storage", "TimescaleSupport.cs"));

var body = MethodBody(source, "public static async Task<int> EnsureRetentionPoliciesAsync");
Assert.False(string.IsNullOrEmpty(body),
"could not locate EnsureRetentionPoliciesAsync — the guard cannot silently pass on a parse miss");

Assert.Contains(nameof(TimescaleSupport.RawTierCoverage), body, StringComparison.Ordinal);

foreach (var (relation, _, _) in TimescaleSupport.RawTierCoverage)
{
Assert.False(
body.Contains('"' + relation + '"', StringComparison.Ordinal),
$"EnsureRetentionPoliciesAsync names \"{relation}\" as a literal. The coverage-gated tiers must " +
"come from RawTierCoverage so the retention policy and the catalog sweep (#1784) cannot drift " +
"apart about which tables are gated — re-hardcoding them passes every behavioural test until " +
"the two lists disagree.");
}
}

/// <summary>
/// The body of a method, by brace matching from its signature. Returns empty when the signature is not
/// found, so a caller can FAIL rather than silently pass on a parse miss.
/// </summary>
private static string MethodBody(string source, string signature)
{
var start = source.IndexOf(signature, StringComparison.Ordinal);
if (start < 0)
{
return string.Empty;
}

var open = source.IndexOf('{', start);
if (open < 0)
{
return string.Empty;
}

var depth = 0;
for (var i = open; i < source.Length; i++)
{
if (source[i] == '{')
{
depth++;
}
else if (source[i] == '}')
{
depth--;
if (depth == 0)
{
return source.Substring(open, i - open + 1);
}
}
}

return string.Empty;
}

/// <summary>
/// Walks up from the test output directory to the repo root — the directory holding
/// <c>PerformanceMonitor.sln</c>. Same walk-up idiom as <c>DocCommentHygieneTests.FindRepoRoot</c>.
Expand Down
Loading