Skip to content

Commit a5cd20e

Browse files
authored
Fixed the thread safety issue with CRT based S3 client (#3806)
1 parent cb3e287 commit a5cd20e

File tree

4 files changed

+14
-18
lines changed

4 files changed

+14
-18
lines changed

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/CopyObjectHelper.java

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,11 @@ public final class CopyObjectHelper {
5656
private static final long MAX_UPLOAD_PARTS = 10_000;
5757

5858
private final S3AsyncClient s3AsyncClient;
59-
private final S3NativeClientConfiguration s3NativeClientConfiguration;
59+
private final long partSizeInBytes;
6060

61-
public CopyObjectHelper(S3AsyncClient s3AsyncClient, S3NativeClientConfiguration s3NativeClientConfiguration) {
61+
public CopyObjectHelper(S3AsyncClient s3AsyncClient, long partSizeInBytes) {
6262
this.s3AsyncClient = s3AsyncClient;
63-
this.s3NativeClientConfiguration = s3NativeClientConfiguration;
63+
this.partSizeInBytes = partSizeInBytes;
6464
}
6565

6666
public CompletableFuture<CopyObjectResponse> copyObject(CopyObjectRequest copyObjectRequest) {
@@ -92,9 +92,7 @@ private void doCopyObject(CopyObjectRequest copyObjectRequest, CompletableFuture
9292
HeadObjectResponse headObjectResponse) {
9393
Long contentLength = headObjectResponse.contentLength();
9494

95-
long partSize = s3NativeClientConfiguration.partSizeBytes();
96-
97-
if (contentLength <= partSize) {
95+
if (contentLength <= partSizeInBytes) {
9896
log.debug(() -> "Starting the copy as a single copy part request");
9997
copyInOneChunk(copyObjectRequest, returnFuture);
10098
} else {
@@ -132,7 +130,7 @@ private void doCopyInParts(CopyObjectRequest copyObjectRequest,
132130
Long contentLength,
133131
CompletableFuture<CopyObjectResponse> returnFuture,
134132
String uploadId) {
135-
long optimalPartSize = calculateOptimalPartSizeForCopy(s3NativeClientConfiguration.partSizeBytes());
133+
long optimalPartSize = calculateOptimalPartSizeForCopy(partSizeInBytes);
136134

137135
int partCount = determinePartCount(contentLength, optimalPartSize);
138136

@@ -281,8 +279,7 @@ private long calculateOptimalPartSizeForCopy(long contentLengthOfSource) {
281279
double optimalPartSize = contentLengthOfSource / (double) MAX_UPLOAD_PARTS;
282280

283281
optimalPartSize = Math.ceil(optimalPartSize);
284-
return (long) Math.max(optimalPartSize,
285-
s3NativeClientConfiguration.partSizeBytes());
282+
return (long) Math.max(optimalPartSize, partSizeInBytes);
286283
}
287284

288285
private void copyInOneChunk(CopyObjectRequest copyObjectRequest,

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClient.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute.SDK_HTTP_EXECUTION_ATTRIBUTES;
1919
import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.HTTP_CHECKSUM;
2020
import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.OPERATION_NAME;
21+
import static software.amazon.awssdk.services.s3.internal.crt.S3NativeClientConfiguration.DEFAULT_PART_SIZE_IN_BYTES;
2122

2223
import java.net.URI;
2324
import java.util.concurrent.CompletableFuture;
@@ -52,12 +53,13 @@
5253
@SdkInternalApi
5354
public final class DefaultS3CrtAsyncClient extends DelegatingS3AsyncClient implements S3CrtAsyncClient {
5455
private static final String CRT_CLIENT_CLASSPATH = "software.amazon.awssdk.crt.s3.S3Client";
55-
private static S3NativeClientConfiguration s3NativeClientConfiguration;
5656
private final CopyObjectHelper copyObjectHelper;
5757

5858
private DefaultS3CrtAsyncClient(DefaultS3CrtClientBuilder builder) {
5959
super(initializeS3AsyncClient(builder));
60-
this.copyObjectHelper = new CopyObjectHelper((S3AsyncClient) delegate(), s3NativeClientConfiguration);
60+
long partSizeInBytes = builder.minimalPartSizeInBytes == null ? DEFAULT_PART_SIZE_IN_BYTES :
61+
builder.minimalPartSizeInBytes;
62+
this.copyObjectHelper = new CopyObjectHelper((S3AsyncClient) delegate(), partSizeInBytes);
6163
}
6264

6365
@Override
@@ -93,7 +95,7 @@ private static S3CrtAsyncHttpClient.Builder initializeS3CrtAsyncHttpClient(Defau
9395
Validate.isPositiveOrNull(builder.targetThroughputInGbps, "targetThroughputInGbps");
9496
Validate.isPositiveOrNull(builder.minimalPartSizeInBytes, "minimalPartSizeInBytes");
9597

96-
s3NativeClientConfiguration =
98+
S3NativeClientConfiguration s3NativeClientConfiguration =
9799
S3NativeClientConfiguration.builder()
98100
.checksumValidationEnabled(builder.checksumValidationEnabled)
99101
.targetThroughputInGbps(builder.targetThroughputInGbps)
@@ -104,6 +106,7 @@ private static S3CrtAsyncHttpClient.Builder initializeS3CrtAsyncHttpClient(Defau
104106
.credentialsProvider(builder.credentialsProvider)
105107
.readBufferSizeInBytes(builder.readBufferSizeInBytes)
106108
.build();
109+
107110
return S3CrtAsyncHttpClient.builder()
108111
.s3ClientConfiguration(s3NativeClientConfiguration);
109112
}

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3NativeClientConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
*/
3030
@SdkInternalApi
3131
public class S3NativeClientConfiguration implements SdkAutoCloseable {
32-
private static final long DEFAULT_PART_SIZE_IN_BYTES = 8L * 1024 * 1024;
32+
static final long DEFAULT_PART_SIZE_IN_BYTES = 8L * 1024 * 1024;
3333
private static final long DEFAULT_TARGET_THROUGHPUT_IN_GBPS = 10;
3434

3535
private final String signingRegion;

services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/CopyObjectHelperTest.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,11 @@ class CopyObjectHelperTest {
5858
private static final String MULTIPART_ID = "multipartId";
5959
private S3AsyncClient s3AsyncClient;
6060
private CopyObjectHelper copyHelper;
61-
private S3NativeClientConfiguration s3NativeClientConfiguration;
6261

6362
@BeforeEach
6463
public void setUp() {
65-
s3NativeClientConfiguration = S3NativeClientConfiguration.builder()
66-
.partSizeInBytes(1024L)
67-
.build();
6864
s3AsyncClient = Mockito.mock(S3AsyncClient.class);
69-
copyHelper = new CopyObjectHelper(s3AsyncClient, s3NativeClientConfiguration);
65+
copyHelper = new CopyObjectHelper(s3AsyncClient, 1024L);
7066
}
7167

7268
@Test

0 commit comments

Comments
 (0)