diff --git a/.changes/next-release/feature-s3-3d1dcf9.json b/.changes/next-release/feature-s3-3d1dcf9.json new file mode 100644 index 00000000000..8351169e98a --- /dev/null +++ b/.changes/next-release/feature-s3-3d1dcf9.json @@ -0,0 +1,6 @@ +{ + "type": "feature", + "category": "s3", + "contributor": "", + "description": "Pass null part-size to CRT when not configured" +} diff --git a/pom.xml b/pom.xml index a73b642367f..75dc5be34cc 100644 --- a/pom.xml +++ b/pom.xml @@ -129,7 +129,7 @@ 3.1.5 1.17.1 1.37 - 0.39.4 + 0.40.1 5.10.0 diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java index 1963e7e7b35..11f318cbff1 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java @@ -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) @@ -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) diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3NativeClientConfiguration.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3NativeClientConfiguration.java index e3c5bea0641..702503cb7b6 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3NativeClientConfiguration.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3NativeClientConfiguration.java @@ -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; @@ -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; @@ -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); @@ -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(); @@ -164,7 +176,7 @@ public TlsContext tlsContext() { return tlsContext; } - public long partSizeBytes() { + public Long partSizeBytes() { return partSizeInBytes; } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java index 8f69ff5b480..9b29569aaff 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java @@ -509,6 +509,25 @@ void build_partSizeConfigured_shouldApplyToThreshold() { } } + @Test + public void build_noPartSize_shouldUseDefaultsForThresholdAndReadWindowSize() { + 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); + assertThat(clientOptions.getMultiPartUploadThreshold()).isEqualTo(defaultPartSizeInBytes); + assertThat(clientOptions.getInitialReadWindowSize()).isEqualTo(defaultPartSizeInBytes * 10); + } + } + @Test void build_nullHttpConfiguration() { S3NativeClientConfiguration configuration = diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtClientGetObjectResourceManagementTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtClientGetObjectResourceManagementTest.java index 6de8ae6f738..d18d93318f1 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtClientGetObjectResourceManagementTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtClientGetObjectResourceManagementTest.java @@ -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)) diff --git a/test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadConfig.java b/test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadConfig.java index cc197a22531..e79dcb96082 100644 --- a/test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadConfig.java +++ b/test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadConfig.java @@ -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) diff --git a/test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadStreamingRegressionTesting.java b/test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadStreamingRegressionTesting.java index aa33a24a6c9..0c06d3474c4 100644 --- a/test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadStreamingRegressionTesting.java +++ b/test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadStreamingRegressionTesting.java @@ -176,6 +176,7 @@ protected TestCallable 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();