Do not check for object existence when deleting repository index files (#31680)

Before deleting a repository index generation file, BlobStoreRepository
checks for the existence of the file and then deletes it. We can save
a request here by using BlobContainer.deleteBlobIgnoringIfNotExists()
 which ignores error when deleting a file that does not exist.

Since there is no way with S3 to know if a non versioned file existed
before being deleted, this pull request also changes S3BlobContainer so
that it now implements deleteBlobIgnoringIfNotExists(). It will now save
 one more request (blobExist?) when appropriate. The tests and fixture
 have been modified to conform the S3 API that always returns a 204/NO
 CONTENT HTTP response on deletions.

Author: tlrx

plugins/repository-s3/qa/amazon-s3/src/test/java/org/elasticsearch/repositories/s3/AmazonS3Fixture.java

@@ -174,10 +174,8 @@ private static PathTrie<RequestHandler> defaultHandlers(final Map<String, Bucket
                 }
 
                 final String objectName = objectName(request.getParameters());
-                if (bucket.objects.remove(objectName) != null) {
-                    return new Response(RestStatus.OK.getStatus(), TEXT_PLAIN_CONTENT_TYPE, EMPTY_BYTE);
-                }
-                return newObjectNotFoundError(request.getId(), objectName);
+                bucket.objects.remove(objectName);
+                return new Response(RestStatus.NO_CONTENT.getStatus(), TEXT_PLAIN_CONTENT_TYPE, EMPTY_BYTE);
             })
         );
 

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

@@ -107,8 +107,13 @@ public void deleteBlob(String blobName) throws IOException {
         if (blobExists(blobName) == false) {
             throw new NoSuchFileException("Blob [" + blobName + "] does not exist");
         }
+        deleteBlobIgnoringIfNotExists(blobName);
+    }
 
+    @Override
+    public void deleteBlobIgnoringIfNotExists(String blobName) throws IOException {
         try (AmazonS3Reference clientReference = blobStore.clientReference()) {
+            // There is no way to know if an non-versioned object existed before the deletion
             SocketAccess.doPrivilegedVoid(() -> clientReference.client().deleteObject(blobStore.bucket(), buildKey(blobName)));
         } catch (final AmazonClientException e) {
             throw new IOException("Exception when deleting blob [" + blobName + "]", e);

plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java

@@ -149,15 +149,9 @@ public ObjectListing listObjects(final ListObjectsRequest request) throws Amazon
     @Override
     public void deleteObject(final DeleteObjectRequest request) throws AmazonClientException {
         assertThat(request.getBucketName(), equalTo(bucket));
-
-        final String blobName = request.getKey();
-        if (blobs.remove(blobName) == null) {
-            AmazonS3Exception exception = new AmazonS3Exception("[" + blobName + "] does not exist.");
-            exception.setStatusCode(404);
-            throw exception;
-        }
+        blobs.remove(request.getKey());
     }
-    
+
     @Override
     public void shutdown() {
         // TODO check close

server/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java

@@ -389,19 +389,6 @@ public BlobStoreIndexShardSnapshot(String snapshot, long indexVersion, List<File
         this.incrementalSize = incrementalSize;
     }
 
-    /**
-     * Special constructor for the prototype
-     */
-    private BlobStoreIndexShardSnapshot() {
-        this.snapshot = "";
-        this.indexVersion = 0;
-        this.indexFiles = Collections.emptyList();
-        this.startTime = 0;
-        this.time = 0;
-        this.incrementalFileCount = 0;
-        this.incrementalSize = 0;
-    }
-
     /**
      * Returns index version
      *

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

@@ -668,9 +668,7 @@ protected void writeIndexGen(final RepositoryData repositoryData, final long rep
         // delete the N-2 index file if it exists, keep the previous one around as a backup
         if (isReadOnly() == false && newGen - 2 >= 0) {
             final String oldSnapshotIndexFile = INDEX_FILE_PREFIX + Long.toString(newGen - 2);
-            if (snapshotsBlobContainer.blobExists(oldSnapshotIndexFile)) {
-                snapshotsBlobContainer.deleteBlob(oldSnapshotIndexFile);
-            }
+            snapshotsBlobContainer.deleteBlobIgnoringIfNotExists(oldSnapshotIndexFile);
         }
 
         // write the current generation to the index-latest file
@@ -679,9 +677,7 @@ protected void writeIndexGen(final RepositoryData repositoryData, final long rep
             bStream.writeLong(newGen);
             genBytes = bStream.bytes();
         }
-        if (snapshotsBlobContainer.blobExists(INDEX_LATEST_BLOB)) {
-            snapshotsBlobContainer.deleteBlob(INDEX_LATEST_BLOB);
-        }
+        snapshotsBlobContainer.deleteBlobIgnoringIfNotExists(INDEX_LATEST_BLOB);
         logger.debug("Repository [{}] updating index.latest with generation [{}]", metadata.name(), newGen);
         writeAtomic(INDEX_LATEST_BLOB, genBytes);
     }
@@ -702,9 +698,7 @@ void writeIncompatibleSnapshots(RepositoryData repositoryData) throws IOExceptio
             }
             bytes = bStream.bytes();
         }
-        if (snapshotsBlobContainer.blobExists(INCOMPATIBLE_SNAPSHOTS_BLOB)) {
-            snapshotsBlobContainer.deleteBlob(INCOMPATIBLE_SNAPSHOTS_BLOB);
-        }
+        snapshotsBlobContainer.deleteBlobIgnoringIfNotExists(INCOMPATIBLE_SNAPSHOTS_BLOB);
         // write the incompatible snapshots blob
         writeAtomic(INCOMPATIBLE_SNAPSHOTS_BLOB, bytes);
     }