fix(api): cap a batch and resolve its objects a few at a time - #134
Conversation
There was a problem hiding this comment.
Solid fix. Ceiling is checked before any storage work, buffered(16) correctly preserves output order (verified by the reordering test with buffer_unordered failing as a sanity check per the PR description), and the budget snapshot semantics for concurrent uploads are unchanged from the prior sequential behavior (still enforced authoritatively at verify), so concurrency doesn't introduce a new quota bypass. Tests cover both ceiling edges (1000 vs 1001) and ordering. No blocking issues.
Nit: the 1000/1001-object tests build large JSON bodies inline per test — a small objects_of(n) helper shared between the two size tests would trim the duplication, but 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 #128. Builds on #125, which was the expensive half of the same loop.
batchlooped overrequest.objectswith no cap, and each iteration costs around trip: a
staton a volume, aHEADagainst a bucket. git-lfs sends at mosta hundred, so nothing had noticed, but the count is the client's to choose and
axum's body limit leaves room for tens of thousands. That made one authenticated
request into as many serial round trips as the caller liked, against storage the
operator pays per request for.
A ceiling of 1000, refused rather than truncated. Well above what any client
sends, so it is a backstop and not a policy. Answering for the first thousand of
two thousand objects would tell the client the rest do not exist, and it would
upload them all again. The refusal names the ceiling, so there is something to do
about it. Not configurable: nothing legitimate comes near it, and a knob here
would be a limit somebody has to reason about.
The objects resolve a few at a time. The loop was sequential, so a legitimate
hundred-object batch against a bucket was a hundred serial round trips before the
client could start uploading anything. Sixteen at once, which hides the latency
without becoming a burst the store answers with 503.
Measured
A download batch of 100 objects the repository already holds, MinIO on localhost:
The invariant concurrency could have broken
The answers no longer arrive in the order they were asked for, so they are
collected in order (
buffered, notbuffer_unordered). A reordered batch is onea client cannot match up against what it asked.
a_batch_answers_in_the_order_it_was_askedcovers it, and it is not vacuous:swapped to
buffer_unorderedit fails on a 200-object batch.Also covered: a batch of 1001 is refused with 422 and a message naming the
ceiling, and a batch of exactly 1000 is answered in full, so the backstop does not
quietly become a policy one object early.
Verification
cargo fmt --check,cargo clippy --all-targets -- -D warnings