Change default retry mechanism

Signed-off-by: Arpit Bandejiya <abandeji@amazon.com>
opensearch-project · Sep 28, 2024 · 73dc7a8 · 73dc7a8
1 parent 7caca26
commit 73dc7a8
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add successfulSearchShardIndices in searchRequestContext ([#15967](https://github.com/opensearch-project/OpenSearch/pull/15967))
 - Remove identity-related feature flagged code from the RestController ([#15430](https://github.com/opensearch-project/OpenSearch/pull/15430))
 - Add support for msearch API to pass search pipeline name - ([#15923](https://github.com/opensearch-project/OpenSearch/pull/15923))
+- [Remote Publication] Change default retry mechanism of s3 clients ([#15978](https://github.com/opensearch-project/OpenSearch/pull/15978))
 
 ### Dependencies
 - Bump `com.azure:azure-identity` from 1.13.0 to 1.13.2 ([#15578](https://github.com/opensearch-project/OpenSearch/pull/15578))

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
@@ -18,6 +18,7 @@
 import software.amazon.awssdk.core.client.config.ClientAsyncConfiguration;
 import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
 import software.amazon.awssdk.core.client.config.SdkAdvancedAsyncClientOption;
+import software.amazon.awssdk.core.retry.RetryMode;
 import software.amazon.awssdk.core.retry.RetryPolicy;
 import software.amazon.awssdk.core.retry.backoff.BackoffStrategy;
 import software.amazon.awssdk.http.async.SdkAsyncHttpClient;
@@ -239,7 +240,9 @@ static ClientOverrideConfiguration buildOverrideConfiguration(final S3ClientSett
                 RetryPolicy.builder()
                     .numRetries(clientSettings.maxRetries)
                     .throttlingBackoffStrategy(
-                        clientSettings.throttleRetries ? BackoffStrategy.defaultThrottlingStrategy() : BackoffStrategy.none()
+                        clientSettings.throttleRetries
+                            ? BackoffStrategy.defaultThrottlingStrategy(RetryMode.STANDARD)
+                            : BackoffStrategy.none()
                     )
                     .build()
             )

diff --git a/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Service.java b/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Service.java
@@ -42,6 +42,7 @@
 import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
 import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption;
 import software.amazon.awssdk.core.exception.SdkException;
+import software.amazon.awssdk.core.retry.RetryMode;
 import software.amazon.awssdk.core.retry.RetryPolicy;
 import software.amazon.awssdk.core.retry.backoff.BackoffStrategy;
 import software.amazon.awssdk.http.SystemPropertyTlsKeyManagersProvider;
@@ -330,6 +331,8 @@ static ClientOverrideConfiguration buildOverrideConfiguration(final S3ClientSett
         );
         if (!clientSettings.throttleRetries) {
             retryPolicy.throttlingBackoffStrategy(BackoffStrategy.none());
+        } else {
+            retryPolicy.throttlingBackoffStrategy(BackoffStrategy.defaultThrottlingStrategy(RetryMode.STANDARD));
         }
         return clientOverrideConfiguration.retryPolicy(retryPolicy.build()).build();
     }

diff --git a/...ins/repository-s3/src/test/java/org/opensearch/repositories/s3/AwsS3ServiceImplTests.java b/...ins/repository-s3/src/test/java/org/opensearch/repositories/s3/AwsS3ServiceImplTests.java
@@ -35,6 +35,7 @@
 import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
 import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
 import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
+import software.amazon.awssdk.core.retry.RetryMode;
 import software.amazon.awssdk.core.retry.backoff.BackoffStrategy;
 import software.amazon.awssdk.http.apache.ProxyConfiguration;
 
@@ -364,7 +365,7 @@ private void launchAWSConfigurationTest(
         if (expectedUseThrottleRetries) {
             assertThat(
                 clientOverrideConfiguration.retryPolicy().get().throttlingBackoffStrategy(),
-                is(BackoffStrategy.defaultThrottlingStrategy())
+                is(BackoffStrategy.defaultThrottlingStrategy(RetryMode.STANDARD))
             );
         } else {
             assertThat(clientOverrideConfiguration.retryPolicy().get().throttlingBackoffStrategy(), is(BackoffStrategy.none()));

diff --git a/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3AsyncServiceTests.java b/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3AsyncServiceTests.java
@@ -8,6 +8,9 @@
 
 package org.opensearch.repositories.s3;
 
+import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
+import software.amazon.awssdk.core.retry.backoff.FullJitterBackoffStrategy;
+
 import org.opensearch.cli.SuppressForbidden;
 import org.opensearch.cluster.metadata.RepositoryMetadata;
 import org.opensearch.common.settings.MockSecureSettings;
@@ -92,4 +95,14 @@ public void testCachedClientsWithCredentialsAreReleased() {
         final S3ClientSettings clientSettingsReloaded = s3AsyncService.settings(metadata1);
         assertNotSame(clientSettings, clientSettingsReloaded);
     }
+
+    public void testS3AsyncServiceDefaultRetryMechanism() {
+        S3AsyncService s3AsyncService = new S3AsyncService(configPath());
+        Settings settings = Settings.builder().put("endpoint", "http://first").put("region", "us-east-2").build();
+        RepositoryMetadata metadata1 = new RepositoryMetadata("first", "s3", settings);
+        S3ClientSettings clientSettings = s3AsyncService.settings(metadata1);
+
+        ClientOverrideConfiguration clientOverrideConfiguration = S3AsyncService.buildOverrideConfiguration(clientSettings);
+        assertTrue(clientOverrideConfiguration.retryPolicy().get().throttlingBackoffStrategy() instanceof FullJitterBackoffStrategy);
+    }
 }