feat(storage): budget how much a single repository may hold - #83
Conversation
There was a problem hiding this comment.
Solid design, but the direct-PUT quota gate in server/src/routes.rs has two gaps that undermine the safety net it's meant to be — both left as line comments:
- Blocking: it doesn't skip already-held objects like the negotiation gate does, so a retried direct PUT of an object the repo already has can be wrongly refused once the repo is near budget — contradicts the PR's own stated invariant and isn't covered by the test suite (the "already held" test only exercises negotiation, not
put()). - Blocking: a PUT with no (or unparseable)
Content-Length— e.g. chunked transfer-encoding — is charged 0 bytes against the quota, so it bypasses the budget entirely. Unlikemax_object_size, there's no live enforcement as bytes stream in.
Everything else — the caching/invalidation story around usage_of/stored/forget, the download-never-refused behavior, dedup-by-content accounting — looks correct and is well covered by quota.rs.
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 |
|
Both findings were right, and the first one caught the code disagreeing with my own PR description. Already-held objects at the PUT gate. Fixed — that gate now checks Undeclared size. Also right, and worse than the first: README corrected on both points. |
There was a problem hiding this comment.
Both blocking findings from the previous review are fixed in 61f2f41 (replies left on the resolved threads): the direct-PUT gate now skips the quota check for already-held objects, and the budget is threaded into stream_to so it's enforced against actual bytes written rather than a client-declared Content-Length — closing the chunked-transfer bypass. No new issues in the fix itself; new tests cover both cases.
Closes #32.
#12 capped a single object. That does not stop one project committing its renders directory a gigabyte at a time, and on a server hosting a team the first symptom is other repositories failing to push.
LFSX_REPO_QUOTA(bytes, unset = no budget) turns that into one repository being told, in its own client, that it is out of room.Refused at negotiation with a per-object
507, and at the directPUTfor clients that skip negotiation. Downloads never — a repository over budget still serves everything it holds, because refusing a checkout punishes the wrong person and fixes nothing.The number is what the repository holds, the same figure
statsand the dashboard report, not what it costs the disk after deduplication. Two projects sharing an asset pack each count it against their own budget. That is the number an operator hands out, and it would be indefensible to tell someone their quota moved because an unrelated project deleted a pack they happened to share.Staleness was the design problem. The cache behind
usage_ofhas a 60-second TTL, which is fine for a dashboard and wrong for enforcement in both directions: stale-low lets a repository push past its budget, stale-high refuses room the client has just freed. So a stored object now adds to the cached figure, and collection drops the entry rather than trying to do arithmetic across hard links — the next reader measures what is really there. A retried transfer of an object already held does not count twice.An object the repository already holds is never refused for want of room, at either gate. It asks for no new space.
Tests
Six, against a repository holding 900 of its 1024 bytes: an object that would not fit is refused at negotiation with no upload link, one that still fits is accepted, a full repository still negotiates and serves downloads, a client skipping negotiation gets
507, an object already held is waved through, andretainfrees room the very next push can use.That last one is the one that matters: it fails if collection does not invalidate the cached figure, because the push would otherwise be refused against a number that is up to a minute out of date.