From 5cad1c0ebbdc7a7a61b79ca51717133a7fcba67c Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 18 Feb 2025 17:50:35 +1100 Subject: [PATCH 1/2] Handle status code 200 for s3 CMU response When a CopmleteMultipartUpload request fails after the initial 200 response, the status code of the failure response use to be not set and hence got translated to status code 0. With #116212, we handle this case accordingly. Since AWS SDK 1.12.691, the status code is now set to 200 instead of 0. This PR changes our error handling code accordingly. Relates: #122431 Relates: #116212 Relevant AWS SDK change https://github.com/aws/aws-sdk-java/blame/430899c217f34fae63b35c77f4d9bfd361c9de77/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/AmazonS3Client.java#L3696-L3709 --- .../org/elasticsearch/repositories/s3/S3BlobContainer.java | 6 +++--- muted-tests.yml | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java index ea13657964016..a3fef296aa84b 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java @@ -899,11 +899,11 @@ public void compareAndExchangeRegister( logger.trace(() -> Strings.format("[%s]: compareAndExchangeRegister failed", key), e); if (e instanceof AmazonS3Exception amazonS3Exception && (amazonS3Exception.getStatusCode() == 404 - || amazonS3Exception.getStatusCode() == 0 && "NoSuchUpload".equals(amazonS3Exception.getErrorCode()))) { + || amazonS3Exception.getStatusCode() == 200 && "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 - // NoSuchUpload... in the response, which the SDK translates to status code 0. Either way, this means - // that our write encountered contention: + // NoSuchUpload... in the response, which the SDK translates to status code 200 since 1.12.691 + // (0 for prior versions). Either way, this means that our write encountered contention: delegate.onResponse(OptionalBytesReference.MISSING); } else { delegate.onFailure(e); diff --git a/muted-tests.yml b/muted-tests.yml index ddcd4a988dbd7..72b01ef7345de 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -394,9 +394,6 @@ tests: issue: https://github.com/elastic/elasticsearch/issues/122680 - class: org.elasticsearch.entitlement.qa.EntitlementsDeniedIT issue: https://github.com/elastic/elasticsearch/issues/122566 -- class: org.elasticsearch.repositories.blobstore.testkit.analyze.S3RepositoryAnalysisRestIT - method: testRepositoryAnalysis - issue: https://github.com/elastic/elasticsearch/issues/122799 - class: org.elasticsearch.xpack.esql.action.EsqlActionBreakerIT issue: https://github.com/elastic/elasticsearch/issues/122810 From d05c44044bdabcc0c2754ea557f41d66964d1737 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 18 Feb 2025 19:08:30 +1100 Subject: [PATCH 2/2] Update modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java Co-authored-by: David Turner --- .../org/elasticsearch/repositories/s3/S3BlobContainer.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java index a3fef296aa84b..d7f5a6e6dfe36 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java @@ -902,8 +902,7 @@ public void compareAndExchangeRegister( || amazonS3Exception.getStatusCode() == 200 && "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 - // NoSuchUpload... in the response, which the SDK translates to status code 200 since 1.12.691 - // (0 for prior versions). Either way, this means that our write encountered contention: + // NoSuchUpload... in the response. Either way, this means that our write encountered contention: delegate.onResponse(OptionalBytesReference.MISSING); } else { delegate.onFailure(e);