From 515a23360d4791499e7675c38bbad9e6e244fc7b Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Wed, 6 Jun 2018 16:38:06 +0200 Subject: [PATCH] Do not check for S3 blob to exist before writing (#31128) In #19749 an extra check was added before writing each blob to ensure that we would not be overriding an existing blob. Due to S3's weak consistency model, this check was best effort. To make matters worse, however, this resulted in a HEAD request to be done before every PUT, in particular also when PUTTING a new object. The approach taken in #19749 worsened our consistency guarantees for follow-up snapshot actions, as it made it less likely for new files that had been written to be available for reads. This commit therefore removes this extra check. Due to the weak consistency model, this check was a best effort thing anyway, and there's currently no way to prevent accidental overrides on S3. --- .../org/elasticsearch/repositories/s3/S3BlobContainer.java | 4 ---- .../repositories/s3/S3BlobStoreContainerTests.java | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java index 401ef0933a8..92050e34a5a 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java @@ -96,10 +96,6 @@ class S3BlobContainer extends AbstractBlobContainer { @Override public void writeBlob(String blobName, InputStream inputStream, long blobSize) throws IOException { - if (blobExists(blobName)) { - throw new FileAlreadyExistsException("Blob [" + blobName + "] already exists, cannot overwrite"); - } - SocketAccess.doPrivilegedIOException(() -> { if (blobSize <= blobStore.bufferSizeInBytes()) { executeSingleUpload(blobStore, buildKey(blobName), inputStream, blobSize); diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java index 453ef3213f0..c760e86d135 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java @@ -64,6 +64,11 @@ public class S3BlobStoreContainerTests extends ESBlobStoreContainerTestCase { return randomMockS3BlobStore(); } + @Override + public void testVerifyOverwriteFails() { + assumeFalse("not implemented because of S3's weak consistency model", true); + } + public void testExecuteSingleUploadBlobSizeTooLarge() { final long blobSize = ByteSizeUnit.GB.toBytes(randomIntBetween(6, 10)); final S3BlobStore blobStore = mock(S3BlobStore.class);