Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/feature-s3-3d1dcf9.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "feature",
"category": "s3",
"contributor": "",
"description": "Pass null part-size to CRT when not configured"
}
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
<rxjava3.version>3.1.5</rxjava3.version>
<commons-codec.verion>1.17.1</commons-codec.verion>
<jmh.version>1.37</jmh.version>
<awscrt.version>0.39.4</awscrt.version>
<awscrt.version>0.40.1</awscrt.version>

<!--Test dependencies -->
<junit5.version>5.10.0</junit5.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ private S3ClientOptions createS3ClientOption() {
.withCredentialsProvider(s3NativeClientConfiguration.credentialsProvider())
.withClientBootstrap(s3NativeClientConfiguration.clientBootstrap())
.withTlsContext(s3NativeClientConfiguration.tlsContext())
.withPartSize(s3NativeClientConfiguration.partSizeBytes())
.withMultipartUploadThreshold(s3NativeClientConfiguration.thresholdInBytes())
.withComputeContentMd5(false)
.withEnableS3Express(true)
Expand All @@ -121,6 +120,7 @@ private S3ClientOptions createS3ClientOption() {
if (Boolean.FALSE.equals(s3NativeClientConfiguration.isUseEnvironmentVariableValues())) {
options.withProxyEnvironmentVariableSetting(disabledHttpProxyEnvironmentVariableSetting());
}
Optional.ofNullable(s3NativeClientConfiguration.partSizeBytes()).ifPresent(options::withPartSize);
Optional.ofNullable(s3NativeClientConfiguration.proxyOptions()).ifPresent(options::withProxyOptions);
Optional.ofNullable(s3NativeClientConfiguration.connectionTimeout())
.map(Duration::toMillis)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class S3NativeClientConfiguration implements SdkAutoCloseable {
private final ClientBootstrap clientBootstrap;
private final CrtCredentialsProviderAdapter credentialProviderAdapter;
private final CredentialsProvider credentialsProvider;
private final long partSizeInBytes;
private final Long partSizeInBytes;
private final long thresholdInBytes;
private final double targetThroughputInGbps;
private final int maxConcurrency;
Expand Down Expand Up @@ -88,10 +88,8 @@ public S3NativeClientConfiguration(Builder builder) {

this.credentialsProvider = credentialProviderAdapter.crtCredentials();

this.partSizeInBytes = builder.partSizeInBytes == null ? DEFAULT_PART_SIZE_IN_BYTES :
builder.partSizeInBytes;
this.thresholdInBytes = builder.thresholdInBytes == null ? this.partSizeInBytes :
builder.thresholdInBytes;
this.partSizeInBytes = builder.partSizeInBytes;
this.thresholdInBytes = resolveThresholdInBytes(builder);
this.targetThroughputInGbps = builder.targetThroughputInGbps == null ?
DEFAULT_TARGET_THROUGHPUT_IN_GBPS : builder.targetThroughputInGbps;

Expand All @@ -101,8 +99,7 @@ public S3NativeClientConfiguration(Builder builder) {

this.endpointOverride = builder.endpointOverride;

this.readBufferSizeInBytes = builder.readBufferSizeInBytes == null ?
partSizeInBytes * 10 : builder.readBufferSizeInBytes;
this.readBufferSizeInBytes = resolveReadBufferSizeInBytes(builder);

if (builder.httpConfiguration != null) {
this.proxyOptions = resolveProxy(builder.httpConfiguration.proxyConfiguration(), tlsContext).orElse(null);
Expand All @@ -120,6 +117,21 @@ public S3NativeClientConfiguration(Builder builder) {
builder.advancedOptions == null ? null : builder.advancedOptions.get(CRT_MEMORY_BUFFER_DISABLED);
}

private Long resolveReadBufferSizeInBytes(Builder builder) {
if (builder.readBufferSizeInBytes != null) {
return builder.readBufferSizeInBytes;
}
long partSize = this.partSizeInBytes == null ? DEFAULT_PART_SIZE_IN_BYTES : this.partSizeInBytes;
return partSize * 10;
}

private long resolveThresholdInBytes(Builder builder) {
if (builder.thresholdInBytes != null) {
return builder.thresholdInBytes;
}
return this.partSizeInBytes == null ? DEFAULT_PART_SIZE_IN_BYTES : this.partSizeInBytes;
}

private static Boolean resolveUseEnvironmentVariableValues(Builder builder) {
if (builder != null && builder.httpConfiguration != null && builder.httpConfiguration.proxyConfiguration() != null) {
return builder.httpConfiguration.proxyConfiguration().isUseEnvironmentVariableValues();
Expand Down Expand Up @@ -164,7 +176,7 @@ public TlsContext tlsContext() {
return tlsContext;
}

public long partSizeBytes() {
public Long partSizeBytes() {
return partSizeInBytes;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,25 @@
}
}

@Test
public void build_noPartSize_shouldUseDefaultsForThresholdAndReadWindowSize() {

Check warning on line 513 in services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this 'public' modifier.

See more on https://sonarcloud.io/project/issues?id=aws_aws-sdk-java-v2&issues=AZrBtrD2sBPLoUX8gQCH&open=AZrBtrD2sBPLoUX8gQCH&pullRequest=6588
S3NativeClientConfiguration configuration =
S3NativeClientConfiguration.builder()
.credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create("test",
"test")))
.signingRegion("us-west-2")
.build();
try (S3CrtAsyncHttpClient client =
(S3CrtAsyncHttpClient) S3CrtAsyncHttpClient.builder()
.s3ClientConfiguration(configuration).build()) {
long defaultPartSizeInBytes = 1024 * 1024L * 8L;
S3ClientOptions clientOptions = client.s3ClientOptions();
assertThat(clientOptions.getPartSize()).isEqualTo(0);

Check warning on line 525 in services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use isZero() instead.

See more on https://sonarcloud.io/project/issues?id=aws_aws-sdk-java-v2&issues=AZrBtrD2sBPLoUX8gQCI&open=AZrBtrD2sBPLoUX8gQCI&pullRequest=6588
assertThat(clientOptions.getMultiPartUploadThreshold()).isEqualTo(defaultPartSizeInBytes);
assertThat(clientOptions.getInitialReadWindowSize()).isEqualTo(defaultPartSizeInBytes * 10);
}
}

@Test
void build_nullHttpConfiguration() {
S3NativeClientConfiguration configuration =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ private static void stubGetObjectCalls() {
}

// final part
String contentRange = "bytes " + PART_SIZE * numOfParts + "-" + (totalContentSize - 1) + "/" + totalContentSize;
String contentRange = "bytes " + PART_SIZE * (numOfParts - 1) + "-" + (totalContentSize - 1) + "/" + totalContentSize;
String range = "bytes=" + PART_SIZE * (numOfParts - 1) + "-" + (totalContentSize - 1);
stubFor(get(anyUrl()).withHeader("Range", equalTo(range)).willReturn(aResponse().withStatus(200)
.withHeader("content-length", String.valueOf(finalPartSize))
Expand Down
Loading