HDFS-328. Improve fs -setrep error message for invalid replication factors. Contributed by Daniel Templeton.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/afc88b39 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/afc88b39 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/afc88b39 Branch: refs/heads/YARN-1197 Commit: afc88b396f06488c331564e0f6987013bb920d3e Parents: c006c3a Author: Andrew Wang <[email protected]> Authored: Wed Sep 2 13:45:20 2015 -0700 Committer: Andrew Wang <[email protected]> Committed: Wed Sep 2 13:46:00 2015 -0700 ---------------------------------------------------------------------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../server/blockmanagement/BlockManager.java | 41 ++++++++------ .../org/apache/hadoop/hdfs/TestDFSShell.java | 56 ++++++++++++++++++++ .../src/test/resources/testHDFSConf.xml | 56 +++++++++++++++++++- 4 files changed, 140 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/afc88b39/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 0f2d713..78bbf26 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -877,6 +877,9 @@ Release 2.8.0 - UNRELEASED HDFS-2070. Add more unit tests for FsShell getmerge (Daniel Templeton via Colin P. McCabe) + HDFS-328. Improve fs -setrep error message for invalid replication factors. + (Daniel Templeton via wang) + OPTIMIZATIONS HDFS-8026. Trace FSOutputSummer#writeChecksumChunks rather than http://git-wip-us.apache.org/repos/asf/hadoop/blob/afc88b39/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java index 1346ab3..08fbd4f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java @@ -953,28 +953,39 @@ public class BlockManager implements BlockStatsMXBean { /** * Check whether the replication parameter is within the range - * determined by system configuration. + * determined by system configuration and throw an exception if it's not. + * + * @param src the path to the target file + * @param replication the requested replication factor + * @param clientName the name of the client node making the request + * @throws java.io.IOException thrown if the requested replication factor + * is out of bounds */ public void verifyReplication(String src, short replication, String clientName) throws IOException { - if (replication >= minReplication && replication <= maxReplication) { - //common case. avoid building 'text' - return; - } - - String text = "file " + src - + ((clientName != null) ? " on client " + clientName : "") - + ".\n" - + "Requested replication " + replication; + if (replication < minReplication || replication > maxReplication) { + StringBuilder msg = new StringBuilder("Requested replication factor of "); - if (replication > maxReplication) - throw new IOException(text + " exceeds maximum " + maxReplication); + msg.append(replication); - if (replication < minReplication) - throw new IOException(text + " is less than the required minimum " + - minReplication); + if (replication > maxReplication) { + msg.append(" exceeds maximum of "); + msg.append(maxReplication); + } else { + msg.append(" is less than the required minimum of "); + msg.append(minReplication); + } + + msg.append(" for ").append(src); + + if (clientName != null) { + msg.append(" from ").append(clientName); + } + + throw new IOException(msg.toString()); + } } /** http://git-wip-us.apache.org/repos/asf/hadoop/blob/afc88b39/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java index 1386124..dda2051 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java @@ -2412,7 +2412,63 @@ public class TestDFSShell { } } + /** + * Test -setrep with a replication factor that is too low. We have to test + * this here because the mini-cluster used with testHDFSConf.xml uses a + * replication factor of 1 (for good reason). + */ + @Test (timeout = 30000) + public void testSetrepLow() throws Exception { + Configuration conf = new Configuration(); + + conf.setInt(DFSConfigKeys.DFS_NAMENODE_REPLICATION_MIN_KEY, 2); + + MiniDFSCluster.Builder builder = new MiniDFSCluster.Builder(conf); + MiniDFSCluster cluster = builder.numDataNodes(2).format(true).build(); + FsShell shell = new FsShell(conf); + + cluster.waitActive(); + + final String testdir = "/tmp/TestDFSShell-testSetrepLow"; + final Path hdfsFile = new Path(testdir, "testFileForSetrepLow"); + final PrintStream origOut = System.out; + final PrintStream origErr = System.err; + + try { + final FileSystem fs = cluster.getFileSystem(); + + assertTrue("Unable to create test directory", + fs.mkdirs(new Path(testdir))); + fs.create(hdfsFile, true).close(); + + // Capture the command output so we can examine it + final ByteArrayOutputStream bao = new ByteArrayOutputStream(); + final PrintStream capture = new PrintStream(bao); + + System.setOut(capture); + System.setErr(capture); + + final String[] argv = new String[] { "-setrep", "1", hdfsFile.toString() }; + + try { + assertEquals("Command did not return the expected exit code", + 1, shell.run(argv)); + } finally { + System.setOut(origOut); + System.setErr(origErr); + } + + assertEquals("Error message is not the expected error message", + "setrep: Requested replication factor of 1 is less than " + + "the required minimum of 2 for /tmp/TestDFSShell-" + + "testSetrepLow/testFileForSetrepLow\n", + bao.toString()); + } finally { + shell.close(); + cluster.shutdown(); + } + } // setrep for file and directory. @Test (timeout = 30000) http://git-wip-us.apache.org/repos/asf/hadoop/blob/afc88b39/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testHDFSConf.xml ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testHDFSConf.xml b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testHDFSConf.xml index 18c68ca..7c3cac9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testHDFSConf.xml +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testHDFSConf.xml @@ -6471,7 +6471,61 @@ </comparator> </comparators> </test> - + + <test> <!-- TESTED --> + <description>setrep: invalid replication factor -- too high</description> + <test-commands> + <command>-fs NAMENODE -mkdir -p /dir0</command> + <command>-fs NAMENODE -touchz /dir0/file0</command> + <command>-fs NAMENODE -setrep 1025 /dir0/file0</command> + </test-commands> + <cleanup-commands> + <command>-fs NAMENODE -rm -r /user</command> + </cleanup-commands> + <comparators> + <comparator> + <type>RegexpComparator</type> + <expected-output>^setrep: Requested replication factor of 1025 exceeds maximum of [0-9]+ for /dir0/file0</expected-output> + </comparator> + </comparators> + </test> + + <test> <!-- TESTED --> + <description>setrep: invalid replication factor -- 0</description> + <test-commands> + <command>-fs NAMENODE -mkdir -p dir0</command> + <command>-fs NAMENODE -touchz dir0/file0</command> + <command>-fs NAMENODE -setrep 0 dir0/file0</command> + </test-commands> + <cleanup-commands> + <command>-fs NAMENODE -rm -r /user</command> + </cleanup-commands> + <comparators> + <comparator> + <type>RegexpComparator</type> + <expected-output>^-setrep: replication must be >= 1</expected-output> + </comparator> + </comparators> + </test> + + <test> <!-- TESTED --> + <description>setrep: invalid replication factor -- NaN</description> + <test-commands> + <command>-fs NAMENODE -mkdir -p dir0</command> + <command>-fs NAMENODE -touchz dir0/file0</command> + <command>-fs NAMENODE -setrep three dir0/file0</command> + </test-commands> + <cleanup-commands> + <command>-fs NAMENODE -rm -r /user</command> + </cleanup-commands> + <comparators> + <comparator> + <type>RegexpComparator</type> + <expected-output>^setrep: Illegal replication, a positive integer expected</expected-output> + </comparator> + </comparators> + </test> + <!-- Tests for touchz--> <test> <!-- TESTED --> <description>touchz: touching file (absolute path) </description>
