Do not create directories if repository is readonly (#21495)

liketic · liketic · commit 489bd71629db · 2017-11-03T10:06:08.000+08:00
For FsBlobStore and HdfsBlobStore, if the repository is read only, the blob store should aware the read only setting and do not create directories even it's not exists.

Do not mkdirs if HDFS repository is read only
diff --git a/core/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobStore.java b/core/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobStore.java
@@ -39,10 +39,15 @@ public class FsBlobStore extends AbstractComponent implements BlobStore {
 
     private final int bufferSizeInBytes;
 
+    private final boolean readOnly;
+
     public FsBlobStore(Settings settings, Path path) throws IOException {
         super(settings);
         this.path = path;
-        Files.createDirectories(path);
+        this.readOnly = settings.getAsBoolean("readonly", false);
+        if (!this.readOnly) {
+            Files.createDirectories(path);
+        }
         this.bufferSizeInBytes = (int) settings.getAsBytesSize("repositories.fs.buffer_size", new ByteSizeValue(100, ByteSizeUnit.KB)).getBytes();
     }
 
@@ -80,7 +85,9 @@ public void close() {
 
     private synchronized Path buildAndCreate(BlobPath path) throws IOException {
         Path f = buildPath(path);
-        Files.createDirectories(f);
+        if (!readOnly) {
+            Files.createDirectories(f);
+        }
         return f;
     }
 
diff --git a/core/src/test/java/org/elasticsearch/common/blobstore/FsBlobStoreTests.java b/core/src/test/java/org/elasticsearch/common/blobstore/FsBlobStoreTests.java
@@ -20,12 +20,14 @@
 
 import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.common.blobstore.fs.FsBlobStore;
+import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeUnit;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.repositories.ESBlobStoreTestCase;
 
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 
 @LuceneTestCase.SuppressFileSystems("ExtrasFS")
@@ -35,4 +37,39 @@ protected BlobStore newBlobStore() throws IOException {
         Settings settings = randomBoolean() ? Settings.EMPTY : Settings.builder().put("buffer_size", new ByteSizeValue(randomIntBetween(1, 100), ByteSizeUnit.KB)).build();
         return new FsBlobStore(settings, tempDir);
     }
+
+    public void testReadOnly() throws Exception {
+        Settings settings = Settings.builder().put("readonly", true).build();
+        Path tempDir = createTempDir();
+        Path path = tempDir.resolve("bar");
+
+        try (FsBlobStore store = new FsBlobStore(settings, path)) {
+            assertFalse(Files.exists(path));
+            BlobPath blobPath = BlobPath.cleanPath().add("foo");
+            store.blobContainer(blobPath);
+            Path storePath = store.path();
+            for (String d : blobPath) {
+                storePath = storePath.resolve(d);
+            }
+            assertFalse(Files.exists(storePath));
+        }
+
+        settings = randomBoolean() ? Settings.EMPTY : Settings.builder().put("readonly", false).build();
+        try (FsBlobStore store = new FsBlobStore(settings, path)) {
+            assertTrue(Files.exists(path));
+            BlobPath blobPath = BlobPath.cleanPath().add("foo");
+            BlobContainer container = store.blobContainer(blobPath);
+            Path storePath = store.path();
+            for (String d : blobPath) {
+                storePath = storePath.resolve(d);
+            }
+            assertTrue(Files.exists(storePath));
+            assertTrue(Files.isDirectory(storePath));
+
+            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"));
+        }
+    }
 }
diff --git a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobStore.java b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobStore.java
@@ -39,17 +39,21 @@ final class HdfsBlobStore implements BlobStore {
     private final FileContext fileContext;
     private final HdfsSecurityContext securityContext;
     private final int bufferSize;
+    private final boolean readOnly;
     private volatile boolean closed;
 
-    HdfsBlobStore(FileContext fileContext, String path, int bufferSize) throws IOException {
+    HdfsBlobStore(FileContext fileContext, String path, int bufferSize, boolean readOnly) throws IOException {
         this.fileContext = fileContext;
         this.securityContext = new HdfsSecurityContext(fileContext.getUgi());
         this.bufferSize = bufferSize;
         this.root = execute(fileContext1 -> fileContext1.makeQualified(new Path(path)));
-        try {
-            mkdirs(root);
-        } catch (FileAlreadyExistsException ok) {
-            // behaves like Files.createDirectories
+        this.readOnly = readOnly;
+        if (!readOnly) {
+            try {
+                mkdirs(root);
+            } catch (FileAlreadyExistsException ok) {
+                // behaves like Files.createDirectories
+            }
         }
     }
 
@@ -80,12 +84,14 @@ public BlobContainer blobContainer(BlobPath path) {
 
     private Path buildHdfsPath(BlobPath blobPath) {
         final Path path = translateToHdfsPath(blobPath);
-        try {
-            mkdirs(path);
-        } catch (FileAlreadyExistsException ok) {
-            // behaves like Files.createDirectories
-        } catch (IOException ex) {
-            throw new ElasticsearchException("failed to create blob container", ex);
+        if (!readOnly) {
+            try {
+                mkdirs(path);
+            } catch (FileAlreadyExistsException ok) {
+                // behaves like Files.createDirectories
+            } catch (IOException ex) {
+                throw new ElasticsearchException("failed to create blob container", ex);
+            }
         }
         return path;
     }
diff --git a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsRepository.java b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsRepository.java
@@ -106,7 +106,7 @@ protected void doStart() {
             SpecialPermission.check();
             FileContext fileContext = AccessController.doPrivileged((PrivilegedAction<FileContext>)
                 () -> createContext(uri, getMetadata().settings()));
-            blobStore = new HdfsBlobStore(fileContext, pathSetting, bufferSize);
+            blobStore = new HdfsBlobStore(fileContext, pathSetting, bufferSize, isReadOnly());
             logger.debug("Using file-system [{}] for URI [{}], path [{}]", fileContext.getDefaultFileSystem(), fileContext.getDefaultFileSystem().getUri(), pathSetting);
         } catch (IOException e) {
             throw new UncheckedIOException(String.format(Locale.ROOT, "Cannot create HDFS repository for uri [%s]", uri), e);
diff --git a/plugins/repository-hdfs/src/test/java/org/elasticsearch/repositories/hdfs/HdfsBlobStoreContainerTests.java b/plugins/repository-hdfs/src/test/java/org/elasticsearch/repositories/hdfs/HdfsBlobStoreContainerTests.java
@@ -19,6 +19,20 @@
 
 package org.elasticsearch.repositories.hdfs;
 
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.AbstractFileSystem;
+import org.apache.hadoop.fs.FileContext;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.UnsupportedFileSystemException;
+import org.elasticsearch.common.SuppressForbidden;
+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.ESBlobStoreContainerTestCase;
+
+import javax.security.auth.Subject;
 import java.io.IOException;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
@@ -29,30 +43,28 @@
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.util.Collections;
-import javax.security.auth.Subject;
 
-import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.AbstractFileSystem;
-import org.apache.hadoop.fs.FileContext;
-import org.apache.hadoop.fs.UnsupportedFileSystemException;
-import org.elasticsearch.common.SuppressForbidden;
-import org.elasticsearch.common.blobstore.BlobStore;
-import org.elasticsearch.repositories.ESBlobStoreContainerTestCase;
+import static org.elasticsearch.repositories.ESBlobStoreTestCase.randomBytes;
+import static org.elasticsearch.repositories.ESBlobStoreTestCase.readBlobFully;
+
 
 @ThreadLeakFilters(filters = {HdfsClientThreadLeakFilter.class})
 public class HdfsBlobStoreContainerTests extends ESBlobStoreContainerTestCase {
 
     @Override
     protected BlobStore newBlobStore() throws IOException {
+        return new HdfsBlobStore(createTestContext(), "temp", 1024, false);
+    }
+
+    private FileContext createTestContext() {
         FileContext fileContext;
         try {
             fileContext = AccessController.doPrivileged((PrivilegedExceptionAction<FileContext>)
                 () -> createContext(new URI("hdfs:///")));
         } catch (PrivilegedActionException e) {
             throw new RuntimeException(e.getCause());
         }
-        return new HdfsBlobStore(fileContext, "temp", 1024);
+        return fileContext;
     }
 
     @SuppressForbidden(reason = "lesser of two evils (the other being a bunch of JNI/classloader nightmares)")
@@ -69,7 +81,7 @@ private FileContext createContext(URI uri) {
             Class<?> clazz = Class.forName("org.apache.hadoop.security.User");
             ctor = clazz.getConstructor(String.class);
             ctor.setAccessible(true);
-        }  catch (ClassNotFoundException | NoSuchMethodException e) {
+        } catch (ClassNotFoundException | NoSuchMethodException e) {
             throw new RuntimeException(e);
         }
 
@@ -98,4 +110,33 @@ private FileContext createContext(URI uri) {
             }
         });
     }
+
+    public void testReadOnly() throws Exception {
+        FileContext fileContext = createTestContext();
+        // Constructor will not create dir if read only
+        HdfsBlobStore hdfsBlobStore = new HdfsBlobStore(fileContext, "dir", 1024, true);
+        FileContext.Util util = fileContext.util();
+        Path root = fileContext.makeQualified(new Path("dir"));
+        assertFalse(util.exists(root));
+        BlobPath blobPath = BlobPath.cleanPath().add("path");
+
+        // blobContainer() will not create path if read only
+        hdfsBlobStore.blobContainer(blobPath);
+        Path hdfsPath = root;
+        for (String p : blobPath) {
+            hdfsPath = new Path(hdfsPath, p);
+        }
+        assertFalse(util.exists(hdfsPath));
+
+        // if not read only, directory will be created
+        hdfsBlobStore = new HdfsBlobStore(fileContext, "dir", 1024, false);
+        assertTrue(util.exists(root));
+        BlobContainer container = hdfsBlobStore.blobContainer(blobPath);
+        assertTrue(util.exists(hdfsPath));
+
+        byte[] data = randomBytes(randomIntBetween(10, scaledRandomIntBetween(1024, 1 << 16)));
+        writeBlob(container, "foo", new BytesArray(data));
+        assertArrayEquals(readBlobFully(container, "foo", data.length), data);
+        assertTrue(container.blobExists("foo"));
+    }
 }
diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java
@@ -142,7 +142,7 @@ public void testVerifyOverwriteFails() throws IOException {
         }
     }
 
-    private void writeBlob(final BlobContainer container, final String blobName, final BytesArray bytesArray) throws IOException {
+    protected void writeBlob(final BlobContainer container, final String blobName, final BytesArray bytesArray) throws IOException {
         try (InputStream stream = bytesArray.streamInput()) {
             container.writeBlob(blobName, stream, bytesArray.length());
         }
diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreTestCase.java
@@ -78,7 +78,7 @@ public static byte[] randomBytes(int length) {
         return data;
     }
 
-    private static void writeBlob(BlobContainer container, String blobName, BytesArray bytesArray) throws IOException {
+    protected static void writeBlob(BlobContainer container, String blobName, BytesArray bytesArray) throws IOException {
         try (InputStream stream = bytesArray.streamInput()) {
             container.writeBlob(blobName, stream, bytesArray.length());
         }

Original file line number	Diff line number	Diff line change
`@@ -142,7 +142,7 @@ public void testVerifyOverwriteFails() throws IOException {`
`142`	`142`	`}`
`143`	`143`	`}`
`144`	`144`
`145`		`- private void writeBlob(final BlobContainer container, final String blobName, final BytesArray bytesArray) throws IOException {`
	`145`	`+ protected void writeBlob(final BlobContainer container, final String blobName, final BytesArray bytesArray) throws IOException {`
`146`	`146`	`try (InputStream stream = bytesArray.streamInput()) {`
`147`	`147`	`container.writeBlob(blobName, stream, bytesArray.length());`
`148`	`148`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ public static byte[] randomBytes(int length) {`
`78`	`78`	`return data;`
`79`	`79`	`}`
`80`	`80`
`81`		`- private static void writeBlob(BlobContainer container, String blobName, BytesArray bytesArray) throws IOException {`
	`81`	`+ protected static void writeBlob(BlobContainer container, String blobName, BytesArray bytesArray) throws IOException {`
`82`	`82`	`try (InputStream stream = bytesArray.streamInput()) {`
`83`	`83`	`container.writeBlob(blobName, stream, bytesArray.length());`
`84`	`84`	`}`