Remove unused BlobStore#deleteBlobsIgnoringIfNotExists (#118245)

This method is never called against a general `BlobStore`, we only use
it in certain implementations for which a bulk delete at the `BlobStore`
level makes sense. This commit removes the unused interface method.
This commit is contained in:
David Turner 2024-12-09 15:46:22 +00:00 committed by GitHub
parent 22a392f1b6
commit 0586cbfb34
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
21 changed files with 17 additions and 135 deletions

View file

@ -144,7 +144,7 @@ public class AzureBlobContainer extends AbstractBlobContainer {
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
blobStore.deleteBlobsIgnoringIfNotExists(purpose, new Iterator<>() {
blobStore.deleteBlobs(purpose, new Iterator<>() {
@Override
public boolean hasNext() {
return blobNames.hasNext();

View file

@ -264,8 +264,7 @@ public class AzureBlobStore implements BlobStore {
return new DeleteResult(blobsDeleted.get(), bytesDeleted.get());
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
void deleteBlobs(OperationPurpose purpose, Iterator<String> blobNames) {
if (blobNames.hasNext() == false) {
return;
}

View file

@ -72,7 +72,7 @@ public class AzureBlobContainerStatsTests extends AbstractAzureServerTestCase {
false
);
case LIST_BLOBS -> blobStore.listBlobsByPrefix(purpose, randomIdentifier(), randomIdentifier());
case BLOB_BATCH -> blobStore.deleteBlobsIgnoringIfNotExists(
case BLOB_BATCH -> blobStore.deleteBlobs(
purpose,
List.of(randomIdentifier(), randomIdentifier(), randomIdentifier()).iterator()
);
@ -113,7 +113,7 @@ public class AzureBlobContainerStatsTests extends AbstractAzureServerTestCase {
os.flush();
});
// BLOB_BATCH
blobStore.deleteBlobsIgnoringIfNotExists(purpose, List.of(randomIdentifier(), randomIdentifier(), randomIdentifier()).iterator());
blobStore.deleteBlobs(purpose, List.of(randomIdentifier(), randomIdentifier(), randomIdentifier()).iterator());
Map<String, BlobStoreActionStats> stats = blobStore.stats();
String statsMapString = stats.toString();
@ -148,10 +148,7 @@ public class AzureBlobContainerStatsTests extends AbstractAzureServerTestCase {
os.flush();
});
// BLOB_BATCH
blobStore.deleteBlobsIgnoringIfNotExists(
purpose,
List.of(randomIdentifier(), randomIdentifier(), randomIdentifier()).iterator()
);
blobStore.deleteBlobs(purpose, List.of(randomIdentifier(), randomIdentifier(), randomIdentifier()).iterator());
}
Map<String, BlobStoreActionStats> stats = blobStore.stats();

View file

@ -114,12 +114,12 @@ class GoogleCloudStorageBlobContainer extends AbstractBlobContainer {
@Override
public DeleteResult delete(OperationPurpose purpose) throws IOException {
return blobStore.deleteDirectory(purpose, path().buildAsString());
return blobStore.deleteDirectory(path().buildAsString());
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
blobStore.deleteBlobsIgnoringIfNotExists(purpose, new Iterator<>() {
blobStore.deleteBlobs(new Iterator<>() {
@Override
public boolean hasNext() {
return blobNames.hasNext();

View file

@ -29,7 +29,6 @@ import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStore;
import org.elasticsearch.common.blobstore.BlobStoreActionStats;
import org.elasticsearch.common.blobstore.DeleteResult;
import org.elasticsearch.common.blobstore.OperationPurpose;
import org.elasticsearch.common.blobstore.OptionalBytesReference;
import org.elasticsearch.common.blobstore.support.BlobContainerUtils;
import org.elasticsearch.common.blobstore.support.BlobMetadata;
@ -491,10 +490,9 @@ class GoogleCloudStorageBlobStore implements BlobStore {
/**
* Deletes the given path and all its children.
*
* @param purpose The purpose of the delete operation
* @param pathStr Name of path to delete
*/
DeleteResult deleteDirectory(OperationPurpose purpose, String pathStr) throws IOException {
DeleteResult deleteDirectory(String pathStr) throws IOException {
return SocketAccess.doPrivilegedIOException(() -> {
DeleteResult deleteResult = DeleteResult.ZERO;
Page<Blob> page = client().list(bucketName, BlobListOption.prefix(pathStr));
@ -502,7 +500,7 @@ class GoogleCloudStorageBlobStore implements BlobStore {
final AtomicLong blobsDeleted = new AtomicLong(0L);
final AtomicLong bytesDeleted = new AtomicLong(0L);
final Iterator<Blob> blobs = page.getValues().iterator();
deleteBlobsIgnoringIfNotExists(purpose, new Iterator<>() {
deleteBlobs(new Iterator<>() {
@Override
public boolean hasNext() {
return blobs.hasNext();
@ -526,11 +524,9 @@ class GoogleCloudStorageBlobStore implements BlobStore {
/**
* Deletes multiple blobs from the specific bucket using a batch request
*
* @param purpose the purpose of the delete operation
* @param blobNames names of the blobs to delete
*/
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
void deleteBlobs(Iterator<String> blobNames) throws IOException {
if (blobNames.hasNext() == false) {
return;
}

View file

@ -342,10 +342,10 @@ class S3BlobContainer extends AbstractBlobContainer {
return summary.getKey();
});
if (list.isTruncated()) {
blobStore.deleteBlobsIgnoringIfNotExists(purpose, blobNameIterator);
blobStore.deleteBlobs(purpose, blobNameIterator);
prevListing = list;
} else {
blobStore.deleteBlobsIgnoringIfNotExists(purpose, Iterators.concat(blobNameIterator, Iterators.single(keyPath)));
blobStore.deleteBlobs(purpose, Iterators.concat(blobNameIterator, Iterators.single(keyPath)));
break;
}
}
@ -357,7 +357,7 @@ class S3BlobContainer extends AbstractBlobContainer {
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
blobStore.deleteBlobsIgnoringIfNotExists(purpose, Iterators.map(blobNames, this::buildKey));
blobStore.deleteBlobs(purpose, Iterators.map(blobNames, this::buildKey));
}
@Override

View file

@ -340,8 +340,7 @@ class S3BlobStore implements BlobStore {
return new S3BlobContainer(path, this);
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
void deleteBlobs(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
if (blobNames.hasNext() == false) {
return;
}

View file

@ -13,7 +13,6 @@ import org.elasticsearch.common.blobstore.BlobContainer;
import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStore;
import org.elasticsearch.common.blobstore.BlobStoreException;
import org.elasticsearch.common.blobstore.OperationPurpose;
import org.elasticsearch.common.blobstore.url.http.HttpURLBlobContainer;
import org.elasticsearch.common.blobstore.url.http.URLHttpClient;
import org.elasticsearch.common.blobstore.url.http.URLHttpClientSettings;
@ -23,10 +22,8 @@ import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.core.CheckedFunction;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Iterator;
import java.util.List;
/**
@ -109,11 +106,6 @@ public class URLBlobStore implements BlobStore {
}
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
throw new UnsupportedOperationException("Bulk deletes are not supported in URL repositories");
}
@Override
public void close() {
// nothing to do here...

View file

@ -16,10 +16,8 @@ import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.blobstore.BlobContainer;
import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStore;
import org.elasticsearch.common.blobstore.OperationPurpose;
import java.io.IOException;
import java.util.Iterator;
final class HdfsBlobStore implements BlobStore {
@ -72,11 +70,6 @@ final class HdfsBlobStore implements BlobStore {
return new HdfsBlobContainer(path, this, buildHdfsPath(path), bufferSize, securityContext, replicationFactor);
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
throw new UnsupportedOperationException("Bulk deletes are not supported in Hdfs repositories");
}
private Path buildHdfsPath(BlobPath blobPath) {
final Path path = translateToHdfsPath(blobPath);
if (readOnly == false) {

View file

@ -46,11 +46,6 @@ public class HdfsBlobStoreRepositoryTests extends ESBlobStoreRepositoryIntegTest
testSnapshotAndRestore(false);
}
@Override
public void testBlobStoreBulkDeletion() throws Exception {
// HDFS does not implement bulk deletion from different BlobContainers
}
@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.singletonList(HdfsPlugin.class);

View file

@ -36,7 +36,6 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.Collection;
import java.util.Iterator;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@ -136,11 +135,6 @@ public class BlobStoreRepositoryOperationPurposeIT extends AbstractSnapshotInteg
return new AssertingBlobContainer(delegateBlobStore.blobContainer(path));
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
delegateBlobStore.deleteBlobsIgnoringIfNotExists(purpose, blobNames);
}
@Override
public void close() throws IOException {
delegateBlobStore.close();

View file

@ -9,9 +9,7 @@
package org.elasticsearch.common.blobstore;
import java.io.Closeable;
import java.io.IOException;
import java.util.Collections;
import java.util.Iterator;
import java.util.Map;
/**
@ -28,14 +26,6 @@ public interface BlobStore extends Closeable {
*/
BlobContainer blobContainer(BlobPath path);
/**
* Delete all the provided blobs from the blob store. Each blob could belong to a different {@code BlobContainer}
*
* @param purpose the purpose of the delete operation
* @param blobNames the blobs to be deleted
*/
void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException;
/**
* Returns statistics on the count of operations that have been performed on this blob store
*/

View file

@ -177,7 +177,7 @@ public class FsBlobContainer extends AbstractBlobContainer {
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
blobStore.deleteBlobsIgnoringIfNotExists(purpose, Iterators.map(blobNames, blobName -> path.resolve(blobName).toString()));
blobStore.deleteBlobs(Iterators.map(blobNames, blobName -> path.resolve(blobName).toString()));
}
@Override

View file

@ -13,7 +13,6 @@ import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.blobstore.BlobContainer;
import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStore;
import org.elasticsearch.common.blobstore.OperationPurpose;
import org.elasticsearch.core.IOUtils;
import java.io.IOException;
@ -70,8 +69,7 @@ public class FsBlobStore implements BlobStore {
return new FsBlobContainer(this, path, f);
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
void deleteBlobs(Iterator<String> blobNames) throws IOException {
IOException ioe = null;
long suppressedExceptions = 0;
while (blobNames.hasNext()) {

View file

@ -35,7 +35,6 @@ import java.io.InputStream;
import java.io.OutputStream;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -100,11 +99,6 @@ public class BlobStoreRepositoryDeleteThrottlingTests extends ESSingleNodeTestCa
return new ConcurrencyLimitingBlobContainer(delegate.blobContainer(path), activeIndices, countDownLatch);
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
delegate.deleteBlobsIgnoringIfNotExists(purpose, blobNames);
}
@Override
public void close() throws IOException {
delegate.close();

View file

@ -24,7 +24,6 @@ import org.elasticsearch.xcontent.NamedXContentRegistry;
import java.io.IOException;
import java.io.InputStream;
import java.util.Iterator;
class LatencySimulatingBlobStoreRepository extends FsRepository {
@ -53,11 +52,6 @@ class LatencySimulatingBlobStoreRepository extends FsRepository {
return new LatencySimulatingBlobContainer(blobContainer);
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
fsBlobStore.deleteBlobsIgnoringIfNotExists(purpose, blobNames);
}
@Override
public void close() throws IOException {
fsBlobStore.close();

View file

@ -48,7 +48,6 @@ import org.hamcrest.CoreMatchers;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.NoSuchFileException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
@ -70,7 +69,6 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcke
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
@ -524,39 +522,6 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase
assertAcked(clusterAdmin().prepareDeleteSnapshot(TEST_REQUEST_TIMEOUT, repoName, "test-snap2").get());
}
public void testBlobStoreBulkDeletion() throws Exception {
Map<BlobPath, List<String>> expectedBlobsPerContainer = new HashMap<>();
try (BlobStore store = newBlobStore()) {
List<String> blobsToDelete = new ArrayList<>();
int numberOfContainers = randomIntBetween(2, 5);
for (int i = 0; i < numberOfContainers; i++) {
BlobPath containerPath = BlobPath.EMPTY.add(randomIdentifier());
final BlobContainer container = store.blobContainer(containerPath);
int numberOfBlobsPerContainer = randomIntBetween(5, 10);
for (int j = 0; j < numberOfBlobsPerContainer; j++) {
byte[] bytes = randomBytes(randomInt(100));
String blobName = randomAlphaOfLength(10);
container.writeBlob(randomPurpose(), blobName, new BytesArray(bytes), false);
if (randomBoolean()) {
blobsToDelete.add(containerPath.buildAsString() + blobName);
} else {
expectedBlobsPerContainer.computeIfAbsent(containerPath, unused -> new ArrayList<>()).add(blobName);
}
}
}
store.deleteBlobsIgnoringIfNotExists(randomPurpose(), blobsToDelete.iterator());
for (var containerEntry : expectedBlobsPerContainer.entrySet()) {
BlobContainer blobContainer = store.blobContainer(containerEntry.getKey());
Map<String, BlobMetadata> blobsInContainer = blobContainer.listBlobs(randomPurpose());
for (String expectedBlob : containerEntry.getValue()) {
assertThat(blobsInContainer, hasKey(expectedBlob));
}
blobContainer.delete(randomPurpose());
}
}
}
public void testDanglingShardLevelBlobCleanup() throws Exception {
final var repoName = createRepository(randomRepositoryName());
final var client = client();

View file

@ -12,15 +12,13 @@ import org.elasticsearch.common.blobstore.BlobContainer;
import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStore;
import org.elasticsearch.common.blobstore.BlobStoreActionStats;
import org.elasticsearch.common.blobstore.OperationPurpose;
import java.io.IOException;
import java.util.Iterator;
import java.util.Map;
public class BlobStoreWrapper implements BlobStore {
private BlobStore delegate;
private final BlobStore delegate;
public BlobStoreWrapper(BlobStore delegate) {
this.delegate = delegate;
@ -31,11 +29,6 @@ public class BlobStoreWrapper implements BlobStore {
return delegate.blobContainer(path);
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) throws IOException {
delegate.deleteBlobsIgnoringIfNotExists(purpose, blobNames);
}
@Override
public void close() throws IOException {
delegate.close();

View file

@ -67,7 +67,6 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -466,12 +465,6 @@ public class SearchableSnapshotsPrewarmingIntegTests extends ESSingleNodeTestCas
return new TrackingFilesBlobContainer(delegate.blobContainer(path));
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames)
throws IOException {
delegate.deleteBlobsIgnoringIfNotExists(purpose, blobNames);
}
@Override
public void close() throws IOException {
delegate.close();

View file

@ -569,11 +569,6 @@ public class RepositoryAnalysisFailureIT extends AbstractSnapshotIntegTestCase {
}
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) {
assertPurpose(purpose);
}
private void deleteContainer(DisruptableBlobContainer container) {
blobContainer = null;
}

View file

@ -287,11 +287,6 @@ public class RepositoryAnalysisSuccessIT extends AbstractSnapshotIntegTestCase {
}
}
@Override
public void deleteBlobsIgnoringIfNotExists(OperationPurpose purpose, Iterator<String> blobNames) {
assertPurpose(purpose);
}
@Override
public void close() {}