You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by rc...@apache.org on 2020/01/13 04:47:11 UTC
[james-project] 05/05: [Refactoring] AWSS3BlobPutter reuse S3Client
and TransferManager
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit f2d7abae2fdbcdaf4c192b1e3d6d4e283417cefd
Author: Tran Tien Duc <dt...@linagora.com>
AuthorDate: Mon Jan 6 18:09:12 2020 +0700
[Refactoring] AWSS3BlobPutter reuse S3Client and TransferManager
As they are documented as ThreadSafe.
And reusing the TransferManager is encouraged - see the Java doc
---
.../james/blob/objectstorage/BlobPutter.java | 3 +-
.../blob/objectstorage/ObjectStorageBlobStore.java | 3 +-
.../objectstorage/StreamCompatibleBlobPutter.java | 6 ++++
.../blob/objectstorage/aws/AwsS3ObjectStorage.java | 32 ++++++++++------------
.../ObjectStorageBlobStoreAWSCryptoTest.java | 4 ++-
.../ObjectStorageBlobStoreAWSNamespaceTest.java | 4 ++-
...tStorageBlobStoreAWSPrefixAndNamespaceTest.java | 4 ++-
.../ObjectStorageBlobStoreAWSPrefixTest.java | 4 ++-
.../ObjectStorageBlobStoreAWSTest.java | 4 ++-
.../objectstorage/ObjectStorageBlobStoreTest.java | 2 +-
10 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/BlobPutter.java b/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/BlobPutter.java
index 8fe1a2b..e521db5 100644
--- a/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/BlobPutter.java
+++ b/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/BlobPutter.java
@@ -19,6 +19,7 @@
package org.apache.james.blob.objectstorage;
+import java.io.Closeable;
import java.util.function.Supplier;
import org.apache.james.blob.api.BlobId;
@@ -36,7 +37,7 @@ import reactor.core.publisher.Mono;
*
*/
-public interface BlobPutter {
+public interface BlobPutter extends Closeable {
Mono<Void> putDirectly(ObjectStorageBucketName bucketName, Blob blob);
diff --git a/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStore.java b/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStore.java
index fbe82ed..fba4fa1 100644
--- a/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStore.java
+++ b/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStore.java
@@ -91,8 +91,9 @@ public class ObjectStorageBlobStore implements BlobStore {
}
@PreDestroy
- public void close() {
+ public void close() throws IOException {
blobStore.getContext().close();
+ blobPutter.close();
}
@Override
diff --git a/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/StreamCompatibleBlobPutter.java b/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/StreamCompatibleBlobPutter.java
index 269a8a4..46d1d2e 100644
--- a/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/StreamCompatibleBlobPutter.java
+++ b/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/StreamCompatibleBlobPutter.java
@@ -19,6 +19,7 @@
package org.apache.james.blob.objectstorage;
+import java.io.IOException;
import java.time.Duration;
import java.util.Optional;
import java.util.function.Supplier;
@@ -112,4 +113,9 @@ public class StreamCompatibleBlobPutter implements BlobPutter {
return Optional.empty();
}
+
+ @Override
+ public void close() throws IOException {
+ // No resource to clean up
+ }
}
\ No newline at end of file
diff --git a/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/aws/AwsS3ObjectStorage.java b/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/aws/AwsS3ObjectStorage.java
index f4a35bb..fa58add 100644
--- a/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/aws/AwsS3ObjectStorage.java
+++ b/server/blob/blob-objectstorage/src/main/java/org/apache/james/blob/objectstorage/aws/AwsS3ObjectStorage.java
@@ -137,12 +137,12 @@ public class AwsS3ObjectStorage {
private static final Duration FIRST_BACK_OFF = Duration.ofMillis(100);
private static final Duration FOREVER = Duration.ofMillis(Long.MAX_VALUE);
- private final AwsS3AuthConfiguration configuration;
- private final ExecutorService executorService;
+ private final AmazonS3 s3Client;
+ private final TransferManager transferManager;
- AwsS3BlobPutter(AwsS3AuthConfiguration configuration, ExecutorService executorService) {
- this.configuration = configuration;
- this.executorService = executorService;
+ AwsS3BlobPutter(AwsS3AuthConfiguration authConfiguration, ExecutorService executorService) {
+ this.s3Client = getS3Client(authConfiguration, getClientConfiguration());
+ this.transferManager = getTransferManager(s3Client, executorService);
}
@Override
@@ -184,7 +184,7 @@ public class AwsS3ObjectStorage {
.exponentialBackoff(FIRST_BACK_OFF, FOREVER)
.withBackoffScheduler(Schedulers.elastic())
.retryMax(MAX_RETRY_ON_EXCEPTION)
- .doOnRetry(retryContext -> createBucket(bucketName, configuration)));
+ .doOnRetry(retryContext -> s3Client.createBucket(bucketName.asString())));
}
private void uploadByFile(ObjectStorageBucketName bucketName, BlobId blobId, File file) throws InterruptedException {
@@ -204,16 +204,9 @@ public class AwsS3ObjectStorage {
}
private void upload(PutObjectRequest request) throws InterruptedException {
- TransferManager transferManager = getTransferManager();
transferManager
.upload(request)
.waitForUploadResult();
- transferManager.shutdownNow();
- }
-
- private void createBucket(ObjectStorageBucketName bucketName, AwsS3AuthConfiguration configuration) {
- getS3Client(configuration, getClientConfiguration())
- .createBucket(bucketName.asString());
}
private boolean needToCreateBucket(Throwable th) {
@@ -226,13 +219,10 @@ public class AwsS3ObjectStorage {
return false;
}
- private TransferManager getTransferManager() {
- ClientConfiguration clientConfiguration = getClientConfiguration();
- AmazonS3 amazonS3 = getS3Client(configuration, clientConfiguration);
-
+ private static TransferManager getTransferManager(AmazonS3 s3Client, ExecutorService executorService) {
return TransferManagerBuilder
.standard()
- .withS3Client(amazonS3)
+ .withS3Client(s3Client)
.withMultipartUploadThreshold(MULTIPART_UPLOAD_THRESHOLD.getValue())
.withExecutorFactory(() -> executorService)
.withShutDownThreadPools(DO_NOT_SHUTDOWN_THREAD_POOL)
@@ -253,5 +243,11 @@ public class AwsS3ObjectStorage {
clientConfiguration.setRetryPolicy(PredefinedRetryPolicies.getDefaultRetryPolicyWithCustomMaxRetries(MAX_ERROR_RETRY));
return clientConfiguration;
}
+
+ @Override
+ public void close() throws IOException {
+ transferManager.shutdownNow();
+ s3Client.shutdown();
+ }
}
}
diff --git a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSCryptoTest.java b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSCryptoTest.java
index d120c24..aba7d13 100644
--- a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSCryptoTest.java
+++ b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSCryptoTest.java
@@ -19,6 +19,8 @@
package org.apache.james.blob.objectstorage;
+import java.io.IOException;
+
import org.apache.james.blob.api.BlobId;
import org.apache.james.blob.api.BlobStore;
import org.apache.james.blob.api.HashBlobId;
@@ -69,7 +71,7 @@ public class ObjectStorageBlobStoreAWSCryptoTest implements MetricableBlobStoreC
}
@AfterEach
- void tearDown() {
+ void tearDown() throws IOException {
objectStorageBlobStore.deleteAllBuckets().block();
objectStorageBlobStore.close();
awsS3ObjectStorage.tearDown();
diff --git a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSNamespaceTest.java b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSNamespaceTest.java
index 8fcc3a2..fdf697b 100644
--- a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSNamespaceTest.java
+++ b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSNamespaceTest.java
@@ -19,6 +19,8 @@
package org.apache.james.blob.objectstorage;
+import java.io.IOException;
+
import org.apache.james.blob.api.BlobId;
import org.apache.james.blob.api.BlobStore;
import org.apache.james.blob.api.BucketName;
@@ -62,7 +64,7 @@ public class ObjectStorageBlobStoreAWSNamespaceTest implements MetricableBlobSto
}
@AfterEach
- void tearDown() {
+ void tearDown() throws IOException {
objectStorageBlobStore.deleteAllBuckets().block();
objectStorageBlobStore.close();
awsS3ObjectStorage.tearDown();
diff --git a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSPrefixAndNamespaceTest.java b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSPrefixAndNamespaceTest.java
index 55b6af6..aea96a0 100644
--- a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSPrefixAndNamespaceTest.java
+++ b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSPrefixAndNamespaceTest.java
@@ -19,6 +19,8 @@
package org.apache.james.blob.objectstorage;
+import java.io.IOException;
+
import org.apache.james.blob.api.BlobId;
import org.apache.james.blob.api.BlobStore;
import org.apache.james.blob.api.BucketName;
@@ -63,7 +65,7 @@ public class ObjectStorageBlobStoreAWSPrefixAndNamespaceTest implements Metricab
}
@AfterEach
- void tearDown() {
+ void tearDown() throws IOException {
objectStorageBlobStore.deleteAllBuckets().block();
objectStorageBlobStore.close();
awsS3ObjectStorage.tearDown();
diff --git a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSPrefixTest.java b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSPrefixTest.java
index 8f3ce20..f82ed8f 100644
--- a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSPrefixTest.java
+++ b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSPrefixTest.java
@@ -19,6 +19,8 @@
package org.apache.james.blob.objectstorage;
+import java.io.IOException;
+
import org.apache.james.blob.api.BlobId;
import org.apache.james.blob.api.BlobStore;
import org.apache.james.blob.api.HashBlobId;
@@ -61,7 +63,7 @@ public class ObjectStorageBlobStoreAWSPrefixTest implements MetricableBlobStoreC
}
@AfterEach
- void tearDown() {
+ void tearDown() throws IOException {
objectStorageBlobStore.deleteAllBuckets().block();
objectStorageBlobStore.close();
awsS3ObjectStorage.tearDown();
diff --git a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSTest.java b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSTest.java
index d076dae..6af3fd3 100644
--- a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSTest.java
+++ b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreAWSTest.java
@@ -19,6 +19,8 @@
package org.apache.james.blob.objectstorage;
+import java.io.IOException;
+
import org.apache.james.blob.api.BlobId;
import org.apache.james.blob.api.BlobStore;
import org.apache.james.blob.api.HashBlobId;
@@ -61,7 +63,7 @@ public class ObjectStorageBlobStoreAWSTest implements MetricableBlobStoreContrac
}
@AfterEach
- void tearDown() {
+ void tearDown() throws IOException {
objectStorageBlobStore.deleteAllBuckets().block();
objectStorageBlobStore.close();
awsS3ObjectStorage.tearDown();
diff --git a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreTest.java b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreTest.java
index 5b05311..e342525 100644
--- a/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreTest.java
+++ b/server/blob/blob-objectstorage/src/test/java/org/apache/james/blob/objectstorage/ObjectStorageBlobStoreTest.java
@@ -95,7 +95,7 @@ public class ObjectStorageBlobStoreTest implements MetricableBlobStoreContract {
}
@AfterEach
- void tearDown() {
+ void tearDown() throws IOException {
objectStorageBlobStore.deleteAllBuckets().block();
objectStorageBlobStore.close();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org