Fix S3BlobStoreRepositoryTests

Signed-off-by: Ashish Singh <[email protected]>
opensearch-project · Dec 17, 2024 · 49d893f · 49d893f
1 parent 1c58299
commit 49d893f
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 13 deletions.
diff --git a/...c/internalClusterTest/java/org/opensearch/repositories/s3/S3BlobStoreRepositoryTests.java b/...c/internalClusterTest/java/org/opensearch/repositories/s3/S3BlobStoreRepositoryTests.java
@@ -96,7 +96,6 @@ public class S3BlobStoreRepositoryTests extends OpenSearchMockAPIBasedRepository
     public void setUp() throws Exception {
         signerOverride = AwsRequestSigner.VERSION_FOUR_SIGNER.getName();
         previousOpenSearchPathConf = SocketAccess.doPrivileged(() -> System.setProperty("opensearch.path.conf", "config"));
-        logger.info("previousOpenSearchPathConf={}", previousOpenSearchPathConf);
         super.setUp();
     }
 

diff --git a/.../internalClusterTest/java/org/opensearch/repositories/s3/S3RepositoryThirdPartyTests.java b/.../internalClusterTest/java/org/opensearch/repositories/s3/S3RepositoryThirdPartyTests.java
@@ -55,11 +55,6 @@
 
 public class S3RepositoryThirdPartyTests extends AbstractThirdPartyRepositoryTestCase {
 
-    @Override
-    protected Settings nodeSettings() {
-        return Settings.builder().put(super.nodeSettings()).build();
-    }
-
     @Override
     @Before
     @SuppressForbidden(reason = "Need to set system property here for AWS SDK v2")

diff --git a/...ns/repository-s3/src/main/java/org/opensearch/repositories/s3/AmazonAsyncS3Reference.java b/...ns/repository-s3/src/main/java/org/opensearch/repositories/s3/AmazonAsyncS3Reference.java
@@ -25,7 +25,7 @@ public class AmazonAsyncS3Reference extends RefCountedReleasable<AmazonAsyncS3Wi
 
     private static final Logger logger = LogManager.getLogger(AmazonAsyncS3Reference.class);
 
-    AmazonAsyncS3Reference(AmazonAsyncS3WithCredentials client) {
+    AmazonAsyncS3Reference(AmazonAsyncS3WithCredentials client, Runnable onClose) {
         super("AWS_S3_CLIENT", client, () -> {
             client.client().close();
             client.priorityClient().close();
@@ -38,6 +38,7 @@ public class AmazonAsyncS3Reference extends RefCountedReleasable<AmazonAsyncS3Wi
                     logger.error("Exception while closing AwsCredentialsProvider", e);
                 }
             }
+            onClose.run();
         });
     }
 }
diff --git a/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3AsyncService.java b/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3AsyncService.java
@@ -119,8 +119,14 @@ public AmazonAsyncS3Reference client(
             if (existing != null && existing.tryIncRef()) {
                 return existing;
             }
+
             final AmazonAsyncS3Reference clientReference = new AmazonAsyncS3Reference(
-                buildClient(clientSettings, urgentExecutorBuilder, priorityExecutorBuilder, normalExecutorBuilder)
+                buildClient(clientSettings, urgentExecutorBuilder, priorityExecutorBuilder, normalExecutorBuilder),
+                () -> {
+                    urgentExecutorBuilder.getAsyncTransferEventLoopGroup().close();
+                    priorityExecutorBuilder.getAsyncTransferEventLoopGroup().close();
+                    normalExecutorBuilder.getAsyncTransferEventLoopGroup().close();
+                }
             );
             clientReference.incRef();
             clientsCache = MapBuilder.newMapBuilder(clientsCache).put(clientSettings, clientReference).immutableMap();
@@ -437,5 +443,6 @@ public AwsCredentials resolveCredentials() {
     @Override
     public void close() {
         releaseCachedClients();
+
     }
 }
diff --git a/...itory-s3/src/test/java/org/opensearch/repositories/s3/S3BlobContainerMockClientTests.java b/...itory-s3/src/test/java/org/opensearch/repositories/s3/S3BlobContainerMockClientTests.java
@@ -294,7 +294,7 @@ public AmazonAsyncS3Reference client(
             AsyncExecutorContainer priorityExecutorBuilder,
             AsyncExecutorContainer normalExecutorBuilder
         ) {
-            return new AmazonAsyncS3Reference(AmazonAsyncS3WithCredentials.create(asyncClient, asyncClient, asyncClient, null));
+            return new AmazonAsyncS3Reference(AmazonAsyncS3WithCredentials.create(asyncClient, asyncClient, asyncClient, null), () -> {});
         }
     }
 

diff --git a/...repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java b/...repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java
@@ -1136,7 +1136,8 @@ public void testReadBlobAsyncMultiPart() throws Exception {
 
         final S3AsyncClient s3AsyncClient = mock(S3AsyncClient.class);
         final AmazonAsyncS3Reference amazonAsyncS3Reference = new AmazonAsyncS3Reference(
-            AmazonAsyncS3WithCredentials.create(s3AsyncClient, s3AsyncClient, s3AsyncClient, null)
+            AmazonAsyncS3WithCredentials.create(s3AsyncClient, s3AsyncClient, s3AsyncClient, null),
+            () -> {}
         );
 
         final S3BlobStore blobStore = mock(S3BlobStore.class);
@@ -1194,7 +1195,8 @@ public void testReadBlobAsyncSinglePart() throws Exception {
 
         final S3AsyncClient s3AsyncClient = mock(S3AsyncClient.class);
         final AmazonAsyncS3Reference amazonAsyncS3Reference = new AmazonAsyncS3Reference(
-            AmazonAsyncS3WithCredentials.create(s3AsyncClient, s3AsyncClient, s3AsyncClient, null)
+            AmazonAsyncS3WithCredentials.create(s3AsyncClient, s3AsyncClient, s3AsyncClient, null),
+            () -> {}
         );
         final S3BlobStore blobStore = mock(S3BlobStore.class);
         final BlobPath blobPath = new BlobPath();
@@ -1249,7 +1251,8 @@ public void testReadBlobAsyncFailure() throws Exception {
 
         final S3AsyncClient s3AsyncClient = mock(S3AsyncClient.class);
         final AmazonAsyncS3Reference amazonAsyncS3Reference = new AmazonAsyncS3Reference(
-            AmazonAsyncS3WithCredentials.create(s3AsyncClient, s3AsyncClient, s3AsyncClient, null)
+            AmazonAsyncS3WithCredentials.create(s3AsyncClient, s3AsyncClient, s3AsyncClient, null),
+            () -> {}
         );
 
         final S3BlobStore blobStore = mock(S3BlobStore.class);
@@ -1292,7 +1295,8 @@ public void testReadBlobAsyncOnCompleteFailureMissingData() throws Exception {
 
         final S3AsyncClient s3AsyncClient = mock(S3AsyncClient.class);
         final AmazonAsyncS3Reference amazonAsyncS3Reference = new AmazonAsyncS3Reference(
-            AmazonAsyncS3WithCredentials.create(s3AsyncClient, s3AsyncClient, s3AsyncClient, null)
+            AmazonAsyncS3WithCredentials.create(s3AsyncClient, s3AsyncClient, s3AsyncClient, null),
+            () -> {}
         );
 
         final S3BlobStore blobStore = mock(S3BlobStore.class);