From 4be9eea1b7f515d178ef1a637f4d6a47850e56d7 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Wed, 9 Aug 2023 15:16:25 -0700 Subject: [PATCH 1/3] Fixed the issue where thresholdInBytes is not the same as minimalPartSizeInBytes if a custom minimalPartSizeInBytes is provided. --- .../MultipartConfigurationResolver.java | 53 +++++++++++++ .../multipart/MultipartS3AsyncClient.java | 19 +---- .../s3/multipart/MultipartConfiguration.java | 16 ++-- .../MultipartConfigurationResolverTest.java | 74 +++++++++++++++++++ 4 files changed, 142 insertions(+), 20 deletions(-) create mode 100644 services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolver.java create mode 100644 services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolverTest.java diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolver.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolver.java new file mode 100644 index 000000000000..7159f3bd20d5 --- /dev/null +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolver.java @@ -0,0 +1,53 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.services.s3.internal.multipart; + +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.services.s3.multipart.MultipartConfiguration; +import software.amazon.awssdk.utils.Validate; + +/** + * Internal utility class to resolve {@link MultipartConfiguration}. + */ +@SdkInternalApi +public final class MultipartConfigurationResolver { + + private static final long DEFAULT_MIN_PART_SIZE = 8L * 1024 * 1024; + private final MultipartConfiguration configuration; + private final long minimalPartSizeInBytes; + private final long apiCallBufferSize; + private final long thresholdInBytes; + + public MultipartConfigurationResolver(MultipartConfiguration multipartConfiguration) { + this.configuration = multipartConfiguration; + this.minimalPartSizeInBytes = Validate.getOrDefault(configuration.minimumPartSizeInBytes(), () -> DEFAULT_MIN_PART_SIZE); + this.apiCallBufferSize = Validate.getOrDefault(configuration.apiCallBufferSizeInBytes(), + () -> minimalPartSizeInBytes * 4); + this.thresholdInBytes = Validate.getOrDefault(configuration.thresholdInBytes(), () -> minimalPartSizeInBytes); + } + + public long minimalPartSizeInBytes() { + return minimalPartSizeInBytes; + } + + public long thresholdInBytes() { + return thresholdInBytes; + } + + public long apiCallBufferSize() { + return apiCallBufferSize; + } +} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartS3AsyncClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartS3AsyncClient.java index 65b26ddec971..98c55259d44d 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartS3AsyncClient.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartS3AsyncClient.java @@ -46,10 +46,6 @@ public final class MultipartS3AsyncClient extends DelegatingS3AsyncClient { private static final ApiName USER_AGENT_API_NAME = ApiName.builder().name("hll").version("s3Multipart").build(); - private static final long DEFAULT_MIN_PART_SIZE = 8L * 1024 * 1024; - private static final long DEFAULT_THRESHOLD = 8L * 1024 * 1024; - private static final long DEFAULT_API_CALL_BUFFER_SIZE = DEFAULT_MIN_PART_SIZE * 4; - private final UploadObjectHelper mpuHelper; private final CopyObjectHelper copyObjectHelper; @@ -57,21 +53,14 @@ private MultipartS3AsyncClient(S3AsyncClient delegate, MultipartConfiguration mu super(delegate); MultipartConfiguration validConfiguration = Validate.getOrDefault(multipartConfiguration, MultipartConfiguration.builder()::build); - long minPartSizeInBytes = Validate.getOrDefault(validConfiguration.minimumPartSizeInBytes(), - () -> DEFAULT_MIN_PART_SIZE); - long threshold = Validate.getOrDefault(validConfiguration.thresholdInBytes(), - () -> DEFAULT_THRESHOLD); - long apiCallBufferSizeInBytes = Validate.getOrDefault(validConfiguration.apiCallBufferSizeInBytes(), - () -> computeApiCallBufferSize(validConfiguration)); + MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(validConfiguration); + long minPartSizeInBytes = resolver.minimalPartSizeInBytes(); + long threshold = resolver.thresholdInBytes(); + long apiCallBufferSizeInBytes = resolver.apiCallBufferSize(); mpuHelper = new UploadObjectHelper(delegate, minPartSizeInBytes, threshold, apiCallBufferSizeInBytes); copyObjectHelper = new CopyObjectHelper(delegate, minPartSizeInBytes, threshold); } - private long computeApiCallBufferSize(MultipartConfiguration multipartConfiguration) { - return multipartConfiguration.minimumPartSizeInBytes() != null ? multipartConfiguration.minimumPartSizeInBytes() * 4 - : DEFAULT_API_CALL_BUFFER_SIZE; - } - @Override public CompletableFuture putObject(PutObjectRequest putObjectRequest, AsyncRequestBody requestBody) { return mpuHelper.uploadObject(putObjectRequest, requestBody); diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java index 28e418974db8..be2500703e15 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java @@ -93,13 +93,19 @@ public Long apiCallBufferSizeInBytes() { public interface Builder extends CopyableBuilder { /** - * Configures the minimum number of bytes of the body of the request required for requests to be converted to their - * multipart equivalent. Only taken into account when converting {@code putObject} and {@code copyObject} requests. - * Any request whose size is less than the configured value will not use multipart operation, - * even if multipart is enabled via {@link S3AsyncClientBuilder#multipartEnabled(Boolean)}. + * Configure the size threshold, in bytes, for when to use multipart upload. Uploads/copies over this size will + * automatically use a multipart upload strategy, while uploads/copies smaller than this threshold will use a single + * connection to upload/copy the whole object. + * *

+ * Multipart uploads are easier to recover from and also potentially faster than single part uploads, especially when the + * upload parts can be uploaded in parallel. Because there are additional network API calls, small objects are still + * recommended to use a single connection for the upload. See + * Uploading and copying objects using + * multipart upload. * - * Default value: 8 Mib + *

+ * By default, it is the same as {@link #minimumPartSizeInBytes(Long)}. * * @param thresholdInBytes the value of the threshold to set. * @return an instance of this builder. diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolverTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolverTest.java new file mode 100644 index 000000000000..b41a6bc66f4a --- /dev/null +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolverTest.java @@ -0,0 +1,74 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.services.s3.internal.multipart; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.services.s3.multipart.MultipartConfiguration; + +public class MultipartConfigurationResolverTest { + + @Test + void resolveThresholdInBytes_valueNotProvided_shouldSameAsPartSize() { + MultipartConfiguration configuration = MultipartConfiguration.builder() + .minimumPartSizeInBytes(10L) + .build(); + MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration); + assertThat(resolver.thresholdInBytes()).isEqualTo(10L); + } + + @Test + void resolveThresholdInBytes_valueProvided_shouldHonor() { + MultipartConfiguration configuration = MultipartConfiguration.builder() + .minimumPartSizeInBytes(1L) + .thresholdInBytes(12L) + .build(); + MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration); + assertThat(resolver.thresholdInBytes()).isEqualTo(12L); + } + + @Test + void resolveApiCallBufferSize_valueProvided_shouldHonor() { + MultipartConfiguration configuration = MultipartConfiguration.builder() + .apiCallBufferSizeInBytes(100L) + .build(); + MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration); + assertThat(resolver.apiCallBufferSize()).isEqualTo(100L); + } + + @Test + void resolveApiCallBufferSize_valueNotProvided_shouldComputeBasedOnPartSize() { + MultipartConfiguration configuration = MultipartConfiguration.builder() + .minimumPartSizeInBytes(10L) + .build(); + MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration); + assertThat(resolver.apiCallBufferSize()).isEqualTo(40L); + } + + @Test + void valueProvidedForAllFields_shouldHonor() { + MultipartConfiguration configuration = MultipartConfiguration.builder() + .minimumPartSizeInBytes(10L) + .thresholdInBytes(8L) + .apiCallBufferSizeInBytes(3L) + .build(); + MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration); + assertThat(resolver.minimalPartSizeInBytes()).isEqualTo(10L); + assertThat(resolver.thresholdInBytes()).isEqualTo(8L); + assertThat(resolver.apiCallBufferSize()).isEqualTo(3L); + } +} From 2f6c1860aad9d43f7aed24b60fc0cc9cc55513c2 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Fri, 11 Aug 2023 11:20:41 -0700 Subject: [PATCH 2/3] Address feedback --- .../MultipartConfigurationResolver.java | 10 +++++----- .../multipart/MultipartS3AsyncClient.java | 3 +-- .../internal/multipart/UploadObjectHelper.java | 16 +++++++--------- .../multipart/UploadObjectHelperTest.java | 8 +++++++- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolver.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolver.java index 7159f3bd20d5..9fc199175bda 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolver.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolver.java @@ -26,17 +26,17 @@ public final class MultipartConfigurationResolver { private static final long DEFAULT_MIN_PART_SIZE = 8L * 1024 * 1024; - private final MultipartConfiguration configuration; private final long minimalPartSizeInBytes; private final long apiCallBufferSize; private final long thresholdInBytes; public MultipartConfigurationResolver(MultipartConfiguration multipartConfiguration) { - this.configuration = multipartConfiguration; - this.minimalPartSizeInBytes = Validate.getOrDefault(configuration.minimumPartSizeInBytes(), () -> DEFAULT_MIN_PART_SIZE); - this.apiCallBufferSize = Validate.getOrDefault(configuration.apiCallBufferSizeInBytes(), + Validate.notNull(multipartConfiguration, "multipartConfiguration"); + this.minimalPartSizeInBytes = Validate.getOrDefault(multipartConfiguration.minimumPartSizeInBytes(), + () -> DEFAULT_MIN_PART_SIZE); + this.apiCallBufferSize = Validate.getOrDefault(multipartConfiguration.apiCallBufferSizeInBytes(), () -> minimalPartSizeInBytes * 4); - this.thresholdInBytes = Validate.getOrDefault(configuration.thresholdInBytes(), () -> minimalPartSizeInBytes); + this.thresholdInBytes = Validate.getOrDefault(multipartConfiguration.thresholdInBytes(), () -> minimalPartSizeInBytes); } public long minimalPartSizeInBytes() { diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartS3AsyncClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartS3AsyncClient.java index 98c55259d44d..8b53099b8683 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartS3AsyncClient.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartS3AsyncClient.java @@ -56,8 +56,7 @@ private MultipartS3AsyncClient(S3AsyncClient delegate, MultipartConfiguration mu MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(validConfiguration); long minPartSizeInBytes = resolver.minimalPartSizeInBytes(); long threshold = resolver.thresholdInBytes(); - long apiCallBufferSizeInBytes = resolver.apiCallBufferSize(); - mpuHelper = new UploadObjectHelper(delegate, minPartSizeInBytes, threshold, apiCallBufferSizeInBytes); + mpuHelper = new UploadObjectHelper(delegate, resolver); copyObjectHelper = new CopyObjectHelper(delegate, minPartSizeInBytes, threshold); } diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelper.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelper.java index 0700e8ade5f9..1ca499b57aa8 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelper.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelper.java @@ -34,30 +34,28 @@ public final class UploadObjectHelper { private final long partSizeInBytes; private final GenericMultipartHelper genericMultipartHelper; - private final long maxMemoryUsageInBytes; + private final long apiCallBufferSize; private final long multipartUploadThresholdInBytes; private final UploadWithKnownContentLengthHelper uploadWithKnownContentLength; private final UploadWithUnknownContentLengthHelper uploadWithUnknownContentLength; public UploadObjectHelper(S3AsyncClient s3AsyncClient, - long partSizeInBytes, - long multipartUploadThresholdInBytes, - long maxMemoryUsageInBytes) { + MultipartConfigurationResolver resolver) { this.s3AsyncClient = s3AsyncClient; - this.partSizeInBytes = partSizeInBytes; + this.partSizeInBytes = resolver.minimalPartSizeInBytes(); this.genericMultipartHelper = new GenericMultipartHelper<>(s3AsyncClient, SdkPojoConversionUtils::toAbortMultipartUploadRequest, SdkPojoConversionUtils::toPutObjectResponse); - this.maxMemoryUsageInBytes = maxMemoryUsageInBytes; - this.multipartUploadThresholdInBytes = multipartUploadThresholdInBytes; + this.apiCallBufferSize = resolver.apiCallBufferSize(); + this.multipartUploadThresholdInBytes = resolver.thresholdInBytes(); this.uploadWithKnownContentLength = new UploadWithKnownContentLengthHelper(s3AsyncClient, partSizeInBytes, multipartUploadThresholdInBytes, - maxMemoryUsageInBytes); + apiCallBufferSize); this.uploadWithUnknownContentLength = new UploadWithUnknownContentLengthHelper(s3AsyncClient, partSizeInBytes, multipartUploadThresholdInBytes, - maxMemoryUsageInBytes); + apiCallBufferSize); } public CompletableFuture uploadObject(PutObjectRequest putObjectRequest, diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelperTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelperTest.java index 11d54a73fb72..b287270515b0 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelperTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelperTest.java @@ -61,6 +61,7 @@ import software.amazon.awssdk.services.s3.model.PutObjectResponse; import software.amazon.awssdk.services.s3.model.UploadPartRequest; import software.amazon.awssdk.services.s3.model.UploadPartResponse; +import software.amazon.awssdk.services.s3.multipart.MultipartConfiguration; import software.amazon.awssdk.testutils.RandomTempFile; import software.amazon.awssdk.utils.CompletableFutureUtils; @@ -97,7 +98,12 @@ public static Stream asyncRequestBody() { @BeforeEach public void beforeEach() { s3AsyncClient = Mockito.mock(S3AsyncClient.class); - uploadHelper = new UploadObjectHelper(s3AsyncClient, PART_SIZE, THRESHOLD, PART_SIZE * 2); + uploadHelper = new UploadObjectHelper(s3AsyncClient, + new MultipartConfigurationResolver(MultipartConfiguration.builder() + .minimumPartSizeInBytes(PART_SIZE) + .thresholdInBytes(THRESHOLD) + .thresholdInBytes(PART_SIZE * 2) + .build())); } @ParameterizedTest From df714992ae550c0cf9c3957c94d613962ed8db50 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Mon, 14 Aug 2023 14:56:31 -0700 Subject: [PATCH 3/3] Add more test --- .../multipart/MultipartConfigurationResolverTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolverTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolverTest.java index b41a6bc66f4a..99e929c09f4e 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolverTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolverTest.java @@ -71,4 +71,13 @@ void valueProvidedForAllFields_shouldHonor() { assertThat(resolver.thresholdInBytes()).isEqualTo(8L); assertThat(resolver.apiCallBufferSize()).isEqualTo(3L); } + + @Test + void noValueProvided_shouldUseDefault() { + MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(MultipartConfiguration.builder() + .build()); + assertThat(resolver.minimalPartSizeInBytes()).isEqualTo(8L * 1024 * 1024); + assertThat(resolver.thresholdInBytes()).isEqualTo(8L * 1024 * 1024); + assertThat(resolver.apiCallBufferSize()).isEqualTo(8L * 1024 * 1024 * 4); + } }