fix(s3/transfermanager): respect MultipartUploadThreshold in upload decision#3408
Open
nghiack7 wants to merge 1 commit into
Open
fix(s3/transfermanager): respect MultipartUploadThreshold in upload decision#3408nghiack7 wants to merge 1 commit into
nghiack7 wants to merge 1 commit into
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Options.MultipartUploadThresholdis documented and defaults to 16 MB, but the upload decision logic innextReader()never reads it. The threshold used isPartSizeBytes(default 8 MB), so:PutObjectfor files below their configured threshold.MultipartUploadThresholdhas no effect.Reported in #3333.
Root Cause
Fix
Read up to
max(MultipartUploadThreshold, PartSizeBytes)bytes. If all data fits within the threshold (EOF), usePutObject. Otherwise use multipart and re-inject any excess lookahead bytes back into the body viaio.MultiReaderso subsequent part reads remain at the normalPartSizeBytesgranularity.Behaviour Changes
MultipartUploadThresholdPart sizes for multipart uploads are unchanged (each part =
PartSizeBytes).Tests
Updated existing tests to reflect the corrected boundary. Added:
TestUploadSingleBelowThreshold— 8 MB body usesPutObjectwith default 16 MB thresholdTestUploadMultiAboveThreshold— 16 MB+1 body uses multipart with correct 8 MB+8 MB+1 partsTestUploadCustomThreshold— table-driven: below/at/above a 5 MB custom threshold