Sweep staging directories left by clones that never finished

GitBranchStateCache.Tests/Mirrors/MirrorMaintenanceServiceTests.cs

@@ -288,4 +288,115 @@
 		Assert.IsFalse(store.Exists(idle));
 		Assert.IsTrue(store.Exists(busy));
 	}
+
+	/// <summary>
+	/// Leaves a clone's staging directory beside where its mirror would go, dated as the clone started.
+	/// </summary>
+	private static string SeedStaging(MirrorStore store, MockFileSystem fileSystem, string repositoryPath, DateTimeOffset startedAt)
+	{
+		store.TryResolve(new MirrorKey("github", repositoryPath), out string? directory);
+		string staging = $"{directory}.tmp-{Guid.NewGuid():N}";
+		fileSystem.Directory.CreateDirectory(fileSystem.Path.Combine(staging, "objects"));
+		fileSystem.File.WriteAllText(fileSystem.Path.Combine(staging, "objects", "pack"), "partial clone");
+		fileSystem.Directory.SetCreationTimeUtc(staging, startedAt.UtcDateTime);
+		fileSystem.Directory.SetLastWriteTimeUtc(staging, startedAt.UtcDateTime);
+		return staging;
+	}
+
+	[TestMethod]
+	public void Sweep_AStagingDirectoryLeftByADeadClone_IsRemoved()
+	{
+		// A process killed mid-clone never runs the finally that discards its staging directory, and
+		// nothing else would ever look for it.
+		(MirrorMaintenanceService service, MirrorStore store, MockFileSystem fileSystem, FakeTimeProvider time) =
+			Build();
+
+		string staging = SeedStaging(store, fileSystem, "studio/game.git", time.GetUtcNow());
+		time.Advance(TimeSpan.FromDays(1));
+
+		service.Sweep();
+
+		Assert.IsFalse(fileSystem.Directory.Exists(staging));
+	}
+
+	[TestMethod]
+	public void Sweep_WithReapingDisabled_StillRemovesADeadClonesStagingDirectory()
+	{
+		// Keeping every mirror is a choice about mirrors. A staging directory is not one.
+		(MirrorMaintenanceService service, MirrorStore store, MockFileSystem fileSystem, FakeTimeProvider time) =
+			Build(TimeSpan.Zero);
+
+		string staging = SeedStaging(store, fileSystem, "studio/game.git", time.GetUtcNow());
+		time.Advance(TimeSpan.FromDays(1));
+
+		service.Sweep();
+
+		Assert.IsFalse(fileSystem.Directory.Exists(staging));
+	}
+
+	[TestMethod]
+	public void Sweep_AStagingDirectoryOfACloneStillRunning_IsKept()
+	{
+		// A clone may run for the whole of FetchTimeout, so its staging directory is live until then.
+		(MirrorMaintenanceService service, MirrorStore store, MockFileSystem fileSystem, FakeTimeProvider time) =
+			Build();
+
+		string staging = SeedStaging(store, fileSystem, "studio/game.git", time.GetUtcNow());
+		time.Advance(new GitBranchStateCacheOptions().FetchTimeout);
+
+		service.Sweep();
+
+		Assert.IsTrue(fileSystem.Directory.Exists(staging));
+	}
+
+	[TestMethod]
+	public void Sweep_ARepositoryPathSegmentThatLooksLikeStaging_IsKept()
+	{
+		// Only the exact name a clone gives its staging directory counts. A repository path segment
+		// that merely starts the same way holds a live mirror.
+		(MirrorMaintenanceService service, MirrorStore store, MockFileSystem fileSystem, FakeTimeProvider time) =
+			Build();
+
+		string directory = Seed(store, fileSystem, "studio/mirror.git.tmp-old/game.git");
+		store.MarkUsed(directory);
+		time.Advance(TimeSpan.FromDays(1));
+		store.MarkUsed(directory);
+
+		service.Sweep();
+
+		Assert.IsTrue(store.Exists(directory));
+	}
+
+	[TestMethod]
+	public void EnumerateStaging_FindsOnlyCloneStagingDirectories()
+	{
+		(_, MirrorStore store, MockFileSystem fileSystem, FakeTimeProvider time) = Build();
+
+		string staging = SeedStaging(store, fileSystem, "studio/game.git", time.GetUtcNow());
+		Seed(store, fileSystem, "studio/tools.git");
+		Seed(store, fileSystem, "studio/mirror.git.tmp-notaguid/game.git");
+
+		CollectionAssert.AreEqual(new[] { staging }, store.EnumerateStaging().ToArray());
+	}
+
+	[TestMethod]
+	public void Sweep_AStagingNamedDirectoryHoldingAMirror_IsKept()
+	{
+		// However unlikely the name, a directory with a mirror inside it is a repository path segment,
+		// and deleting it would take a live mirror with it.
+		(MirrorMaintenanceService service, MirrorStore store, MockFileSystem fileSystem, FakeTimeProvider time) =
+			Build();
+
+		string segment = $"mirror.git.tmp-{Guid.NewGuid():N}";
+		string directory = Seed(store, fileSystem, $"studio/{segment}/game.git");
+		string parent = fileSystem.Path.GetDirectoryName(directory)!;
+		fileSystem.Directory.SetCreationTimeUtc(parent, time.GetUtcNow().UtcDateTime);
+		fileSystem.Directory.SetLastWriteTimeUtc(parent, time.GetUtcNow().UtcDateTime);
+		time.Advance(TimeSpan.FromDays(1));
+		store.MarkUsed(directory);
+
+		service.Sweep();
+
+		Assert.IsTrue(store.Exists(directory));
+	}
 }

GitBranchStateCache/Mirrors/IMirrorStore.cs

@@ -57,6 +57,17 @@ public interface IMirrorStore
 	/// <returns>The directories.</returns>
 	public IReadOnlyList<string> Enumerate();
 
+	/// <summary>
+	/// Lists every directory a clone is staging into, or was when it died.
+	/// </summary>
+	/// <remarks>
+	/// A clone is written beside its mirror under a name only a clone uses, and moved into place when it
+	/// finishes. One whose process was killed first is never moved or removed by that clone, so it is
+	/// left for the maintenance sweep, which is the only thing that looks for it.
+	/// </remarks>
+	/// <returns>The directories.</returns>
+	public IReadOnlyList<string> EnumerateStaging();
+
 	/// <summary>Removes a mirror and everything under it.</summary>
 	/// <param name="directory">The mirror directory.</param>
 	public void Delete(string directory);

GitBranchStateCache/Mirrors/MirrorFetcher.cs

@@ -33,7 +33,7 @@
 /// <param name="options">The configured options.</param>
 /// <param name="timeProvider">Clock, injected so freshness is testable.</param>
 /// <param name="logger">Logger.</param>
 public sealed class MirrorFetcher(
 	IGitRunner runner,
 	IMirrorStore mirrors,
 	IFileSystem fileSystem,
@@ -134,7 +134,7 @@
 
 		fileSystem.Directory.CreateDirectory(parent);
 
-		string staging = fileSystem.Path.Combine(parent, $"{MirrorStore.MirrorDirectoryName}.tmp-{Guid.NewGuid():N}");
+		string staging = fileSystem.Path.Combine(parent, $"{MirrorStore.StagingPrefix}{Guid.NewGuid():N}");
 
 		metrics.RecordClone(key.Upstream);
 		MirrorLog.Cloning(logger, key.RepositoryPath, key.Upstream);

GitBranchStateCache/Mirrors/MirrorLog.cs

@@ -44,4 +44,16 @@ internal static partial class MirrorLog
 		Level = LogLevel.Warning,
 		Message = "Could not remove the idle mirror at '{Directory}'.")]
 	public static partial void ReapFailed(ILogger logger, Exception exception, string directory);
+
+	[LoggerMessage(
+		EventId = 1006,
+		Level = LogLevel.Information,
+		Message = "Removed the staging directory '{Staging}', left by a clone that did not finish.")]
+	public static partial void SweptStaging(ILogger logger, string staging);
+
+	[LoggerMessage(
+		EventId = 1007,
+		Level = LogLevel.Warning,
+		Message = "Could not remove the staging directory '{Staging}', left by a clone that did not finish.")]
+	public static partial void SweepStagingFailed(ILogger logger, Exception exception, string staging);
 }

GitBranchStateCache/Mirrors/MirrorMaintenanceService.cs

@@ -58,7 +58,16 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
 	}
 
 	/// <summary>
-	/// Measures every mirror and reaps the idle ones.
+	/// How much longer than the longest possible clone a staging directory must be left alone.
+	/// </summary>
+	/// <remarks>
+	/// Covers the time a timed-out git process takes to be killed and the clone's own bookkeeping
+	/// either side of it, so that a clone near the end of its timeout is never swept from under it.
+	/// </remarks>
+	private static readonly TimeSpan StagingMargin = TimeSpan.FromMinutes(10);
+
+	/// <summary>
+	/// Removes abandoned clone staging directories, then measures every mirror and reaps the idle ones.
 	/// </summary>
 	internal void Sweep()
 	{
@@ -68,6 +77,8 @@ internal void Sweep()
 		long bytes = 0;
 		int kept = 0;
 
+		SweepStaging(settings, directories, now);
+
 		foreach (string directory in directories)
 		{
 			// A mirror can sit inside another when a repository path runs through a mirror.git segment
@@ -92,6 +103,58 @@ internal void Sweep()
 		ReadinessLog.ReportedMirrorSize(logger, bytes, kept);
 	}
 
+	/// <summary>
+	/// Removes the staging directories of clones that can no longer be running.
+	/// </summary>
+	/// <remarks>
+	/// A clone stages into a directory beside its mirror and discards it in a <c>finally</c>, which
+	/// never runs when the process is killed partway through: an OOM kill, an eviction or a rollout
+	/// during the clone of a large repository. The partial clone, possibly gigabytes, would otherwise
+	/// stay on the volume for good, and uncounted, since it is not a mirror.
+	/// <para>
+	/// A clone runs for at most <see cref="GitBranchStateCacheOptions.FetchTimeout"/> and then
+	/// configures the result within <see cref="GitBranchStateCacheOptions.ProbeTimeout"/>, so a staging
+	/// directory older than both, plus a margin, belongs to no clone still running. Age is the later of
+	/// creation and last write, so a filesystem that cannot report creation times errs towards keeping.
+	/// This runs whether or not idle mirrors are reaped, because keeping every mirror is not a reason
+	/// to keep what is not one.
+	/// </para>
+	/// </remarks>
+	private void SweepStaging(GitBranchStateCacheOptions settings, IReadOnlyList<string> directories, DateTimeOffset now)
+	{
+		TimeSpan maxAge = settings.FetchTimeout + settings.ProbeTimeout + StagingMargin;
+
+		foreach (string staging in mirrors.EnumerateStaging())
+		{
+			// A mirror inside means this is a repository path segment that happens to share the name,
+			// not a staging directory.
+			if (directories.Any(directory => MirrorStore.IsInside(directory, staging)))
+			{
+				continue;
+			}
+
+			try
+			{
+				IDirectoryInfo info = fileSystem.DirectoryInfo.New(staging);
+				DateTime touched = info.CreationTimeUtc > info.LastWriteTimeUtc
+					? info.CreationTimeUtc
+					: info.LastWriteTimeUtc;
+
+				if (now - new DateTimeOffset(touched, TimeSpan.Zero) <= maxAge)
+				{
+					continue;
+				}
+
+				mirrors.Delete(staging);
+				MirrorLog.SweptStaging(logger, staging);
+			}
+			catch (Exception failure) when (failure is IOException or UnauthorizedAccessException)
+			{
+				MirrorLog.SweepStagingFailed(logger, failure, staging);
+			}
+		}
+	}
+
 	/// <summary>
 	/// Reports when a mirror was last useful to anyone.
 	/// </summary>

GitBranchStateCache/Mirrors/MirrorStore.cs

@@ -30,6 +30,12 @@ public sealed class MirrorStore(
 	/// <summary>The directory name every mirror repository has.</summary>
 	internal const string MirrorDirectoryName = "mirror.git";
 
+	/// <summary>What a clone's staging directory is named before the suffix that makes it unique.</summary>
+	internal const string StagingPrefix = MirrorDirectoryName + ".tmp-";
+
+	/// <summary>The length of the unique suffix: a GUID written as 32 hexadecimal digits.</summary>
+	private const int StagingSuffixLength = 32;
+
 	private const string FetchedMarker = ".refs-fetched-at";
 	private const string UsedMarker = ".last-used";
 
@@ -111,6 +117,36 @@ public IReadOnlyList<string> Enumerate()
 		return [.. candidates.Where(candidate => !IsRepositoryPathSegment(candidate, candidates))];
 	}
 
+	/// <inheritdoc />
+	/// <remarks>
+	/// Matched on the whole name a clone gives its staging directory, the prefix and a GUID's 32
+	/// hexadecimal digits, rather than on the prefix alone. A repository path segment may begin
+	/// <c>mirror.git.tmp-</c> too, and that directory holds live mirrors.
+	/// </remarks>
+	public IReadOnlyList<string> EnumerateStaging()
+	{
+		string root = options.Value.MirrorRoot;
+
+		if (!fileSystem.Directory.Exists(root))
+		{
+			return [];
+		}
+
+		return [.. fileSystem.Directory
+			.GetDirectories(root, StagingPrefix + "*", SearchOption.AllDirectories)
+			.Where(candidate => IsStagingName(fileSystem.Path.GetFileName(candidate)))];
+	}
+
+	/// <summary>
+	/// Reports whether a directory name is one a clone gives its staging directory.
+	/// </summary>
+	/// <param name="name">The directory name.</param>
+	/// <returns><see langword="true"/> when it is the staging prefix followed by 32 hexadecimal digits.</returns>
+	internal static bool IsStagingName(string name) =>
+		name.Length == StagingPrefix.Length + StagingSuffixLength
+		&& name.StartsWith(StagingPrefix, StringComparison.Ordinal)
+		&& name[StagingPrefix.Length..].All(char.IsAsciiHexDigit);
+
 	/// <summary>
 	/// Reports whether a path lies strictly inside a directory.
 	/// </summary>

README.md

@@ -162,7 +162,7 @@ Startup validation refuses to run with no upstreams, an upstream with an empty o
 
 Deploy **adjacent to the forge, not on-premises**. Its cost is round trips and its clients are worst served on residential links, which is the opposite placement from an object cache. `deploy/k8s` is a kustomize base: a StatefulSet with a read-write-once volume, a service, an ingress and a configmap. Start at one replica; each replica holds its own mirrors and diff cache, so replicas multiply fetch traffic and disk without improving the hit rate.
 
-The volume is sized by the allow-list rather than by traffic, which is what makes it possible to provision in advance. Mirrors that go unqueried for longer than `MirrorIdleMaxAge` are deleted, because a deleted mirror costs one clone if it is asked for again, which is the cheapest possible way to be wrong.
+The volume is sized by the allow-list rather than by traffic, which is what makes it possible to provision in advance. Mirrors that go unqueried for longer than `MirrorIdleMaxAge` are deleted, because a deleted mirror costs one clone if it is asked for again, which is the cheapest possible way to be wrong. The same sweep removes the partial clone a killed process leaves behind (`mirror.git.tmp-*`), once it is older than `FetchTimeout` plus `ProbeTimeout` plus ten minutes and so cannot belong to a clone still running.
 
 ## Instrumentation