Remove blobExists Method from BlobContainer (#44472) (#44475)

* We only use this method in one place in production code and can replace that with a read -> remove it to simplify the interface
   * Keep it as an implementation detail in the Azure repository
This commit is contained in:
Armin Braun 2019-07-17 11:56:02 +02:00 committed by GitHub
parent e423b7341a
commit c8db0e9b7e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 32 additions and 90 deletions

View File

@ -101,14 +101,6 @@ public class URLBlobContainer extends AbstractBlobContainer {
throw new UnsupportedOperationException("URL repository is read only");
}
/**
* This operation is not supported by URLBlobContainer
*/
@Override
public boolean blobExists(String blobName) {
throw new UnsupportedOperationException("URL repository doesn't support this operation");
}
@Override
public InputStream readBlob(String name) throws IOException {
try {

View File

@ -57,8 +57,7 @@ public class AzureBlobContainer extends AbstractBlobContainer {
this.threadPool = threadPool;
}
@Override
public boolean blobExists(String blobName) {
private boolean blobExists(String blobName) {
logger.trace("blobExists({})", blobName);
try {
return blobStore.blobExists(buildKey(blobName));

View File

@ -22,7 +22,6 @@ package org.elasticsearch.repositories.gcs;
import org.elasticsearch.common.blobstore.BlobContainer;
import org.elasticsearch.common.blobstore.BlobMetaData;
import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStoreException;
import org.elasticsearch.common.blobstore.support.AbstractBlobContainer;
import java.io.IOException;
@ -42,15 +41,6 @@ class GoogleCloudStorageBlobContainer extends AbstractBlobContainer {
this.path = path.buildAsString();
}
@Override
public boolean blobExists(String blobName) {
try {
return blobStore.blobExists(buildKey(blobName));
} catch (Exception e) {
throw new BlobStoreException("Failed to check if blob [" + blobName + "] exists", e);
}
}
@Override
public Map<String, BlobMetaData> listBlobs() throws IOException {
return blobStore.listBlobs(path);

View File

@ -163,18 +163,6 @@ class GoogleCloudStorageBlobStore implements BlobStore {
return mapBuilder.immutableMap();
}
/**
* Returns true if the blob exists in the specific bucket
*
* @param blobName name of the blob
* @return true iff the blob exists
*/
boolean blobExists(String blobName) throws IOException {
final BlobId blobId = BlobId.of(bucketName, blobName);
final Blob blob = SocketAccess.doPrivilegedIOException(() -> client().get(blobId));
return blob != null;
}
/**
* Returns an {@link java.io.InputStream} for the given blob name
*

View File

@ -58,15 +58,6 @@ final class HdfsBlobContainer extends AbstractBlobContainer {
this.bufferSize = bufferSize;
}
@Override
public boolean blobExists(String blobName) {
try {
return store.execute(fileContext -> fileContext.util().exists(new Path(path, blobName)));
} catch (Exception e) {
return false;
}
}
@Override
public void deleteBlob(String blobName) throws IOException {
try {

View File

@ -31,6 +31,7 @@ import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStore;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.repositories.ESBlobStoreContainerTestCase;
import org.elasticsearch.repositories.blobstore.BlobStoreTestUtil;
import javax.security.auth.Subject;
import java.io.IOException;
@ -137,6 +138,6 @@ public class HdfsBlobStoreContainerTests extends ESBlobStoreContainerTestCase {
byte[] data = randomBytes(randomIntBetween(10, scaledRandomIntBetween(1024, 1 << 16)));
writeBlob(container, "foo", new BytesArray(data), randomBoolean());
assertArrayEquals(readBlobFully(container, "foo", data.length), data);
assertTrue(container.blobExists("foo"));
assertTrue(BlobStoreTestUtil.blobExists(container, "foo"));
}
}

View File

@ -42,7 +42,6 @@ import org.elasticsearch.common.Strings;
import org.elasticsearch.common.blobstore.BlobContainer;
import org.elasticsearch.common.blobstore.BlobMetaData;
import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStoreException;
import org.elasticsearch.common.blobstore.support.AbstractBlobContainer;
import org.elasticsearch.common.blobstore.support.PlainBlobMetaData;
import org.elasticsearch.common.collect.Tuple;
@ -79,15 +78,6 @@ class S3BlobContainer extends AbstractBlobContainer {
this.keyPath = path.buildAsString();
}
@Override
public boolean blobExists(String blobName) {
try (AmazonS3Reference clientReference = blobStore.clientReference()) {
return SocketAccess.doPrivileged(() -> clientReference.client().doesObjectExist(blobStore.bucket(), buildKey(blobName)));
} catch (final Exception e) {
throw new BlobStoreException("Failed to check if blob [" + blobName +"] exists", e);
}
}
@Override
public InputStream readBlob(String blobName) throws IOException {
try (AmazonS3Reference clientReference = blobStore.clientReference()) {

View File

@ -38,15 +38,6 @@ public interface BlobContainer {
*/
BlobPath path();
/**
* Tests whether a blob with the given blob name exists in the container.
*
* @param blobName
* The name of the blob whose existence is to be determined.
* @return {@code true} if a blob exists in the {@link BlobContainer} with the given name, and {@code false} otherwise.
*/
boolean blobExists(String blobName);
/**
* Creates a new {@link InputStream} for the given blob name.
*

View File

@ -127,11 +127,6 @@ public class FsBlobContainer extends AbstractBlobContainer {
IOUtils.rm(path);
}
@Override
public boolean blobExists(String blobName) {
return Files.exists(path.resolve(blobName));
}
@Override
public InputStream readBlob(String name) throws IOException {
final Path resolvedPath = path.resolve(name);

View File

@ -1059,7 +1059,7 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
}
} else {
BlobContainer testBlobContainer = blobStore().blobContainer(basePath().add(testBlobPrefix(seed)));
if (testBlobContainer.blobExists("master.dat")) {
try (InputStream ignored = testBlobContainer.readBlob("master.dat")) {
try {
BytesArray bytes = new BytesArray(seed);
try (InputStream stream = bytes.streamInput()) {
@ -1069,11 +1069,13 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
throw new RepositoryVerificationException(metadata.name(), "store location [" + blobStore() +
"] is not accessible on the node [" + localNode + "]", exp);
}
} else {
} catch (NoSuchFileException e) {
throw new RepositoryVerificationException(metadata.name(), "a file written by master to the store [" + blobStore() +
"] cannot be accessed on the node [" + localNode + "]. " +
"This might indicate that the store [" + blobStore() + "] is not shared between this node and the master node or " +
"that permissions on the store don't allow reading files written by the master node");
"that permissions on the store don't allow reading files written by the master node", e);
} catch (IOException e) {
throw new RepositoryVerificationException(metadata.name(), "Failed to verify repository", e);
}
}
}

View File

@ -27,6 +27,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.repositories.ESBlobStoreTestCase;
import org.elasticsearch.repositories.blobstore.BlobStoreTestUtil;
import java.io.IOException;
import java.nio.file.Files;
@ -74,7 +75,7 @@ public class FsBlobStoreTests extends ESBlobStoreTestCase {
byte[] data = randomBytes(randomIntBetween(10, scaledRandomIntBetween(1024, 1 << 16)));
writeBlob(container, "test", new BytesArray(data));
assertArrayEquals(readBlobFully(container, "test", data.length), data);
assertTrue(container.blobExists("test"));
assertTrue(BlobStoreTestUtil.blobExists(container, "test"));
}
}
}

View File

@ -36,6 +36,7 @@ import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.translog.BufferedChecksumStreamOutput;
import org.elasticsearch.repositories.blobstore.BlobStoreTestUtil;
import org.elasticsearch.repositories.blobstore.ChecksumBlobStoreFormat;
import org.elasticsearch.snapshots.mockstore.BlobContainerWrapper;
@ -193,11 +194,12 @@ public class BlobStoreFormatIT extends AbstractSnapshotIntegTestCase {
return null;
}
});
// signalling
block.await(5, TimeUnit.SECONDS);
assertFalse(blobContainer.blobExists("test-blob"));
assertFalse(BlobStoreTestUtil.blobExists(blobContainer, "test-blob"));
unblock.countDown();
future.get();
assertTrue(blobContainer.blobExists("test-blob"));
assertTrue(BlobStoreTestUtil.blobExists(blobContainer, "test-blob"));
} finally {
threadPool.shutdown();
}

View File

@ -38,11 +38,6 @@ public class BlobContainerWrapper implements BlobContainer {
return delegate.path();
}
@Override
public boolean blobExists(String blobName) {
return delegate.blobExists(blobName);
}
@Override
public InputStream readBlob(String name) throws IOException {
return delegate.readBlob(name);

View File

@ -311,11 +311,6 @@ public class MockRepository extends FsRepository {
super(delegate);
}
@Override
public boolean blobExists(String blobName) {
return super.blobExists(blobName);
}
@Override
public InputStream readBlob(String name) throws IOException {
maybeIOExceptionOrBlock(name);

View File

@ -259,9 +259,9 @@ public abstract class AbstractThirdPartyRepositoryTestCase extends ESSingleNodeT
final BlobStore blobStore = repo.blobStore();
future.onResponse(
blobStore.blobContainer(BlobPath.cleanPath().add("indices")).children().containsKey("foo")
&& blobStore.blobContainer(BlobPath.cleanPath().add("indices").add("foo")).blobExists("bar")
&& blobStore.blobContainer(BlobPath.cleanPath()).blobExists("meta-foo.dat")
&& blobStore.blobContainer(BlobPath.cleanPath()).blobExists("snap-foo.dat")
&& BlobStoreTestUtil.blobExists(blobStore.blobContainer(BlobPath.cleanPath().add("indices").add("foo")), "bar")
&& BlobStoreTestUtil.blobExists(blobStore.blobContainer(BlobPath.cleanPath()), "meta-foo.dat")
&& BlobStoreTestUtil.blobExists(blobStore.blobContainer(BlobPath.cleanPath()), "snap-foo.dat")
);
}
});

View File

@ -22,6 +22,7 @@ import org.elasticsearch.common.blobstore.BlobContainer;
import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStore;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.repositories.blobstore.BlobStoreTestUtil;
import org.elasticsearch.test.ESTestCase;
import java.io.IOException;
@ -47,8 +48,8 @@ public abstract class ESBlobStoreTestCase extends ESTestCase {
assertArrayEquals(readBlobFully(containerFoo, "test", data1.length), data1);
assertArrayEquals(readBlobFully(containerBar, "test", data2.length), data2);
assertTrue(containerFoo.blobExists("test"));
assertTrue(containerBar.blobExists("test"));
assertTrue(BlobStoreTestUtil.blobExists(containerFoo, "test"));
assertTrue(BlobStoreTestUtil.blobExists(containerBar, "test"));
}
}

View File

@ -38,6 +38,7 @@ import org.elasticsearch.threadpool.ThreadPool;
import java.io.DataInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.NoSuchFileException;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
@ -60,6 +61,14 @@ public final class BlobStoreTestUtil {
BlobStoreTestUtil.assertConsistency(repo, repo.threadPool().executor(ThreadPool.Names.GENERIC));
}
public static boolean blobExists(BlobContainer container, String blobName) throws IOException {
try (InputStream ignored = container.readBlob(blobName)) {
return true;
} catch (NoSuchFileException e) {
return false;
}
}
/**
* Assert that there are no unreferenced indices or unreferenced root-level metadata blobs in any repository.
* TODO: Expand the logic here to also check for unreferenced segment blobs and shard level metadata
@ -74,11 +83,11 @@ public final class BlobStoreTestUtil {
@Override
protected void doRun() throws Exception {
final BlobContainer blobContainer = repository.blobContainer();
assertTrue(
"Could not find index.latest blob for repo [" + repository + "]", blobContainer.blobExists("index.latest"));
final long latestGen;
try (DataInputStream inputStream = new DataInputStream(blobContainer.readBlob("index.latest"))) {
latestGen = inputStream.readLong();
} catch (NoSuchFileException e) {
throw new AssertionError("Could not find index.latest blob for repo [" + repository + "]");
}
assertIndexGenerations(blobContainer, latestGen);
final RepositoryData repositoryData;

View File

@ -269,7 +269,7 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase
latch.await();
for (IndexId indexId : repositoryData.get().getIndices().values()) {
if (indexId.getName().equals("test-idx-3")) {
assertFalse(indicesBlobContainer.get().blobExists(indexId.getId())); // deleted index
assertFalse(BlobStoreTestUtil.blobExists(indicesBlobContainer.get(), indexId.getId())); // deleted index
}
}
}