fix: harden S3 multipart uploads against CompleteMultipartUpload InvalidPart by sylr · Pull Request #1631 · formancehq/ledger

sylr · 2026-07-23T11:27:18Z

Problem

Full S3 backups of a large ledger fail with CompleteMultipartUpload … InvalidPart: One or more of the specified parts could not be found. Investigation against the customer's dev bucket showed the bucket/KMS/S3 are healthy for multipart from a clean path — the failure is specific to the client's multipart execution, on files large enough to go multipart.

Change

32 MiB multipart part size on both the backup (internal/infra/backup/s3.go) and cold-storage (internal/infra/coldstorage/s3.go) uploaders (SDK default is 5 MiB). Cuts part count ~6.4× for multi-GB objects (smaller failure surface) and lifts the 10,000-part single-object ceiling from ~48.8 GiB to ~312 GiB.
RequestChecksumCalculation=WhenRequired on the shared S3 client (coldstorage.NewS3Client, reused by the backup client). Recent aws-sdk-go-v2 defaults to WhenSupported, which attaches a trailing CRC via aws-chunked encoding on every part — the leading suspect for the InvalidPart at Complete, and the older binary that predates the default uploaded fine.

Object integrity is preserved end-to-end without the SDK checksums: checkpoint keys are content-addressed by sha256, and cold-storage archives carry a verified sha256 metadata checksum.

Verification

go build -tags s3 ./... and go vet clean.
Multipart + trailing-CRC upload verified working against the exact bucket from a clean network path (bucket/KMS are not the problem).
⚠️ s3 integration tests (MinIO testcontainer) require Docker → CI.
⚠️ Confidence on the checksum lever is medium (hypothesis, not reproduced from a clean path); the part-size change is unambiguously beneficial.

Notes

Operational: with 32 MiB parts, a non-seekable upload can buffer ~160 MiB (concurrency × part size), up from ~25 MiB.

…lidPart Full and incremental backups (and cold-storage archives) upload via the aws-sdk-go-v2 manager Uploader with all defaults. Against the customer S3 path a full backup of a multi-GB Pebble checkpoint blob fails at CompleteMultipartUpload with `InvalidPart: One or more of the specified parts could not be found`, and never completes. Two changes: - Set the multipart PartSize to 32 MiB (default 5 MiB) on both the backup and cold-storage uploaders. This cuts the part count ~6.4x for large objects — smaller failure surface for a part to go missing, and lifts the 10,000-part single-object ceiling from ~48.8 GiB to ~312 GiB. Per-part memory stays bounded (partSize x concurrency). - Set RequestChecksumCalculation=WhenRequired on the shared S3 client (coldstorage.NewS3Client, reused by the backup client). The recent aws-sdk-go-v2 default (WhenSupported) attaches a trailing CRC via aws-chunked encoding to every UploadPart; this is the suspected trigger for the InvalidPart at Complete, and the older binary that predates the default uploaded fine. End-to-end integrity is unaffected: checkpoint object keys are content-addressed by sha256 and cold-storage archives carry a verified sha256 metadata checksum. Constraint: bucket, KMS key and real S3 proven healthy for multipart from a clean path — failure is client/path-side, so the fix targets the client Rejected: application-level whole-object retry only | does not address a deterministic (non-transient) InvalidPart; kept as a follow-up Rejected: extend hash chain / change integrity model | out of scope, sha256 content-addressing already covers object integrity Confidence: medium Scope-risk: narrow Directive: if InvalidPart persists after this, the next lever is the checksum path — capture a wire trace of UploadPart/CompleteMultipartUpload before reverting the checksum default Not-tested: live multi-GB multipart upload against the customer S3 path (no repro from a clean network path)

coderabbitai · 2026-07-23T11:27:28Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)

main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 07e50a38-a473-4df8-907e-697f7111cf5a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/s3-multipart-invalidpart

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

NumaryBot · 2026-07-23T11:28:12Z

🛑 Changes requested — automated review

The PR raises the S3 multipart upload part-size threshold to 32 MiB and correctly adds a test-only 5 MiB part-size override for the backup-package multipart tests. However, the equivalent cold-storage regression test (TestS3Storage_ArchiveMultipartLargeObject) still uploads a 12 MiB payload with no part-size override, so the AWS SDK uploader now silently takes the single PutObject path. The CreateMultipartUpload / UploadPart / CompleteMultipartUpload code path that this PR is specifically hardening is therefore unexercised in CI for cold storage. The fix requires either a matching part-size override or a payload larger than 32 MiB in that test. No other confirmed issues were found.

NumaryBot

NumaryBot posted 1 new inline finding.

Summary: #1631 (comment)

codecov · 2026-07-23T11:35:22Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.66%. Comparing base (e430164) to head (3ae39c0).

Additional details and impacted files

@@               Coverage Diff                @@
##           release/v3.0    #1631      +/-   ##
================================================
- Coverage         74.75%   74.66%   -0.09%     
================================================
  Files               446      446              
  Lines             47485    47485              
================================================
- Hits              35497    35455      -42     
- Misses             8777     8814      +37     
- Partials           3211     3216       +5

Flag	Coverage Δ
e2e	`74.66% <ø> (-0.09%)`	⬇️
scenario	`74.66% <ø> (-0.09%)`	⬇️
unit	`74.66% <ø> (-0.09%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… parts Raising the production part size to 32 MiB routed the existing 12 MiB multipart tests through a single PutObject, leaving the multipart path this PR hardens unexercised in CI (NumaryBot finding). Add an unexported newS3StorageWithPartSize constructor and have the two multipart tests force a 5 MiB part size, so their 12 MiB payloads still exercise CreateMultipartUpload/UploadPart/CompleteMultipartUpload regardless of the production default. Confidence: high Scope-risk: narrow Not-tested: run requires the MinIO testcontainer (Docker) — compile-checked locally, executed in CI

NumaryBot

NumaryBot posted 1 new inline finding.

Summary: #1631 (comment)

Same fix as the backup uploader: with s3UploadPartSize raised to 32 MiB, TestS3Storage_ArchiveMultipartLargeObject's 12 MiB payload fell onto the single-PutObject path, leaving the multipart archive path unexercised (NumaryBot finding). Add an unexported newS3StorageWithPartSize and force a 5 MiB part size in that test so it still drives CreateMultipartUpload/UploadPart/CompleteMultipartUpload. Confidence: high Scope-risk: narrow Not-tested: run requires the MinIO testcontainer (Docker) — compile-checked locally, executed in CI

NumaryBot

NumaryBot posted 1 new inline finding.

Summary: #1631 (comment)

NumaryBot · 2026-07-23T11:46:49Z

+	return newS3StorageWithPartSize(client, bucket, s3UploadPartSize)
+}
+
+// newS3StorageWithPartSize is NewS3Storage with an explicit multipart part


🟠 [major] Cold-storage multipart test no longer exercises the multipart upload path
_{reported by NumaryBot, codex}

With s3UploadPartSize raised to 32 MiB, TestS3Storage_ArchiveMultipartLargeObject uses a 12 MiB payload, so the AWS SDK uploader silently falls through to the single PutObject path instead of CreateMultipartUpload / UploadPart / CompleteMultipartUpload. The backup package tests received a test-only 5 MiB part-size override to compensate, but no equivalent treatment was applied to the cold-storage test, leaving the hardened multipart path entirely unexercised in CI.

Suggestion: Either raise the cold-storage test payload above 32 MiB, or inject a test-only part-size override (e.g. 5 MiB, matching the backup test approach) so that TestS3Storage_ArchiveMultipartLargeObject actually exercises the CreateMultipartUpload / UploadPart / CompleteMultipartUpload code path this PR is intended to harden.

flemzord · 2026-07-23T12:06:13Z

Reviewed live head 3ae39c05. The existing NumaryBot multipart-test finding is no longer active in the current tree: both backup and cold-storage constructors now accept a test-only part size, and all three 12 MiB multipart tests force 5 MiB parts while production stays at 32 MiB. Main build/test checks and Codecov are green; the remaining NumaryBot status does not match the current source. I found no correctness/runtime blocker on this head; GitHub approval is intentionally disabled by this cron configuration.

sylr requested review from Azorlogh, gfyrag and paul-nicolas as code owners July 23, 2026 11:27

sylr mentioned this pull request Jul 23, 2026

fix(backup): skip re-hashing unchanged files and retry transient uploads #1632

Open

NumaryBot reviewed Jul 23, 2026

View reviewed changes

Comment thread internal/infra/backup/s3.go

NumaryBot reviewed Jul 23, 2026

View reviewed changes

Comment thread internal/infra/coldstorage/s3.go Outdated

NumaryBot reviewed Jul 23, 2026

View reviewed changes

Azorlogh approved these changes Jul 30, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix: harden S3 multipart uploads against CompleteMultipartUpload InvalidPart - #1631

fix: harden S3 multipart uploads against CompleteMultipartUpload InvalidPart#1631
sylr wants to merge 3 commits into
release/v3.0from
fix/s3-multipart-invalidpart

sylr commented Jul 23, 2026

Uh oh!

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

Review skipped

Uh oh!

NumaryBot commented Jul 23, 2026 •

edited

Loading

Uh oh!

NumaryBot left a comment

Uh oh!

Uh oh!

codecov Bot commented Jul 23, 2026 •

edited

Loading

Uh oh!

NumaryBot left a comment

Uh oh!

Uh oh!

NumaryBot left a comment

Uh oh!

NumaryBot Jul 23, 2026

Uh oh!

flemzord commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

sylr commented Jul 23, 2026

Problem

Change

Verification

Notes

Uh oh!

coderabbitai Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

NumaryBot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 Changes requested — automated review

Uh oh!

NumaryBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NumaryBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NumaryBot left a comment

Choose a reason for hiding this comment

Uh oh!

NumaryBot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

flemzord commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

NumaryBot commented Jul 23, 2026 •

edited

Loading

codecov Bot commented Jul 23, 2026 •

edited

Loading