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