Przeglądaj źródła

Remove blobExists Method from BlobContainer (#44472)

* 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
Armin Braun 6 lat temu
rodzic
commit
4b8fd4e76f
18 zmienionych plików z 32 dodań i 90 usunięć
  1. 0 8
      modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java
  2. 1 2
      plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java
  3. 0 10
      plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java
  4. 0 12
      plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java
  5. 0 9
      plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java
  6. 2 1
      plugins/repository-hdfs/src/test/java/org/elasticsearch/repositories/hdfs/HdfsBlobStoreContainerTests.java
  7. 0 10
      plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java
  8. 0 9
      server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java
  9. 0 5
      server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java
  10. 5 3
      server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java
  11. 2 1
      server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreTests.java
  12. 4 2
      server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java
  13. 0 5
      server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java
  14. 0 5
      server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java
  15. 3 3
      test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java
  16. 3 2
      test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreTestCase.java
  17. 11 2
      test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java
  18. 1 1
      test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java

+ 0 - 8
modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java

@@ -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 {

+ 1 - 2
plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java

@@ -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));

+ 0 - 10
plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java

@@ -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);

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

@@ -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
      *

+ 0 - 9
plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java

@@ -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 {

+ 2 - 1
plugins/repository-hdfs/src/test/java/org/elasticsearch/repositories/hdfs/HdfsBlobStoreContainerTests.java

@@ -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"));
     }
 }

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

@@ -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()) {

+ 0 - 9
server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java

@@ -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.
      *

+ 0 - 5
server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java

@@ -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);

+ 5 - 3
server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

@@ -1033,7 +1033,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()) {
@@ -1043,11 +1043,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);
             }
         }
     }

+ 2 - 1
server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreTests.java

@@ -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"));
         }
     }
 }

+ 4 - 2
server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java

@@ -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();
         }

+ 0 - 5
server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java

@@ -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);

+ 0 - 5
server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java

@@ -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);

+ 3 - 3
test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java

@@ -261,9 +261,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")
                 );
             }
         });

+ 3 - 2
test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreTestCase.java

@@ -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"));
         }
     }
 

+ 11 - 2
test/framework/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreTestUtil.java

@@ -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;

+ 1 - 1
test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java

@@ -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
             }
         }
     }