Skip to content

fix(s3/transfermanager): respect MultipartUploadThreshold in upload decision#3408

Open
nghiack7 wants to merge 1 commit into
aws:mainfrom
nghiack7:fix/s3-multipart-upload-threshold
Open

fix(s3/transfermanager): respect MultipartUploadThreshold in upload decision#3408
nghiack7 wants to merge 1 commit into
aws:mainfrom
nghiack7:fix/s3-multipart-upload-threshold

Conversation

@nghiack7
Copy link
Copy Markdown

@nghiack7 nghiack7 commented May 9, 2026

Problem

Options.MultipartUploadThreshold is documented and defaults to 16 MB, but the upload decision logic in nextReader() never reads it. The threshold used is PartSizeBytes (default 8 MB), so:

  • Files between 8 MB and 16 MB are uploaded via multipart even though the caller expects a single PutObject for files below their configured threshold.
  • Setting a custom MultipartUploadThreshold has no effect.

Reported in #3333.

Root Cause

// Before: threshold is PartSizeBytes, MultipartUploadThreshold ignored
r := io.LimitReader(u.in.Body, u.options.PartSizeBytes)
firstPart, _ := io.ReadAll(r)
if int64(len(firstPart)) < u.options.PartSizeBytes {
    return ..., io.EOF  // → PutObject
}
return ..., nil  // → multipart

Fix

Read up to max(MultipartUploadThreshold, PartSizeBytes) bytes. If all data fits within the threshold (EOF), use PutObject. Otherwise use multipart and re-inject any excess lookahead bytes back into the body via io.MultiReader so subsequent part reads remain at the normal PartSizeBytes granularity.

// After: use MultipartUploadThreshold as the decision boundary
readLimit := u.options.MultipartUploadThreshold
if u.options.PartSizeBytes > readLimit {
    readLimit = u.options.PartSizeBytes
}
lookahead, _ := io.ReadAll(io.LimitReader(u.in.Body, readLimit))
if int64(len(lookahead)) < u.options.MultipartUploadThreshold {
    return ..., io.EOF  // → PutObject
}
// re-inject overflow so part sizes stay at PartSizeBytes
if int64(len(lookahead)) > u.options.PartSizeBytes {
    overflow := lookahead[u.options.PartSizeBytes:]
    u.in.Body = io.MultiReader(bytes.NewReader(overflow), u.in.Body)
    lookahead = lookahead[:u.options.PartSizeBytes]
}
return ..., nil  // → multipart

Behaviour Changes

File size Before After
< PartSizeBytes (8 MB) PutObject PutObject
PartSizeBytes ≤ size < MultipartUploadThreshold (8–16 MB) Multipart (bug) PutObject (fixed)
≥ MultipartUploadThreshold (16 MB) Multipart Multipart
Custom MultipartUploadThreshold Ignored Respected

Part sizes for multipart uploads are unchanged (each part = PartSizeBytes).

Tests

Updated existing tests to reflect the corrected boundary. Added:

  • TestUploadSingleBelowThreshold — 8 MB body uses PutObject with default 16 MB threshold
  • TestUploadMultiAboveThreshold — 16 MB+1 body uses multipart with correct 8 MB+8 MB+1 parts
  • TestUploadCustomThreshold — table-driven: below/at/above a 5 MB custom threshold
ok github.com/aws/aws-sdk-go-v2/feature/s3/transfermanager 7.2s

…ecision

Options.MultipartUploadThreshold was documented and defaulted to 16 MB but
was never read; the upload path used PartSizeBytes (8 MB) as the sole
threshold. Files between 8 MB and 16 MB were therefore uploaded via multipart
even though callers expected single PutObject for files below their chosen
threshold.

Fix: expand the initial lookahead read to max(MultipartUploadThreshold,
PartSizeBytes) bytes. If all data arrives before the threshold is reached
(EOF), fall back to PutObject. When the threshold is exceeded, re-inject any
lookahead bytes beyond the first PartSizeBytes back into the body reader via
io.MultiReader so that subsequent part reads remain at the expected PartSizeBytes
granularity — preserving existing part-size behaviour for callers who rely on
consistent chunk sizes.

Updated tests to reflect the corrected decision boundary and added three new
tests: TestUploadSingleBelowThreshold, TestUploadMultiAboveThreshold, and
TestUploadCustomThreshold (table-driven, covers below/at/above a 5 MB
custom threshold).

Fixes aws#3333
@nghiack7 nghiack7 requested a review from a team May 9, 2026 05:58
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