Add a new setting for s3 API call timeout #138072

ywangd · 2025-11-14T06:22:44Z

This PR adds a new setting to configure AWS SDK API call timeout (https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/timeouts.html). Defaults to 0, i.e. no timeout.

DaveCTurner

I think this'd be better as an internal-cluster test akin to e.g. S3BlobStoreRepositoryTests or S3BlobContainerRetriesTests. That way we can call writeBlob directly, rather than having to construct a shard of the right sort of size to trigger such a call during the snapshot, and also we can cancel the sleep when the request times out.

elasticsearchmachine · 2025-11-28T07:34:24Z

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

ywangd · 2025-11-28T07:37:19Z

Replaced the REST test with an internal cluster test as suggested in 3c1a680

Also added a new api_call_timeout setting. It defaults to 0, i.e. disabled, so that it is a no-op for existing clusters. We can look into enabling it separately.

elasticsearchmachine · 2025-11-28T07:38:30Z

Hi @ywangd, I've created a changelog YAML for you.

DaveCTurner

Some suggestions/nits but otherwise looks like a sensible approach to me.

DaveCTurner · 2025-11-28T08:59:10Z

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

        static final TimeValue CONNECTION_MAX_IDLE_TIME = TimeValue.timeValueSeconds(60);
        static final int MAX_CONNECTIONS = 50;
        static final int RETRY_COUNT = 3;
+        static final TimeValue API_CALL_TIMEOUT = TimeValue.ZERO; // default to no API call timeout


Could we use TimeValue.MINUS_ONE to mean "no timeout", leaving TimeValue.ZERO to mean "time out immediately"? Or at least to defer the meaning of zero down to the SDK, I haven't followed through to determine whether zero means zero or infinity within the SDK but I'd prefer we had some way to say "don't even invoke clientOverrideConfiguration.apiCallTimeout".

Turns out zero is invalid here:

org.elasticsearch.repositories.RepositoryVerificationException: [repository] path [base_path_integration_tests] is not accessible on master node Caused by: java.lang.IllegalArgumentException: apiCallTimeout must be positive at software.amazon.awssdk.utils.Validate.isPositive(Validate.java:694) ~[?:?] at software.amazon.awssdk.utils.Validate.isPositiveOrNull(Validate.java:711) ~[?:?] at software.amazon.awssdk.core.client.config.ClientOverrideConfiguration.<init>(ClientOverrideConfiguration.java:172) ~[?:?] at software.amazon.awssdk.core.client.config.ClientOverrideConfiguration$DefaultBuilder.build(ClientOverrideConfiguration.java:1126) ~[?:?] at software.amazon.awssdk.core.client.config.ClientOverrideConfiguration$DefaultBuilder.build(ClientOverrideConfiguration.java:786) ~[?:?] at org.elasticsearch.repositories.s3.S3Service.buildConfiguration(S3Service.java:347) ~[?:?] at org.elasticsearch.repositories.s3.S3Service.buildClientBuilder(S3Service.java:200) ~[?:?] at org.elasticsearch.repositories.s3.S3Service.buildClient(S3Service.java:193) ~[?:?] at org.elasticsearch.repositories.s3.S3Service.buildClientReference(S3Service.java:178) ~[?:?] at org.elasticsearch.repositories.s3.S3ClientsManager$ClientsHolder.client(S3ClientsManager.java:326) ~[?:?] at org.elasticsearch.repositories.s3.S3ClientsManager.client(S3ClientsManager.java:192) ~[?:?] at org.elasticsearch.repositories.s3.S3Service.client(S3Service.java:163) ~[?:?] at org.elasticsearch.repositories.s3.S3BlobStore.clientReference(S3BlobStore.java:270) ~[?:?] at org.elasticsearch.repositories.s3.S3BlobContainer.executeSingleUpload(S3BlobContainer.java:550) ~[?:?] at org.elasticsearch.repositories.s3.S3BlobContainer.writeBlob(S3BlobContainer.java:148) ~[?:?] at org.elasticsearch.common.blobstore.BlobContainer.writeBlob(BlobContainer.java:123) ~[elasticsearch-9.3.0-SNAPSHOT.jar:?] at org.elasticsearch.repositories.s3.S3BlobContainer.writeBlobAtomic(S3BlobContainer.java:332) ~[?:?] at org.elasticsearch.repositories.blobstore.BlobStoreRepository.startVerification(BlobStoreRepository.java:2314) ~[elasticsearch-9.3.0-SNAPSHOT.jar:?] at org.elasticsearch.repositories.RepositoriesService.lambda$validatePutRepositoryRequest$11(RepositoriesService.java:403) ~[elasticsearch-9.3.0-SNAPSHOT.jar:?] at org.elasticsearch.action.ActionRunnable$1.doRun(ActionRunnable.java:37) ~[elasticsearch-9.3.0-SNAPSHOT.jar:?] at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:1113) ~[elasticsearch-9.3.0-SNAPSHOT.jar:?] at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:27) ~[elasticsearch-9.3.0-SNAPSHOT.jar:?] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1090) ~[?:?] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:614) ~[?:?] at java.lang.Thread.run(Thread.java:1474) ~[?:?]

Still I'd prefer to use -1 to mean "missing".

DaveCTurner · 2025-11-28T08:59:29Z

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

+    /**
+     * The maximum time for a single attempt of an API operation
+     */
+    final int apiCallTimeoutMillis;


Could we keep this as a TimeValue until it's actually used?

DaveCTurner · 2025-11-28T09:00:47Z

...nalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTimeoutTests.java

+                new BytesArray(randomBytes((int) ByteSizeValue.ofMb(10).getBytes())),
+                randomBoolean()
+            );
+            fail("should have timed out");


Could we use expectThrows() here?

DaveCTurner · 2025-11-28T09:02:40Z

...nalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTimeoutTests.java

+        } catch (IOException e) {
+            final var cause = ExceptionsHelper.unwrap(e, ApiCallTimeoutException.class);
+            assertNotNull(cause);
+            assertThat(cause.getMessage(), containsString("Client execution did not complete before the specified timeout configuration"));


Could we assert something about how these outcomes are captured in the metrics? Should we add something specific to S3RepositoriesMetrics to track this case separately from other exceptions?

DaveCTurner · 2025-11-28T09:04:00Z

...nalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTimeoutTests.java

+                    headerDecodedContentLength
+                );
+                try {
+                    final var released = latch.await(60, TimeUnit.SECONDS);


Could we use safeAwait()? At least, I'd like us to be asserting that the latch is released in a reasonably timely manner.

Add a test to demostrate the issue

a92d132

ywangd added >non-issue :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v9.3.0 labels Nov 14, 2025

ywangd and others added 2 commits November 14, 2025 17:52

forbidden API

e36b2bd

[CI] Auto commit changes from spotless

02101a0

DaveCTurner reviewed Nov 18, 2025

View reviewed changes

ywangd added 3 commits November 28, 2025 17:15

Merge remote-tracking branch 'origin/main' into s3-write-hanging-fix

3153b52

Replace REST test with internal cluster test

3c1a680

add new apiCallTimoutSetting

a123c80

ywangd marked this pull request as ready for review November 28, 2025 07:34

elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Nov 28, 2025

ywangd requested a review from DaveCTurner November 28, 2025 07:37

ywangd changed the title ~~Test for S3 upload time unresponsiveness~~ Add a new setting for s3 API call timeout Nov 28, 2025

ywangd added >enhancement and removed >non-issue labels Nov 28, 2025

ywangd and others added 2 commits November 28, 2025 18:38

Update docs/changelog/138072.yaml

c38f377

Merge branch 'main' into s3-write-hanging-fix

e8745de

DaveCTurner reviewed Nov 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add a new setting for s3 API call timeout #138072

Add a new setting for s3 API call timeout #138072

ywangd commented Nov 14, 2025 •

edited

Loading

Uh oh!

DaveCTurner left a comment

Uh oh!

elasticsearchmachine commented Nov 28, 2025

Uh oh!

ywangd commented Nov 28, 2025

Uh oh!

elasticsearchmachine commented Nov 28, 2025

Uh oh!

DaveCTurner left a comment

Uh oh!

DaveCTurner Nov 28, 2025

Uh oh!

DaveCTurner Nov 28, 2025

Uh oh!

DaveCTurner Nov 28, 2025

Uh oh!

DaveCTurner Nov 28, 2025

Uh oh!

DaveCTurner Nov 28, 2025

Uh oh!

DaveCTurner Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add a new setting for s3 API call timeout #138072

Are you sure you want to change the base?

Add a new setting for s3 API call timeout #138072

Conversation

ywangd commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Nov 28, 2025

Uh oh!

ywangd commented Nov 28, 2025

Uh oh!

elasticsearchmachine commented Nov 28, 2025

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

DaveCTurner Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

DaveCTurner Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

DaveCTurner Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

DaveCTurner Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

DaveCTurner Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

DaveCTurner Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ywangd commented Nov 14, 2025 •

edited

Loading