Skip to content

fix(s3/transfermanager): use MultipartUploadThreshold for upload decision#3336

Open
abhu85 wants to merge 2 commits intoaws:mainfrom
abhu85:fix/transfermanager-multipart-upload-threshold
Open

fix(s3/transfermanager): use MultipartUploadThreshold for upload decision#3336
abhu85 wants to merge 2 commits intoaws:mainfrom
abhu85:fix/transfermanager-multipart-upload-threshold

Conversation

@abhu85
Copy link
Copy Markdown

@abhu85 abhu85 commented Mar 2, 2026

Summary

Fixes #3333 - MultipartUploadThreshold was defined but never used in upload decision logic.

Problem

The MultipartUploadThreshold field in transfermanager.Options was defined and resolved with a default value (16 MiB), but it was never actually referenced in the upload decision logic. The single-upload vs multipart-upload decision was solely determined by PartSizeBytes, making MultipartUploadThreshold dead code with no effect on behavior.

Solution

Use min(MultipartUploadThreshold, PartSizeBytes) as the cutoff for single vs multipart upload decision in nextReader(). This ensures:

  1. MultipartUploadThreshold now works: Setting a lower threshold (e.g., 1KB) will trigger multipart uploads for smaller files
  2. Backward compatibility: With default values (PartSizeBytes=8MB, MultipartUploadThreshold=16MB), the effective threshold becomes 8MB - identical to current behavior since the threshold was previously dead code
  3. No data truncation: The threshold is capped to PartSizeBytes since we can only observe that much data in the first read

Changes

  • api_op_UploadObject.go: Modified nextReader() to use threshold-based decision
  • api_op_UploadObject_test.go: Added 3 new tests for threshold behavior

Test Plan

  • TestUploadMultipartTriggeredByThreshold - verifies low threshold triggers multipart
  • TestUploadSingleWhenBelowThreshold - verifies files below threshold use single upload
  • TestUploadThresholdCappedByPartSize - verifies backward compatibility when threshold > partSize
  • All 33 existing upload tests pass
  • Full transfermanager test suite passes

Reproduction

Before fix:

// Set threshold to 1KB — expect multipart upload for anything > 1KB
tm := transfermanager.New(s3Client, func(o *transfermanager.Options) {
    o.MultipartUploadThreshold = 1024 // 1 KB
    o.PartSizeBytes = 5 * 1024 * 1024 // 5 MB
})
// Upload 100KB — SHOULD use multipart (> 1KB threshold)
// but ACTUALLY uses single PutObject (< 5MB PartSizeBytes)

After fix:

// Same config — now works as expected
// 100KB upload uses multipart because 100KB > 1KB threshold

🤖 Generated with Claude Code

…sion

The MultipartUploadThreshold option was defined but never referenced in
the upload decision logic. The single-upload vs multipart-upload decision
was solely based on PartSizeBytes, making MultipartUploadThreshold dead
code with no effect on behavior.

This fix uses min(MultipartUploadThreshold, PartSizeBytes) as the cutoff
for the single vs multipart upload decision. This allows users to set a
lower threshold to trigger multipart uploads earlier.

With default values (PartSizeBytes=8MB, MultipartUploadThreshold=16MB),
the effective threshold becomes 8MB, which is identical to the current
behavior since the threshold was previously dead code.

Fixes aws#3333

Signed-off-by: abhu85 <60182103+abhu85@users.noreply.github.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: abhu85 <60182103+abhu85@users.noreply.github.com>
@abhu85 abhu85 requested a review from a team as a code owner March 2, 2026 14: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.

S3 Transfer Manager v2: Options.MultipartUploadThreshold is defined but never referenced in upload decision logic

1 participant