feat(storage): fold objects stored before the shared store into it - #88
Conversation
|
CI caught something my machine could not: It was not the test being wrong. Fixed where it belongs rather than in the test: |
![ferrfleet[bot]](https://avatars.githubusercontent.com/in/4354888?s=60&v=4){kind=small}
There was a problem hiding this comment.
Solid change: atomic rename-over-original, digest verification on both sides before admitting/adopting, admin-gated, and the test suite actually exercises the interesting cases (corrupt shared entry, corrupt local file, re-run idempotency, dry-run no-op).
Nit: in adopt() (server/src/storage/dedupe.rs), self.link(content, &staged).await? propagates a raw NotFound error if the shared content vanishes between the fs::metadata check and the link call (e.g. a concurrent retain/gc on another repo drops the last other reference to that shared object). Unlike link_or_move in storage/mod.rs, which retries by restaging on NotFound, this just aborts the whole dedupe call with a 500 — no corruption, but the client gets an opaque error and loses the report for everything already processed in that run instead of the object being counted as refused and the walk continuing. Narrow race, not worth blocking on.
SonarQube — aucune nouvelle issueComparaison entre le projet bac à sable de cette PR et la branche par défaut : SonarQube Community n'analyse pas les PR, ce delta est calculé côté CI. Détail |
Closes #87.
Deduplication arrived in 0.20.0. Objects written before it are plain files with a single link — they serve correctly and never collapse, so a server running since 0.17.x keeps paying full price for every pack two projects share. The homelab is that server: 11 827 objects, 3.46 GB, all written the old way.
POST /{org}/{repo}/objects/dedupe, andlfsx dedupe --repo <org/repo> [--dry-run]in front of it. Each object is either moved into.contentand linked back, or linked to what is already there with its copy freed.Bytes come back on the second repository, not the first. The first to run moves its objects into the shared store and frees nothing; the one that follows and holds the same pack is where the disk gives anything back. Worth knowing before reading a report that says
reclaimed: 0and concluding it did not work.What it refuses to do
An object whose bytes do not hash to its own name never enters the shared store — everything that links there afterwards would inherit them. A shared entry that does not match its name is never adopted, so a repository holding a good copy keeps it. Both are counted as
refusedand named in the log rather than silently skipped.The repository's file is never removed before its replacement exists: the link is created under a temporary name and renamed over the original, which is atomic. An interrupted run leaves either the old file or the new link, never a gap where a repository has no object.
Admin only. This rewrites every object in place, so it asks for the rights of someone who could delete them instead — a push token is not enough.
Re-running
An object already sharing its inode with the shared store is recognised and skipped, so a second run reports
already_sharedand does nothing. A migration that cannot be re-run is one nobody dares run at all, and that is a test rather than a claim.On a platform without inode numbers there is no way to tell a link from a copy, so the portable path relinks every time — same result, repeated work. The server ships on Linux; this only affects tests elsewhere, and the test says so.
Tests
Six at the storage level: an old-style object is folded in and still reads; a second repository sharing the bytes drops the disk usage measurably; a re-run is a no-op; a corrupt shared entry is refused and the good local copy survives; an object that lies about its digest never reaches the shared store; a dry run touches nothing.
Two through the API: the endpoint folds a repository in and it still serves afterwards, and a push token is refused with the repository left untouched.