From 20fc34edacf8d6970cc210c574dd04f572ba104d Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Mon, 13 Oct 2014 18:28:34 -0700 Subject: [PATCH] HDFS-7237. The command "hdfs namenode -rollingUpgrade" throws ArrayIndexOutOfBoundsException. --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../server/common/HdfsServerConstants.java | 9 ++++ .../hadoop/hdfs/server/namenode/NameNode.java | 13 +++--- .../datanode/TestHdfsServerConstants.java | 17 ++++--- .../namenode/TestNameNodeOptionParsing.java | 45 +++++++++++++++++++ 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 05894720113..f93181629a0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -555,6 +555,9 @@ Release 2.6.0 - UNRELEASED HDFS-6544. Broken Link for GFS in package.html. (Suraj Nayak M via wheat9) + HDFS-7237. The command "hdfs namenode -rollingUpgrade" throws + ArrayIndexOutOfBoundsException. (szetszwo) + BREAKDOWN OF HDFS-6134 AND HADOOP-10150 SUBTASKS AND RELATED JIRAS HDFS-6387. HDFS CLI admin tool for creating & deleting an diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java index 767c1b559f9..3e8c842b34e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java @@ -72,6 +72,15 @@ static RollingUpgradeStartupOption fromString(String s) { throw new IllegalArgumentException("Failed to convert \"" + s + "\" to " + RollingUpgradeStartupOption.class.getSimpleName()); } + + public static String getAllOptionString() { + final StringBuilder b = new StringBuilder("<"); + for(RollingUpgradeStartupOption opt : VALUES) { + b.append(opt.name().toLowerCase()).append("|"); + } + b.setCharAt(b.length() - 1, '>'); + return b.toString(); + } } /** Startup options */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java index ac86e6b4acb..cb182c97a0c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java @@ -215,10 +215,8 @@ public static enum OperationCategory { " [" + StartupOption.CLUSTERID.getName() + " cid]" + " [" + StartupOption.RENAMERESERVED.getName() + "] ] | \n\t[" + StartupOption.ROLLBACK.getName() + "] | \n\t[" - + StartupOption.ROLLINGUPGRADE.getName() + " <" - + RollingUpgradeStartupOption.DOWNGRADE.name().toLowerCase() + "|" - + RollingUpgradeStartupOption.ROLLBACK.name().toLowerCase() + "|" - + RollingUpgradeStartupOption.STARTED.name().toLowerCase() + "> ] | \n\t[" + + StartupOption.ROLLINGUPGRADE.getName() + " " + + RollingUpgradeStartupOption.getAllOptionString() + " ] | \n\t[" + StartupOption.FINALIZE.getName() + "] | \n\t[" + StartupOption.IMPORT.getName() + "] | \n\t[" + StartupOption.INITIALIZESHAREDEDITS.getName() + "] | \n\t[" @@ -1249,6 +1247,11 @@ static StartupOption parseArguments(String args[]) { } else if (StartupOption.ROLLINGUPGRADE.getName().equalsIgnoreCase(cmd)) { startOpt = StartupOption.ROLLINGUPGRADE; ++i; + if (i >= argsLen) { + LOG.fatal("Must specify a rolling upgrade startup option " + + RollingUpgradeStartupOption.getAllOptionString()); + return null; + } startOpt.setRollingUpgradeStartupOption(args[i]); } else if (StartupOption.ROLLBACK.getName().equalsIgnoreCase(cmd)) { startOpt = StartupOption.ROLLBACK; @@ -1503,7 +1506,7 @@ public static void main(String argv[]) throws Exception { namenode.join(); } } catch (Throwable e) { - LOG.fatal("Exception in namenode join", e); + LOG.fatal("Failed to start namenode.", e); terminate(1, e); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestHdfsServerConstants.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestHdfsServerConstants.java index a1471035525..2e76b250b8f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestHdfsServerConstants.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestHdfsServerConstants.java @@ -17,12 +17,12 @@ */ package org.apache.hadoop.hdfs.server.datanode; -import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.*; -import org.junit.Test; - +import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertThat; + +import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.RollingUpgradeStartupOption; +import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.StartupOption; +import org.junit.Test; /** @@ -43,10 +43,10 @@ private static void verifyStartupOptionResult(String value, RollingUpgradeStartupOption expectedRollupOption) { StartupOption option = StartupOption.getEnum(value); - assertThat(option, is(expectedOption)); + assertEquals(expectedOption, option); if (expectedRollupOption != null) { - assertThat(option.getRollingUpgradeStartupOption(), is(expectedRollupOption)); + assertEquals(expectedRollupOption, option.getRollingUpgradeStartupOption()); } } @@ -86,6 +86,9 @@ public void testRollingUpgradeStartupOptionParsing() { verifyStartupOptionResult("ROLLINGUPGRADE(DOWNGRADE)", StartupOption.ROLLINGUPGRADE, RollingUpgradeStartupOption.DOWNGRADE); + verifyStartupOptionResult("ROLLINGUPGRADE(STARTED)", + StartupOption.ROLLINGUPGRADE, + RollingUpgradeStartupOption.STARTED); try { verifyStartupOptionResult("ROLLINGUPGRADE(UNKNOWNOPTION)", StartupOption.ROLLINGUPGRADE, null); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeOptionParsing.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeOptionParsing.java index 6ef1e57cf85..f540253ed77 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeOptionParsing.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeOptionParsing.java @@ -23,7 +23,9 @@ import static org.junit.Assert.assertTrue; import org.apache.hadoop.hdfs.protocol.HdfsConstants; +import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.RollingUpgradeStartupOption; import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.StartupOption; +import org.junit.Assert; import org.junit.Test; public class TestNameNodeOptionParsing { @@ -102,4 +104,47 @@ public void testUpgrade() { assertNull(opt); } + @Test(timeout = 10000) + public void testRollingUpgrade() { + { + final String[] args = {"-rollingUpgrade"}; + final StartupOption opt = NameNode.parseArguments(args); + assertNull(opt); + } + + { + final String[] args = {"-rollingUpgrade", "started"}; + final StartupOption opt = NameNode.parseArguments(args); + assertEquals(StartupOption.ROLLINGUPGRADE, opt); + assertEquals(RollingUpgradeStartupOption.STARTED, opt.getRollingUpgradeStartupOption()); + assertTrue(RollingUpgradeStartupOption.STARTED.matches(opt)); + } + + { + final String[] args = {"-rollingUpgrade", "downgrade"}; + final StartupOption opt = NameNode.parseArguments(args); + assertEquals(StartupOption.ROLLINGUPGRADE, opt); + assertEquals(RollingUpgradeStartupOption.DOWNGRADE, opt.getRollingUpgradeStartupOption()); + assertTrue(RollingUpgradeStartupOption.DOWNGRADE.matches(opt)); + } + + { + final String[] args = {"-rollingUpgrade", "rollback"}; + final StartupOption opt = NameNode.parseArguments(args); + assertEquals(StartupOption.ROLLINGUPGRADE, opt); + assertEquals(RollingUpgradeStartupOption.ROLLBACK, opt.getRollingUpgradeStartupOption()); + assertTrue(RollingUpgradeStartupOption.ROLLBACK.matches(opt)); + } + + { + final String[] args = {"-rollingUpgrade", "foo"}; + try { + NameNode.parseArguments(args); + Assert.fail(); + } catch(IllegalArgumentException iae) { + // the exception is expected. + } + } + } + }