diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-3077.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-3077.txt index da8b9d6e1b7..4fd8bacf490 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-3077.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-3077.txt @@ -16,3 +16,5 @@ HDFS-3741. Exhaustive failure injection test for skipped RPCs (todd) HDFS-3773. TestNNWithQJM fails after HDFS-3741. (atm) HDFS-3793. Implement genericized format() in QJM (todd) + +HDFS-3795. QJM: validate journal dir at startup (todd) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java index 67bebbce82c..e64fccdcf70 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java @@ -55,6 +55,8 @@ public class JournalNode implements Tool, Configurable { private JournalNodeHttpServer httpServer; private Map journalsById = Maps.newHashMap(); + private File localDir; + static { HdfsConfiguration.init(); } @@ -82,8 +84,32 @@ public class JournalNode implements Tool, Configurable { @Override public void setConf(Configuration conf) { this.conf = conf; + this.localDir = new File( + conf.get(DFSConfigKeys.DFS_JOURNALNODE_EDITS_DIR_KEY, + DFSConfigKeys.DFS_JOURNALNODE_EDITS_DIR_DEFAULT).trim()); } + private static void validateAndCreateJournalDir(File dir) throws IOException { + if (!dir.isAbsolute()) { + throw new IllegalArgumentException( + "Journal dir '" + dir + "' should be an absolute path"); + } + + if (!dir.exists() && !dir.mkdirs()) { + throw new IOException("Could not create journal dir '" + + dir + "'"); + } else if (!dir.isDirectory()) { + throw new IOException("Journal directory '" + dir + "' is not " + + "a directory"); + } + + if (!dir.canWrite()) { + throw new IOException("Unable to write to journal dir '" + + dir + "'"); + } + } + + @Override public Configuration getConf() { return conf; @@ -101,6 +127,8 @@ public class JournalNode implements Tool, Configurable { public void start() throws IOException { Preconditions.checkState(!isStarted(), "JN already running"); + validateAndCreateJournalDir(localDir); + DefaultMetricsSystem.initialize("JournalNode"); JvmMetrics.create("JournalNode", conf.get(DFSConfigKeys.DFS_METRICS_SESSION_ID_KEY), diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/MiniJournalCluster.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/MiniJournalCluster.java index 559de47642b..840f4939aeb 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/MiniJournalCluster.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/MiniJournalCluster.java @@ -156,7 +156,7 @@ public class MiniJournalCluster { } public File getStorageDir(int idx) { - return new File(baseDir, "journalnode-" + idx); + return new File(baseDir, "journalnode-" + idx).getAbsoluteFile(); } public File getCurrentDir(int idx, String jid) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/TestMiniJournalCluster.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/TestMiniJournalCluster.java index 8d7cc66accb..fbb51e12dc8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/TestMiniJournalCluster.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/TestMiniJournalCluster.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hdfs.qjournal; import static org.junit.Assert.*; +import java.io.File; import java.io.IOException; import java.net.URI; @@ -42,7 +43,9 @@ public class TestMiniJournalCluster { JournalNode node = c.getJournalNode(0); String dir = node.getConf().get(DFSConfigKeys.DFS_JOURNALNODE_EDITS_DIR_KEY); - assertEquals(MiniDFSCluster.getBaseDirectory() + "journalnode-0", + assertEquals( + new File(MiniDFSCluster.getBaseDirectory() + "journalnode-0") + .getAbsolutePath(), dir); } finally { c.shutdown(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java index 23a819376f1..dee1e20b28e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNode.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.io.IOException; import java.net.HttpURLConnection; import java.net.InetSocketAddress; import java.net.URL; @@ -234,6 +235,33 @@ public class TestJournalNode { } } + @Test + public void testFailToStartWithBadConfig() throws Exception { + Configuration conf = new Configuration(); + conf.set(DFSConfigKeys.DFS_JOURNALNODE_EDITS_DIR_KEY, "non-absolute-path"); + assertJNFailsToStart(conf, "should be an absolute path"); + + // Existing file which is not a directory + conf.set(DFSConfigKeys.DFS_JOURNALNODE_EDITS_DIR_KEY, "/dev/null"); + assertJNFailsToStart(conf, "is not a directory"); + + // Directory which cannot be created + conf.set(DFSConfigKeys.DFS_JOURNALNODE_EDITS_DIR_KEY, "/proc/does-not-exist"); + assertJNFailsToStart(conf, "Could not create"); + + } + + private static void assertJNFailsToStart(Configuration conf, + String errString) { + try { + JournalNode jn = new JournalNode(); + jn.setConf(conf); + jn.start(); + } catch (Exception e) { + GenericTestUtils.assertExceptionContains(errString, e); + } + } + // TODO: // - add test that checks formatting behavior // - add test that checks rejects newEpoch if nsinfo doesn't match