Ignore NoSuchKey exceptions when deleting container

Author: DaveCTurner

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

@@ -412,7 +412,7 @@ public DeleteResult delete(OperationPurpose purpose) throws IOException {
                     deletedBytes.addAndGet(s3Object.size());
                     return s3Object.key();
                 });
-                blobStore.deleteBlobs(purpose, false, blobNameIterator);
+                blobStore.deleteBlobs(purpose, blobNameIterator);
                 prevListing = listObjectsResponse;
             }
         } catch (final SdkException e) {
@@ -423,7 +423,7 @@ public DeleteResult delete(OperationPurpose purpose) throws IOException {
 
     @Override
     public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
-        blobStore.deleteBlobs(purpose, true, Iterators.map(blobNames, this::buildKey));
+        blobStore.deleteBlobs(purpose, Iterators.map(blobNames, this::buildKey));
     }
 
     @Override

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

@@ -306,11 +306,6 @@ public BlobContainer blobContainer(BlobPath path) {
     private static class DeletionExceptions {
         Exception exception = null;
         private int count = 0;
-        final boolean ignoreNoSuchKey;
-
-        DeletionExceptions(boolean ignoreNoSuchKey) {
-            this.ignoreNoSuchKey = ignoreNoSuchKey;
-        }
 
         void useOrMaybeSuppress(Exception e) {
             if (count < MAX_DELETE_EXCEPTIONS) {
@@ -320,15 +315,15 @@ void useOrMaybeSuppress(Exception e) {
         }
     }
 
-    void deleteBlobs(OperationPurpose purpose, boolean ignoreNoSuchKey, Iterator<String> blobNames) throws IOException {
+    void deleteBlobs(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
         if (blobNames.hasNext() == false) {
             return;
         }
 
         final List<ObjectIdentifier> partition = new ArrayList<>();
         try {
             // S3 API only allows 1k blobs per delete so we split up the given blobs into requests of max. 1k deletes
-            final var deletionExceptions = new DeletionExceptions(ignoreNoSuchKey);
+            final var deletionExceptions = new DeletionExceptions();
             blobNames.forEachRemaining(key -> {
                 partition.add(ObjectIdentifier.builder().key(key).build());
                 if (partition.size() == bulkDeletionBatchSize) {
@@ -360,7 +355,7 @@ private void deletePartition(OperationPurpose purpose, List<ObjectIdentifier> pa
         while (true) {
             try (AmazonS3Reference clientReference = clientReference()) {
                 final var response = clientReference.client().deleteObjects(bulkDelete(purpose, this, partition));
-                if (maybeRecordDeleteErrors(response, deletionExceptions) == false) {
+                if (maybeRecordDeleteErrors(purpose, response, deletionExceptions) == false) {
                     s3RepositoriesMetrics.retryDeletesHistogram().record(retryCounter);
                 }
                 return;
@@ -384,7 +379,11 @@ private void deletePartition(OperationPurpose purpose, List<ObjectIdentifier> pa
         }
     }
 
-    private static boolean maybeRecordDeleteErrors(DeleteObjectsResponse response, DeletionExceptions deletionExceptions) {
+    private static boolean maybeRecordDeleteErrors(
+        OperationPurpose purpose,
+        DeleteObjectsResponse response,
+        DeletionExceptions deletionExceptions
+    ) {
         if (response.hasErrors() == false) {
             return false;
         }
@@ -394,8 +393,9 @@ private static boolean maybeRecordDeleteErrors(DeleteObjectsResponse response, D
         StringBuilder sb = null;
 
         for (final var err : errors) {
-            if (deletionExceptions.ignoreNoSuchKey && "NoSuchKey".equals(err.code())) {
+            if (purpose != OperationPurpose.REPOSITORY_ANALYSIS && "NoSuchKey".equals(err.code())) {
                 // The blob does not exist, which is what we wanted, so let's count that as a win
+                // (except for repo analysis where we can be certain that the blobs being deleted do all exist)
                 continue;
             }