diff --git a/core/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java b/core/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java index 5d00e36ddbd..10f5154d863 100644 --- a/core/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java +++ b/core/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java @@ -27,60 +27,138 @@ import java.util.Collection; import java.util.Map; /** - * + * An interface for managing a repository of blob entries, where each blob entry is just a named group of bytes. */ public interface BlobContainer { + /** + * Gets the {@link BlobPath} that defines the implementation specific paths to where the blobs are contained. + * + * @return the BlobPath where the blobs are contained + */ 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. + * @throws IOException if any error occurred while attempting to ascertain if the blob exists + */ boolean blobExists(String blobName); /** - * Creates a new InputStream for the given blob name + * Creates a new {@link InputStream} for the given blob name. + * + * @param blobName + * The name of the blob to get an {@link InputStream} for. + * @return The {@code InputStream} to read the blob. + * @throws IOException if the blob does not exist or can not be read. */ InputStream readBlob(String blobName) throws IOException; /** - * Reads blob content from the input stream and writes it to the blob store + * Reads blob content from the input stream and writes it to the container in a new blob with the given name. + * This method assumes the container does not already contain a blob of the same blobName. If a blob by the + * same name already exists, the operation will fail and an {@link IOException} will be thrown. + * + * @param blobName + * The name of the blob to write the contents of the input stream to. + * @param inputStream + * The input stream from which to retrieve the bytes to write to the blob. + * @param blobSize + * The size of the blob to be written, in bytes. It is implementation dependent whether + * this value is used in writing the blob to the repository. + * @throws IOException if the input stream could not be read, a blob by the same name already exists, + * or the target blob could not be written to. */ void writeBlob(String blobName, InputStream inputStream, long blobSize) throws IOException; /** - * Writes bytes to the blob + * Writes the input bytes to a new blob in the container with the given name. This method assumes the + * container does not already contain a blob of the same blobName. If a blob by the same name already + * exists, the operation will fail and an {@link IOException} will be thrown. + * + * TODO: Remove this in favor of a single {@link #writeBlob(String, InputStream, long)} method. + * See https://github.com/elastic/elasticsearch/issues/18528 + * + * @param blobName + * The name of the blob to write the contents of the input stream to. + * @param bytes + * The bytes to write to the blob. + * @throws IOException if a blob by the same name already exists, or the target blob could not be written to. */ void writeBlob(String blobName, BytesReference bytes) throws IOException; /** - * Deletes a blob with giving name. + * Deletes a blob with giving name, if the blob exists. If the blob does not exist, this method throws an IOException. * - * If a blob exists but cannot be deleted an exception has to be thrown. + * @param blobName + * The name of the blob to delete. + * @throws IOException if the blob does not exist, or if the blob exists but could not be deleted. */ void deleteBlob(String blobName) throws IOException; /** - * Deletes blobs with giving names. + * Deletes blobs with the given names. If any subset of the names do not exist in the container, this method has no + * effect for those names, and will delete the blobs for those names that do exist. If any of the blobs failed + * to delete, those blobs that were processed before it and successfully deleted will remain deleted. An exception + * is thrown at the first blob entry that fails to delete (TODO: is this the right behavior? Should we collect + * all the failed deletes into a single IOException instead?) * - * If a blob exists but cannot be deleted an exception has to be thrown. + * TODO: remove, see https://github.com/elastic/elasticsearch/issues/18529 + * + * @param blobNames + * The collection of blob names to delete from the container. + * @throws IOException if any of the blobs in the collection exists but could not be deleted. */ void deleteBlobs(Collection blobNames) throws IOException; /** - * Deletes all blobs in the container that match the specified prefix. + * Deletes all blobs in the container that match the specified prefix. If any of the blobs failed to delete, + * those blobs that were processed before it and successfully deleted will remain deleted. An exception is + * thrown at the first blob entry that fails to delete (TODO: is this the right behavior? Should we collect + * all the failed deletes into a single IOException instead?) + * + * TODO: remove, see: https://github.com/elastic/elasticsearch/issues/18529 + * + * @param blobNamePrefix + * The prefix to match against blob names in the container. Any blob whose name has the prefix will be deleted. + * @throws IOException if any of the matching blobs failed to delete. */ void deleteBlobsByPrefix(String blobNamePrefix) throws IOException; /** - * Lists all blobs in the container + * Lists all blobs in the container. + * + * @return A map of all the blobs in the container. The keys in the map are the names of the blobs and + * the values are {@link BlobMetaData}, containing basic information about each blob. + * @throws IOException if there were any failures in reading from the blob container. */ Map listBlobs() throws IOException; /** - * Lists all blobs in the container that match specified prefix + * Lists all blobs in the container that match the specified prefix. + * + * @param blobNamePrefix + * The prefix to match against blob names in the container. + * @return A map of the matching blobs in the container. The keys in the map are the names of the blobs + * and the values are {@link BlobMetaData}, containing basic information about each blob. + * @throws IOException if there were any failures in reading from the blob container. */ Map listBlobsByPrefix(String blobNamePrefix) throws IOException; /** - * Atomically renames source blob into target blob + * Atomically renames the source blob into the target blob. If the source blob does not exist or the + * target blob already exists, an exception is thrown. + * + * @param sourceBlobName + * The blob to rename. + * @param targetBlobName + * The name of the blob after the renaming. + * @throws IOException if the source blob does not exist, the target blob already exists, + * or there were any failures in reading from the blob container. */ void move(String sourceBlobName, String targetBlobName) throws IOException; } diff --git a/core/src/main/java/org/elasticsearch/common/blobstore/BlobMetaData.java b/core/src/main/java/org/elasticsearch/common/blobstore/BlobMetaData.java index 3f69e268034..da6c277aa2a 100644 --- a/core/src/main/java/org/elasticsearch/common/blobstore/BlobMetaData.java +++ b/core/src/main/java/org/elasticsearch/common/blobstore/BlobMetaData.java @@ -20,11 +20,17 @@ package org.elasticsearch.common.blobstore; /** - * + * An interface for providing basic metadata about a blob. */ public interface BlobMetaData { + /** + * Gets the name of the blob. + */ String name(); + /** + * Gets the size of the blob in bytes. + */ long length(); } diff --git a/core/src/main/java/org/elasticsearch/common/blobstore/BlobPath.java b/core/src/main/java/org/elasticsearch/common/blobstore/BlobPath.java index 858486d282c..9092e13eb1b 100644 --- a/core/src/main/java/org/elasticsearch/common/blobstore/BlobPath.java +++ b/core/src/main/java/org/elasticsearch/common/blobstore/BlobPath.java @@ -19,14 +19,13 @@ package org.elasticsearch.common.blobstore; - import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.List; /** - * + * The list of paths where a blob can reside. The contents of the paths are dependent upon the implementation of {@link BlobContainer}. */ public class BlobPath implements Iterable { diff --git a/core/src/main/java/org/elasticsearch/common/blobstore/BlobStore.java b/core/src/main/java/org/elasticsearch/common/blobstore/BlobStore.java index 9275b379158..e4cdb148a15 100644 --- a/core/src/main/java/org/elasticsearch/common/blobstore/BlobStore.java +++ b/core/src/main/java/org/elasticsearch/common/blobstore/BlobStore.java @@ -22,12 +22,18 @@ import java.io.Closeable; import java.io.IOException; /** - * + * An interface for storing blobs. */ 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; } diff --git a/core/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java b/core/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java index c62166a23a3..822f8d1721a 100644 --- a/core/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java +++ b/core/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java @@ -41,7 +41,12 @@ import java.util.Map; import static java.util.Collections.unmodifiableMap; /** + * A file system based implementation of {@link org.elasticsearch.common.blobstore.BlobContainer}. + * All blobs in the container are stored on a file system, the location of which is specified by the {@link BlobPath}. * + * Note that the methods in this implementation of {@link org.elasticsearch.common.blobstore.BlobContainer} may + * additionally throw a {@link java.lang.SecurityException} if the configured {@link java.lang.SecurityManager} + * does not permit read and/or write access to the underlying files. */ public class FsBlobContainer extends AbstractBlobContainer { diff --git a/core/src/main/java/org/elasticsearch/common/blobstore/support/AbstractBlobContainer.java b/core/src/main/java/org/elasticsearch/common/blobstore/support/AbstractBlobContainer.java index 8f83bbf8098..9929de2674d 100644 --- a/core/src/main/java/org/elasticsearch/common/blobstore/support/AbstractBlobContainer.java +++ b/core/src/main/java/org/elasticsearch/common/blobstore/support/AbstractBlobContainer.java @@ -30,7 +30,7 @@ import java.util.Collection; import java.util.Map; /** - * + * A base abstract blob container that implements higher level container methods. */ public abstract class AbstractBlobContainer implements BlobContainer { @@ -55,11 +55,11 @@ public abstract class AbstractBlobContainer implements BlobContainer { @Override public void deleteBlobs(Collection blobNames) throws IOException { - for(String blob: blobNames) { + for (String blob: blobNames) { deleteBlob(blob); } } - + @Override public void writeBlob(String blobName, BytesReference bytes) throws IOException { try (InputStream stream = bytes.streamInput()) { diff --git a/core/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java b/core/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java index 666ef9dfe39..b2b9e780205 100644 --- a/core/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java +++ b/core/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java @@ -283,6 +283,7 @@ public class BlobStoreFormatIT extends AbstractSnapshotIntegTestCase { int location = randomIntBetween(0, buffer.length - 1); buffer[location] = (byte) (buffer[location] ^ 42); } while (originalChecksum == checksum(buffer)); + blobContainer.deleteBlob(blobName); // delete original before writing new blob blobContainer.writeBlob(blobName, new BytesArray(buffer)); } diff --git a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java index 135e2f77810..6ba726e2b24 100644 --- a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java +++ b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java @@ -154,4 +154,4 @@ final class HdfsBlobContainer extends AbstractBlobContainer { public Map listBlobs() throws IOException { return listBlobsByPrefix(null); } -} \ No newline at end of file +} diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java index 8462cf007f0..6ff0b71cdcc 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java @@ -112,5 +112,19 @@ public abstract class ESBlobStoreContainerTestCase extends ESTestCase { } } + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/15579") + public void testOverwriteFails() throws IOException { + try (final BlobStore store = newBlobStore()) { + final String blobName = "foobar"; + final BlobContainer container = store.blobContainer(new BlobPath()); + byte[] data = randomBytes(randomIntBetween(10, scaledRandomIntBetween(1024, 1 << 16))); + final BytesArray bytesArray = new BytesArray(data); + container.writeBlob(blobName, bytesArray); + expectThrows(IOException.class, () -> container.writeBlob(blobName, bytesArray)); + container.deleteBlob(blobName); + container.writeBlob(blobName, bytesArray); // deleted it, so should be able to write it again + } + } + protected abstract BlobStore newBlobStore() throws IOException; }