Sweep staging directories left by clones that never finished - #61
Merged
Merged
Conversation
…tch] A clone stages into mirror.git.tmp-<guid> beside its mirror and discards it in a finally, which never runs when the process is killed mid-clone (OOM, eviction, rollout). Nothing else looked for the pattern, so each crash left a partial bare clone on the volume for good, uncounted by the mirror metrics. The maintenance sweep now removes staging directories older than FetchTimeout + ProbeTimeout + ten minutes, the longest a clone can still be running, whether or not idle-mirror reaping is enabled. Only the exact name a clone uses (the prefix plus 32 hex digits) is matched, and a candidate that holds a mirror is skipped, so a repository path segment that happens to share the prefix is never deleted. Fixes #45 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015LsGhhi4Vq9pNq2fr5T75T
|
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
2 participants
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 #45
Problem
MirrorFetcher.CloneAsyncstages each clone intomirror.git.tmp-<guid>and discards it in afinally. Thatfinallynever runs when the process is killed mid-clone, for example by an OOM kill, an eviction or a rollout.MirrorStore.Enumerate()only finds directories named exactlymirror.git, and nothing else looks for the staging pattern. So each crash left a partial bare clone on the volume permanently, and the mirror byte metrics never counted it.Change
IMirrorStore.EnumerateStaging()(implemented inMirrorStore) lists staging directories underMirrorRoot.mirror.git.tmp-followed by 32 hex digits. The prefix alone isn't enough, because a repository path segment may start the same way.CloneAsyncnow builds its staging name from the sameStagingPrefixconstant.MirrorMaintenanceService.Sweepnow starts by removing staging directories older thanFetchTimeout + ProbeTimeout + 10 min. That is the longest a clone can still be running (the clone, then the config step, plus a margin for killing a timed-out process).MirrorIdleMaxAgeis zero (idle reaping disabled).I chose the age-bounded sweep over a startup-only cleanup because it also catches a staging directory whose
DiscardStagingfailed while the process stayed up.DiscardStaging's existing comment already promised "until the next sweep". The first sweep runs at startup anyway.Tests
Six new tests in
MirrorMaintenanceServiceTests:FetchTimeoutkeeps its staging directorymirror.git.tmp-oldrepository segment is keptEnumerateStagingmatches only real staging namesI checked that the tests catch the bug by temporarily disabling the staging sweep: the two removal tests failed, and they pass with the fix. Full suite: 225/225 passed locally.
🤖 Generated with Claude Code
https://claude.ai/code/session_015LsGhhi4Vq9pNq2fr5T75T
Generated by Claude Code