From bdad86b70714529cd9bed88277ac31c03bf84bd2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 05:32:15 +0000 Subject: [PATCH] Prune old runs after each build poll so stale branches stop keeping rows [patch] Every poll added the newest runs a provider reported and nothing removed them, so every branch that ever ran kept its own table row and the app data file grew without bound. After each build poll, Build.PruneRuns keeps ongoing runs, the 50 newest runs, the estimator's 20 newest successful runs, and the newest run of each branch that ran within 30 days, and drops the rest. Fixes #313 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW2iZzaJLpHHri8a4vS --- BuildMonitor.Test/RunRetentionTests.cs | 171 +++++++++++++++++++++++++ BuildMonitor/Build.cs | 63 +++++++++ BuildMonitor/BuildSync.cs | 8 ++ BuildMonitor/DurationEstimator.cs | 2 +- CLAUDE.md | 11 +- 5 files changed, 253 insertions(+), 2 deletions(-) create mode 100644 BuildMonitor.Test/RunRetentionTests.cs diff --git a/BuildMonitor.Test/RunRetentionTests.cs b/BuildMonitor.Test/RunRetentionTests.cs new file mode 100644 index 0000000..b6e9b28 --- /dev/null +++ b/BuildMonitor.Test/RunRetentionTests.cs @@ -0,0 +1,171 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.BuildMonitor.Test; + +using ktsu.Semantics.Strings; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers . Every poll adds the newest runs a provider reports and nothing +/// removed them, so every branch that ever ran kept a table row forever and the app data file grew +/// without bound (ktsu-dev/BuildMonitor#313). +/// +[TestClass] +public sealed class RunRetentionTests +{ + private static readonly DateTimeOffset Now = new(2026, 6, 1, 0, 0, 0, TimeSpan.Zero); + + private static Build NewBuild() => new() { Name = "ci".As(), Id = "1".As() }; + + private static Run AddRun(Build build, string id, string branch, DateTimeOffset started, RunStatus status = RunStatus.Success) + { + Run run = new() + { + Id = id.As(), + Name = id.As(), + Branch = branch.As(), + Status = status, + Started = started, + LastUpdated = started.AddMinutes(5), + Build = build, + }; + build.Runs[run.Id] = run; + return run; + } + + /// + /// Adds runs on main, an hour apart, the newest starting at . + /// + private static List AddMainRuns(Build build, int count, DateTimeOffset newest, RunStatus status = RunStatus.Success) => + [.. Enumerable.Range(0, count).Select(i => AddRun(build, $"main-{status}-{i}", "main", newest.AddHours(-i), status))]; + + [TestMethod] + public void MoreRunsThanTheCapAreTrimmedToTheNewest() + { + // Arrange + Build build = NewBuild(); + List runs = AddMainRuns(build, Build.RetainedNewestRuns + 30, Now.AddDays(-60)); + + // Act + int removed = build.PruneRuns(Now); + + // Assert + Assert.AreEqual(30, removed); + Assert.HasCount(Build.RetainedNewestRuns, build.Runs); + CollectionAssert.AreEquivalent( + runs.Take(Build.RetainedNewestRuns).Select(run => run.Id).ToList(), + build.Runs.Keys.ToList(), + "The newest runs should be the ones kept"); + } + + [TestMethod] + public void ABuildUnderTheCapKeepsEveryRun() + { + // Arrange + Build build = NewBuild(); + _ = AddMainRuns(build, Build.RetainedNewestRuns, Now.AddDays(-400)); + + // Act + int removed = build.PruneRuns(Now); + + // Assert + Assert.AreEqual(0, removed); + Assert.HasCount(Build.RetainedNewestRuns, build.Runs); + } + + [TestMethod] + public void AnOldBranchOutsideTheNewestRunsLosesItsRow() + { + // Arrange - a dependabot branch ran once, long ago, and has since been merged and deleted + Build build = NewBuild(); + _ = AddRun(build, "old-branch", "dependabot/nuget/Foo-1.2.3", Now - Build.BranchRetention - TimeSpan.FromDays(1)); + _ = AddMainRuns(build, Build.RetainedNewestRuns, Now); + + // Act + _ = build.PruneRuns(Now); + + // Assert + Assert.IsFalse(build.Runs.ContainsKey("old-branch".As())); + Assert.IsFalse(build.Runs.Values.Any(run => run.Branch == "dependabot/nuget/Foo-1.2.3".As()), "The branch should no longer have a row"); + } + + [TestMethod] + public void ABranchThatRanWithinTheRetentionWindowKeepsItsNewestRun() + { + // Arrange + Build build = NewBuild(); + Run olderFeatureRun = AddRun(build, "feature-old", "feature", Now.AddDays(-3)); + Run newestFeatureRun = AddRun(build, "feature-new", "feature", Now.AddDays(-2)); + _ = AddMainRuns(build, Build.RetainedNewestRuns, Now); + + // Act + _ = build.PruneRuns(Now); + + // Assert + Assert.IsTrue(build.Runs.ContainsKey(newestFeatureRun.Id), "The branch's newest run keeps its row"); + Assert.IsFalse(build.Runs.ContainsKey(olderFeatureRun.Id), "Only the branch's newest run is kept for it"); + } + + [TestMethod] + public void AnOngoingRunIsNeverDropped() + { + // Arrange - a run queued long ago that is still waiting + Build build = NewBuild(); + Run stuck = AddRun(build, "stuck", "old-branch", Now.AddDays(-90), RunStatus.Pending); + _ = AddMainRuns(build, Build.RetainedNewestRuns + 10, Now); + + // Act + _ = build.PruneRuns(Now); + + // Assert + Assert.IsTrue(build.Runs.ContainsKey(stuck.Id)); + } + + [TestMethod] + public void TheEstimatorKeepsItsFullSampleWhenRecentRunsFailed() + { + // Arrange - every recent run failed, so the successful ones the estimator reads are all older + Build build = NewBuild(); + _ = AddMainRuns(build, Build.RetainedNewestRuns, Now, RunStatus.Failure); + List successes = AddMainRuns(build, DurationEstimator.MaxSamplesToConsider + 5, Now.AddDays(-30), RunStatus.Success); + TimeSpan estimateBefore = DurationEstimator.EstimateDuration(build); + + // Act + _ = build.PruneRuns(Now); + + // Assert + Assert.AreEqual(Build.RetainedNewestRuns + DurationEstimator.MaxSamplesToConsider, build.Runs.Count); + foreach (Run success in successes.Take(DurationEstimator.MaxSamplesToConsider)) + { + Assert.IsTrue(build.Runs.ContainsKey(success.Id), $"Successful run {success.Id} is an estimator sample and should be kept"); + } + + Assert.AreEqual(estimateBefore, DurationEstimator.EstimateDuration(build)); + } + + [TestMethod] + public void RepeatedPollsOfABusyWorkflowStayBounded() + { + // Arrange - a poll adds ten new runs on fresh branches, and the build is pruned after each one + Build build = NewBuild(); + int maxSeen = 0; + + // Act - a year of daily polls + for (int day = 0; day < 365; day++) + { + DateTimeOffset pollTime = Now.AddDays(day); + for (int i = 0; i < 10; i++) + { + _ = AddRun(build, $"run-{day}-{i}", $"branch-{day}-{i}", pollTime.AddMinutes(-i)); + } + + _ = build.PruneRuns(pollTime); + maxSeen = Math.Max(maxSeen, build.Runs.Count); + } + + // Assert - at most the newest runs plus one per branch that ran within the window + int bound = Build.RetainedNewestRuns + (((int)Build.BranchRetention.TotalDays + 1) * 10); + Assert.IsLessThanOrEqualTo(bound, maxSeen, $"Runs should stay bounded, but reached {maxSeen}"); + } +} diff --git a/BuildMonitor/Build.cs b/BuildMonitor/Build.cs index bbba8c9..a91ca2e 100644 --- a/BuildMonitor/Build.cs +++ b/BuildMonitor/Build.cs @@ -64,6 +64,69 @@ internal bool HasRecentBranchFailure(DateTimeOffset since) /// The estimated duration, or TimeSpan.Zero if insufficient data. internal TimeSpan CalculateEstimatedDuration(BranchName branch) => DurationEstimator.EstimateDuration(this, branch); + /// + /// How many of a build's newest runs are always kept, whatever their branch or status. + /// + internal const int RetainedNewestRuns = 50; + + /// + /// How long a branch keeps its newest run, and so its table row, after that run started. + /// + internal static readonly TimeSpan BranchRetention = TimeSpan.FromDays(30); + + /// + /// Drops runs that nothing needs any more, so and the app data file stay bounded + /// while a busy workflow is polled for months (ktsu-dev/BuildMonitor#313). + /// + /// The current instant, which the branch retention window is measured back from. + /// The number of runs removed. + /// + /// A run is kept when any of these holds: + /// + /// it is still pending or running; + /// it is one of the build's newest runs; + /// it is one of the newest successful runs, + /// which is every sample the duration estimator reads; + /// it is the newest run on a branch that ran within . + /// + /// A branch none of whose runs is kept disappears from the table. + /// + internal int PruneRuns(DateTimeOffset now) + { + List runs = [.. Runs.Values]; + if (runs.Count <= RetainedNewestRuns) + { + return 0; + } + + HashSet kept = [.. runs.Where(run => run.IsOngoing).Select(run => run.Id)]; + kept.UnionWith(runs + .OrderByDescending(run => run.Started) + .Take(RetainedNewestRuns) + .Select(run => run.Id)); + kept.UnionWith(runs + .Where(run => run.Status == RunStatus.Success) + .OrderByDescending(run => run.Started) + .Take(DurationEstimator.MaxSamplesToConsider) + .Select(run => run.Id)); + kept.UnionWith(runs + .GroupBy(run => run.Branch) + .Select(branch => branch.MaxBy(run => run.Started)!) + .Where(newest => now - newest.Started <= BranchRetention) + .Select(newest => newest.Id)); + + int removed = 0; + foreach (Run run in runs.Where(run => !kept.Contains(run.Id))) + { + if (Runs.TryRemove(run.Id, out _)) + { + removed++; + } + } + + return removed; + } + internal TimeSpan CalculateETA() { TimeSpan estimate = CalculateEstimatedDuration(); diff --git a/BuildMonitor/BuildSync.cs b/BuildMonitor/BuildSync.cs index 3ff3052..d64757f 100644 --- a/BuildMonitor/BuildSync.cs +++ b/BuildMonitor/BuildSync.cs @@ -119,6 +119,14 @@ private async Task UpdateBuildAsync() { int runsBefore = Build.Runs.Count; await Build.Owner.BuildProvider.UpdateBuildAsync(Build).ConfigureAwait(false); + + int pruned = Build.PruneRuns(Clock.UtcNow); + if (pruned > 0) + { + Log.Debug($"BuildSync: {Build.Owner.Name}/{Build.Repository.Name}/{Build.Name} pruned {pruned} old runs"); + BuildMonitor.QueueSaveAppData(); + } + int runsAfter = Build.Runs.Count; if (runsAfter != runsBefore) diff --git a/BuildMonitor/DurationEstimator.cs b/BuildMonitor/DurationEstimator.cs index bfafc0f..4a19caa 100644 --- a/BuildMonitor/DurationEstimator.cs +++ b/BuildMonitor/DurationEstimator.cs @@ -15,7 +15,7 @@ internal static class DurationEstimator /// /// Maximum number of recent runs to consider for estimation. /// - private const int MaxSamplesToConsider = 20; + internal const int MaxSamplesToConsider = 20; /// /// Decay factor for exponential weighting (0-1). Higher values give more weight to recent runs. diff --git a/CLAUDE.md b/CLAUDE.md index 9088e3a..13d2ff2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,10 +116,19 @@ The application uses a **GetOrAdd pattern** to ensure existing entities are upda - Errors are cleared when run status changes from failure to non-failure - Triggers `Build.UpdateFromRun()` to keep parent build state in sync +**Run Retention:** +- After each build poll, `BuildSync` calls `Build.PruneRuns()`, which keeps a run when any of these holds: + - it is still pending or running + - it is one of the build's `Build.RetainedNewestRuns` (50) newest runs + - it is one of the newest `DurationEstimator.MaxSamplesToConsider` (20) successful runs, so the estimator keeps its full sample + - it is the newest run on a branch that ran within `Build.BranchRetention` (30 days) +- Every other run is removed, so a merged and deleted branch loses its table row once it falls outside both the + newest runs and the retention window, and the app data file stays bounded + **Benefits of Update Strategy:** - Repository visibility changes are reflected without manual intervention - Renamed workflows/build definitions don't create duplicate entries -- No data accumulation from stale entities +- No data accumulation from stale entities, including runs (see Run Retention) - Efficient AppData saves (only when changes occur) - Better logging distinguishes between discovery and updates