Repository navigation
Evict stale and excess lock snapshots [minor] - #100
Merged
Merged
Conversation
LockSnapshotStore held one snapshot per (upstream, repository, ref) for the life of the process, and the ref comes from the client's ?refspec=, so every branch ever listed kept a full lock list in memory and a client could grow the store without limit. Each publish now drops snapshots older than Locks:ListTtl, which would be refetched before being served anyway, then the oldest survivors until at most the new Locks:MaxSnapshots (default 1000) remain. The snapshot just published is never evicted. Read and Invalidate are unchanged. Fixes #50 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01K6bDUGMFsAQews3TnA2jXr
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #50
Summary
LockSnapshotStorekept one snapshot per(upstream, repository, ref)for as long as the process ran. The ref comes from the client's?refspec=, so every branch anyone had listed kept a full lock list in memory, and a client could grow the store without limit.The store now evicts on every
Publish. Publishing is the only thing that grows it, and it follows a full upstream walk, so the O(n) scan costs little by comparison:Locks:ListTtl. These would be refetched before being served anyway.TakenAt, until at mostLocks:MaxSnapshotsare left. This is a new option, default 1000, and the validator rejects values of zero or less.Removal checks the value as well as the key, so a snapshot republished under the same key during the scan is not lost. Only eviction is serialised.
ReadandInvalidatestill take no lock, and their behaviour is unchanged. In particular,LockFanOutcan still resolve lock ids from a stale snapshot until the next publish, so a large batch unlock does not start doing more upstream walks. Worst-case memory is nowMaxSnapshots × MaxSnapshotLocks.TimeProvideris now passed into the store, which DI already registers. The README settings table documentsLocks:MaxSnapshots.Out of scope: normalising the refspec or repository path in the key. Whether a path is case-sensitive depends on the forge, so this is not a small change. It is also tied to the refspec-keyed design that #46 and #47 cover. The cap bounds memory either way.
How it was tested
New tests in
GitLfsCache.Tests/Locks/LockSnapshotStoreTests.csuseFakeTimeProvider:Publish_AfterManyRefsHaveOutlivedTheListTtl_DropsEveryStaleSnapshot: publishes 200 distinct refspecs, advances time byListTtl, then publishes once more. Afterwards the store holds only that last snapshot.Publish_BeyondMaxSnapshots_EvictsTheOldestFirst: with a cap of 3, publishes 5 snapshots. The 2 oldest are evicted.Publish_ASnapshotThatIsAlreadyOld_KeepsIt,Publish_RepublishingOneKey_HoldsOneSnapshotandInvalidate_DropsEveryRefOfTheRepositoryOnlycheck the guard and the existing behaviour.MaxSnapshots <= 0.Revert proof: with eviction short-circuited in
LockSnapshotStore, the two eviction tests fail (352/354 pass). With the fix restored, all 354 pass.Full suite:
dotnet buildfinished with 0 warnings and 0 errors, anddotnet testpassed 354 of 354.🤖 Generated with Claude Code
https://claude.ai/code/session_01K6bDUGMFsAQews3TnA2jXr
Generated by Claude Code