Skip to content

Expose S3 connection max idle time as a setting #125552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/125552.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 125552
summary: Expose S3 connection max idle time as a setting
area: Snapshot/Restore
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,12 @@ final class S3ClientSettings {
key -> Setting.simpleString(key, 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)
);
Comment on lines +176 to +180
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to update the docs for this. But was not able to find the source file anymore (asciidoc or md). Not sure whether this is a bug introduced in #123507. I asked in the es-docs channel.


/** Credentials to authenticate with s3. */
final S3BasicCredentials credentials;

Expand Down Expand Up @@ -223,6 +229,11 @@ final class S3ClientSettings {
/** Signer override to use or empty string to use default. */
final String signerOverride;

/**
* The maximum idle time (in millis) of a connection before it is discarded from the connection pool.
*/
final long connectionMaxIdleTimeMillis;

private S3ClientSettings(
S3BasicCredentials credentials,
String endpoint,
Expand All @@ -239,7 +250,8 @@ private S3ClientSettings(
boolean pathStyleAccess,
boolean disableChunkedEncoding,
String region,
String signerOverride
String signerOverride,
long connectionMaxIdleTimeMillis
) {
this.credentials = credentials;
this.endpoint = endpoint;
Expand All @@ -257,6 +269,7 @@ private S3ClientSettings(
this.disableChunkedEncoding = disableChunkedEncoding;
this.region = region;
this.signerOverride = signerOverride;
this.connectionMaxIdleTimeMillis = connectionMaxIdleTimeMillis;
}

/**
Expand Down Expand Up @@ -297,6 +310,11 @@ S3ClientSettings refine(Settings repositorySettings) {
}
final String newRegion = getRepoSettingOrDefault(REGION, normalizedSettings, region);
final String newSignerOverride = getRepoSettingOrDefault(SIGNER_OVERRIDE, normalizedSettings, signerOverride);
final long newConnectionMaxIdleTimeMillis = getRepoSettingOrDefault(
CONNECTION_MAX_IDLE_TIME_SETTING,
normalizedSettings,
TimeValue.timeValueMillis(connectionMaxIdleTimeMillis)
).millis();
if (Objects.equals(endpoint, newEndpoint)
&& protocol == newProtocol
&& Objects.equals(proxyHost, newProxyHost)
Expand All @@ -310,7 +328,8 @@ S3ClientSettings refine(Settings repositorySettings) {
&& newPathStyleAccess == pathStyleAccess
&& newDisableChunkedEncoding == disableChunkedEncoding
&& Objects.equals(region, newRegion)
&& Objects.equals(signerOverride, newSignerOverride)) {
&& Objects.equals(signerOverride, newSignerOverride)
&& Objects.equals(connectionMaxIdleTimeMillis, newConnectionMaxIdleTimeMillis)) {
return this;
}
return new S3ClientSettings(
Expand All @@ -329,7 +348,8 @@ S3ClientSettings refine(Settings repositorySettings) {
newPathStyleAccess,
newDisableChunkedEncoding,
newRegion,
newSignerOverride
newSignerOverride,
newConnectionMaxIdleTimeMillis
);
}

Expand Down Expand Up @@ -438,7 +458,8 @@ static S3ClientSettings getClientSettings(final Settings settings, final String
getConfigValue(settings, clientName, USE_PATH_STYLE_ACCESS),
getConfigValue(settings, clientName, DISABLE_CHUNKED_ENCODING),
getConfigValue(settings, clientName, REGION),
getConfigValue(settings, clientName, SIGNER_OVERRIDE)
getConfigValue(settings, clientName, SIGNER_OVERRIDE),
getConfigValue(settings, clientName, CONNECTION_MAX_IDLE_TIME_SETTING).millis()
);
}
}
Expand Down Expand Up @@ -466,7 +487,8 @@ public boolean equals(final Object o) {
&& Objects.equals(proxyPassword, that.proxyPassword)
&& Objects.equals(disableChunkedEncoding, that.disableChunkedEncoding)
&& Objects.equals(region, that.region)
&& Objects.equals(signerOverride, that.signerOverride);
&& Objects.equals(signerOverride, that.signerOverride)
&& Objects.equals(connectionMaxIdleTimeMillis, that.connectionMaxIdleTimeMillis);
}

@Override
Expand All @@ -486,7 +508,8 @@ public int hashCode() {
throttleRetries,
disableChunkedEncoding,
region,
signerOverride
signerOverride,
connectionMaxIdleTimeMillis
);
}

Expand All @@ -507,5 +530,6 @@ static final class Defaults {
static final int MAX_CONNECTIONS = ClientConfiguration.DEFAULT_MAX_CONNECTIONS;
static final int RETRY_COUNT = ClientConfiguration.DEFAULT_RETRY_POLICY.getMaxErrorRetry();
static final boolean THROTTLE_RETRIES = ClientConfiguration.DEFAULT_THROTTLE_RETRIES;
static final TimeValue CONNECTION_MAX_IDLE_TIME = TimeValue.timeValueMillis(ClientConfiguration.DEFAULT_CONNECTION_MAX_IDLE_MILLIS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public List<Setting<?>> getSettings() {
S3ClientSettings.USE_THROTTLE_RETRIES_SETTING,
S3ClientSettings.USE_PATH_STYLE_ACCESS,
S3ClientSettings.SIGNER_OVERRIDE,
S3ClientSettings.CONNECTION_MAX_IDLE_TIME_SETTING,
S3ClientSettings.REGION,
S3Service.REPOSITORY_S3_CAS_TTL_SETTING,
S3Service.REPOSITORY_S3_CAS_ANTI_CONTENTION_DELAY_SETTING,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ static ClientConfiguration buildConfiguration(S3ClientSettings clientSettings, b
clientConfiguration.setMaxErrorRetry(clientSettings.maxRetries);
clientConfiguration.setUseThrottleRetries(clientSettings.throttleRetries);
clientConfiguration.setSocketTimeout(clientSettings.readTimeoutMillis);
clientConfiguration.setConnectionMaxIdleMillis(clientSettings.connectionMaxIdleTimeMillis);

if (isStateless) {
clientConfiguration.setRetryPolicy(RETRYABLE_403_RETRY_POLICY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.watcher.ResourceWatcherService;
Expand Down Expand Up @@ -46,6 +47,7 @@ public void testThereIsADefaultClientByDefault() {
assertThat(defaultSettings.maxConnections, is(ClientConfiguration.DEFAULT_MAX_CONNECTIONS));
assertThat(defaultSettings.maxRetries, is(ClientConfiguration.DEFAULT_RETRY_POLICY.getMaxErrorRetry()));
assertThat(defaultSettings.throttleRetries, is(ClientConfiguration.DEFAULT_THROTTLE_RETRIES));
assertThat(defaultSettings.connectionMaxIdleTimeMillis, is(ClientConfiguration.DEFAULT_CONNECTION_MAX_IDLE_MILLIS));
}

public void testDefaultClientSettingsCanBeSet() {
Expand Down Expand Up @@ -200,6 +202,22 @@ public void testSignerOverrideCanBeSet() {
assertThat(configuration.getSignerOverride(), is(signerOverride));
}

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()));
ClientConfiguration defaultConfiguration = S3Service.buildConfiguration(settings.get("default"), randomBoolean());
assertThat(defaultConfiguration.getConnectionMaxIdleMillis(), is(S3ClientSettings.Defaults.CONNECTION_MAX_IDLE_TIME.millis()));
ClientConfiguration configuration = S3Service.buildConfiguration(settings.get("other"), randomBoolean());
assertThat(configuration.getConnectionMaxIdleMillis(), is(connectionMaxIdleTimeValue.millis()));
}

public void testMaxConnectionsCanBeSet() {
final int maxConnections = between(1, 100);
final Map<String, S3ClientSettings> settings = S3ClientSettings.load(
Expand Down
Loading