feat(server): collect objects no longer referenced by any commit - #23
Conversation
There was a problem hiding this comment.
Solid implementation. sweep_directory's three safety checks (non-oid filenames like .part staging files, retained oids, and the grace period) are each independently correct and well-tested — retained objects are kept regardless of age, staging files are never touched, and nothing under LFSX_GC_GRACE is swept. Directory cleanup (remove_dir on emptied fanout/prefix dirs) is race-safe against concurrent uploads since create_dir_all is idempotent and remove_dir on a non-empty directory just fails silently. The six new tests exercise exactly the properties the PR description claims.
Nit: in sweep_directory (server/src/storage.rs), entry.metadata().await? and fs::remove_file(entry.path()).await? propagate a hard error and abort the entire sweep on any IO hiccup — e.g. if two retain calls race on the same repo and one removes a file the other already listed. Objects deleted earlier in the loop stay deleted, but the caller just gets a 500 with no report of what actually happened. Consider treating NotFound there as already-swept-by-someone-else and continuing, mirroring the best-effort let _ = fs::remove_dir(...) pattern already used elsewhere in this function, so a transient/concurrent error doesn't throw away an otherwise-accurate report.
Closes #4.
Objects were written and never removed. A repository that rewrites history, drops a branch or replaces a large asset left the old blobs on disk forever.
The server never sees the Git history, so it cannot decide what is unreferenced.
POST /{org}/{repo}/objects/retaintakes the set of oids the repository still references and sweeps the rest:{ "oids": ["a1b2…", "c3d4…"], "dry_run": true } → { "swept": 42, "bytes": 3221225472, "within_grace": 3, "dry_run": true }Nothing is deleted while
dry_runis set, and the report is the same either way, so the dry run is a real preview rather than a different code path.The two things that stop it eating live data
An object is uploaded before the commit referencing it is pushed. A naive sweep deletes objects mid-push, and the client then fails on a
404for something it just uploaded. Anything touched withinLFSX_GC_GRACEis therefore never taken — that is thewithin_gracecount. The default is two weeks, matching git's owngc.pruneExpire.Transfers in flight are skipped too:
sweeponly considers filenames that are valid oids, so the{oid}.{ticket}.partstaging files are invisible to it. That also keeps this out of the way of #8.Sweeping requires push rights, like any other write.
Notes
Empty fanout directories are removed as they are emptied, otherwise a long-lived repository accumulates thousands of empty two-character directories.
An empty
oidsset legitimately means nothing is referenced any more, and outside the grace window that sweeps the repository clean. That is the honest semantic — the README says so and tells you to run it dry first.Tests
Six, covering both directions: an unreferenced object is swept, one still inside the grace period survives, a listed oid is kept however old it is, a dry run reports the same numbers and frees nothing, a staging file is never collected, and a read-only token gets
403.cargo test: 30 passing. Clippy clean. The README documents the flow with agit lfs ls-files --all --longpipeline, which I checked against a real repository rather than writing from memory.