feat: upload S3 multipart parts in parallel by digizeph · Pull Request #87 · bgpkit/oneio

digizeph · 2026-10-24T00:17:04Z

Summary

Multipart uploads sent parts one at a time. With a single TCP stream to S3
capping around 12 MiB/s, a 4.3 GB upload takes roughly seven minutes, and a part
request that hangs blocks every remaining part until the 300 s request timeout
fires (three times, if it keeps hanging).

This changes the multipart path to upload parts from a small worker pool, four
by default, and makes the per-request timeout configurable.

Changes

upload_multipart runs parts on a std::thread::scope worker pool. Each
worker opens its own File, reads only the byte range it uploads, signs its
own part URL and writes the resulting ETag into a pre-sized slot, so the
CompleteMultipartUpload list is ordered by construction rather than by
completion order.
On the first unrecoverable part error a shared stop flag makes the other
workers exit at their next part boundary; the multipart upload is then aborted
once and the first error is returned.
ONEIO_S3_MAX_CONCURRENCY (default 4, clamped to 1..=16) and
ONEIO_S3_REQUEST_TIMEOUT_SECS (default 300, the previous hardcoded value)
are read once per process, mirroring the existing s3_retry_config pattern.
No public API change: s3_upload's signature and the S3Config struct are
untouched, so existing struct literals keep compiling.
Parts are read from one open handle, with range reads serialized through it:
a file replaced at the same path during the upload can no longer contribute
bytes from two versions to one object, and cloned handles would share a seek
position instead.
Peak memory for a multipart upload is concurrency * part_size (4 x 8 MiB
with the defaults), documented in the module docs and README.
Spec under specs/04-s3-concurrent-multipart/.

Behaviour that stays the same: single PUT below the 5 MiB threshold, per-part
retry with exponential backoff, abort on failure, and the ONEIO_S3_CHUNK_SIZE
/ ONEIO_S3_MULTIPART_THRESHOLD knobs.

Known limit: a part that stalls still costs one request timeout at the end of
the upload, because the completion step waits for every part. Lowering
ONEIO_S3_REQUEST_TIMEOUT_SECS bounds that cost; concurrency keeps the rest of
the upload moving while it happens.

Testing

Local gates, all passing:

cargo fmt --check
cargo clippy --all-features -- -D warnings
cargo clippy --no-default-features -- -D warnings
cargo test --all-features
cargo test --no-default-features
cargo test --features "https,s3"
cargo doc --no-deps --all-features

New unit tests cover the concurrency clamp, ETag ordering and the missing-part
error.

Against Cloudflare R2 with a 4.3 GB local file:

4 workers: 90 s end to end, object stored exactly 4,334,649,344 bytes
ONEIO_S3_MAX_CONCURRENCY=1 (previous behaviour): 420 s, same byte count
failure path, 32 MB parts with ONEIO_S3_REQUEST_TIMEOUT_SECS=1: returns an
error, and no in-progress multipart upload is left behind (the abort runs)

Multipart uploads ran one part per request, so a single TCP stream set the throughput ceiling and a single stalled part blocked the whole upload until the 300 s request timeout fired. Measured against Cloudflare R2, one stream reaches about 12 MiB/s while four concurrent streams reach about 90 MiB/s. Parts now upload from a small scoped-thread worker pool. Each worker opens its own file handle and reads only the range it uploads, signs its own part URL and stores the ETag in a pre-sized slot, so the completion list is ordered by construction. On the first unrecoverable part error the other workers stop at their next part boundary, the multipart upload is aborted once, and the first error is returned. Two settings are now readable from the environment, following the existing retry-config pattern so no public struct or signature changes: - ONEIO_S3_MAX_CONCURRENCY (default 4, capped at 16) - ONEIO_S3_REQUEST_TIMEOUT_SECS (default 300, the previous hardcoded value) Peak memory per multipart upload becomes concurrency * part_size. A 4.3 GB upload takes 90 s with the default four workers and 420 s with one.

Copilot

🟡 Changes recommended

Worker failures do not signal cancellation promptly, allowing queued parts to continue uploading before the failed upload is aborted.

1 open issue

Signal cancellation immediately when any worker fails · New

What changed in this PR

This PR parallelizes OneIO’s blocking S3 multipart uploads to improve throughput while preserving the public API.

Changes:

Adds a bounded worker pool with independent file handles and ordered ETag collection.
Adds cached environment settings for upload concurrency and request timeout.
Adds helper tests and updates documentation, the implementation spec, and changelog.

File	Description
`src/s3/mod.rs`	Implements concurrent uploads, tuning settings, and helper tests.
`src/lib.rs`	Documents upload settings and memory usage.
`specs/04-s3-concurrent-multipart/README.md`	Records design, validation, and operational risks.
`README.md`	Updates upload behavior and configuration guidance.
`CHANGELOG.md`	Records parallel uploads and new tuning options.

_{Review effort: Balanced}

Workers were joined in creation order, so a failure in a later worker only set the stop flag once the coordinator reached its handle, after joining the slower workers ahead of it. Parts queued behind the failure kept uploading, and the reported error was the first in join order rather than the first to occur. Part selection, cancellation and the first error now live in a shared PartUploadState that the failing worker updates itself; the join loop is only there to surface a panic. Adds unit tests for part claiming, first-error retention, and cancellation being visible while another worker is still running.

Copilot

🟡 Changes recommended

Worker creation failure can panic after multipart initiation and bypass cleanup, leaving an unfinished upload behind.

1 open issue

Handle scoped worker spawn failures without leaking multipart uploads · New

1 resolved since last review

Signal cancellation immediately when any worker fails

_{Review effort: Balanced}

`Scope::spawn` panics when the OS refuses to create a thread. At that point the multipart upload is already open, so unwinding skips the abort path and leaves an incomplete upload behind while `s3_upload` panics. Workers now start through `Builder::spawn_scoped`. A spawn failure is recorded in the pool state like any other part error, no further workers are started, the ones already running stop at their next part boundary, and the regular abort path runs before the error is returned.

Copilot

🔵 Needs a closer look

Independent worker file opens can silently mix source versions when the file path is atomically replaced during startup.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Open source once to prevent mixed-version reads

src/s3/mod.rs:620

Opening the source path independently in each worker can silently mix file versions if the path is atomically replaced during worker startup, such as during backup rotation. With equal-sized versions, all part-length checks pass and the completed object can contain parts from both files. Open the source once and use synchronized range reads or positional I/O for both initial reads and retries; cloning handles alone is insufficient because they share the seek position.

1 resolved since last review

Handle scoped worker spawn failures without leaking multipart uploads

_{Review effort: Balanced}

Each worker opened the source path itself. A writer that replaces the file atomically between two worker startups gives the pool two different versions: all part-length checks still pass when the versions have the same size, and the completed object mixes bytes from both. Opening the path once per upload and reading ranges from that handle keeps every part, the total size and the retry reads on the same file version. Handles cloned from one open would share a seek position, so range reads are serialized through the handle instead. Adds tests for exact-range reads, a short read at the end of the file, and keeping the opened version after the path is replaced.

Copilot

🔵 Needs a closer look

The concurrent upload lifecycle warrants human validation of cancellation, out-of-order completion, and failure cleanup beyond helper tests and reported manual checks.

1 open issue

Correct stale description of shared-handle multipart reads · New

_{Review effort: Balanced}

The comments and the changelog still described one file handle per worker. Since the upload now reads its parts from a single open source, they have to say that range reads are serialized by that handle's lock and the uploads run in parallel after the read returns.

Copilot

🔵 Needs a closer look

Concurrent cancellation and abort behavior warrant human validation beyond the helper tests and reported manual checks.

No open issues 🎉

1 resolved since last review

Correct stale description of shared-handle multipart reads

_{Review effort: Balanced}

Copilot AI balanced review requested due to automatic review settings October 24, 2026 00:17

Copilot started reviewing on behalf of digizeph October 24, 2026 00:17

View session