Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void setPayloadSigning(boolean payloadSigning) {

@Override
public String toString() {
return ToString.builder("FlattenUploadConfig")
return ToString.builder("UploadConfig")
.add("bucketType", bucketType)
.add("forcePathStyle", forcePathStyle)
.add("requestChecksumValidation", requestChecksumValidation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ protected TestCallable<PutObjectResponse> callTmUpload(PutObjectRequest request,
CompletedUpload completedUpload = CompletableFutureUtils.joinLikeSync(upload.completionFuture());
return completedUpload.response();
} catch (Exception e) {
LOG.error(() -> "Upload failed for " + config.toString());
throw new RuntimeException(e);
} finally {
transferManager.close();
Expand Down
Loading