HDFS-5139. Remove redundant -R option from setrep.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1525659 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/SetReplication.java

@@ -39,11 +39,14 @@ class SetReplication extends FsCommand {
   }
   
   public static final String NAME = "setrep";
-  public static final String USAGE = "[-R] [-w] <rep> <path/file> ...";
+  public static final String USAGE = "[-R] [-w] <rep> <path> ...";
   public static final String DESCRIPTION =
-    "Set the replication level of a file.\n" +
+    "Set the replication level of a file. If <path> is a directory\n" +
-    "The -R flag requests a recursive change of replication level\n" +
+    "then the command recursively changes the replication factor of\n" +
-    "for an entire tree.";
+    "all files under the directory tree rooted at <path>.\n" +
+    "The -w flag requests that the command wait for the replication\n" +
+    "to complete. This can potentially take a very long time.\n" +
+    "The -R flag is accepted for backwards compatibility. It has no effect.";
   
   protected short newRep = 0;
   protected List<PathData> waitList = new LinkedList<PathData>();
@@ -54,7 +57,7 @@ class SetReplication extends FsCommand {
     CommandFormat cf = new CommandFormat(2, Integer.MAX_VALUE, "R", "w");
     cf.parse(args);
     waitOpt = cf.getOpt("w");
-    setRecursive(cf.getOpt("R"));
+    setRecursive(true);
     
     try {
       newRep = Short.parseShort(args.removeFirst());
@@ -126,4 +129,4 @@ class SetReplication extends FsCommand {
       out.println(" done");
     }
   }
-}
+}

hadoop-common-project/hadoop-common/src/site/apt/FileSystemShell.apt.vm

@@ -381,17 +381,22 @@ rmr
 
 setrep
 
-   Usage: <<<hdfs dfs -setrep [-R] <path> >>>
+   Usage: <<<hdfs dfs -setrep [-R] [-w] <numRepicas> <path> >>>
 
-   Changes the replication factor of a file.
+   Changes the replication factor of a file. If <path> is a directory then
+   the command recursively changes the replication factor of all files under
+   the directory tree rooted at <path>.
 
    Options:
 
-     * The -R option will recursively increase the replication factor of files within a directory.
+     * The -w flag requests that the command wait for the replication
+       to complete. This can potentially take a very long time.
+
+     * The -R flag is accepted for backwards compatibility. It has no effect.
 
    Example:
 
-     * <<<hdfs dfs -setrep -w 3 -R /user/hadoop/dir1>>>
+     * <<<hdfs dfs -setrep -w 3 /user/hadoop/dir1>>>
 
    Exit Code:
 

hadoop-common-project/hadoop-common/src/test/resources/testConf.xml

@@ -601,16 +601,28 @@
       <comparators>
         <comparator>
           <type>RegexpComparator</type>
-          <expected-output>^-setrep \[-R\] \[-w\] &lt;rep&gt; &lt;path/file&gt; \.\.\.:( |\t)*Set the replication level of a file.( )*</expected-output>
+          <expected-output>^-setrep \[-R\] \[-w\] &lt;rep&gt; &lt;path&gt; \.\.\.:( |\t)*Set the replication level of a file. If &lt;path&gt; is a directory( )*</expected-output>
         </comparator>
         <comparator>
           <type>RegexpComparator</type>
-          <expected-output>^( |\t)*The -R flag requests a recursive change of replication level( )*</expected-output>
+          <expected-output>^( |\t)*then the command recursively changes the replication factor of( )*</expected-output>
         </comparator>
         <comparator>
           <type>RegexpComparator</type>
-          <expected-output>^( |\t)*for an entire tree.( )*</expected-output>
+          <expected-output>^( |\t)*all files under the directory tree rooted at &lt;path&gt;\.( )*</expected-output>
         </comparator>
+        <comparator>
+            <type>RegexpComparator</type>
+            <expected-output>^( |\t)*The -w flag requests that the command wait for the replication( )*</expected-output>
+        </comparator>
+        <comparator>
+            <type>RegexpComparator</type>
+            <expected-output>^( |\t)*to complete. This can potentially take a very long time\.( )*</expected-output>
+        </comparator>
+          <comparator>
+              <type>RegexpComparator</type>
+              <expected-output>^( |\t)*The -R flag is accepted for backwards compatibility\. It has no effect\.( )*</expected-output>
+          </comparator>
       </comparators>
     </test>
 

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java

@@ -61,6 +61,8 @@ import static org.junit.Assert.*;
 public class TestDFSShell {
   private static final Log LOG = LogFactory.getLog(TestDFSShell.class);
   private static AtomicInteger counter = new AtomicInteger();
+  private final int SUCCESS = 0;
+  private final int ERROR = 1;
 
   static final String TEST_ROOT_DIR = PathUtils.getTestDirName(TestDFSShell.class);
 
@@ -1619,9 +1621,6 @@ public class TestDFSShell {
   // force Copy Option is -f
   @Test (timeout = 30000)
   public void testCopyCommandsWithForceOption() throws Exception {
-    final int SUCCESS = 0;
-    final int ERROR = 1;
-
     Configuration conf = new Configuration();
     MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1)
         .format(true).build();
@@ -1682,7 +1681,55 @@ public class TestDFSShell {
       }
       cluster.shutdown();
     }
+  }
 
+  // setrep for file and directory.
+  @Test (timeout = 30000)
+  public void testSetrep() throws Exception {
+
+    Configuration conf = new Configuration();
+    MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1)
+                                                             .format(true).build();
+    FsShell shell = null;
+    FileSystem fs = null;
+
+    final String testdir1 = "/tmp/TestDFSShell-testSetrep-" + counter.getAndIncrement();
+    final String testdir2 = testdir1 + "/nestedDir";
+    final Path hdfsFile1 = new Path(testdir1, "testFileForSetrep");
+    final Path hdfsFile2 = new Path(testdir2, "testFileForSetrep");
+    final Short oldRepFactor = new Short((short) 1);
+    final Short newRepFactor = new Short((short) 3);
+    try {
+      String[] argv;
+      cluster.waitActive();
+      fs = cluster.getFileSystem();
+      assertThat(fs.mkdirs(new Path(testdir2)), is(true));
+      shell = new FsShell(conf);
+
+      fs.create(hdfsFile1, true).close();
+      fs.create(hdfsFile2, true).close();
+
+      // Tests for setrep on a file.
+      argv = new String[] { "-setrep", newRepFactor.toString(), hdfsFile1.toString() };
+      assertThat(shell.run(argv), is(SUCCESS));
+      assertThat(fs.getFileStatus(hdfsFile1).getReplication(), is(newRepFactor));
+      assertThat(fs.getFileStatus(hdfsFile2).getReplication(), is(oldRepFactor));
+
+      // Tests for setrep
+
+      // Tests for setrep on a directory and make sure it is applied recursively.
+      argv = new String[] { "-setrep", newRepFactor.toString(), testdir1 };
+      assertThat(shell.run(argv), is(SUCCESS));
+      assertThat(fs.getFileStatus(hdfsFile1).getReplication(), is(newRepFactor));
+      assertThat(fs.getFileStatus(hdfsFile2).getReplication(), is(newRepFactor));
+
+    } finally {
+      if (shell != null) {
+        shell.close();
+      }
+
+      cluster.shutdown();
+    }
   }
 
   /**

hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testHDFSConf.xml

@@ -6049,7 +6049,7 @@
         <command>-fs NAMENODE -mkdir /dir0</command>
         <command>-fs NAMENODE -touchz /dir0/file0</command>
         <command>-fs NAMENODE -touchz /dir0/file1</command>
-        <command>-fs NAMENODE -setrep -R 2 /dir0</command>
+        <command>-fs NAMENODE -setrep 2 /dir0</command>
       </test-commands>
       <cleanup-commands>
         <command>-fs NAMENODE -rm -r /user</command>
@@ -6072,7 +6072,7 @@
         <command>-fs NAMENODE -mkdir -p dir0</command>
         <command>-fs NAMENODE -touchz dir0/file0</command>
         <command>-fs NAMENODE -touchz dir0/file1</command>
-        <command>-fs NAMENODE -setrep -R 2 dir0</command>
+        <command>-fs NAMENODE -setrep 2 dir0</command>
       </test-commands>
       <cleanup-commands>
         <command>-fs NAMENODE -rm -r /user</command>
@@ -6089,6 +6089,24 @@
       </comparators>
     </test>
     
+    <test> <!-- TESTED -->
+      <description>setrep: -R ignored for existing file</description>
+      <test-commands>
+        <command>-fs NAMENODE -mkdir -p dir0</command>
+        <command>-fs NAMENODE -touchz dir0/file0</command>
+        <command>-fs NAMENODE -setrep -R 2 dir0/file0</command>
+      </test-commands>
+      <cleanup-commands>
+        <command>-fs NAMENODE -rm -r /user</command>
+      </cleanup-commands>
+      <comparators>
+        <comparator>
+          <type>RegexpComparator</type>
+          <expected-output>^Replication 2 set: dir0/file0</expected-output>
+        </comparator>
+      </comparators>
+    </test>
+
     <test> <!-- TESTED -->
       <description>setrep: non existent file (absolute path)</description>
       <test-commands>
@@ -6145,7 +6163,7 @@
         <command>-fs NAMENODE -mkdir hdfs:///dir0/</command>
         <command>-fs NAMENODE -touchz hdfs:///dir0/file0</command>
         <command>-fs NAMENODE -touchz hdfs:///dir0/file1</command>
-        <command>-fs NAMENODE -setrep -R 2 hdfs:///dir0</command>
+        <command>-fs NAMENODE -setrep 2 hdfs:///dir0</command>
       </test-commands>
       <cleanup-commands>
         <command>-fs NAMENODE -rm -r hdfs:///*</command>
@@ -6203,7 +6221,7 @@
         <command>-fs NAMENODE -mkdir -p NAMENODE/dir0</command>
         <command>-fs NAMENODE -touchz NAMENODE/dir0/file0</command>
         <command>-fs NAMENODE -touchz NAMENODE/dir0/file1</command>
-        <command>-fs NAMENODE -setrep -R 2 NAMENODE/dir0</command>
+        <command>-fs NAMENODE -setrep 2 NAMENODE/dir0</command>
       </test-commands>
       <cleanup-commands>
         <command>-fs NAMENODE -rm -r NAMENODE/*</command>