From c0a5c73a5527f0f5832a21b20232f319ef7d5cdc Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 22 Oct 2026 22:31:28 +1100 Subject: [PATCH 1/2] Cache merge bases, work on branches in parallel, and fetch in the background [minor] A state request naming a few hundred branches ran one git merge-base per branch, one after another, on every request. Merge bases are now cached by mirror, client base and branch tip, uncached branches are worked on several at a time, and each clone and fetch writes a commit-graph so an uncached merge base stays quick on a long history. Refs a little past RefsTtl are answered from while one fetch runs in the background, capped so they are never older than AdmissionTtl. New settings: RefsServeStaleFor (30 seconds), MaxCachedMergeBases (20,000) and MaxConcurrentBranchQueries (4). --- ...itBranchStateCacheOptionsValidatorTests.cs | 58 ++++++ .../Integration/ScriptedGit.cs | 6 + .../Integration/StateFlowTests.cs | 170 ++++++++++++++++++ .../Mirrors/MirrorFetcherRealGitTests.cs | 73 ++++++++ .../Mirrors/MirrorQueriesTests.cs | 98 ++++++++++ GitBranchStateCache.Tool/appsettings.json | 3 + .../GitBranchStateCacheOptions.cs | 35 ++++ .../GitBranchStateCacheOptionsValidator.cs | 19 ++ .../Endpoints/BranchStateHandler.cs | 65 ++++++- GitBranchStateCache/Mirrors/MirrorFetcher.cs | 122 ++++++++++++- GitBranchStateCache/Mirrors/MirrorLog.cs | 6 + GitBranchStateCache/Mirrors/MirrorQueries.cs | 64 ++++++- README.md | 7 + deploy/k8s/configmap.yaml | 13 ++ 14 files changed, 729 insertions(+), 10 deletions(-) create mode 100644 GitBranchStateCache.Tests/Mirrors/MirrorQueriesTests.cs diff --git a/GitBranchStateCache.Tests/Configuration/GitBranchStateCacheOptionsValidatorTests.cs b/GitBranchStateCache.Tests/Configuration/GitBranchStateCacheOptionsValidatorTests.cs index e0b0400..efbfd33 100644 --- a/GitBranchStateCache.Tests/Configuration/GitBranchStateCacheOptionsValidatorTests.cs +++ b/GitBranchStateCache.Tests/Configuration/GitBranchStateCacheOptionsValidatorTests.cs @@ -106,6 +106,64 @@ public void Validate_RefsTtlAboveAdmissionTtl_Fails() Assert.Contains("AdmissionTtl", result.FailureMessage); } + [TestMethod] + public void Validate_RefsServeStaleForPastAdmissionTtl_Succeeds() + { + // Capped where it is used rather than refused here, so a deployment whose RefsTtl already sits + // close to AdmissionTtl keeps starting after an upgrade that introduced the default. + GitBranchStateCacheOptions options = Valid(); + options.RefsTtl = TimeSpan.FromMinutes(1); + options.AdmissionTtl = TimeSpan.FromMinutes(1); + + Assert.IsTrue(Validate(options).Succeeded); + } + + [TestMethod] + public void Validate_NegativeRefsServeStaleFor_Fails() + { + GitBranchStateCacheOptions options = Valid(); + options.RefsServeStaleFor = TimeSpan.FromSeconds(-1); + + ValidateOptionsResult result = Validate(options); + + Assert.IsTrue(result.Failed); + Assert.Contains("RefsServeStaleFor", result.FailureMessage); + } + + [TestMethod] + public void Validate_ZeroRefsServeStaleFor_Succeeds() + { + // Zero is how a deployment makes every request past RefsTtl wait on its fetch. + GitBranchStateCacheOptions options = Valid(); + options.RefsServeStaleFor = TimeSpan.Zero; + + Assert.IsTrue(Validate(options).Succeeded); + } + + [TestMethod] + public void Validate_NonPositiveMaxCachedMergeBases_Fails() + { + GitBranchStateCacheOptions options = Valid(); + options.MaxCachedMergeBases = 0; + + ValidateOptionsResult result = Validate(options); + + Assert.IsTrue(result.Failed); + Assert.Contains("MaxCachedMergeBases", result.FailureMessage); + } + + [TestMethod] + public void Validate_NonPositiveMaxConcurrentBranchQueries_Fails() + { + GitBranchStateCacheOptions options = Valid(); + options.MaxConcurrentBranchQueries = 0; + + ValidateOptionsResult result = Validate(options); + + Assert.IsTrue(result.Failed); + Assert.Contains("MaxConcurrentBranchQueries", result.FailureMessage); + } + [TestMethod] [DataRow("RefsTtl")] [DataRow("FetchTimeout")] diff --git a/GitBranchStateCache.Tests/Integration/ScriptedGit.cs b/GitBranchStateCache.Tests/Integration/ScriptedGit.cs index d6e37d2..83bea40 100644 --- a/GitBranchStateCache.Tests/Integration/ScriptedGit.cs +++ b/GitBranchStateCache.Tests/Integration/ScriptedGit.cs @@ -40,6 +40,9 @@ internal sealed class ScriptedGit(MockFileSystem fileSystem) : IGitRunner /// Gets or sets whether a diff times out rather than answering. public bool DiffTimesOut { get; set; } + /// Gets or sets whether writing a commit-graph fails. + public bool CommitGraphFails { get; set; } + /// Gets or sets work to run before a given subcommand answers, for concurrency tests. public Func? Before { get; set; } @@ -70,6 +73,9 @@ public async Task RunAsync(GitInvocation invocation, CancellationToke "clone" => Clone(arguments), "config" => Success(string.Empty), "fetch" => Success(string.Empty), + "commit-graph" => CommitGraphFails + ? new GitResult(128, string.Empty, "fatal: could not write commit-graph", TimedOut: false) + : Success(string.Empty), "for-each-ref" => Success(ForEachRef()), "cat-file" => CatFile(arguments), "merge-base" => MergeBase(arguments), diff --git a/GitBranchStateCache.Tests/Integration/StateFlowTests.cs b/GitBranchStateCache.Tests/Integration/StateFlowTests.cs index 656b932..48fe198 100644 --- a/GitBranchStateCache.Tests/Integration/StateFlowTests.cs +++ b/GitBranchStateCache.Tests/Integration/StateFlowTests.cs @@ -359,6 +359,176 @@ public async Task Post_State_AfterTheRefsTtl_FetchesAgain() Assert.AreEqual(1, fixture.Git.CountOf("fetch")); } + [TestMethod] + public async Task Post_State_WithinTheStaleWindow_AnswersWithoutWaitingForTheFetch() + { + // The first editor to ask after RefsTtl would otherwise wait on a round trip to the forge, and + // with a studio polling on a heartbeat that is a steady share of every request. + await using ServiceFixture fixture = await ServiceFixture.StartAsync(); + Seed(fixture.Git); + + using HttpResponseMessage first = await fixture.Client.SendAsync(StateRequest(Body())); + DateTimeOffset cloned = fixture.Time.GetUtcNow(); + + TaskCompletionSource release = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource fetched = new(TaskCreationOptions.RunContinuationsAsynchronously); + fixture.Git.Before = async invocation => + { + if (invocation.Arguments[0] == "fetch") + { + await release.Task.ConfigureAwait(false); + fetched.TrySetResult(); + } + }; + + fixture.Time.Advance(TimeSpan.FromSeconds(45)); + + // A deadline, so a request that waits on the held fetch fails here rather than hanging the run. + using CancellationTokenSource deadline = new(TimeSpan.FromSeconds(10)); + using HttpResponseMessage second = await fixture.Client.SendAsync(StateRequest(Body()), deadline.Token); + + Assert.AreEqual(HttpStatusCode.OK, second.StatusCode); + Assert.AreEqual(cloned, (await ReadAsync(second)).GetProperty("refsAsOf").GetDateTimeOffset()); + + // A third request while that fetch is still running neither waits on it nor starts another. + using HttpResponseMessage third = await fixture.Client.SendAsync(StateRequest(Body()), deadline.Token); + Assert.AreEqual(HttpStatusCode.OK, third.StatusCode); + + release.SetResult(); + await fetched.Task.WaitAsync(TimeSpan.FromSeconds(10)); + Assert.AreEqual(1, fixture.Git.CountOf("fetch")); + } + + [TestMethod] + public async Task Post_State_ServingStaleRefsTurnedOff_WaitsForTheFetch() + { + await using ServiceFixture fixture = await ServiceFixture.StartAsync(new Dictionary + { + ["GitBranchStateCache:RefsServeStaleFor"] = "00:00:00", + }); + Seed(fixture.Git); + + using HttpResponseMessage first = await fixture.Client.SendAsync(StateRequest(Body())); + fixture.Time.Advance(TimeSpan.FromSeconds(45)); + using HttpResponseMessage second = await fixture.Client.SendAsync(StateRequest(Body())); + + Assert.AreEqual(1, fixture.Git.CountOf("fetch")); + Assert.AreEqual(fixture.Time.GetUtcNow(), (await ReadAsync(second)).GetProperty("refsAsOf").GetDateTimeOffset()); + } + + [TestMethod] + public async Task Post_State_RepeatedRequests_FindEachMergeBaseOnce() + { + // Every heartbeat asks for the same merge bases until someone pulls or a branch moves. Asking git + // again for each one is a process per branch per request, which is most of a warm request's cost. + await using ServiceFixture fixture = await ServiceFixture.StartAsync(); + Seed(fixture.Git); + + using HttpResponseMessage first = await fixture.Client.SendAsync(StateRequest(Body(patterns: "\"origin/*\""))); + using HttpResponseMessage second = await fixture.Client.SendAsync(StateRequest(Body(patterns: "\"origin/*\""))); + + Assert.AreEqual(HttpStatusCode.OK, second.StatusCode); + Assert.AreEqual(2, fixture.Git.CountOf("merge-base")); + } + + [TestMethod] + public async Task Post_State_WorksOnSeveralBranchesAtOnce() + { + await using ServiceFixture fixture = await ServiceFixture.StartAsync(); + Seed(fixture.Git); + + int running = 0; + int peak = 0; + TaskCompletionSource bothStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); + fixture.Git.Before = async invocation => + { + if (invocation.Arguments[0] != "merge-base") + { + return; + } + + int now = Interlocked.Increment(ref running); + InterlockedMax(ref peak, now); + + if (now >= 2) + { + bothStarted.TrySetResult(); + } + + // Run one after the other, each would wait out the whole delay alone. + await Task.WhenAny(bothStarted.Task, Task.Delay(TimeSpan.FromSeconds(2))).ConfigureAwait(false); + Interlocked.Decrement(ref running); + }; + + using HttpResponseMessage response = await fixture.Client.SendAsync(StateRequest(Body(patterns: "\"origin/*\""))); + + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + Assert.AreEqual(2, Volatile.Read(ref peak)); + } + + [TestMethod] + public async Task Post_State_BranchesFinishingOutOfOrder_AreReportedInTheOrderTheyMatched() + { + await using ServiceFixture sequential = await ServiceFixture.StartAsync(new Dictionary + { + ["GitBranchStateCache:MaxConcurrentBranchQueries"] = "1", + }); + Seed(sequential.Git); + using HttpResponseMessage expected = await sequential.Client.SendAsync(StateRequest(Body(patterns: "\"origin/*\""))); + string[] expectedOrder = BranchNames(await ReadAsync(expected)); + + await using ServiceFixture fixture = await ServiceFixture.StartAsync(); + Seed(fixture.Git); + + // Hold back whichever branch matched first, so it finishes last. + string firstTip = fixture.Git.Branches[expectedOrder[0]["origin/".Length..]]; + fixture.Git.Before = async invocation => + { + if (invocation.Arguments[0] == "merge-base" && invocation.Arguments[^1] == firstTip) + { + await Task.Delay(200).ConfigureAwait(false); + } + }; + + using HttpResponseMessage response = await fixture.Client.SendAsync(StateRequest(Body(patterns: "\"origin/*\""))); + + CollectionAssert.AreEqual(expectedOrder, BranchNames(await ReadAsync(response))); + } + + [TestMethod] + public async Task Post_State_ACommitGraphThatCannotBeWritten_StillAnswers() + { + // The graph only makes merge bases faster. A mirror without one answers the same questions. + await using ServiceFixture fixture = await ServiceFixture.StartAsync(); + Seed(fixture.Git); + fixture.Git.CommitGraphFails = true; + + using HttpResponseMessage response = await fixture.Client.SendAsync(StateRequest(Body())); + + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + Assert.AreEqual(1, fixture.Git.CountOf("commit-graph")); + } + + private static string[] BranchNames(JsonElement body) => + [.. body.GetProperty("branches").EnumerateArray().Select(branch => branch.GetProperty("name").GetString()!)]; + + private static void InterlockedMax(ref int target, int value) + { + int current = Volatile.Read(ref target); + + while (value > current) + { + int seen = Interlocked.CompareExchange(ref target, value, current); + + if (seen == current) + { + return; + } + + current = seen; + } + } + [TestMethod] public async Task Post_State_ReportsHowOldTheRefsAre() { diff --git a/GitBranchStateCache.Tests/Mirrors/MirrorFetcherRealGitTests.cs b/GitBranchStateCache.Tests/Mirrors/MirrorFetcherRealGitTests.cs index be03f18..9cdf1f2 100644 --- a/GitBranchStateCache.Tests/Mirrors/MirrorFetcherRealGitTests.cs +++ b/GitBranchStateCache.Tests/Mirrors/MirrorFetcherRealGitTests.cs @@ -200,6 +200,79 @@ public async Task EnsureCurrentAsync_AfterTheRefsTtl_FetchesWhatTheUpstreamHasSi Assert.AreEqual(moved, main.Tip); } + [TestMethod] + public async Task EnsureCurrentAsync_FirstRequest_WritesACommitGraph() + { + // Without one, every merge base is a commit walk that parses each commit out of the packs, and a + // state request asks for one per branch. + (MirrorFetcher fetcher, _, _, string directory) = Build(); + + await EnsureAsync(fetcher, directory); + + Assert.IsTrue(HasCommitGraph(directory)); + } + + [TestMethod] + public async Task EnsureCurrentAsync_AfterAFetch_TheCommitGraphCoversTheNewCommits() + { + (MirrorFetcher fetcher, _, FakeTimeProvider time, string directory) = Build(); + + await EnsureAsync(fetcher, directory); + string before = CommitGraphFingerprint(directory); + Commit("Content/Maps/Foo.umap", "two", "second"); + + time.Advance(TimeSpan.FromMinutes(1)); + await EnsureAsync(fetcher, directory); + + // A graph that skipped the fetch would be byte for byte the one the clone wrote. + Assert.AreNotEqual(before, CommitGraphFingerprint(directory)); + Git(directory, "commit-graph", "verify"); + } + + /// Names every commit-graph file with its length, so a rewritten graph reads differently. + private static string CommitGraphFingerprint(string directory) + { + string info = Path.Combine(directory, "objects", "info"); + + return string.Join( + '\n', + Directory.EnumerateFiles(info, "*graph*", SearchOption.AllDirectories) + .Order(StringComparer.Ordinal) + .Select(file => $"{Path.GetRelativePath(info, file)} {new FileInfo(file).Length}")); + } + + [TestMethod] + public async Task EnsureCurrentAsync_WithinTheStaleWindow_AnswersFromDiskAndFetchesInTheBackground() + { + (MirrorFetcher fetcher, MirrorStore store, FakeTimeProvider time, string directory) = Build(); + + await EnsureAsync(fetcher, directory); + DateTimeOffset cloned = time.GetUtcNow(); + Commit("Content/Maps/Foo.umap", "two", "second"); + string moved = Git(_source, "rev-parse", "HEAD").Trim(); + + time.Advance(TimeSpan.FromSeconds(45)); + MirrorFetchResult answered = await EnsureAsync(fetcher, directory); + + // Answered with the refs on disk, and says how old they are. + Assert.AreEqual(MirrorFetchStatus.Current, answered.Status); + Assert.AreEqual(cloned, answered.RefsAsOf); + + // The fetch it started lands without anything else asking. + using CancellationTokenSource deadline = new(TimeSpan.FromSeconds(30)); + while (store.RefsFetchedAt(directory) == cloned) + { + await Task.Delay(50, deadline.Token); + } + + BranchRef main = (await BranchesAsync(directory)).Single(branch => branch.Name == "origin/main"); + Assert.AreEqual(moved, main.Tip); + } + + private static bool HasCommitGraph(string directory) => + File.Exists(Path.Combine(directory, "objects", "info", "commit-graph")) + || Directory.Exists(Path.Combine(directory, "objects", "info", "commit-graphs")); + [TestMethod] public async Task EnsureCurrentAsync_ABranchDeletedUpstream_IsPrunedRatherThanKept() { diff --git a/GitBranchStateCache.Tests/Mirrors/MirrorQueriesTests.cs b/GitBranchStateCache.Tests/Mirrors/MirrorQueriesTests.cs new file mode 100644 index 0000000..4175072 --- /dev/null +++ b/GitBranchStateCache.Tests/Mirrors/MirrorQueriesTests.cs @@ -0,0 +1,98 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitBranchStateCache.Tests.Mirrors; + +using ktsu.GitBranchStateCache.Configuration; +using ktsu.GitBranchStateCache.Git; +using ktsu.GitBranchStateCache.Mirrors; +using ktsu.GitBranchStateCache.Tests.Fakes; +using Microsoft.Extensions.Options; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +public class MirrorQueriesTests +{ + private const string Mirror = "/mirrors/github/studio/game"; + private const string Base = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + private const string Tip = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + private const string OtherTip = "cccccccccccccccccccccccccccccccccccccccc"; + private const string ForkPoint = "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"; + + private static FakeGitRunner Answering(string mergeBase) => new() + { + Respond = _ => new GitResult(0, $"{mergeBase}\n", string.Empty, TimedOut: false), + }; + + private static MirrorQueries Build(IGitRunner runner, int maxCachedMergeBases = 100) => + new(runner, Options.Create(new GitBranchStateCacheOptions { MaxCachedMergeBases = maxCachedMergeBases })); + + [TestMethod] + public async Task FindMergeBaseAsync_TheSameTwoCommits_RunsGitOnce() + { + // Every heartbeat from every editor asks for the same merge bases until someone pulls or a + // branch moves. Both ids are immutable, so the second answer is the first one. + FakeGitRunner runner = Answering(ForkPoint); + MirrorQueries queries = Build(runner); + + await queries.FindMergeBaseAsync(Mirror, Base, Tip, CancellationToken.None); + string? second = await queries.FindMergeBaseAsync(Mirror, Base, Tip, CancellationToken.None); + + Assert.AreEqual(ForkPoint, second); + Assert.AreEqual(1, runner.CountOf("merge-base")); + } + + [TestMethod] + public async Task FindMergeBaseAsync_ADifferentTip_RunsGitAgain() + { + FakeGitRunner runner = Answering(ForkPoint); + MirrorQueries queries = Build(runner); + + await queries.FindMergeBaseAsync(Mirror, Base, Tip, CancellationToken.None); + await queries.FindMergeBaseAsync(Mirror, Base, OtherTip, CancellationToken.None); + + Assert.AreEqual(2, runner.CountOf("merge-base")); + } + + [TestMethod] + public async Task FindMergeBaseAsync_ADifferentMirror_RunsGitAgain() + { + FakeGitRunner runner = Answering(ForkPoint); + MirrorQueries queries = Build(runner); + + await queries.FindMergeBaseAsync(Mirror, Base, Tip, CancellationToken.None); + await queries.FindMergeBaseAsync("/mirrors/github/studio/other", Base, Tip, CancellationToken.None); + + Assert.AreEqual(2, runner.CountOf("merge-base")); + } + + [TestMethod] + public async Task FindMergeBaseAsync_AFailure_IsNotRemembered() + { + // A timeout says nothing about the commits, so the next request has to try again rather than + // be told there is no merge base. + FakeGitRunner runner = new() + { + Respond = _ => new GitResult(-1, string.Empty, "timed out", TimedOut: true), + }; + MirrorQueries queries = Build(runner); + + string? first = await queries.FindMergeBaseAsync(Mirror, Base, Tip, CancellationToken.None); + await queries.FindMergeBaseAsync(Mirror, Base, Tip, CancellationToken.None); + + Assert.IsNull(first); + Assert.AreEqual(2, runner.CountOf("merge-base")); + } + + [TestMethod] + public async Task FindMergeBaseAsync_OverTheLimit_ForgetsTheLeastRecentlyUsed() + { + FakeGitRunner runner = Answering(ForkPoint); + MirrorQueries queries = Build(runner, maxCachedMergeBases: 1); + + await queries.FindMergeBaseAsync(Mirror, Base, Tip, CancellationToken.None); + await queries.FindMergeBaseAsync(Mirror, Base, OtherTip, CancellationToken.None); + await queries.FindMergeBaseAsync(Mirror, Base, Tip, CancellationToken.None); + + Assert.AreEqual(3, runner.CountOf("merge-base")); + } +} diff --git a/GitBranchStateCache.Tool/appsettings.json b/GitBranchStateCache.Tool/appsettings.json index b82c06b..52703b2 100644 --- a/GitBranchStateCache.Tool/appsettings.json +++ b/GitBranchStateCache.Tool/appsettings.json @@ -7,11 +7,14 @@ }, "GitBranchStateCache": { "RefsTtl": "00:00:30", + "RefsServeStaleFor": "00:00:30", "AdmissionTtl": "00:01:00", "FetchTimeout": "00:02:00", "DiffTimeout": "00:02:00", "ProbeTimeout": "00:00:30", "MaxCachedDiffs": 2000, + "MaxCachedMergeBases": 20000, + "MaxConcurrentBranchQueries": 4, "MaxPathsPerRequest": 20000, "MirrorIdleMaxAge": "30.00:00:00", "MaintenanceInterval": "01:00:00" diff --git a/GitBranchStateCache/Configuration/GitBranchStateCacheOptions.cs b/GitBranchStateCache/Configuration/GitBranchStateCacheOptions.cs index 403c5e7..d591166 100644 --- a/GitBranchStateCache/Configuration/GitBranchStateCacheOptions.cs +++ b/GitBranchStateCache/Configuration/GitBranchStateCacheOptions.cs @@ -48,6 +48,22 @@ public sealed class GitBranchStateCacheOptions /// public TimeSpan RefsTtl { get; set; } = TimeSpan.FromSeconds(30); + /// + /// Gets or sets how much longer than a request may be answered from the refs + /// on disk while a fetch runs in the background. + /// + /// + /// Without this, the first request after waits on a round trip to the forge, + /// and with editors polling on a heartbeat that is a steady fraction of every request. Answered + /// refs are never older than plus this, which the validator holds within + /// . Refs older than that are fetched before answering, as before. + /// + /// Set to to make every request past wait on its + /// fetch. + /// + /// + public TimeSpan RefsServeStaleFor { get; set; } = TimeSpan.FromSeconds(30); + /// /// Gets or sets how long an upstream authorization is trusted before it is proven again. /// @@ -83,6 +99,25 @@ public sealed class GitBranchStateCacheOptions /// public int MaxCachedDiffs { get; set; } = 2000; + /// + /// Gets or sets how many merge bases are retained before the least recently used is evicted. + /// + /// + /// One entry per client base and branch tip, so the working set is roughly the number of editors + /// times the number of branches they ask about. Each entry is a few hundred bytes. + /// + public int MaxCachedMergeBases { get; set; } = 20_000; + + /// + /// Gets or sets how many branches one request works on at once when their merge base or diff is + /// not cached. + /// + /// + /// Each is a git process, so this bounds how many one request starts at a time. A warm request + /// starts none. + /// + public int MaxConcurrentBranchQueries { get; set; } = 4; + /// /// Gets or sets the most paths one request may name, and the most one response may return. /// diff --git a/GitBranchStateCache/Configuration/GitBranchStateCacheOptionsValidator.cs b/GitBranchStateCache/Configuration/GitBranchStateCacheOptionsValidator.cs index c5a52d7..f304039 100644 --- a/GitBranchStateCache/Configuration/GitBranchStateCacheOptionsValidator.cs +++ b/GitBranchStateCache/Configuration/GitBranchStateCacheOptionsValidator.cs @@ -128,6 +128,15 @@ private static void ValidateTimings(GitBranchStateCacheOptions options, List BuildStateAsync( bool partial = false; bool truncated = false; - foreach (BranchRef branch in branches) + BranchWork[] work = await WorkOnAllAsync(repository, request.Base!, branches, cancellationToken) + .ConfigureAwait(false); + + for (int index = 0; index < branches.Count; index++) { - string? mergeBase = await queries - .FindMergeBaseAsync(repository.Directory, request.Base!, branch.Tip, cancellationToken) - .ConfigureAwait(false); + BranchRef branch = branches[index]; + string? mergeBase = work[index].MergeBase; if (mergeBase is null) { @@ -244,8 +246,7 @@ private async Task BuildStateAsync( continue; } - DiffOutcome diff = await ComputeAsync(repository, mergeBase, branch, cancellationToken) - .ConfigureAwait(false); + DiffOutcome diff = work[index].Diff!; if (!diff.Succeeded) { @@ -280,6 +281,58 @@ private async Task BuildStateAsync( BranchStateJsonContext.Default.StateResponse); } + /// + /// Finds every branch's merge base and diff, several at a time. + /// + /// + /// Each branch's work is independent of every other's, and on a cold cache each step is a git + /// process, so they run side by side. The results are indexed like the branches, so the answer is + /// assembled in the order the branches matched however the work happened to finish. + /// + private async Task WorkOnAllAsync( + ResolvedRepository repository, + string clientBase, + IReadOnlyList branches, + CancellationToken cancellationToken) + { + BranchWork[] work = new BranchWork[branches.Count]; + + await Parallel.ForEachAsync( + Enumerable.Range(0, branches.Count), + new ParallelOptions + { + MaxDegreeOfParallelism = options.Value.MaxConcurrentBranchQueries, + CancellationToken = cancellationToken, + }, + async (index, token) => work[index] = await WorkOnAsync(repository, clientBase, branches[index], token) + .ConfigureAwait(false)).ConfigureAwait(false); + + return work; + } + + /// + /// Finds one branch's merge base with the client's base, and its diff from there. + /// + private async Task WorkOnAsync( + ResolvedRepository repository, + string clientBase, + BranchRef branch, + CancellationToken cancellationToken) + { + string? mergeBase = await queries + .FindMergeBaseAsync(repository.Directory, clientBase, branch.Tip, cancellationToken) + .ConfigureAwait(false); + + return mergeBase is null + ? new BranchWork(null, null) + : new BranchWork(mergeBase, await ComputeAsync(repository, mergeBase, branch, cancellationToken).ConfigureAwait(false)); + } + + /// + /// One branch's merge base, and its diff when there is a merge base to diff from. + /// + private readonly record struct BranchWork(string? MergeBase, DiffOutcome? Diff); + /// /// Computes one branch's diff. /// diff --git a/GitBranchStateCache/Mirrors/MirrorFetcher.cs b/GitBranchStateCache/Mirrors/MirrorFetcher.cs index 59babe6..6fad77e 100644 --- a/GitBranchStateCache/Mirrors/MirrorFetcher.cs +++ b/GitBranchStateCache/Mirrors/MirrorFetcher.cs @@ -28,6 +28,17 @@ namespace ktsu.GitBranchStateCache.Mirrors; /// still bounds it, because enforces the invocation's timeout itself. /// /// +/// Refs a little past are answered from as they are +/// while one fetch runs in the background, for up to +/// and never past +/// . Older refs are fetched before answering. +/// +/// +/// Every clone and fetch is followed by a commit-graph write. A state request finds one merge base per +/// branch, and without a graph each is a walk that parses every commit it passes out of the packs. +/// Failing to write one costs speed, never an answer, so it is not treated as a failure. +/// +/// /// A clone lands in a temporary directory and is moved into place only once it has finished. A crash /// part way through a clone of a large repository would otherwise leave a directory that looks like a /// mirror, and every later request would be answered from a repository missing most of its history. @@ -69,10 +80,20 @@ public async Task EnsureCurrentAsync( bool exists = mirrors.Exists(directory); DateTimeOffset? fetchedAt = exists ? mirrors.RefsFetchedAt(directory) : null; - if (exists && fetchedAt is DateTimeOffset current - && timeProvider.GetUtcNow() - current < settings.RefsTtl) + if (exists && fetchedAt is DateTimeOffset current) { - return MirrorFetchResult.Current(current); + TimeSpan age = timeProvider.GetUtcNow() - current; + + if (age < settings.RefsTtl) + { + return MirrorFetchResult.Current(current); + } + + if (age < settings.RefsTtl + StaleWindow(settings)) + { + RefreshInBackground(key, directory, repositoryUrl, upstreamBase, authorization, fetchedAt); + return MirrorFetchResult.Current(current); + } } IWorkTicket ticket = flights.Acquire(key.ToFlightKey()); @@ -90,6 +111,72 @@ public async Task EnsureCurrentAsync( return await work.WaitAsync(cancellationToken).ConfigureAwait(false); } + /// + /// How long past refs may be answered from while a + /// fetch runs in the background. + /// + /// + /// Capped so the refs answered from are never older than + /// , the same rule the validator holds + /// to. + /// + private static TimeSpan StaleWindow(GitBranchStateCacheOptions settings) + { + TimeSpan room = settings.AdmissionTtl - settings.RefsTtl; + TimeSpan window = settings.RefsServeStaleFor < room ? settings.RefsServeStaleFor : room; + return window > TimeSpan.Zero ? window : TimeSpan.Zero; + } + + /// + /// Starts a fetch nobody waits on, unless one is already running for this repository. + /// + /// + /// Runs under the requesting caller's credential, which the admission gate has just proven, exactly + /// as a fetch the request waited on would. A request arriving while it runs is answered from disk + /// rather than queued behind it. + /// + [System.Diagnostics.CodeAnalysis.SuppressMessage( + "Reliability", + "CA2025:Ensure tasks using 'IDisposable' instances complete before the instances are disposed", + Justification = "The ticket is handed to LeadAsync, which owns it and disposes it when the fetch finishes. Nothing here disposes it.")] + private void RefreshInBackground( + MirrorKey key, + string directory, + Uri repositoryUrl, + Uri upstreamBase, + string? authorization, + DateTimeOffset? fetchedAt) + { + IWorkTicket ticket = flights.Acquire(key.ToFlightKey()); + + if (!ticket.IsLeader) + { + ticket.Dispose(); + return; + } + + _ = ObserveAsync(LeadAsync(key, directory, repositoryUrl, upstreamBase, authorization, exists: true, fetchedAt, ticket)); + } + + /// + /// Waits on background work so its failure is observed rather than left for the finalizer. + /// + /// + /// A fetch that fails has already recorded and logged it. What reaches here is host shutdown + /// cancelling it, which is nobody's problem. + /// + private static async Task ObserveAsync(Task work) + { + try + { + await work.ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // The host is stopping. + } + } + /// /// Does the clone or fetch on behalf of every request coalesced onto it, and reports the outcome. /// @@ -216,6 +303,7 @@ private async Task CloneAsync( return MirrorFetchResult.Unavailable($"Could not configure the mirror: {configure.Summary}"); } + await WriteCommitGraphAsync(key, staging, cancellationToken).ConfigureAwait(false); MoveIntoPlace(staging, directory); } finally @@ -261,10 +349,38 @@ private async Task FetchAsync( return MirrorFetchResult.Stale(fetchedAt, $"The fetch did not succeed: {fetch.Summary}"); } + await WriteCommitGraphAsync(key, directory, cancellationToken).ConfigureAwait(false); + mirrors.MarkFetched(directory); return MirrorFetchResult.Current(mirrors.RefsFetchedAt(directory) ?? timeProvider.GetUtcNow()); } + /// + /// Brings the mirror's commit-graph up to date with its refs. + /// + /// + /// Split, so after a fetch only the new commits are written as a layer of their own rather than the + /// whole history again. Bounded by the fetch timeout because it is part of the same work, and its + /// failure is logged and otherwise ignored, because a mirror without a graph answers the same + /// questions more slowly. + /// + private async Task WriteCommitGraphAsync(MirrorKey key, string directory, CancellationToken cancellationToken) + { + GitResult graph = await runner.RunAsync( + new GitInvocation + { + WorkingDirectory = directory, + Arguments = ["commit-graph", "write", "--reachable", "--split", "--no-progress"], + Timeout = options.Value.FetchTimeout, + }, + cancellationToken).ConfigureAwait(false); + + if (!graph.Succeeded) + { + MirrorLog.CommitGraphNotWritten(logger, key.RepositoryPath, key.Upstream, graph.Summary); + } + } + /// /// Publishes a finished clone. /// diff --git a/GitBranchStateCache/Mirrors/MirrorLog.cs b/GitBranchStateCache/Mirrors/MirrorLog.cs index f01fb47..15ff39b 100644 --- a/GitBranchStateCache/Mirrors/MirrorLog.cs +++ b/GitBranchStateCache/Mirrors/MirrorLog.cs @@ -56,4 +56,10 @@ internal static partial class MirrorLog 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); + + [LoggerMessage( + EventId = 1008, + Level = LogLevel.Warning, + Message = "Could not write a commit-graph for '{Repository}' from upstream '{Upstream}', so its merge bases are found more slowly: {Reason}")] + public static partial void CommitGraphNotWritten(ILogger logger, string repository, string upstream, string reason); } diff --git a/GitBranchStateCache/Mirrors/MirrorQueries.cs b/GitBranchStateCache/Mirrors/MirrorQueries.cs index 4e6edec..b7922ea 100644 --- a/GitBranchStateCache/Mirrors/MirrorQueries.cs +++ b/GitBranchStateCache/Mirrors/MirrorQueries.cs @@ -9,10 +9,22 @@ namespace ktsu.GitBranchStateCache.Mirrors; /// /// Answers commit questions against a mirror. /// +/// +/// Merge bases are remembered. Both ids are immutable, so an answer is never wrong and never needs +/// invalidating, and every editor's heartbeat asks for the same few hundred of them until someone +/// pulls or a branch moves. Without this, a request naming a few hundred branches starts a few hundred +/// git processes before it can answer anything from the diff cache. +/// /// Runs git. /// The configured options. public sealed class MirrorQueries(IGitRunner runner, IOptions options) : IMirrorQueries { + private readonly Dictionary>> _mergeBases = + new(StringComparer.Ordinal); + + private readonly LinkedList> _recency = new(); + private readonly Lock _gate = new(); + /// public async Task ContainsCommitAsync( string directory, @@ -50,6 +62,15 @@ public async Task ContainsCommitAsync( return null; } + // The separator cannot appear in a directory or an object id, so no two different triples can + // produce the same key. + string cacheKey = $"{directory}\n{first}\n{second}"; + + if (TryRead(cacheKey) is string cached) + { + return cached; + } + GitResult result = await runner.RunAsync( new GitInvocation { @@ -70,6 +91,47 @@ public async Task ContainsCommitAsync( } string merged = result.StandardOutput.Trim(); - return ObjectId.IsValid(merged) ? merged : null; + + if (!ObjectId.IsValid(merged)) + { + return null; + } + + Publish(cacheKey, merged); + return merged; + } + + private string? TryRead(string cacheKey) + { + lock (_gate) + { + if (!_mergeBases.TryGetValue(cacheKey, out LinkedListNode>? node)) + { + return null; + } + + _recency.Remove(node); + _recency.AddFirst(node); + return node.Value.Value; + } + } + + private void Publish(string cacheKey, string mergeBase) + { + lock (_gate) + { + if (_mergeBases.TryGetValue(cacheKey, out LinkedListNode>? existing)) + { + _recency.Remove(existing); + } + + _mergeBases[cacheKey] = _recency.AddFirst(new KeyValuePair(cacheKey, mergeBase)); + + while (_mergeBases.Count > options.Value.MaxCachedMergeBases && _recency.Last is { } oldest) + { + _recency.RemoveLast(); + _mergeBases.Remove(oldest.Value.Key); + } + } } } diff --git a/README.md b/README.md index 0a24507..b955ad3 100644 --- a/README.md +++ b/README.md @@ -135,11 +135,14 @@ Resolves wildcard patterns against the mirror's refs and returns names with tip "GitBranchStateCache": { "MirrorRoot": "/var/lib/gitbranchstatecache", "RefsTtl": "00:00:30", + "RefsServeStaleFor": "00:00:30", "AdmissionTtl": "00:01:00", "FetchTimeout": "00:02:00", "DiffTimeout": "00:02:00", "ProbeTimeout": "00:00:30", "MaxCachedDiffs": 2000, + "MaxCachedMergeBases": 20000, + "MaxConcurrentBranchQueries": 4, "MaxPathsPerRequest": 20000, "MirrorIdleMaxAge": "30.00:00:00", "Upstreams": { @@ -158,6 +161,10 @@ Resolves wildcard patterns against the mirror's refs and returns names with tip Startup validation refuses to run with no upstreams, an upstream with an empty or missing `Repositories` list, a pattern naming no literal segment, an unwritable mirror root, a `RefsTtl` above `AdmissionTtl`, or a non-positive timeout, and reports every problem at once. +Refs younger than `RefsTtl` are answered from as they are. Past it, and for up to `RefsServeStaleFor` more, a request is still answered from the refs on disk while one fetch runs in the background, so editors on a heartbeat do not take turns waiting on the forge. That window is capped where `RefsTtl` plus it would pass `AdmissionTtl`, and refs older than that are fetched before answering. Set `RefsServeStaleFor` to zero to make every request past `RefsTtl` wait on its fetch. Every response carries `refsAsOf`, so a client can always see how old an answer is. + +Merge bases are cached by mirror, client base and branch tip, up to `MaxCachedMergeBases`, and diffs by merge base and tip, up to `MaxCachedDiffs`. A request whose answers are all cached starts no git processes beyond reading the refs. One that is not works on up to `MaxConcurrentBranchQueries` branches at a time. Each clone and fetch also writes a commit-graph, which is what keeps an uncached merge base quick on a long history. + ## Deployment 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. diff --git a/deploy/k8s/configmap.yaml b/deploy/k8s/configmap.yaml index eb9170b..4e3affd 100644 --- a/deploy/k8s/configmap.yaml +++ b/deploy/k8s/configmap.yaml @@ -13,6 +13,11 @@ data: # thirty second heartbeat actually gets, and it must not exceed AdmissionTtl below, or branch state # could outlive the authorization that permits reading it. GitBranchStateCache__RefsTtl: "00:00:30" + + # How much longer refs may be answered from while one fetch runs in the background, so the first + # editor after RefsTtl does not wait on the forge. Capped where RefsTtl plus this would pass + # AdmissionTtl. Set to 0 to make every request past RefsTtl wait on its fetch. + GitBranchStateCache__RefsServeStaleFor: "00:00:30" GitBranchStateCache__AdmissionTtl: "00:01:00" # A clone of a repository this service has never seen is the slow one; an incremental fetch is not. @@ -24,6 +29,14 @@ data: # ratio means the team is spread across many integration points, and this is the number to raise. GitBranchStateCache__MaxCachedDiffs: "2000" + # One merge base per editor's base per branch. A few hundred bytes each, so the default covers a + # studio of dozens of editors asking about hundreds of branches. + GitBranchStateCache__MaxCachedMergeBases: "20000" + + # How many branches one request works on at once when their answers are not cached. Each is a git + # process, so keep this near the CPU limit. + GitBranchStateCache__MaxConcurrentBranchQueries: "4" + # Bounds one response. A long-lived branch can differ in a hundred thousand paths, so a client that # knows which assets it cares about should name them rather than omit the field. GitBranchStateCache__MaxPathsPerRequest: "20000" From 177fbef4fb5a1d907b6d85a0ea687ad0486bed48 Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 22 Oct 2026 23:17:45 +1100 Subject: [PATCH 2/2] Check the cached merge base node for null explicitly [patch] --- GitBranchStateCache/Mirrors/MirrorQueries.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GitBranchStateCache/Mirrors/MirrorQueries.cs b/GitBranchStateCache/Mirrors/MirrorQueries.cs index b7922ea..dbe5379 100644 --- a/GitBranchStateCache/Mirrors/MirrorQueries.cs +++ b/GitBranchStateCache/Mirrors/MirrorQueries.cs @@ -105,7 +105,7 @@ public async Task ContainsCommitAsync( { lock (_gate) { - if (!_mergeBases.TryGetValue(cacheKey, out LinkedListNode>? node)) + if (!_mergeBases.TryGetValue(cacheKey, out LinkedListNode>? node) || node is null) { return null; }