-
Notifications
You must be signed in to change notification settings - Fork 1k
Bump CRT version and expose setting memory limits for S3 calls #4885
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.
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
Changes from 2 commits
43a1cae
387853a
b2a47f2
7791d63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "type": "feature", | ||
| "category": "AWS CRT-based S3 Client", | ||
| "contributor": "", | ||
| "description": "Bump `aws-crt` version to `0.29.9`" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "type": "feature", | ||
| "category": "Amazon S3", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be |
||
| "contributor": "", | ||
| "description": "Exposes a setting to set the memory limit when making asynchronous calls with the CRT-based S3 client" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,12 +114,30 @@ default S3CrtAsyncClientBuilder credentialsProvider(IdentityProvider<? extends A | |
| */ | ||
| S3CrtAsyncClientBuilder minimumPartSizeInBytes(Long uploadPartSize); | ||
|
|
||
| /** | ||
| * The amount of memory that CRT is allowed to use when making requests to S3. | ||
| * <p> | ||
| * If not provided, the CRT attempts to limit memory usage in an optimal way, based on a number of parameters | ||
| * such as target throughput. Therefore, only configure the memory limit explicitly when needed. | ||
| * <p> | ||
| * Supported range: | ||
| * <ul> | ||
| * <li><b>Min: </b>1 GB</li> | ||
| * <li><b>Max: </b>The lowest value of the supplied value and the SIZE_MAX of the system</li> | ||
| * </ul> | ||
| * | ||
| * @param memoryLimitInBytes the memory limit in bytes | ||
| * @return this builder for method chaining. | ||
| * @see #targetThroughputInGbps(Double) | ||
| */ | ||
| S3CrtAsyncClientBuilder memoryLimitInBytes(Long memoryLimitInBytes); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion |
||
|
|
||
| /** | ||
| * The target throughput for transfer requests. Higher value means more connections will be established with S3. | ||
| * | ||
| * <p> | ||
| * Whether the transfer manager can achieve the configured target throughput depends on various factors such as the network | ||
| * bandwidth of the environment and whether {@link #maxConcurrency} is configured. | ||
| * bandwidth of the environment, whether {@link #maxConcurrency} is configured and amount of available memory. | ||
| * | ||
| * <p> | ||
| * By default, it is 10 gigabits per second. If users want to transfer as fast as possible, it's recommended to set it to the | ||
|
|
@@ -128,10 +146,14 @@ default S3CrtAsyncClientBuilder credentialsProvider(IdentityProvider<? extends A | |
| * instance type in <a href="https://aws.amazon.com/ec2/instance-types/">Amazon EC2 instance type page</a>. | ||
| * If you are running into out of file descriptors error, consider using {@link #maxConcurrency(Integer)} to limit the | ||
| * number of connections. | ||
| * <p> | ||
| * <b>Note: </b> this setting affects the memory usage of CRT; a higher throughput value will result in a larger memory | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, will update |
||
| * usage. Typically, a range of throughput values maps to a discrete memory limit value in CRT, with a maximum upper limit. | ||
| * | ||
| * @param targetThroughputInGbps the target throughput in Gbps | ||
| * @return this builder for method chaining. | ||
| * @see #maxConcurrency(Integer) | ||
| * @see #memoryLimitInBytes(Long) | ||
| */ | ||
| S3CrtAsyncClientBuilder targetThroughputInGbps(Double targetThroughputInGbps); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically affects other CRT related components like signing right? Might be better just as "AWS SDK for Java v2"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at other entries in the changelog, but perhaps we're not consistent - not surprising.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I thought it only affects s3 client, is that not the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh nvm, this is version bump, yeah, should be
AWS SDK for Java v2