Remove Delete Method from BlobStore (#41619) (#42574)

* Remove Delete Method from BlobStore (#41619)
* The delete method on the blob store was used almost nowhere and just duplicates the delete method on the blob containers
  * The fact that it provided for some recursive delete logic (that did not behave the same way on all implementations) was not used and not properly tested either

Author: original-brownbear

modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobStore.java

@@ -57,9 +57,6 @@ public URLBlobStore(Settings settings, URL path) {
             new ByteSizeValue(100, ByteSizeUnit.KB)).getBytes();
     }
 
-    /**
-     * {@inheritDoc}
-     */
     @Override
     public String toString() {
         return path.toString();
@@ -83,9 +80,6 @@ public int bufferSizeInBytes() {
         return this.bufferSizeInBytes;
     }
 
-    /**
-     * {@inheritDoc}
-     */
     @Override
     public BlobContainer blobContainer(BlobPath path) {
         try {
@@ -95,17 +89,6 @@ public BlobContainer blobContainer(BlobPath path) {
         }
     }
 
-    /**
-     * This operation is not supported by URL Blob Store
-     */
-    @Override
-    public void delete(BlobPath path) {
-        throw new UnsupportedOperationException("URL repository is read only");
-    }
-
-    /**
-     * {@inheritDoc}
-     */
     @Override
     public void close() {
         // nothing to do here...

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java

@@ -22,8 +22,6 @@
 import com.microsoft.azure.storage.LocationMode;
 import com.microsoft.azure.storage.StorageException;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
 import org.elasticsearch.cluster.metadata.RepositoryMetaData;
 import org.elasticsearch.common.blobstore.BlobContainer;
 import org.elasticsearch.common.blobstore.BlobMetaData;
@@ -40,8 +38,6 @@
 import static java.util.Collections.emptyMap;
 
 public class AzureBlobStore implements BlobStore {
-    
-    private static final Logger logger = LogManager.getLogger(AzureBlobStore.class);
 
     private final AzureStorageService service;
 
@@ -82,17 +78,6 @@ public BlobContainer blobContainer(BlobPath path) {
         return new AzureBlobContainer(path, this);
     }
 
-    @Override
-    public void delete(BlobPath path) throws IOException {
-        final String keyPath = path.buildAsString();
-        try {
-            service.deleteFiles(clientName, container, keyPath);
-        } catch (URISyntaxException | StorageException e) {
-            logger.warn("cannot access [{}] in container {{}}: {}", keyPath, container, e.getMessage());
-            throw new IOException(e);
-        }
-    }
-
     @Override
     public void close() {
     }

plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java

@@ -50,11 +50,11 @@
 import java.nio.channels.WritableByteChannel;
 import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.NoSuchFileException;
+import java.util.Map;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
@@ -91,11 +91,6 @@ public BlobContainer blobContainer(BlobPath path) {
         return new GoogleCloudStorageBlobContainer(path, this);
     }
 
-    @Override
-    public void delete(BlobPath path) throws IOException {
-        deleteBlobsByPrefix(path.buildAsString());
-    }
-
     @Override
     public void close() {
     }
@@ -291,15 +286,6 @@ void deleteBlob(String blobName) throws IOException {
         }
     }
 
-    /**
-     * Deletes multiple blobs from the specific bucket all of which have prefixed names
-     *
-     * @param prefix prefix of the blobs to delete
-     */
-    private void deleteBlobsByPrefix(String prefix) throws IOException {
-        deleteBlobsIgnoringIfNotExists(listBlobsByPrefix("", prefix).keySet());
-    }
-
     /**
      * Deletes multiple blobs from the specific bucket using a batch request
      *

plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobStore.java

@@ -66,14 +66,6 @@ private void mkdirs(Path path) throws IOException {
         });
     }
 
-    @Override
-    public void delete(BlobPath path) throws IOException {
-        execute((Operation<Void>) fc -> {
-            fc.delete(translateToHdfsPath(path), true);
-            return null;
-        });
-    }
-
     @Override
     public String toString() {
         return root.toUri().toString();

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

@@ -20,10 +20,6 @@
 package org.elasticsearch.repositories.s3;
 
 import com.amazonaws.services.s3.model.CannedAccessControlList;
-import com.amazonaws.services.s3.model.DeleteObjectsRequest;
-import com.amazonaws.services.s3.model.DeleteObjectsRequest.KeyVersion;
-import com.amazonaws.services.s3.model.ObjectListing;
-import com.amazonaws.services.s3.model.S3ObjectSummary;
 import com.amazonaws.services.s3.model.StorageClass;
 import org.elasticsearch.cluster.metadata.RepositoryMetaData;
 import org.elasticsearch.common.blobstore.BlobContainer;
@@ -33,7 +29,6 @@
 import org.elasticsearch.common.unit.ByteSizeValue;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Locale;
 
 class S3BlobStore implements BlobStore {
@@ -90,50 +85,6 @@ public BlobContainer blobContainer(BlobPath path) {
         return new S3BlobContainer(path, this);
     }
 
-    @Override
-    public void delete(BlobPath path) {
-        try (AmazonS3Reference clientReference = clientReference()) {
-            ObjectListing prevListing = null;
-            // From
-            // http://docs.amazonwebservices.com/AmazonS3/latest/dev/DeletingMultipleObjectsUsingJava.html
-            // we can do at most 1K objects per delete
-            // We don't know the bucket name until first object listing
-            DeleteObjectsRequest multiObjectDeleteRequest = null;
-            final ArrayList<KeyVersion> keys = new ArrayList<>();
-            while (true) {
-                ObjectListing list;
-                if (prevListing != null) {
-                    final ObjectListing finalPrevListing = prevListing;
-                    list = SocketAccess.doPrivileged(() -> clientReference.client().listNextBatchOfObjects(finalPrevListing));
-                } else {
-                    list = SocketAccess.doPrivileged(() -> clientReference.client().listObjects(bucket, path.buildAsString()));
-                    multiObjectDeleteRequest = new DeleteObjectsRequest(list.getBucketName());
-                }
-                for (final S3ObjectSummary summary : list.getObjectSummaries()) {
-                    keys.add(new KeyVersion(summary.getKey()));
-                    // Every 500 objects batch the delete request
-                    if (keys.size() > 500) {
-                        multiObjectDeleteRequest.setKeys(keys);
-                        final DeleteObjectsRequest finalMultiObjectDeleteRequest = multiObjectDeleteRequest;
-                        SocketAccess.doPrivilegedVoid(() -> clientReference.client().deleteObjects(finalMultiObjectDeleteRequest));
-                        multiObjectDeleteRequest = new DeleteObjectsRequest(list.getBucketName());
-                        keys.clear();
-                    }
-                }
-                if (list.isTruncated()) {
-                    prevListing = list;
-                } else {
-                    break;
-                }
-            }
-            if (!keys.isEmpty()) {
-                multiObjectDeleteRequest.setKeys(keys);
-                final DeleteObjectsRequest finalMultiObjectDeleteRequest = multiObjectDeleteRequest;
-                SocketAccess.doPrivilegedVoid(() -> clientReference.client().deleteObjects(finalMultiObjectDeleteRequest));
-            }
-        }
-    }
-
     @Override
     public void close() throws IOException {
         this.service.close();

server/src/main/java/org/elasticsearch/common/blobstore/BlobStore.java

@@ -19,7 +19,6 @@
 package org.elasticsearch.common.blobstore;
 
 import java.io.Closeable;
-import java.io.IOException;
 
 /**
  * An interface for storing blobs.
@@ -30,10 +29,4 @@ public interface BlobStore extends Closeable {
      * Get a blob container instance for storing blobs at the given {@link BlobPath}.
      */
     BlobContainer blobContainer(BlobPath path);
-
-    /**
-     * Delete the blob store at the given {@link BlobPath}.
-     */
-    void delete(BlobPath path) throws IOException;
-
 }

server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobStore.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.common.blobstore.fs;
 
-import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.blobstore.BlobContainer;
 import org.elasticsearch.common.blobstore.BlobPath;
@@ -72,16 +71,6 @@ public BlobContainer blobContainer(BlobPath path) {
         }
     }
 
-    @Override
-    public void delete(BlobPath path) throws IOException {
-        assert readOnly == false : "should not delete anything from a readonly repository: " + path;
-        //noinspection ConstantConditions in case assertions are disabled
-        if (readOnly) {
-            throw new ElasticsearchException("unexpectedly deleting [" + path + "] from a readonly repository");
-        }
-        IOUtils.rm(buildPath(path));
-    }
-
     @Override
     public void close() {
         // nothing to do here...

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

@@ -629,7 +629,10 @@ public String startVerification() {
     public void endVerification(String seed) {
         if (isReadOnly() == false) {
             try {
-                blobStore().delete(basePath().add(testBlobPrefix(seed)));
+                final String testPrefix = testBlobPrefix(seed);
+                final BlobContainer container = blobStore().blobContainer(basePath().add(testPrefix));
+                container.deleteBlobsIgnoringIfNotExists(new ArrayList<>(container.listBlobs().keySet()));
+                blobStore().blobContainer(basePath()).deleteBlobIgnoringIfNotExists(testPrefix);
             } catch (IOException exp) {
                 throw new RepositoryVerificationException(metadata.name(), "cannot delete test data at " + basePath(), exp);
             }

server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobStoreWrapper.java

@@ -37,11 +37,6 @@ public BlobContainer blobContainer(BlobPath path) {
         return delegate.blobContainer(path);
     }
 
-    @Override
-    public void delete(BlobPath path) throws IOException {
-        delegate.delete(path);
-    }
-
     @Override
     public void close() throws IOException {
         delegate.close();

test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreTestCase.java

@@ -49,9 +49,6 @@ public void testContainerCreationAndDeletion() throws IOException {
 
             assertTrue(containerFoo.blobExists("test"));
             assertTrue(containerBar.blobExists("test"));
-            store.delete(new BlobPath());
-            assertFalse(containerFoo.blobExists("test"));
-            assertFalse(containerBar.blobExists("test"));
         }
     }