Pass null part-size to CRT when not configured (#6588)

* pass null part size to CRT when not configured

* add changelog

* fix crt resource management test - wrong final part content-range

* regression test logs

Author: L-Applin

.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"
+}

pom.xml

@@ -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>

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)

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;
     }
 

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 =

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))

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)

test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadStreamingRegressionTesting.java

@@ -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();