HADOOP-738. Change 'dfs -get' command to not create CRC files by default. Contributed by Milind.

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

CHANGES.txt

@@ -23,6 +23,10 @@ Trunk (unreleased changes)
     processing very large invalid block lists can tie up both the
     namenode and datanode for too long.  (Dhruba Borthakur via cutting) 
 
+ 7. HADOOP-738. Change 'dfs -get' command to not create CRC files by
+    default, adding a -crc option to force their creation.
+    (Milind Bhandarkar via cutting)
+
 
 Release 0.9.1 - 2006-12-06
 

src/java/org/apache/hadoop/dfs/DFSShell.java

@@ -62,12 +62,23 @@ public class DFSShell extends ToolBase {
      * and copy them to the local name. srcf is kept.
      * When copying mutiple files, the destination must be a directory. 
      * Otherwise, IOException is thrown.
-     * @param srcf: a file pattern specifying source files
-     * @param dstf: a destination local file/directory 
+     * @param argv: arguments
+     * @param pos: Ignore everything before argv[pos]  
      * @exception: IOException  
      * @see org.apache.hadoop.fs.FileSystem.globPaths 
      */
-    void copyToLocal(String srcf, String dstf) throws IOException {
+    void copyToLocal(String[]argv, int pos) throws IOException {
+      if(argv.length-pos<2 || (argv.length-pos==2 && argv[pos].equalsIgnoreCase("-crc"))) {
+        System.err.println("Usage: -get [-crc] <src> <dst>");
+        System.exit(-1);
+      }
+      boolean copyCrc = false;
+      if ("-crc".equalsIgnoreCase(argv[pos])) {
+        pos++;
+        copyCrc = true;
+      }
+      String srcf = argv[pos++];
+      String dstf = argv[pos++];
       Path [] srcs = fs.globPaths( new Path(srcf) );
       if( srcs.length > 1 && !new File( dstf ).isDirectory()) {
         throw new IOException( "When copy multiple files, " 
@@ -75,7 +86,7 @@ public class DFSShell extends ToolBase {
       }
       Path dst = new Path( dstf );
       for( int i=0; i<srcs.length; i++ ) {
-        fs.copyToLocalFile( srcs[i], dst );
+        fs.copyToLocalFile( srcs[i], dst, copyCrc );
       }
     }
     
@@ -431,7 +442,7 @@ public class DFSShell extends ToolBase {
             + "destination should be a directory." );
       }
       for( int i=0; i<srcs.length; i++ ) {
-        FileUtil.copy(fs, srcs[i], fs, dst, false, conf);
+        FileUtil.copy(fs, srcs[i], fs, dst, false, true, conf);
       }
     }
 
@@ -644,7 +655,7 @@ public class DFSShell extends ToolBase {
           } else if ("-get".equals(cmd) || "-copyToLocal".equals(cmd) ||
                    "-moveToLocal".equals(cmd)) {
             System.err.println("Usage: java DFSShell" + 
-                " [" + cmd + " <src> <localdst>]");
+                " [" + cmd + " [-crc] <src> <localdst>]");
           } else if ("-cat".equals(cmd)) {
             System.out.println("Usage: java DFSShell" + 
                 " [" + cmd + " <src>]");
@@ -669,11 +680,11 @@ public class DFSShell extends ToolBase {
             System.err.println("           [-put <localsrc> <dst>]");
             System.err.println("           [-copyFromLocal <localsrc> <dst>]");
             System.err.println("           [-moveFromLocal <localsrc> <dst>]");
-            System.err.println("           [-get <src> <localdst>]");
+            System.err.println("           [-get [-crc] <src> <localdst>]");
             System.err.println("           [-getmerge <src> <localdst> [addnl]]");
             System.err.println("           [-cat <src>]");
-            System.err.println("           [-copyToLocal <src> <localdst>]");
-            System.err.println("           [-moveToLocal <src> <localdst>]");
+            System.err.println("           [-copyToLocal [-crc] <src> <localdst>]");
+            System.err.println("           [-moveToLocal [-crc] <src> <localdst>]");
             System.err.println("           [-mkdir <path>]");
             System.err.println("           [-setrep [-R] <rep> <path/file>]");
           }
@@ -696,13 +707,18 @@ public class DFSShell extends ToolBase {
         //
         // verify that we have enough command line parameters
         //
-        if ("-put".equals(cmd) || "-get".equals(cmd) || 
-            "-copyFromLocal".equals(cmd) || "-moveFromLocal".equals(cmd) || 
-            "-copyToLocal".equals(cmd) || "-moveToLocal".equals(cmd)) {
+        if ("-put".equals(cmd) || 
+            "-copyFromLocal".equals(cmd) || "-moveFromLocal".equals(cmd)) {
                 if (argv.length != 3) {
                   printUsage(cmd);
                   return exitCode;
                 }
+        } else if ("-get".equals(cmd) || 
+            "-copyToLocal".equals(cmd) || "-moveToLocal".equals(cmd)) {
+                if (argv.length < 3) {
+                  printUsage(cmd);
+                  return exitCode;
+                }
         } else if ("-mv".equals(cmd) || "-cp".equals(cmd)) {
                 if (argv.length < 3) {
                   printUsage(cmd);
@@ -735,7 +751,7 @@ public class DFSShell extends ToolBase {
             } else if ("-moveFromLocal".equals(cmd)) {
                 moveFromLocal(new Path(argv[i++]), argv[i++]);
             } else if ("-get".equals(cmd) || "-copyToLocal".equals(cmd)) {
-                copyToLocal(argv[i++], argv[i++]);
+                copyToLocal(argv, i);
             } else if ("-getmerge".equals(cmd)) {
                 if(argv.length>i+2)
                     copyMergeToLocal(argv[i++], new Path(argv[i++]), Boolean.parseBoolean(argv[i++]));

src/java/org/apache/hadoop/dfs/DistributedFileSystem.java

@@ -181,15 +181,15 @@ public class DistributedFileSystem extends FileSystem {
     }
 
     public void moveFromLocalFile(Path src, Path dst) throws IOException {
-      FileUtil.copy(localFs, src, this, dst, true, getConf());
+      FileUtil.copy(localFs, src, this, dst, true, true, getConf());
     }
 
     public void copyFromLocalFile(Path src, Path dst) throws IOException {
-      FileUtil.copy(localFs, src, this, dst, false, getConf());
+      FileUtil.copy(localFs, src, this, dst, false, true, getConf());
     }
 
-    public void copyToLocalFile(Path src, Path dst) throws IOException {
-      FileUtil.copy(this, src, localFs, dst, false, getConf());
+    public void copyToLocalFile(Path src, Path dst, boolean copyCrc) throws IOException {
+      FileUtil.copy(this, src, localFs, dst, false, copyCrc, getConf());
     }
 
     public Path startLocalOutput(Path fsOutputFile, Path tmpLocalFile)

src/java/org/apache/hadoop/fs/FileSystem.java

@@ -731,8 +731,19 @@ public abstract class FileSystem extends Configured {
     /**
      * The src file is under FS, and the dst is on the local disk.
      * Copy it from FS control to the local dst name.
+     * If src and dst are directories, copy crc files as well.
      */
-    public abstract void copyToLocalFile(Path src, Path dst) throws IOException;
+    public void copyToLocalFile(Path src, Path dst) throws IOException {
+      copyToLocalFile(src, dst, true);
+    }
+    
+    /**
+     * The src file is under FS, and the dst is on the local disk.
+     * Copy it from FS control to the local dst name.
+     * If src and dst are directories, the copyCrc parameter
+     * determines whether to copy CRC files.
+     */
+    public abstract void copyToLocalFile(Path src, Path dst, boolean copyCrc) throws IOException;
 
     /**
      * Returns a local File that the user can write output to.  The caller

src/java/org/apache/hadoop/fs/FileUtil.java

@@ -52,11 +52,20 @@ public class FileUtil {
     return dir.delete();
   }
 
+  /** Copy files between FileSystems. */
+  public static boolean copy(FileSystem srcFS, Path src, 
+                             FileSystem dstFS, Path dst, 
+                             boolean deleteSource,
+                             Configuration conf ) throws IOException {
+    return copy(srcFS, src, dstFS, dst, deleteSource, true, conf);
+  
+  }
 
   /** Copy files between FileSystems. */
   public static boolean copy(FileSystem srcFS, Path src, 
                              FileSystem dstFS, Path dst, 
                              boolean deleteSource,
+                             boolean copyCrc,
                              Configuration conf ) throws IOException {
     dst = checkDest(src.getName(), dstFS, dst);
 
@@ -67,12 +76,16 @@ public class FileUtil {
       Path contents[] = srcFS.listPaths(src);
       for (int i = 0; i < contents.length; i++) {
         copy(srcFS, contents[i], dstFS, new Path(dst, contents[i].getName()),
-             deleteSource, conf);
+             deleteSource, copyCrc, conf);
       }
     } else if (srcFS.isFile(src)) {
       InputStream in = srcFS.open(src);
       try {
-        copyContent(in, dstFS.create(dst), conf);
+        OutputStream out = (copyCrc) ?
+          dstFS.create(dst) :
+          dstFS.createRaw(dst, true, dstFS.getDefaultReplication(),
+            dstFS.getDefaultBlockSize());
+        copyContent(in, out, conf);
       } finally {
         in.close();
       } 

src/java/org/apache/hadoop/fs/LocalFileSystem.java

@@ -338,8 +338,8 @@ public class LocalFileSystem extends FileSystem {
     }
 
     // We can't delete the src file in this case.  Too bad.
-    public void copyToLocalFile(Path src, Path dst) throws IOException {
-      FileUtil.copy(this, src, this, dst, false, getConf());
+    public void copyToLocalFile(Path src, Path dst, boolean copyCrc) throws IOException {
+      FileUtil.copy(this, src, this, dst, false, copyCrc, getConf());
     }
 
     // We can write output directly to the final location

src/java/org/apache/hadoop/mapred/PhasedFileSystem.java

@@ -361,7 +361,7 @@ public class PhasedFileSystem extends FileSystem {
 
   @Override
   public void copyToLocalFile(
-      Path src, Path dst)
+      Path src, Path dst, boolean copyCrc)
       throws IOException {
     throw new UnsupportedOperationException("Operation not supported");  
   }

src/test/org/apache/hadoop/dfs/TestDFSShell.java

@@ -29,7 +29,10 @@ import org.apache.hadoop.fs.Path;
  * @author Dhruba Borthakur
  */
 public class TestDFSShell extends TestCase {
-
+  private static String TEST_ROOT_DIR =
+    new Path(System.getProperty("test.build.data","/tmp"))
+    .toString().replace(' ', '+');
+  
   private void writeFile(FileSystem fileSys, Path name) throws IOException {
     DataOutputStream stm = fileSys.create(name);
     stm.writeBytes("dhruba");
@@ -72,6 +75,58 @@ public class TestDFSShell extends TestCase {
           assertTrue(val == 0);
         }
 
+        // Verify that we can get with and without crc
+        {
+          File testFile = new File(TEST_ROOT_DIR, "mkdirs/myFile");
+          File checksumFile = new File(fileSys.getChecksumFile(
+              new Path(testFile.getAbsolutePath())).toString());
+          testFile.delete();
+          checksumFile.delete();
+          new File(TEST_ROOT_DIR, "mkdirs").delete();
+          
+          String[] args = new String[3];
+          args[0] = "-get";
+          args[1] = "/test/mkdirs";
+          args[2] = TEST_ROOT_DIR;
+          int val = -1;
+          try {
+            val = shell.run(args);
+            } catch (Exception e) {
+            System.err.println("Exception raised from DFSShell.run " +
+                               e.getLocalizedMessage()); 
+          }
+          assertTrue(val == 0);
+          assertTrue("Copying failed.", testFile.exists());
+          assertTrue("Checksum file " + checksumFile+" is copied.", !checksumFile.exists());
+          testFile.delete();
+        }
+        {
+          File testFile = new File(TEST_ROOT_DIR, "mkdirs/myFile");
+          File checksumFile = new File(fileSys.getChecksumFile(
+              new Path(testFile.getAbsolutePath())).toString());
+          testFile.delete();
+          checksumFile.delete();
+          new File(TEST_ROOT_DIR, "mkdirs").delete();
+          
+          String[] args = new String[4];
+          args[0] = "-get";
+          args[1] = "-crc";
+          args[2] = "/test/mkdirs";
+          args[3] = TEST_ROOT_DIR;
+          int val = -1;
+          try {
+            val = shell.run(args);
+            } catch (Exception e) {
+            System.err.println("Exception raised from DFSShell.run " +
+                               e.getLocalizedMessage()); 
+          }
+          assertTrue(val == 0);
+          
+          assertTrue("Copying data file failed.", testFile.exists());
+          assertTrue("Checksum file " + checksumFile+" not copied.", checksumFile.exists());
+          testFile.delete();
+          checksumFile.delete();
+        }
         // Verify that we get an error while trying to read an nonexistent file
         {
           String[] args = new String[2];