Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve S3 upload speeds using aws transfer manager #17674

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Improve S3 upload speeds using aws transfer manager #17674

wants to merge 5 commits into from

Conversation

suraj-goel
Copy link
Contributor

@suraj-goel suraj-goel commented Jan 28, 2025 •
edited
Loading

Fixes #17669.

Description

  • Improve S3 Upload speeds using AWS Transfer Manager.
  • Benchmark Results:

Screenshot 2025-01-28 at 11 45 04 AM

  • As per the benchmark results, minimumUploadPartSize/multipartUploadThreshold should be set to 20 MB for best performance.

Release note

Improved S3 Upload Performance in Druid

  • Integrated S3 Transfer Manager to enhance the speed and efficiency of S3 uploads within Druid. This update significantly reduces upload times of segments to S3.
  • The feature can be enabled by adding following configs in common.runtime.properties
    druid.storage.transfer.useTransferManager=true
    druid.storage.transfer.minimumUploadPartSize=20971520
    druid.storage.transfer.multipartUploadThreshold=20971520

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 28, 2025
View reviewed changes
...ons-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentMoverTest.java
@@ -201,7 +201,7 @@

private MockAmazonS3Client()
{
super(new AmazonS3Client(), new NoopServerSideEncryption());
super(new AmazonS3Client(), new NoopServerSideEncryption(), new S3TransferConfig());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note test

Invoking
AmazonS3Client.AmazonS3Client
Loading
should be avoided because it has been deprecated.
@suraj-goel suraj-goel marked this pull request as ready for review January 28, 2025 07:01
cryptoe
cryptoe reviewed Feb 25, 2025
View reviewed changes
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3Utils.java
@@ -96,6 +96,9 @@ public boolean apply(Throwable e)
// This can happen sometimes when AWS isn't able to obtain the credentials for some service:
// https://github.com/aws/aws-sdk-java/issues/2285
return true;
} else if (e instanceof InterruptedException) {
Thread.interrupted(); // Clear interrupted state and not retry
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to clear the interrupted flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention is to gracefully stop the retry operations.
Although, The flag would already be cleared when the InterruptedException is thrown.

Let me know if your suggestion is to modify it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cryptoe cryptoe cryptoe left review comments

Assignees
No one assigned
Labels
Area - Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add the capability to speed up S3 uploads using AWS transfer manager
2 participants
@suraj-goel @cryptoe