fix(controlplane): retention keep window counts Sealed rows; the newest Sealed blob is never pruned

The window used to count every terminal row, so 50+ failed deploys pushed the last good artifact out of it. Now the newest KeepSealed (>= 1) Sealed rows keep their blob, failed rows are prunable unless referenced in the ReferencedDays window. Pinned: 60 TimedOut + 1 old Sealed -> Sealed retained, 60 TimedOut emptied. Refs #733 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 13:39:34 -04:00
parent 88268ea3a6
commit f1c52385ed
3 changed files with 83 additions and 10 deletions
@@ -97,8 +97,9 @@ public sealed class DeploymentRetentionActor : ReceiveActor, IWithTimers
    }

    /// <summary>
-    ///     The retention rule. Empties the blob of every terminal deployment that is (a) older than the
-    ///     newest <see cref="DeploymentRetentionOptions.KeepSealed"/> terminal rows, (b) not referenced by a
+    ///     The retention rule. Empties the blob of every terminal deployment that is (a) not one of the
+    ///     newest <see cref="DeploymentRetentionOptions.KeepSealed"/> SEALED rows (at least one — the newest
+    ///     Sealed blob is never pruned), (b) not referenced by a
    ///     <c>NodeDeploymentState</c> row started or applied within
    ///     <see cref="DeploymentRetentionOptions.ReferencedDays"/>, and (c) still carrying bytes — at most
    ///     <see cref="DeploymentRetentionOptions.MaxRowsPerSweep"/> per call.
@@ -125,11 +126,23 @@ public sealed class DeploymentRetentionActor : ReceiveActor, IWithTimers
                        || d.Status == DeploymentStatus.PartiallyFailed
                        || d.Status == DeploymentStatus.TimedOut)
            .OrderByDescending(d => d.CreatedAtUtc)
-            .Select(d => new { d.DeploymentId, HasBlob = d.ArtifactBlob.Length > 0 })
+            .Select(d => new { d.DeploymentId, d.Status, HasBlob = d.ArtifactBlob.Length > 0 })
            .ToListAsync(ct)
            .ConfigureAwait(false);

-        var aged = terminal.Skip(keep).Where(d => d.HasBlob).Select(d => d.DeploymentId).ToList();
+        // The keep window counts SEALED rows only (fix round 1 / I3). Counting every terminal row let a burst
+        // of failed deploys (TimedOut / PartiallyFailed) fill the window by itself and push the fleet's last
+        // good artifact — the newest Sealed row, which boot, drift detection and the publish fallback all
+        // read — out of it. `keep` is clamped to ≥ 1, so the newest Sealed row is never a candidate.
+        var keptSealed = terminal
+            .Where(d => d.Status == DeploymentStatus.Sealed)
+            .Take(keep)
+            .Select(d => d.DeploymentId)
+            .ToHashSet();
+        var aged = terminal
+            .Where(d => d.HasBlob && !keptSealed.Contains(d.DeploymentId))
+            .Select(d => d.DeploymentId)
+            .ToList();
        if (aged.Count == 0) return 0;

        var referenced = (await db.NodeDeploymentStates
@@ -17,9 +17,10 @@ public sealed class DeploymentRetentionOptions
    public bool Enabled { get; set; } = true;

    /// <summary>
-    ///     How many of the most recent terminal (Sealed / PartiallyFailed / TimedOut) deployments keep their
-    ///     blob regardless of age. A node booting from central SQL reads the LATEST sealed row's blob, so this
-    ///     must stay well above 1; 50 also keeps a comfortable re-deploy history.
+    ///     How many of the most recent SEALED deployments keep their blob regardless of age (clamped to ≥ 1).
+    ///     Failed deploys (PartiallyFailed / TimedOut) do not count toward it, so a burst of them can never push
+    ///     the last good artifact out. A node booting from central SQL reads the LATEST sealed row's blob, so
+    ///     this must stay well above 1; 50 also keeps a comfortable re-deploy history.
    /// </summary>
    public int KeepSealed { get; set; } = 50;

@@ -24,7 +24,7 @@ public sealed class DeploymentRetentionTests : ControlPlaneActorTestBase
    public async Task Sweep_empties_blobs_beyond_the_keep_window_and_leaves_the_newest_intact()
    {
        var f = NewInMemoryDbFactory();
-        var ids = Seed(f, sealedCount: 12);   // ids[0] newest … ids[11] oldest
+        var ids = Seed(f, sealedCount: 12, mixInTimedOut: false);   // ids[0] newest … ids[11] oldest
        var options = new DeploymentRetentionOptions { KeepSealed = 5, ReferencedDays = 30, MaxRowsPerSweep = 200 };

        int pruned;
@@ -91,6 +91,65 @@ public sealed class DeploymentRetentionTests : ControlPlaneActorTestBase
        (await verify.Deployments.SingleAsync(d => d.DeploymentId == inFlight)).ArtifactBlob.ShouldNotBeEmpty("in flight");
    }

+    /// <summary>
+    ///     Fix round 1 / I3 — the keep window counts SEALED rows, not "terminal" ones. A burst of 60 failed
+    ///     deploys used to fill a 50-row terminal window on its own and push the fleet's last good (Sealed)
+    ///     artifact out of it — the one every boot / drift / fallback path reads. The newest Sealed blob is
+    ///     never pruned; the unreferenced TimedOut rows are (positive control: the sweep did run).
+    /// </summary>
+    [Fact]
+    public async Task Sixty_timed_out_deploys_never_prune_the_newest_sealed_blob()
+    {
+        var f = NewInMemoryDbFactory();
+        var sealedId = Guid.NewGuid();
+        var timedOut = new List<Guid>();
+        await using (var db = f.CreateDbContext())
+        {
+            db.Deployments.Add(new Deployment
+            {
+                DeploymentId = sealedId, RevisionHash = new string('s', 64), Status = DeploymentStatus.Sealed,
+                CreatedBy = "test", CreatedAtUtc = SweepNow.AddDays(-200), SealedAtUtc = SweepNow.AddDays(-200),
+                ArtifactBlob = new byte[] { 1, 2, 3 },
+            });
+            for (var i = 0; i < 60; i++)
+            {
+                var id = Guid.NewGuid();
+                timedOut.Add(id);
+                db.Deployments.Add(new Deployment
+                {
+                    DeploymentId = id, RevisionHash = new string('t', 64), Status = DeploymentStatus.TimedOut,
+                    CreatedBy = "test", CreatedAtUtc = SweepNow.AddHours(-i), ArtifactBlob = new byte[] { 9 },
+                });
+            }
+            await db.SaveChangesAsync();
+        }
+        var options = new DeploymentRetentionOptions { KeepSealed = 50, ReferencedDays = 30, MaxRowsPerSweep = 200 };
+
+        await using (var db = f.CreateDbContext())
+            (await DeploymentRetentionActor.SweepAsync(db, options, SweepNow)).ShouldBe(60);
+
+        await using var verify = f.CreateDbContext();
+        (await verify.Deployments.SingleAsync(d => d.DeploymentId == sealedId)).ArtifactBlob
+            .ShouldNotBeEmpty("the newest Sealed artifact is never pruned");
+        (await verify.Deployments.CountAsync(d => timedOut.Contains(d.DeploymentId) && d.ArtifactBlob.Length == 0))
+            .ShouldBe(60);
+    }
+
+    /// <summary>Even with KeepSealed misconfigured to 0, the newest Sealed row keeps its blob.</summary>
+    [Fact]
+    public async Task The_newest_sealed_blob_survives_a_zero_keep_window()
+    {
+        var f = NewInMemoryDbFactory();
+        var ids = Seed(f, sealedCount: 3, mixInTimedOut: false);
+
+        await using (var db = f.CreateDbContext())
+            (await DeploymentRetentionActor.SweepAsync(db, new DeploymentRetentionOptions { KeepSealed = 0 }, SweepNow))
+                .ShouldBe(2);
+
+        await using var verify = f.CreateDbContext();
+        (await verify.Deployments.SingleAsync(d => d.DeploymentId == ids[0])).ArtifactBlob.ShouldNotBeEmpty();
+    }
+
    [Fact]
    public async Task Sweep_is_bounded_per_call_and_drains_a_backlog_over_calls()
    {
@@ -149,7 +208,7 @@ public sealed class DeploymentRetentionTests : ControlPlaneActorTestBase
    }

    /// <summary>Seeds sealed deployments, newest first; each one hour older than the previous.</summary>
-    private static List<Guid> Seed(IDbContextFactory<OtOpcUaConfigDbContext> f, int sealedCount)
+    private static List<Guid> Seed(IDbContextFactory<OtOpcUaConfigDbContext> f, int sealedCount, bool mixInTimedOut = true)
    {
        var ids = new List<Guid>();
        using var db = f.CreateDbContext();
@@ -161,7 +220,7 @@ public sealed class DeploymentRetentionTests : ControlPlaneActorTestBase
            {
                DeploymentId = id,
                RevisionHash = new string('a', 64),
-                Status = i % 3 == 2 ? DeploymentStatus.TimedOut : DeploymentStatus.Sealed,
+                Status = mixInTimedOut && i % 3 == 2 ? DeploymentStatus.TimedOut : DeploymentStatus.Sealed,
                CreatedBy = "test",
                CreatedAtUtc = SweepNow.AddHours(-i),
                SealedAtUtc = SweepNow.AddHours(-i),