Add "CRT_MEMORY_BUFFER_DISABLED" advanced client option #6579
Conversation
…anced client option
|
| Optional.ofNullable(s3NativeClientConfiguration.httpMonitoringOptions()) | ||
| .ifPresent(options::withHttpMonitoringOptions); | ||
| Optional.ofNullable(s3NativeClientConfiguration.memoryBufferDisabled()) | ||
| .ifPresent(memoryBufferDisabled -> options.withFileIoOptions(new FileIoOptions(memoryBufferDisabled, 0.0, false))); |
There was a problem hiding this comment.
Should we be setting the other FileIOptions here? I think the diskThroughputGbps (the 0.0?) applies only shouldStream/memoryBufferDisabled is true - so what if the user has set memoryBufferDisabled=true and we now set the diskThroughput to 0?
Ditto on setting directIO - I assume false is the default and we don't expose configuration for this anywhere else?
There was a problem hiding this comment.
when disk throughput is set to 0, it uses the default value, which is the same as the throughputTargetGbps config. We decided to only expose shouldStream/memoryBufferDisabled so I am just setting the default value s here for he other FileIoOptions as they can't be left null.
The default for directIO is indeed false and yes we don't expose it.
|
|
||
| /** | ||
| * Advanced configuration for the native S3CrtAsyncClient which only applies for multipart uploads. When set to true, | ||
| * the client will skip buffering the part in native memory before sending the request. Default to false on small objects, |
There was a problem hiding this comment.
I find this a little hard to parse and I'm not quite sure I understand the actual behavior... what about:
When set to true the the client will never buffer parts in native memory before sending the request. When set to false the client will buffer parts for large objects. Parts for small objects are never buffered.
| * and true when the object size exceed a certain threshold. When set to true, the client will also skip | ||
| * buffering for small objects. | ||
| */ | ||
| public static final SdkAdvancedAsyncClientOption<Boolean> CRT_MEMORY_BUFFER_DISABLED = |
There was a problem hiding this comment.
Is there as reason this should be on SdkAdvanced options? That seems to imply that it might apply to any async client rather than specifically to S3 CRT only?
Is there a reason to not put it on S3CrtAsyncClientBuilder only? I assume its because we want to "hide" it in "advanced" options rather than increasing the discoverability by having it on the S3 CRT client's builder direclty?
There was a problem hiding this comment.
exactly, we don't want this to be as discoverable as other options. This is what we decided when we discussed about it this week.
2 participants
Add CRT "shouldStream" config as "CRT_MEMORY_BUFFER_DISABLED" SDK advanced client option
Motivation and Context
Add the ability to configure the FileIO config available for the CRT client.
Modifications
Added support for advanced options methods for the
S3CrtAsyncClientBuilderTesting