feat(object-store): upload large packs through S3 multipart - #50
Merged
Merged
Conversation
A single S3 PUT cannot exceed 5 GiB, so a repository whose pack grew past that could not be pushed at all. Files above CODE_S3_MULTIPART_THRESHOLD_BYTES (100 MiB by default) now go through InitiateMultipartUpload, UploadPart and CompleteMultipartUpload, in CODE_S3_MULTIPART_PART_SIZE_BYTES parts (64 MiB by default, validated against S3's 5 MiB to 5 GiB bounds at boot). Parts stream off disk in the same 1 MiB sub-chunks as a single PUT, so no pack is ever held in memory. The ceiling becomes part size times 10,000. Create-only is enforced atomically with If-None-Match: * on the completion. A HEAD before initiate only avoids re-sending a pack already stored; a 403 to it, which AWS gives credentials without s3:ListBucket, is treated as unknown. Everything after initiate either completes or aborts, including on raised exceptions, which are then re-raised; a failed abort is logged, and the docs recommend an incomplete-upload lifecycle rule for processes that die. Preconditions with no atomic multipart equivalent (if_match, an ETag-valued if_none_match) are refused rather than silently dropped. A 409 on a pack upload is now a conflict, not a precondition failure, on both paths: it can follow a concurrent delete, so reporting it as "already present" could let the log reference a missing pack. put_file reports a precondition failure only when a HEAD finds the object; otherwise the push fails for the client to retry. Reviewed adversarially in three rounds with Pi and Codex; their findings (abort skipped on exceptions, unconditional completion, unvalidated part size, the 409 mapping) are all addressed and covered by Req-stubbed unit tests. Conditional completion was confirmed against RustFS, which answers 412; other stores are documented as unverified. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both e2e nodes now start with a 5 MiB multipart threshold and part size, so every pack above that in any example goes through multipart against RustFS; the existing 64 MiB replication example uses 13 parts. A new spec pushes an 8 MiB pack, clones it back from the other node byte-identically, and checks code_object_store_multipart_upload_count moved, and checks that a small push leaves it alone. This suite caught that CompleteMultipartUpload returns the final ETag in its XML body rather than as a header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pepicrft
marked this pull request as ready for review
September 26, 2026 14:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Packs larger than a configurable threshold are now uploaded to object storage with S3 multipart upload instead of a single
PUT. S3 is Amazon Simple Storage Service, whose API every store Code supports implements.Code.ObjectStore.S3.put_file/4routes files aboveCODE_S3_MULTIPART_THRESHOLD_BYTES(100 MiB by default) throughInitiateMultipartUpload, oneUploadPartperCODE_S3_MULTIPART_PART_SIZE_BYTESslice (64 MiB by default), andCompleteMultipartUpload, aborting the upload on any failure. Smaller files keep the existing singlePUT.PUTpaths (see Root cause).[:code, :object_store, :multipart_upload], exported to Prometheus ascode_object_store_multipart_upload_count,_bytesand_parts. A failed abort is logged as a warning withoperation=multipart_abort.docs/architecture.mdanddocs/operations.mddescribe the new behaviour, the two environment variables, the store requirements, and a recommended bucket lifecycle rule for incomplete uploads.spec/multipart_upload_spec.shcovers a large and a small push.Why
A single S3
PUTcannot exceed 5 GiB, anddocs/architecture.mdlisted multipart upload as not implemented. A repository whose pack grew past 5 GiB could not be pushed at all. Of the gaps the docs call out, this was the one with the clearest contract and the smallest blast radius: it stays inside the object store abstraction and does not touch how pushes are ordered. The ceiling is now the part size times S3's 10,000-part limit, about 625 GiB at the default part size.Root cause
Two bugs in the first version of this change were found by the new tests and reviews rather than by production, and are worth recording.
CompleteMultipartUploadreturns the final ETag (the object's entity tag) inside its XML body, not as a response header. The first version read the header, so every multipart push failed. The end-to-end suite caught this on its first run against RustFS.The existing single
PUTpath mapped both 409 and 412 to:precondition_failed, and the multipart completion copied that. For packs,Code.WALturns:precondition_failedinto "already stored". But S3 returns 409 (ConditionalRequestConflict) after a concurrent write or delete of the same key, so a 409 is not evidence that the pack exists, and treating it as success could let the log reference a missing pack.put_file/4now reports a precondition failure after a 409 only when a follow-upHEADfinds the object. Otherwise it returns{:error, {:conflict, 409}}, which fails the push for the client to retry.put/3, which the index compare-and-swap uses, is unchanged: its caller re-reads and retries anyway.Approach
The invariant that matters is that pack writes are create-only and idempotent. The first version enforced that with a
HEADbefore initiating the upload. That leaves a window where two writers both see the key as absent and both complete. The fix is to putIf-None-Match: *onCompleteMultipartUploaditself, which S3 evaluates atomically, and to keep theHEADonly to avoid re-sending a pack that is already stored. A 403 on thatHEADis treated as "unknown", because AWS returns 403 for absent keys to credentials withouts3:ListBucket, and the conditional completion still arbitrates.Everything after initiate either completes or aborts. That includes raised exceptions, for example the source file disappearing between parts, which are then re-raised unchanged. Only a killed process skips the abort, which is what the documented
AbortIncompleteMultipartUploadlifecycle rule is for.Preconditions that multipart cannot enforce atomically (
if_match, or an ETag-valuedif_none_match) are refused rather than silently dropped. No caller uses them today.Parts stream off disk in the same 1 MiB sub-chunks as a single
PUT, so the "never hold a pack in memory" rule still holds.Two options were considered and left out. Retrying individual parts would be more resilient, but I have not confirmed that Req re-signs a retried request, and a retry of the whole upload is already idempotent. The first review round also asked for an end-to-end test of two writers racing on completion. That is covered by unit tests plus a manual probe (below) rather than a new spec, because forcing the race through
git pushis fragile.The change went through three rounds of adversarial review with Pi and Codex. Their findings were: the abort was skipped when an exception was raised; completion was unconditional; part sizes were not validated;
HEADbehaves differently under least-privilege credentials; non-*preconditions were dropped silently; and the 409 mapping above. All are fixed and covered by tests, and both reviewers consider the final state mergeable.Impact
If-None-Match: *onCompleteMultipartUpload. AWS S3 documents it and RustFS enforces it. MinIO, Cloudflare R2, Ceph and Tigris are unverified, whichdocs/operations.mdstates. A store that ignores the header is harmless for content-addressed packs. A store that rejects it would fail packs above the threshold.docs/operations.md).Validation
mise run lint: format and Credo strict, no issues.mise run test: 484 tests pass, including 20 new Req-stubbed tests intest/code/object_store/s3_test.exs. They cover threshold routing and clamping, part boundaries, the conditional completion header,HEAD200 and 403, a 412 and a 409 at completion, a failed part, a part with no ETag, a 200 response carrying an<Error>body, a missing upload identifier, a source vanishing mid-upload, abort failures, refused preconditions, invalid part sizes, and the size ceiling.mise run typecheck: no errors.mise run e2e: 56 examples, 0 failures, against RustFS with a 5 MiB threshold. That includes the new multipart spec and the existing 64 MiB replication example, which now uploads in 13 parts.If-None-Match: *returns412 PreconditionFailed.🤖 Generated with Claude Code