Skip to content
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.

Sign up for GitHub

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

[8.x] Handle status code 0 in S3 CMU response (#116212) #116232

Merged
merged 1 commit into from
Nov 5, 2024
Merged

[8.x] Handle status code 0 in S3 CMU response (#116212) #116232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions docs/changelog/116212.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 116212
summary: Handle status code 0 in S3 CMU response
area: Snapshot/Restore
type: bug
issues:
- 102294
9 changes: 7 additions & 2 deletions modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,8 +882,13 @@ public void compareAndExchangeRegister(
final var clientReference = blobStore.clientReference();
ActionListener.run(ActionListener.releaseAfter(listener.delegateResponse((delegate, e) -> {
logger.trace(() -> Strings.format("[%s]: compareAndExchangeRegister failed", key), e);
if (e instanceof AmazonS3Exception amazonS3Exception && amazonS3Exception.getStatusCode() == 404) {
// an uncaught 404 means that our multipart upload was aborted by a concurrent operation before we could complete it
if (e instanceof AmazonS3Exception amazonS3Exception
&& (amazonS3Exception.getStatusCode() == 404
|| amazonS3Exception.getStatusCode() == 0 && "NoSuchUpload".equals(amazonS3Exception.getErrorCode()))) {
// An uncaught 404 means that our multipart upload was aborted by a concurrent operation before we could complete it.
// Also (rarely) S3 can start processing the request during a concurrent abort and this can result in a 200 OK with an
// <Error><Code>NoSuchUpload</Code>... in the response, which the SDK translates to status code 0. Either way, this means
// that our write encountered contention:
delegate.onResponse(OptionalBytesReference.MISSING);
} else {
delegate.onFailure(e);
Expand Down
17 changes: 16 additions & 1 deletion test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.sun.net.httpserver.HttpHandler;

import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.bytes.BytesReference;
Expand Down Expand Up @@ -168,7 +169,21 @@ public void handle(final HttpExchange exchange) throws IOException {
RestUtils.decodeQueryString(request, request.indexOf('?') + 1, params);
final var upload = uploads.remove(params.get("uploadId"));
if (upload == null) {
exchange.sendResponseHeaders(RestStatus.NOT_FOUND.getStatus(), -1);
if (Randomness.get().nextBoolean()) {
exchange.sendResponseHeaders(RestStatus.NOT_FOUND.getStatus(), -1);
} else {
byte[] response = ("""
<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>NoSuchUpload</Code>
<Message>No such upload</Message>
<RequestId>test-request-id</RequestId>
<HostId>test-host-id</HostId>
</Error>""").getBytes(StandardCharsets.UTF_8);
exchange.getResponseHeaders().add("Content-Type", "application/xml");
exchange.sendResponseHeaders(RestStatus.OK.getStatus(), response.length);
exchange.getResponseBody().write(response);
}
} else {
final var blobContents = upload.complete(extractPartEtags(Streams.readFully(exchange.getRequestBody())));
blobs.put(requestComponents.path, blobContents);
Expand Down
7 changes: 0 additions & 7 deletions .../org/elasticsearch/repositories/blobstore/testkit/analyze/S3RepositoryAnalysisRestIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@ public class S3RepositoryAnalysisRestIT extends AbstractRepositoryAnalysisRestTe
.setting("s3.client.repo_test_kit.protocol", () -> "http", (n) -> USE_FIXTURE)
.setting("s3.client.repo_test_kit.endpoint", s3Fixture::getAddress, (n) -> USE_FIXTURE)
.setting("xpack.security.enabled", "false")
// Additional tracing related to investigation into https://github.com/elastic/elasticsearch/issues/102294
.setting("logger.org.elasticsearch.repositories.s3", "TRACE")
.setting("logger.org.elasticsearch.repositories.blobstore.testkit", "TRACE")
.setting("logger.com.amazonaws.request", "DEBUG")
.setting("logger.org.apache.http.wire", "DEBUG")
// Necessary to permit setting the above two restricted loggers to DEBUG
.jvmArg("-Des.insecure_network_trace_enabled=true")
.build();

@ClassRule
Expand Down