Skip to content

Commit 5982373

Browse files
zoewanggzoewangg
authored
Fixed the issue where thresholdInBytes is not the same as minimalPart… (#4289)
* Fixed the issue where thresholdInBytes is not the same as minimalPartSizeInBytes if a custom minimalPartSizeInBytes is provided. * Address feedback * Add more test
1 parent 8f064e7 commit 5982373

File tree

6 files changed

+165
-31
lines changed

6 files changed

+165
-31
lines changed

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolver.java

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.services.s3.internal.multipart;
17+
18+
import software.amazon.awssdk.annotations.SdkInternalApi;
19+
import software.amazon.awssdk.services.s3.multipart.MultipartConfiguration;
20+
import software.amazon.awssdk.utils.Validate;
21+
22+
/**
23+
* Internal utility class to resolve {@link MultipartConfiguration}.
24+
*/
25+
@SdkInternalApi
26+
public final class MultipartConfigurationResolver {
27+
28+
private static final long DEFAULT_MIN_PART_SIZE = 8L * 1024 * 1024;
29+
private final long minimalPartSizeInBytes;
30+
private final long apiCallBufferSize;
31+
private final long thresholdInBytes;
32+
33+
public MultipartConfigurationResolver(MultipartConfiguration multipartConfiguration) {
34+
Validate.notNull(multipartConfiguration, "multipartConfiguration");
35+
this.minimalPartSizeInBytes = Validate.getOrDefault(multipartConfiguration.minimumPartSizeInBytes(),
36+
() -> DEFAULT_MIN_PART_SIZE);
37+
this.apiCallBufferSize = Validate.getOrDefault(multipartConfiguration.apiCallBufferSizeInBytes(),
38+
() -> minimalPartSizeInBytes * 4);
39+
this.thresholdInBytes = Validate.getOrDefault(multipartConfiguration.thresholdInBytes(), () -> minimalPartSizeInBytes);
40+
}
41+
42+
public long minimalPartSizeInBytes() {
43+
return minimalPartSizeInBytes;
44+
}
45+
46+
public long thresholdInBytes() {
47+
return thresholdInBytes;
48+
}
49+
50+
public long apiCallBufferSize() {
51+
return apiCallBufferSize;
52+
}
53+
}

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartS3AsyncClient.java

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,32 +46,20 @@ public final class MultipartS3AsyncClient extends DelegatingS3AsyncClient {
4646

4747
private static final ApiName USER_AGENT_API_NAME = ApiName.builder().name("hll").version("s3Multipart").build();
4848

49-
private static final long DEFAULT_MIN_PART_SIZE = 8L * 1024 * 1024;
50-
private static final long DEFAULT_THRESHOLD = 8L * 1024 * 1024;
51-
private static final long DEFAULT_API_CALL_BUFFER_SIZE = DEFAULT_MIN_PART_SIZE * 4;
52-
5349
private final UploadObjectHelper mpuHelper;
5450
private final CopyObjectHelper copyObjectHelper;
5551

5652
private MultipartS3AsyncClient(S3AsyncClient delegate, MultipartConfiguration multipartConfiguration) {
5753
super(delegate);
5854
MultipartConfiguration validConfiguration = Validate.getOrDefault(multipartConfiguration,
5955
MultipartConfiguration.builder()::build);
60-
long minPartSizeInBytes = Validate.getOrDefault(validConfiguration.minimumPartSizeInBytes(),
61-
() -> DEFAULT_MIN_PART_SIZE);
62-
long threshold = Validate.getOrDefault(validConfiguration.thresholdInBytes(),
63-
() -> DEFAULT_THRESHOLD);
64-
long apiCallBufferSizeInBytes = Validate.getOrDefault(validConfiguration.apiCallBufferSizeInBytes(),
65-
() -> computeApiCallBufferSize(validConfiguration));
66-
mpuHelper = new UploadObjectHelper(delegate, minPartSizeInBytes, threshold, apiCallBufferSizeInBytes);
56+
MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(validConfiguration);
57+
long minPartSizeInBytes = resolver.minimalPartSizeInBytes();
58+
long threshold = resolver.thresholdInBytes();
59+
mpuHelper = new UploadObjectHelper(delegate, resolver);
6760
copyObjectHelper = new CopyObjectHelper(delegate, minPartSizeInBytes, threshold);
6861
}
6962

70-
private long computeApiCallBufferSize(MultipartConfiguration multipartConfiguration) {
71-
return multipartConfiguration.minimumPartSizeInBytes() != null ? multipartConfiguration.minimumPartSizeInBytes() * 4
72-
: DEFAULT_API_CALL_BUFFER_SIZE;
73-
}
74-
7563
@Override
7664
public CompletableFuture<PutObjectResponse> putObject(PutObjectRequest putObjectRequest, AsyncRequestBody requestBody) {
7765
return mpuHelper.uploadObject(putObjectRequest, requestBody);

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelper.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,30 +34,28 @@ public final class UploadObjectHelper {
3434
private final long partSizeInBytes;
3535
private final GenericMultipartHelper<PutObjectRequest, PutObjectResponse> genericMultipartHelper;
3636

37-
private final long maxMemoryUsageInBytes;
37+
private final long apiCallBufferSize;
3838
private final long multipartUploadThresholdInBytes;
3939
private final UploadWithKnownContentLengthHelper uploadWithKnownContentLength;
4040
private final UploadWithUnknownContentLengthHelper uploadWithUnknownContentLength;
4141

4242
public UploadObjectHelper(S3AsyncClient s3AsyncClient,
43-
long partSizeInBytes,
44-
long multipartUploadThresholdInBytes,
45-
long maxMemoryUsageInBytes) {
43+
MultipartConfigurationResolver resolver) {
4644
this.s3AsyncClient = s3AsyncClient;
47-
this.partSizeInBytes = partSizeInBytes;
45+
this.partSizeInBytes = resolver.minimalPartSizeInBytes();
4846
this.genericMultipartHelper = new GenericMultipartHelper<>(s3AsyncClient,
4947
SdkPojoConversionUtils::toAbortMultipartUploadRequest,
5048
SdkPojoConversionUtils::toPutObjectResponse);
51-
this.maxMemoryUsageInBytes = maxMemoryUsageInBytes;
52-
this.multipartUploadThresholdInBytes = multipartUploadThresholdInBytes;
49+
this.apiCallBufferSize = resolver.apiCallBufferSize();
50+
this.multipartUploadThresholdInBytes = resolver.thresholdInBytes();
5351
this.uploadWithKnownContentLength = new UploadWithKnownContentLengthHelper(s3AsyncClient,
5452
partSizeInBytes,
5553
multipartUploadThresholdInBytes,
56-
maxMemoryUsageInBytes);
54+
apiCallBufferSize);
5755
this.uploadWithUnknownContentLength = new UploadWithUnknownContentLengthHelper(s3AsyncClient,
5856
partSizeInBytes,
5957
multipartUploadThresholdInBytes,
60-
maxMemoryUsageInBytes);
58+
apiCallBufferSize);
6159
}
6260

6361
public CompletableFuture<PutObjectResponse> uploadObject(PutObjectRequest putObjectRequest,

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,19 @@ public Long apiCallBufferSizeInBytes() {
9393
public interface Builder extends CopyableBuilder<Builder, MultipartConfiguration> {
9494

9595
/**
96-
* Configures the minimum number of bytes of the body of the request required for requests to be converted to their
97-
* multipart equivalent. Only taken into account when converting {@code putObject} and {@code copyObject} requests.
98-
* Any request whose size is less than the configured value will not use multipart operation,
99-
* even if multipart is enabled via {@link S3AsyncClientBuilder#multipartEnabled(Boolean)}.
96+
* Configure the size threshold, in bytes, for when to use multipart upload. Uploads/copies over this size will
97+
* automatically use a multipart upload strategy, while uploads/copies smaller than this threshold will use a single
98+
* connection to upload/copy the whole object.
99+
*
100100
* <p>
101+
* Multipart uploads are easier to recover from and also potentially faster than single part uploads, especially when the
102+
* upload parts can be uploaded in parallel. Because there are additional network API calls, small objects are still
103+
* recommended to use a single connection for the upload. See
104+
* <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/mpuoverview.html">Uploading and copying objects using
105+
* multipart upload</a>.
101106
*
102-
* Default value: 8 Mib
107+
* <p>
108+
* By default, it is the same as {@link #minimumPartSizeInBytes(Long)}.
103109
*
104110
* @param thresholdInBytes the value of the threshold to set.
105111
* @return an instance of this builder.

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartConfigurationResolverTest.java

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.services.s3.internal.multipart;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import org.junit.jupiter.api.Test;
21+
import software.amazon.awssdk.services.s3.multipart.MultipartConfiguration;
22+
23+
public class MultipartConfigurationResolverTest {
24+
25+
@Test
26+
void resolveThresholdInBytes_valueNotProvided_shouldSameAsPartSize() {
27+
MultipartConfiguration configuration = MultipartConfiguration.builder()
28+
.minimumPartSizeInBytes(10L)
29+
.build();
30+
MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration);
31+
assertThat(resolver.thresholdInBytes()).isEqualTo(10L);
32+
}
33+
34+
@Test
35+
void resolveThresholdInBytes_valueProvided_shouldHonor() {
36+
MultipartConfiguration configuration = MultipartConfiguration.builder()
37+
.minimumPartSizeInBytes(1L)
38+
.thresholdInBytes(12L)
39+
.build();
40+
MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration);
41+
assertThat(resolver.thresholdInBytes()).isEqualTo(12L);
42+
}
43+
44+
@Test
45+
void resolveApiCallBufferSize_valueProvided_shouldHonor() {
46+
MultipartConfiguration configuration = MultipartConfiguration.builder()
47+
.apiCallBufferSizeInBytes(100L)
48+
.build();
49+
MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration);
50+
assertThat(resolver.apiCallBufferSize()).isEqualTo(100L);
51+
}
52+
53+
@Test
54+
void resolveApiCallBufferSize_valueNotProvided_shouldComputeBasedOnPartSize() {
55+
MultipartConfiguration configuration = MultipartConfiguration.builder()
56+
.minimumPartSizeInBytes(10L)
57+
.build();
58+
MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration);
59+
assertThat(resolver.apiCallBufferSize()).isEqualTo(40L);
60+
}
61+
62+
@Test
63+
void valueProvidedForAllFields_shouldHonor() {
64+
MultipartConfiguration configuration = MultipartConfiguration.builder()
65+
.minimumPartSizeInBytes(10L)
66+
.thresholdInBytes(8L)
67+
.apiCallBufferSizeInBytes(3L)
68+
.build();
69+
MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(configuration);
70+
assertThat(resolver.minimalPartSizeInBytes()).isEqualTo(10L);
71+
assertThat(resolver.thresholdInBytes()).isEqualTo(8L);
72+
assertThat(resolver.apiCallBufferSize()).isEqualTo(3L);
73+
}
74+
75+
@Test
76+
void noValueProvided_shouldUseDefault() {
77+
MultipartConfigurationResolver resolver = new MultipartConfigurationResolver(MultipartConfiguration.builder()
78+
.build());
79+
assertThat(resolver.minimalPartSizeInBytes()).isEqualTo(8L * 1024 * 1024);
80+
assertThat(resolver.thresholdInBytes()).isEqualTo(8L * 1024 * 1024);
81+
assertThat(resolver.apiCallBufferSize()).isEqualTo(8L * 1024 * 1024 * 4);
82+
}
83+
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelperTest.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
6262
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
6363
import software.amazon.awssdk.services.s3.model.UploadPartResponse;
64+
import software.amazon.awssdk.services.s3.multipart.MultipartConfiguration;
6465
import software.amazon.awssdk.testutils.RandomTempFile;
6566
import software.amazon.awssdk.utils.CompletableFutureUtils;
6667

@@ -97,7 +98,12 @@ public static Stream<AsyncRequestBody> asyncRequestBody() {
9798
@BeforeEach
9899
public void beforeEach() {
99100
s3AsyncClient = Mockito.mock(S3AsyncClient.class);
100-
uploadHelper = new UploadObjectHelper(s3AsyncClient, PART_SIZE, THRESHOLD, PART_SIZE * 2);
101+
uploadHelper = new UploadObjectHelper(s3AsyncClient,
102+
new MultipartConfigurationResolver(MultipartConfiguration.builder()
103+
.minimumPartSizeInBytes(PART_SIZE)
104+
.thresholdInBytes(THRESHOLD)
105+
.thresholdInBytes(PART_SIZE * 2)
106+
.build()));
101107
}
102108

103109
@ParameterizedTest

0 commit comments

Comments
 (0)