Post step: bounded incremental mirror maintenance; maintenance failure no longer vetoes the commit

[piob-io]

Summary

The post step ran git gc --auto on the mirror with a 120 s timeout, and a GC failure/timeout set shouldCommit=false. On a large mirror this is a livelock: each synced job adds a pack; once gc.autoPackLimit (50) is crossed, gc --auto does a full repack of every pack including the multi-GB base pack. When that takes >120 s (I/O-bound, so it does on slower disks/arches for a ~2 GB pack), the job discards the synced mirror, the snapshot never advances, and the next job trips the exact same repack — every job pays ~120 s in the post step and nothing ever improves until the disk is purged.

Two changes:

1. Bounded maintenance instead of gc --auto (runMirrorMaintenance, replaces runMirrorGC):

   markKeepPacks(mirror, 256 MiB)   # -> {kept, deferred}
   timeout 120 git -c repack.writeBitmaps=false -C mirror \
       repack -d -l -n --geometric=2 --write-midx
   timeout 120 git -C mirror pack-refs --all
   

   markKeepPacks writes a permanent pack-*.keep for every pack ≥ 256 MiB, and — because the geometric repack picks packs to roll up by object count, not size — additionally gives the largest of the remaining packs a per-run .keep (marker content blacksmith-checkout: deferred to a later run) until the packs left for the roll-up add up to < 256 MiB. So a run rewrites at most 256 MiB of existing packs plus the new loose objects, regardless of how many medium packs have accumulated. Deferred .keeps are removed in a finally; a stale one from a killed run is recognised by its content and cleared on the next run. A rolled-up pack that grows past the threshold becomes another kept pack. Kept packs also don't count toward gc.autoPackLimit, so an older action version sharing the mirror won't trip a full repack on them either. A multi-pack-index keeps lookups fast across kept packs.

   .keep files rather than repack --keep-pack=: with git 2.34 geometric repack still folds a --keep-pack pack into the roll-up and -d then deletes it, losing objects (reproduced on a valid fixture: clean fsck before, dangling refs after).

   Reclaim. Kept packs never lose objects, so history that became unreachable (deleted/rewritten branches) stays on disk. Once per MAINTENANCE_RECLAIM_INTERVAL_MS (14 d) the run instead lifts every .keep and does

   timeout 600 git -c repack.writeBitmaps=false -C mirror repack -a -d -l -n --write-midx
   timeout <remaining> git -C mirror prune --expire=2.weeks.ago
   

   under MAINTENANCE_RECLAIM_TIMEOUT_SECS (600 s). The commit-graph (single file and split chain) is removed before the repack — it would otherwise still list the pruned commits and fsck/incremental graph writes fail on those in every later job if prune then fails or times out — and a run that cannot remove it falls back to an incremental repack instead of reclaiming; a fresh --reachable graph is written only after a successful prune. The stamp file (<mirror>/blacksmith-maintenance-reclaim, epoch ms) is written before the attempt, so a mirror too large for the deadline does not retry it every job — it goes back to bounded incremental runs for another interval. A mirror without a stamp starts its interval rather than reclaiming at once, so a rollout doesn't make every job in the fleet rewrite its mirror simultaneously.

2. Maintenance is optional and never vetoes the commit. cleanup() keeps shouldCommit=false / vmHydratedGitMirror=false for mirror sync failure or timeout, but a maintenance failure or timeout — of repack, prune or pack-refs, each reported as success:false with timedOut set only for exit 124 — is only logged (still reported via the existing git_mirror_gc_failure metric) and the synced mirror is committed. On failure/timeout, stale tmp_pack_*, .tmp-*, *.lock and packed-refs.lock left by the killed process are removed so the next job's sync doesn't hit a lock nobody holds. Maintenance now runs only when shouldCommit is true (same gate the commit-graph catch-up already had).

The sync fetch runs with fetch.unpackLimit=1 so received objects always land in a pack (a large blob left loose would not be counted by markKeepPacks's byte budget).

CleanupResult.gcResult → maintenanceResult. runMirrorMaintenance(mirrorPath, {timeoutSecs, keepBytes, reclaimIntervalMs, reclaimTimeoutSecs, now}).

Tests

• __test__/mirror-maintenance-git.test.ts (real git): base pack + small packs + loose objects → base pack kept (same inode/size), small packs and loose objects rolled up, midx written, fsck --strict clean, all refs resolve; second run is a no-op; a kept pack is excluded from the roll-up even when larger packs arrive later; threshold marking is idempotent. Mixed sizes: 2 packs of ~1 MiB with few objects + 8 tiny packs with many objects under a 1 MiB threshold → bytes written < threshold, the two medium packs untouched and their .keeps gone afterwards (the assertion fails with 2.2 MiB written if the deferral is disabled); markKeepPacks deferral order and cleanup. Reclaim: first sight only writes the stamp; when due, a branch deleted after the base pack was kept has its objects dropped, mirror collapses to one pack, fsck clean, other refs intact; a reclaim that times out leaves the mirror intact, updates the stamp, and the next run is incremental again; prune failing after the reclaim repack leaves no commit-graph behind and fsck stays clean. __test__/mirror-sync-negotiation-git.test.ts: the sync fetch adds exactly one pack and no loose objects. A git shim (installed via a timeout shim, since jest's sandboxed process.env isn't what children inherit) simulates a hung repack → timedOut: true, leftovers removed, mirror untouched; failing repack → success:false, timedOut:false; pack-refs exit 3 → success:false, timedOut:false; hung pack-refs → timedOut:true, packed-refs.lock removed.
• __test__/mirror-cleanup.test.ts (mocked exec + gRPC): repack timeout/failure and pack-refs timeout/failure each still commit with shouldCommit=true, vmHydratedGitMirror=true; sync failure/timeout skips maintenance and commits false/false; shouldCommit=false input skips maintenance; no gc invocation.

Rollout notes

Existing mirrors carry whatever pack layout gc --auto left; the first post step on such a mirror marks its big pack(s) kept, defers the medium ones and rolls up the rest, so even the first run is bounded by the threshold. The first reclaim happens 14 days after the first run of this version on each mirror (600 s budget; on timeout the mirror keeps working from its kept packs and the next attempt is another 14 days out). Verified locally on git 2.34.1; --geometric/--write-midx exist since 2.31/2.32.

CI

• The real-git fixture disables receive-side auto maintenance (gc.auto=0, receive.autogc=false, maintenance.auto=false) on the bare mirror: git 2.55's receive-pack otherwise consolidates the pushed packs itself, so the fixture's "6 small packs" was 2 packs on the CI runner. Verified on git 2.34.1 and 2.55.0.
• test-git-mirror-container / test-git-mirror-after-container: with sticky disk branch protection on, pull_request runs cannot hydrate a mirror (the agent declines the fresh entity and the action falls back to a direct clone). When the disk isn't mounted on a non-push event, the jobs now verify the fallback checkout and emit a notice instead of failing; on push (trusted, may hydrate) the mirror assertions remain hard.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Link to Devin session: https://app.devin.ai/sessions/a3b42ef15226438faf3f9cdde9662918
Open in Devin Desktop: https://app.devin.ai/desktop/session/a3b42ef15226438faf3f9cdde9662918?variant=devin
Requested by: @piob-io

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[ajwerner]

I found three issues with the maintenance changes. The inline comments explain the impact, the checks I ran, and suggested fixes.

[ajwerner]

[P2] Limit the total bytes rewritten in one run

The 256 MiB cutoff limits each pack, but Git chooses which packs to combine by object count. Several packs below the cutoff can therefore be combined into a much larger write.

I reproduced this on Git 2.50.1 with a 1 MiB cutoff. Six packs of about 0.75 MiB, containing 4, 8, 16, 32, 64 and 128 objects, survived successful maintenance after every addition. Adding another 128-object pack made the next run rewrite all seven into one roughly 5.3 MiB pack. At the default cutoff, the same proportions would mean about 1.3 GiB of output, which can bring back repeated 120-second maintenance timeouts.

Please choose packs using a limit on their combined size and add a test with different object sizes. The per-pack cutoff alone does not provide the claimed limit on work.

[devin-ai-integration]

Agreed — the per-pack cutoff only bounded the largest input, not the roll-up. Fixed in 53d9017: markKeepPacks now returns {kept, deferred}. Packs ≥ threshold get a permanent .keep as before; of the rest, the largest get a per-run .keep (marker content, so it's distinguishable from a real keep) until the packs left for the geometric roll-up sum to < threshold. Those .keeps are removed in a finally, and a stale one left by a killed run is recognised by its content and cleared next time. Since git chooses by object count, this is the only way I found to bound the bytes without reimplementing the selection.

Test never rewrites more than the keep threshold per run, whatever the object counts: 2 × ~1 MiB packs with few objects + 8 tiny packs with many objects, 1 MiB threshold → bytes written < 1 MiB, the two medium packs untouched; with the deferral disabled the same test measures 2.2 MiB written. Deferred packs are reconsidered on the next run once the small ones around them have been folded.

[ajwerner]

[P2] Add a way to reclaim deleted history

These .keep files are permanent, and replacing gc --auto removes the action's way to discard objects that no branch or tag needs anymore. Geometric repacking keeps those objects, while .keep prevents later garbage collection from deleting their packs.

I tested a kept pack after deleting its only branch. Repeated maintenance kept the whole pack. Even git gc --prune=now kept it; removing the .keep file let GC reclaim it.

As branches are deleted or rewritten, a long-lived mirror can keep growing even if the live repository stays small. Please add a way to reclaim this space or periodically rebuild the mirror. If the backend already guarantees such rebuilds, document that dependency and verify that it keeps disk usage under control.

[devin-ai-integration]

There's no external rebuild I could rely on (mirrors only get replaced by a purge), so 53d9017 adds an explicit reclaim: once per MAINTENANCE_RECLAIM_INTERVAL_MS (14 d) the run lifts every .keep and does repack -a -d -l -n --write-midx + prune --expire=2.weeks.ago under its own 600 s deadline instead of the incremental step. The stamp (<mirror>/blacksmith-maintenance-reclaim) is written before the attempt, so a mirror too large for the deadline doesn't retry every job — it just stays on bounded incremental runs for another interval (that's the failure mode this PR is fixing, so I'd rather leak unreachable objects for 14 days than reintroduce it). A mirror with no stamp starts its interval rather than reclaiming at once, so a release doesn't make the whole fleet rewrite mirrors on the same day.

Tests: first sight only stamps; when due, a branch deleted after its pack was kept has its objects gone, one pack left, fsck clean, other refs intact; a reclaim that times out leaves the mirror intact and the next run is incremental again.

Open to a different interval or a size cap on the reclaim if you'd rather not have any unbounded step at all — the alternative I considered was repack -a -d only when total mirror size < N GiB, but that leaves the largest mirrors (the ones that accumulate the most) never reclaiming.

[ajwerner]

[P2] Report pack-refs failures as maintenance failures

After logging this failure, the function returns {success: true, timedOut: false} below. I simulated both a timeout (exit 124) and another failure (exit 128), and both were reported as success.

This means main.ts never emits the maintenance-failure metric for this step, even when it spends the full 120 seconds timing out.

Please return success: false and preserve whether the command timed out, with tests for both cases. The synced mirror can still be saved: the new cleanup logic already allows that when maintenance fails.

[devin-ai-integration]

Fixed in 53d9017: pack-refs (and the reclaim's prune) now go through the same fail() path as repack — exit 124 → {success:false, timedOut:true}, any other nonzero → {success:false, timedOut:false}, leftovers removed in both cases. cleanup() still commits the synced mirror either way. Covered by real-git tests (pack-refs exit 3; hung pack-refs → timeout + packed-refs.lock removed) and the mocked cleanup tests (both still commit with shouldCommit=true).

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 53d9017. Configure here.}

[ajwerner]

Approving. A couple of follow-ups worth keeping in mind:

• The new commit-graph rebuild covers successful cleanup. One failure case remains: the full repack can delete old commits, then prune can fail or time out before the graph is removed. Since we still save the mirror, later jobs running git fsck can fail on those stale entries. Removing both graph formats before the full repack, and skipping reclamation if that removal fails, would cover this too. The existing cleanup code can rebuild the graph afterward; leaving it absent if rebuilding fails is safe. A test where prune fails after repacking would catch this case.

• The byte limit still covers existing packs only. Git can store a small fetch as loose objects even when it contains large binaries, so a few files totaling more than 256 MiB can bypass the limit. I reproduced a roughly 2 MiB repack with a 1 MiB threshold this way. The likely symptom is another slow post step or 120-second maintenance timeout; I haven't reproduced incorrect checkout contents. Accounting for loose objects in the budget, or keeping incoming objects packed, would close that gap.

[devin-ai-integration]

@ajwerner both follow-ups from your approval are in 6572b50:

• The commit-graph (objects/info/commit-graph and the commit-graphs/ split chain) is now removed before the reclaim repack; if that removal fails the run does an ordinary incremental repack instead of reclaiming (the stamp is still advanced, so the next attempt is an interval away rather than every job). The graph is only rewritten after a successful prune; on prune failure/timeout the mirror is committed without a graph. New real-git test: prune shimmed to exit 7 after the repack has already dropped the unreachable commit → success:false, no graph on disk, fsck clean (fails on the previous code with the stale graph still present).
• Loose objects: the sync fetch now runs with fetch.unpackLimit=1, so whatever a sync receives lands as a pack and is visible to markKeepPacks's byte budget. The negotiation real-git test asserts the fetch adds exactly one pack and no loose objects. Loose objects already present on hydrated mirrors are folded by the first incremental run, a one-off.