Expose S3 connection max idle time as a setting (#125552)

Resolves: ES-10815

Author: ywangd

docs/changelog/125552.yaml

@@ -0,0 +1,5 @@
+pr: 125552
+summary: Expose S3 connection max idle time as a setting
+area: Snapshot/Restore
+type: enhancement
+issues: []

modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java

@@ -182,6 +182,13 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
         if (region != null) {
             builder.put(S3ClientSettings.REGION.getConcreteSettingForNamespace("test").getKey(), region);
         }
+        if (randomBoolean()) {
+            // Sometimes explicitly set connection max idle time to ensure it is configurable
+            builder.put(
+                S3ClientSettings.CONNECTION_MAX_IDLE_TIME_SETTING.getConcreteSettingForNamespace("test").getKey(),
+                S3ClientSettings.Defaults.CONNECTION_MAX_IDLE_TIME
+            );
+        }
         return builder.build();
     }
 

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3ClientSettings.java

@@ -186,6 +186,12 @@ final class S3ClientSettings {
         key -> Setting.boolSetting(key, false, Property.NodeScope)
     );
 
+    static final Setting.AffixSetting<TimeValue> CONNECTION_MAX_IDLE_TIME_SETTING = Setting.affixKeySetting(
+        PREFIX,
+        "connection_max_idle_time",
+        key -> Setting.timeSetting(key, Defaults.CONNECTION_MAX_IDLE_TIME, Property.NodeScope)
+    );
+
     /** Credentials to authenticate with s3. */
     final AwsCredentials credentials;
 
@@ -215,6 +221,11 @@ final class S3ClientSettings {
     /** The read timeout for the s3 client. */
     final int readTimeoutMillis;
 
+    /**
+     * The maximum idle time (in millis) of a connection before it is discarded from the connection pool.
+     */
+    final long connectionMaxIdleTimeMillis;
+
     /** The maximum number of concurrent connections to use. */
     final int maxConnections;
 
@@ -243,6 +254,7 @@ private S3ClientSettings(
         String proxyUsername,
         String proxyPassword,
         int readTimeoutMillis,
+        long connectionMaxIdleTimeMillis,
         int maxConnections,
         int maxRetries,
         boolean pathStyleAccess,
@@ -259,6 +271,7 @@ private S3ClientSettings(
         this.proxyUsername = proxyUsername;
         this.proxyPassword = proxyPassword;
         this.readTimeoutMillis = readTimeoutMillis;
+        this.connectionMaxIdleTimeMillis = connectionMaxIdleTimeMillis;
         this.maxConnections = maxConnections;
         this.maxRetries = maxRetries;
         this.pathStyleAccess = pathStyleAccess;
@@ -308,12 +321,18 @@ S3ClientSettings refine(Settings repositorySettings) {
             newCredentials = credentials;
         }
         final String newRegion = getRepoSettingOrDefault(REGION, normalizedSettings, region);
+        final long newConnectionMaxIdleTimeMillis = getRepoSettingOrDefault(
+            CONNECTION_MAX_IDLE_TIME_SETTING,
+            normalizedSettings,
+            TimeValue.timeValueMillis(connectionMaxIdleTimeMillis)
+        ).millis();
         if (Objects.equals(protocol, newProtocol)
             && Objects.equals(endpoint, newEndpoint)
             && Objects.equals(proxyHost, newProxyHost)
             && proxyPort == newProxyPort
             && proxyScheme == newProxyScheme
             && newReadTimeoutMillis == readTimeoutMillis
+            && Objects.equals(connectionMaxIdleTimeMillis, newConnectionMaxIdleTimeMillis)
             && maxConnections == newMaxConnections
             && maxRetries == newMaxRetries
             && Objects.equals(credentials, newCredentials)
@@ -333,6 +352,7 @@ S3ClientSettings refine(Settings repositorySettings) {
             proxyUsername,
             proxyPassword,
             newReadTimeoutMillis,
+            newConnectionMaxIdleTimeMillis,
             newMaxConnections,
             newMaxRetries,
             newPathStyleAccess,
@@ -441,6 +461,7 @@ static S3ClientSettings getClientSettings(final Settings settings, final String
                 proxyUsername.toString(),
                 proxyPassword.toString(),
                 Math.toIntExact(getConfigValue(settings, clientName, READ_TIMEOUT_SETTING).millis()),
+                getConfigValue(settings, clientName, CONNECTION_MAX_IDLE_TIME_SETTING).millis(),
                 getConfigValue(settings, clientName, MAX_CONNECTIONS_SETTING),
                 getConfigValue(settings, clientName, MAX_RETRIES_SETTING),
                 getConfigValue(settings, clientName, USE_PATH_STYLE_ACCESS),
@@ -462,6 +483,7 @@ public boolean equals(final Object o) {
         final S3ClientSettings that = (S3ClientSettings) o;
         return proxyPort == that.proxyPort
             && readTimeoutMillis == that.readTimeoutMillis
+            && Objects.equals(connectionMaxIdleTimeMillis, that.connectionMaxIdleTimeMillis)
             && maxConnections == that.maxConnections
             && maxRetries == that.maxRetries
             && Objects.equals(credentials, that.credentials)
@@ -488,6 +510,7 @@ public int hashCode() {
             proxyUsername,
             proxyPassword,
             readTimeoutMillis,
+            connectionMaxIdleTimeMillis,
             maxRetries,
             maxConnections,
             disableChunkedEncoding,
@@ -510,6 +533,7 @@ private static <T> T getRepoSettingOrDefault(Setting.AffixSetting<T> setting, Se
 
     static final class Defaults {
         static final TimeValue READ_TIMEOUT = TimeValue.timeValueSeconds(50);
+        static final TimeValue CONNECTION_MAX_IDLE_TIME = TimeValue.timeValueSeconds(60);
         static final int MAX_CONNECTIONS = 50;
         static final int RETRY_COUNT = 3;
     }

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java

@@ -152,6 +152,7 @@ public List<Setting<?>> getSettings() {
             S3ClientSettings.UNUSED_SIGNER_OVERRIDE,
             S3ClientSettings.ADD_PURPOSE_CUSTOM_QUERY_PARAMETER,
             S3ClientSettings.REGION,
+            S3ClientSettings.CONNECTION_MAX_IDLE_TIME_SETTING,
             S3Service.REPOSITORY_S3_CAS_TTL_SETTING,
             S3Service.REPOSITORY_S3_CAS_ANTI_CONTENTION_DELAY_SETTING,
             S3Repository.ACCESS_KEY_SETTING,

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java

@@ -310,6 +310,7 @@ private SdkHttpClient buildHttpClient(
 
         httpClientBuilder.maxConnections(clientSettings.maxConnections);
         httpClientBuilder.socketTimeout(Duration.ofMillis(clientSettings.readTimeoutMillis));
+        httpClientBuilder.connectionMaxIdleTime(Duration.ofMillis(clientSettings.connectionMaxIdleTimeMillis));
 
         Optional<ProxyConfiguration> proxyConfiguration = buildProxyConfiguration(clientSettings);
         if (proxyConfiguration.isPresent()) {

modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java

@@ -18,6 +18,7 @@
 import org.elasticsearch.common.settings.MockSecureSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue;
+import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ClusterServiceUtils;
 import org.elasticsearch.test.ESTestCase;
@@ -49,6 +50,7 @@ public void testThereIsADefaultClientByDefault() {
         assertThat(defaultSettings.readTimeoutMillis, is(Math.toIntExact(S3ClientSettings.Defaults.READ_TIMEOUT.millis())));
         assertThat(defaultSettings.maxConnections, is(S3ClientSettings.Defaults.MAX_CONNECTIONS));
         assertThat(defaultSettings.maxRetries, is(S3ClientSettings.Defaults.RETRY_COUNT));
+        assertThat(defaultSettings.connectionMaxIdleTimeMillis, is(S3ClientSettings.Defaults.CONNECTION_MAX_IDLE_TIME.millis()));
     }
 
     public void testDefaultClientSettingsCanBeSet() {
@@ -206,6 +208,21 @@ public void testRegionCanBeSet() {
         }
     }
 
+    public void testConnectionMaxIdleTimeCanBeSet() {
+        final TimeValue connectionMaxIdleTimeValue = randomValueOtherThan(
+            S3ClientSettings.Defaults.CONNECTION_MAX_IDLE_TIME,
+            ESTestCase::randomTimeValue
+        );
+        final Map<String, S3ClientSettings> settings = S3ClientSettings.load(
+            Settings.builder().put("s3.client.other.connection_max_idle_time", connectionMaxIdleTimeValue).build()
+        );
+        assertThat(settings.get("default").connectionMaxIdleTimeMillis, is(S3ClientSettings.Defaults.CONNECTION_MAX_IDLE_TIME.millis()));
+        assertThat(settings.get("other").connectionMaxIdleTimeMillis, is(connectionMaxIdleTimeValue.millis()));
+
+        // the default appears in the docs so let's make sure it doesn't change:
+        assertEquals(TimeValue.timeValueSeconds(60), S3ClientSettings.Defaults.CONNECTION_MAX_IDLE_TIME);
+    }
+
     public void testMaxConnectionsCanBeSet() {
         final int maxConnections = between(1, 100);
         final Map<String, S3ClientSettings> settings = S3ClientSettings.load(