Skip to content

fix(s3stream): preserve pending multipart buffers through copy and release - #3652

Open
ashsamantaray-stripe wants to merge 3 commits into
AutoMQ:mainfrom
ashsamantaray-stripe:ashsamantaray/fix-proxywriter-review-upstream
Open

ashsamantaray-stripe wants to merge 3 commits into
AutoMQ:mainfrom
ashsamantaray-stripe:ashsamantaray/fix-proxywriter-review-upstream

Conversation

@ashsamantaray-stripe

@ashsamantaray-stripe ashsamantaray-stripe commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Why

Upstream #3528 followed the original #3527 and is already in this PR's base. It replaced the deferred handoff and addressed several review concerns before this PR was opened.

ProxyWriter replays small copy and write operations into MultiPartWriter once the object exceeds the single-upload threshold. A range read for the current multipart part can still be pending when the writer is released. MultiPartWriter.release() currently frees that part buffer and completes before the read finishes, so the read can later append to a released buffer. A write queued behind the read also remains backed by the caller's buffer after copyOnWrite() returns.

Replay can also leave a 28 MiB write buffered before an 8 MiB copy. The copy path then subtracts 28 MiB from the minimum part size and constructs an invalid range end.

Repeated PutObject close calls can submit an empty overwrite. Release can also return while a PutObject replay still owns buffers and its upload is pending.

What

Already fixed by upstream #3528

  • Replace the deferred buffered handoff with ordered write/copy replay, preserving byte order across multipart selection.
  • Release queued writes when an earlier range read fails, and snapshot pending writes before multipart selection on copyOnWrite().
  • Avoid the oversized handoff behind the seven 4 MiB copies plus 8 MiB copy invalid-range case. Remove the inaccurate claim that S3 cannot copy a small final part.

Fixed in this PR

  • Wait for pending multipart reads before releasing the current part's buffer and completing cleanup.
  • Snapshot writes queued behind multipart reads, including when the part was submitted before the read completed.
  • Flush an already-large buffered write before deriving a copy range, covering the 28 MiB write plus 8 MiB copy case.
  • Make PutObject close() idempotent and make release() wait for active replay and upload cleanup.
  • Add regression tests for both the already-fixed replay boundary and the new cases; clarify that reads are serialized within each part.

How

Release joins the already submitted part uploads with cleanup of the current, unsubmitted part. The current part releases its buffer only after its ordered range-read chain finishes. Pending writes retain their own buffer reference until that chain appends or rejects them; copyOnWrite() replaces those references with private copies. Access to the part buffer and pending-write list is synchronized across caller and range-read completion threads.

When a part is submitted while a range read is pending, it snapshots cached buffers before the writer drops its reference to that part. Copy replay flushes an existing part that already meets the minimum size before deriving a new copy range.

The PutObject close path returns the same future on later calls. Release waits for that future, which completes after the replay and storage write have cleaned up their owned buffers.

Reviewer notes

Start with MultiPartWriter.ObjectPart for pending buffer ownership, then MultiPartWriter.release() for cleanup completion and copyWrite0() for the range boundary. ProxyWriter.close() and release() cover the PutObject lifecycle. ProxyWriterTest exercises both paths.

Verification: ./gradlew --build-cache metadata:S3UnitTest core:S3UnitTest s3stream:test; ./gradlew --build-cache rat checkstyleMain checkstyleTest spotlessJavaCheck; ./gradlew :s3stream:spotbugsMain.

…lease

Committed-By-Agent: codex
Co-authored-by: codex <noreply@openai.com>
ashsamantaray-stripe and others added 2 commits October 12, 2026 14:59
…arts

Committed-By-Agent: codex
Co-authored-by: codex <noreply@openai.com>
Committed-By-Agent: codex
Co-authored-by: codex <noreply@openai.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant