Sitelet https://github.com/aws/aws-sdk-ruby/pull/3407
Skip to content

fix: Address unbounded memory growth in upload_stream when source outpaces upload - #3407

Merged
richardwang1124 merged 13 commits into
version-3from
fix/upload_stream
Sep 30, 2026
Merged

richardwang1124 merged 13 commits into
version-3from
fix/upload_stream

Conversation

@richardwang1124

@richardwang1124 richardwang1124 commented Jul 9, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #3393.

Description

upload_stream held memory proportional to the whole object when the source wrote faster than parts uploaded. Since #3302, the reader offloads each part into DefaultExecutor, whose unbounded queue let it read ahead of the upload threads without limit (#3393).

  • Streaming a fast local source to a slower sink buffered almost the entire object. In a repro, peak RSS was 1045 MB for a 1 GB source and 2078 MB for 2 GB.
  • IO.copy_stream into a StringIO grows the buffer by doubling, so each 5 MB part held an 8 MB string and fragmented the heap.
  • A part task that raised a non-StandardError (NoMemoryError, SystemStackError, LoadError) killed its worker and was left out of the completed parts. On real S3, upload_stream returned normally and committed a 15 MB object missing part 2 of a 20 MB source. upload_file was protected, because S3 rejects its incomplete completion through mpu_object_size.
  • If every worker died that way, queued tasks never ran and upload_stream and ranged downloads hung.
  • A task rejected by a shared executor shut down mid-upload hung upload_stream and left upload_file's multipart upload in progress with no abort.

Fix

DefaultExecutor accepts an optional max_queue, and upload_stream sets it to thread_count, so post blocks while the queue is full.

# before
executor = DefaultExecutor.new(max_threads: thread_count)
IO.copy_stream(read_pipe, StringIO.new(String.new), @part_size)

# after
executor = DefaultExecutor.new(max_threads: thread_count, max_queue: thread_count)
StringIO.new(read_pipe.read(@part_size))
  • Buffered data is capped at about 2 * thread_count + 1 parts. Peak RSS is about 110 MB regardless of source size, with no change in elapsed time. The tempfile path keeps copy_stream.
  • shutdown and kill close the queue instead of pushing sentinels, and post pushes outside the mutex, so a producer waiting on a full queue can't delay them. Without this, shutdown(1) and kill each took 10 s with a bounded queue.
  • Rejected posts raise DefaultExecutor::RejectedExecutionError, a RuntimeError subclass, and both uploaders abort the multipart upload when a post is rejected.
  • Uploader part tasks treat any exception as a failed part, so the upload aborts with MultipartUploadError instead of completing without the part.
  • A worker killed by its task starts a replacement before exiting. shutdown keeps joining until the pool is empty, ignores errors re-raised by joining a dead worker, then re-raises the first fatal error once all tasks finish.
  • Caller-provided executors are still unbounded, which is now documented on :thread_count. The same FileDownloader issues (a zero-filled hole at the destination on a fatal part error, and rejected-post handling) predate this PR and will be fixed in a follow-up.

Testing

aws-sdk-s3 1681 examples, 0 failures. That includes default_executor 12, multipart_stream_uploader 19, multipart_file_uploader 11 and object/upload_stream 5. Each new spec fails with its fix reverted. Repro scripts were run against version-3 and this branch for memory, rejected posts, worker death and non-StandardError part failures. The missing-part case was also checked on real S3.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

  1. To make sure we include your contribution in the release notes, please make sure to add description entry for your changes in the "unreleased changes" section of the CHANGELOG.md file (at corresponding gem). For the description entry, please make sure it lives in one line and starts with Feature or Issue in the correct format.

  2. For generated code changes, please checkout below instructions first:
    https://github.com/aws/aws-sdk-ruby/blob/version-3/CONTRIBUTING.md

Thank you for your contribution!

@richardwang1124
richardwang1124 requested a review from a team as a code owner July 9, 2026 17:53
@github-actions

github-actions Bot commented Jul 9, 2026 •

Copy link
Copy Markdown

Detected 12 possible performance regressions:

  • aws-sdk-cloudwatchlogs.put_log_events_small_allocated_kb - z-score regression: 56.66 -> 57.15. Z-score: Infinity
  • aws-sdk-cloudwatchlogs.put_log_events_large_allocated_kb - z-score regression: 5990.62 -> 5991.11. Z-score: Infinity
  • aws-sdk-cloudwatchlogs.get_log_events_small_allocated_kb - z-score regression: 52.17 -> 52.66. Z-score: Infinity
  • aws-sdk-cloudwatchlogs.get_log_events_large_allocated_kb - z-score regression: 7607.98 -> 7608.48. Z-score: Infinity
  • aws-sdk-cloudwatchlogs.filter_log_events_large_allocated_kb - z-score regression: 7804.05 -> 7804.54. Z-score: Infinity
  • aws-sdk-cloudwatch.put_metric_data_small_allocated_kb - z-score regression: 48.0 -> 48.51. Z-score: 23.69
  • aws-sdk-dynamodb.get_item_small_allocated_kb - z-score regression: 72.34 -> 72.83. Z-score: Infinity
  • aws-sdk-dynamodb.get_item_large_allocated_kb - z-score regression: 19164.15 -> 19164.64. Z-score: Infinity
  • aws-sdk-dynamodb.put_item_small_allocated_kb - z-score regression: 79.71 -> 80.21. Z-score: Infinity
  • aws-sdk-dynamodb.put_item_large_allocated_kb - z-score regression: 11937.5 -> 11937.99. Z-score: Infinity
  • aws-sdk-kinesis.put_record_small_allocated_kb - z-score regression: 47.95 -> 48.44. Z-score: Infinity
  • aws-sdk-kinesis.put_record_large_allocated_kb - z-score regression: 163.97 -> 164.46. Z-score: Infinity

@jterapin jterapin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up! this is a real gap and a good catch. I've left a few questions on the current implementation inline. We're also missing a CHANGELOG entry, so we'll want to add one before this merges.

Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/default_executor.rb Outdated
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/default_executor.rb Outdated
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_stream_uploader.rb

@jterapin jterapin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall approach makes sense. I would like to see a fuller PR description for historical purposes (aside from the issue link).

Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/default_executor.rb Outdated
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/customizations/object.rb
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_stream_uploader.rb
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/default_executor.rb
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/customizations/object.rb
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/default_executor.rb Outdated
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb Outdated
Comment thread gems/aws-sdk-s3/CHANGELOG.md Outdated
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb
Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/default_executor.rb
@oriolbcn

Copy link
Copy Markdown

Are there plans to complete and merge the fix soon? We are experiencing OOM kills in our production system due to it.

@jterapin jterapin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I approved on a condition that we will need to amend the changelog entries.
Additionally, we should create a new ticket to do a fast-follow on the file downloader as well. (also double check any classes that utilizes executors).

Comment thread gems/aws-sdk-s3/lib/aws-sdk-s3/default_executor.rb
Comment thread gems/aws-sdk-s3/CHANGELOG.md Outdated
@richardwang1124
richardwang1124 merged commit a56ab3f into version-3 Sep 30, 2026
51 of 52 checks passed
@richardwang1124
richardwang1124 deleted the fix/upload_stream branch September 30, 2026 21:34
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.

Object#upload_stream retains unbounded memory when the source outpaces the upload

4 participants