reduce the multipart part size on the opendal backends - #6779
Open
DeviousCardi wants to merge 1 commit into
Open
reduce the multipart part size on the opendal backends#6779DeviousCardi wants to merge 1 commit into
DeviousCardi wants to merge 1 commit into
Conversation
OpenDAL buffers a whole part before flushing it, so the target part size is also the peak memory a single upload holds. At the shared default of 5 GiB a large split is sent as one part and resides in memory in full. The S3 implementation streams each part straight from the payload, so a larger part there only buys fewer billed PUT requests and costs no memory. Lower the part size for the OpenDAL backends alone rather than the shared default, which would raise S3 request costs for no benefit.
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
1 participant
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.
Description
Closes #6494.
OpendalStorage::putpasses the multipart part size towriter_with(..).chunk(..), and OpenDAL buffers a whole chunk before flushing it. The part size is therefore also the peak amount of memory a single upload holds. At the sharedMultiPartPolicy::default()target of 5 GiB, a large split is uploaded as one part and stays in memory in full.The S3 implementation does not have this exposure:
upload_partbuilds each part withpayload.range_byte_stream(part.range), so the part is streamed from the payload and never held. There, a larger part only means fewer parts and so fewer billed PUT requests.Because the two backends pay different prices for the same setting, this lowers the part size for the OpenDAL backends alone rather than changing
MultiPartPolicy::default(). Changing the shared default would increase PUT request costs on S3 without saving any memory there.The other limits are left as they are, since they describe what the backend accepts rather than how much is buffered.
How was this PR tested?
Three unit tests in
opendal_storage::base:opendal_policy_bounds_the_part_size— a 4 GiB split is a single 5 GiB part under the shared default and 1 GiB parts under the OpenDAL policy.opendal_policy_keeps_the_other_limits— threshold, maximum part count, maximum object size and upload concurrency still match the default.opendal_policy_leaves_small_uploads_single_part— an upload below the multipart threshold is still sent whole.make fmtis clean.One thing worth a maintainer's opinion: 1 GiB is the value suggested on the issue. It bounds an upload to a fifth of what it could previously hold while keeping the request count low, but if you would rather trade more PUT requests for a smaller ceiling, the constant is the only thing that needs to change.
Generated with Claude Opus 5, then reviewed and tested locally before submission.