From 23b3741618b016623cdc6a92afbf825d1bcbaef9 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 25 Apr 2019 18:25:03 +0200 Subject: [PATCH] Remove Exists Check from S3 Repository Deletes (#40931) (#41534) * The check doesn't add much if anything practically, since the S3 repository is eventually consistent and we only log the non-existence of a blob anyway * We don't do the check on writes for this very reason and documented it as such * Removing the check saves one API call per single delete speeding up the deletion process and lowering costs --- .../org/elasticsearch/repositories/s3/S3BlobContainer.java | 3 --- .../repositories/s3/S3BlobStoreContainerTests.java | 5 +++++ .../org/elasticsearch/common/blobstore/BlobContainer.java | 6 ++---- 3 files changed, 7 insertions(+), 7 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 f98382e5526..652fa6a3601 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 @@ -119,9 +119,6 @@ class S3BlobContainer extends AbstractBlobContainer { @Override public void deleteBlob(String blobName) throws IOException { - if (blobExists(blobName) == false) { - throw new NoSuchFileException("Blob [" + blobName + "] does not exist"); - } deleteBlobIgnoringIfNotExists(blobName); } 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 b2afd826c5b..422fab45e43 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 @@ -65,6 +65,11 @@ public class S3BlobStoreContainerTests extends ESBlobStoreContainerTestCase { return randomMockS3BlobStore(); } + @Override + public void testDeleteBlob() { + assumeFalse("not implemented because of S3's weak consistency model", true); + } + @Override public void testVerifyOverwriteFails() { assumeFalse("not implemented because of S3's weak consistency model", true); diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java index 19d3a66a87d..9c1bacb51ba 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java @@ -100,7 +100,7 @@ public interface BlobContainer { /** * Deletes the blob with the given name, if the blob exists. If the blob does not exist, - * this method throws a NoSuchFileException. + * this method may throw a {@link NoSuchFileException} if the underlying implementation supports an existence check before delete. * * @param blobName * The name of the blob to delete. @@ -120,9 +120,7 @@ public interface BlobContainer { IOException ioe = null; for (String blobName : blobNames) { try { - deleteBlob(blobName); - } catch (NoSuchFileException e) { - // ignored + deleteBlobIgnoringIfNotExists(blobName); } catch (IOException e) { if (ioe == null) { ioe = e;