HADOOP-1620. Reduce the number of abstract FileSystem methods, simplifying implementations.

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

CHANGES.txt

@@ -373,6 +373,9 @@ Trunk (unreleased changes)
 116. HADOOP-1564.  Add unit tests for HDFS block-level checksums.
      (Dhruba Borthakur via cutting)
 
+117. HADOOP-1620.  Reduce the number of abstract FileSystem methods,
+     simplifying implementations.  (cutting)
+
 
 Release 0.13.0 - 2007-06-08
 

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

@@ -198,31 +198,6 @@ public class DistributedFileSystem extends FileSystem {
     dfs.release(getPath(f));
   }
 
-  @Override
-  public void copyFromLocalFile(boolean delSrc, Path src, Path dst)
-  throws IOException {
-    FileUtil.copy(localFs, src, this, dst, delSrc, getConf());
-  }
-
-  @Override
-  public void copyToLocalFile(boolean delSrc, Path src, Path dst)
-  throws IOException {
-    FileUtil.copy(this, src, localFs, dst, delSrc, getConf());
-  }
-
-  public Path startLocalOutput(Path fsOutputFile, Path tmpLocalFile)
-  throws IOException {
-    return tmpLocalFile;
-  }
-
-  /**
-   * Move completed local data to DFS destination
-   */
-  public void completeLocalOutput(Path fsOutputFile, Path tmpLocalFile)
-  throws IOException {
-    moveFromLocalFile(tmpLocalFile, fsOutputFile);
-  }
-
   public void close() throws IOException {
     super.close();
     dfs.close();

src/java/org/apache/hadoop/filecache/DistributedCache.java

@@ -168,7 +168,7 @@ public class DistributedCache {
                                     Configuration conf, boolean isArchive, String md5, Path currentWorkDir) throws IOException {
     boolean b = true;
     boolean doSymlink = getSymlink(conf);
-    FileSystem dfs = getFileSystem(cache, conf);
+    FileSystem dfs = FileSystem.get(cache, conf);
     b = ifExistsAndFresh(cacheStatus, cache, dfs, md5, conf);
     String link = currentWorkDir.toString() + Path.SEPARATOR + cache.getFragment();
     File flink = new File(link);
@@ -290,7 +290,7 @@ public class DistributedCache {
     byte[] b = new byte[CRC_BUFFER_SIZE];
     byte[] digest = null;
 
-    FileSystem fileSystem = getFileSystem(cache, conf);
+    FileSystem fileSystem = FileSystem.get(cache, conf);
     if (!(fileSystem instanceof ChecksumFileSystem)) {
       throw new IOException("Not a checksummed file system: "
                             +fileSystem.getUri());
@@ -358,26 +358,6 @@ public class DistributedCache {
       }
     }  
   }
-  
-  private static String getFileSysName(URI url) {
-    String fsname = url.getScheme();
-    if ("hdfs".equals(fsname)) {
-      String host = url.getHost();
-      int port = url.getPort();
-      return (port == (-1)) ? host : (host + ":" + port);
-    } else {
-      return null;
-    }
-  }
-
-  private static FileSystem getFileSystem(URI cache, Configuration conf)
-    throws IOException {
-    String fileSysName = getFileSysName(cache);
-    if (fileSysName != null)
-      return FileSystem.getNamed(fileSysName, conf);
-    else
-      return FileSystem.get(conf);
-  }
 
   /**
    * Set the configuration with the given set of archives

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

@@ -87,7 +87,7 @@ public abstract class FileSystem extends Configured {
 
   /** Returns the configured filesystem implementation.*/
   public static FileSystem get(Configuration conf) throws IOException {
-    return getNamed(conf.get("fs.default.name", "local"), conf);
+    return getNamed(conf.get("fs.default.name", "file:///"), conf);
   }
 
   /** Called after a new FileSystem instance is constructed.
@@ -102,7 +102,7 @@ public abstract class FileSystem extends Configured {
   public abstract URI getUri();
   
   /** @deprecated call #getUri() instead.*/
-  public abstract String getName();
+  public String getName() { return getUri().toString(); }
 
   /** @deprecated call #get(URI,Configuration) instead. */
   public static FileSystem getNamed(String name, Configuration conf)
@@ -216,7 +216,17 @@ public abstract class FileSystem extends Configured {
    *
    * The FileSystem will simply return an elt containing 'localhost'.
    */
-  public abstract String[][] getFileCacheHints(Path f, long start, long len) throws IOException;
+  public String[][] getFileCacheHints(Path f, long start, long len)
+    throws IOException {
+    if (!exists(f)) {
+      return null;
+    } else {
+      String result[][] = new String[1][];
+      result[0] = new String[1];
+      result[0][0] = "localhost";
+      return result;
+    }
+  }
 
   /**
    * Opens an FSDataInputStream at the indicated Path.
@@ -387,7 +397,10 @@ public abstract class FileSystem extends Configured {
    * @return true if successful;
    *         false if file does not exist or is a directory
    */
-  public abstract boolean setReplication(Path src, short replication) throws IOException;
+  public boolean setReplication(Path src, short replication)
+    throws IOException {
+    return true;
+  }
 
   /**
    * Renames Path src to Path dst.  Can take place on local fs
@@ -725,7 +738,7 @@ public abstract class FileSystem extends Configured {
    * @deprecated FS does not support file locks anymore.
    */
   @Deprecated
-  public abstract void lock(Path f, boolean shared) throws IOException;
+  public void lock(Path f, boolean shared) throws IOException {}
 
   /**
    * Release the lock
@@ -733,7 +746,7 @@ public abstract class FileSystem extends Configured {
    * @deprecated FS does not support file locks anymore.     
    */
   @Deprecated
-  public abstract void release(Path f) throws IOException;
+  public void release(Path f) throws IOException {}
 
   /**
    * The src file is on the local disk.  Add it to FS at
@@ -758,8 +771,10 @@ public abstract class FileSystem extends Configured {
    * the given dst name.
    * delSrc indicates if the source should be removed
    */
-  public abstract void copyFromLocalFile(boolean delSrc, Path src, Path dst)
-    throws IOException;
+  public void copyFromLocalFile(boolean delSrc, Path src, Path dst)
+    throws IOException {
+    FileUtil.copy(getLocal(getConf()), src, this, dst, delSrc, getConf());
+  }
     
   /**
    * The src file is under FS, and the dst is on the local disk.
@@ -783,8 +798,10 @@ public abstract class FileSystem extends Configured {
    * Copy it from FS control to the local dst name.
    * delSrc indicates if the src will be removed or not.
    */   
-  public abstract void copyToLocalFile(boolean delSrc, Path src, Path dst)
-    throws IOException;
+  public void copyToLocalFile(boolean delSrc, Path src, Path dst)
+    throws IOException {
+    FileUtil.copy(this, src, getLocal(getConf()), dst, delSrc, getConf());
+  }
 
   /**
    * Returns a local File that the user can write output to.  The caller
@@ -792,7 +809,10 @@ public abstract class FileSystem extends Configured {
    * file.  If the FS is local, we write directly into the target.  If
    * the FS is remote, we write into the tmp local area.
    */
-  public abstract Path startLocalOutput(Path fsOutputFile, Path tmpLocalFile) throws IOException;
+  public Path startLocalOutput(Path fsOutputFile, Path tmpLocalFile)
+    throws IOException {
+    return tmpLocalFile;
+  }
 
   /**
    * Called when we're all done writing to the target.  A local FS will
@@ -800,7 +820,10 @@ public abstract class FileSystem extends Configured {
    * FS will copy the contents of tmpLocalFile to the correct target at
    * fsOutputFile.
    */
-  public abstract void completeLocalOutput(Path fsOutputFile, Path tmpLocalFile) throws IOException;
+  public void completeLocalOutput(Path fsOutputFile, Path tmpLocalFile)
+    throws IOException {
+    moveFromLocalFile(tmpLocalFile, fsOutputFile);
+  }
 
   /**
    * No more filesystem operations are needed.  Will
@@ -846,7 +869,7 @@ public abstract class FileSystem extends Configured {
   /**
    * Get the default replication.
    */
-  public abstract short getDefaultReplication();
+  public short getDefaultReplication() { return 1; }
 
   /* 
    * Return a file status object that represents the

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

@@ -80,11 +80,6 @@ public class InMemoryFileSystem extends ChecksumFileSystem {
       return uri;
     }
 
-    /** @deprecated */
-    public String getName() {
-      return uri.toString();
-    }
-
     /**
      * Return 1x1 'inmemory' cell if the file exists.
      * Return null if otherwise.
@@ -229,13 +224,6 @@ public class InMemoryFileSystem extends ChecksumFileSystem {
       }
     }
 
-    /**
-     * Replication is not supported for the inmemory file system.
-     */
-    public short getDefaultReplication() {
-      return 1;
-    }
-
     public boolean setReplication(Path src, short replication)
       throws IOException {
       return true;
@@ -290,32 +278,6 @@ public class InMemoryFileSystem extends ChecksumFileSystem {
       return true;
     }
   
-    /** lock operations are not supported */
-    public void lock(Path f, boolean shared) throws IOException {}
-    public void release(Path f) throws IOException {}
-  
-    /** copy/move operations are not supported */
-    public void copyFromLocalFile(boolean delSrc, Path src, Path dst)
-      throws IOException {
-    }
-
-    public void copyToLocalFile(boolean delSrc, Path src, Path dst)
-      throws IOException {
-    }
-
-    public Path startLocalOutput(Path fsOutputFile, Path tmpLocalFile)
-      throws IOException {
-      return fsOutputFile;
-    }
-
-    public void completeLocalOutput(Path fsOutputFile, Path tmpLocalFile)
-      throws IOException {
-    }
-
-    public long getDefaultBlockSize() {
-      return 32 * 1024; //some random large number. can be anything actually
-    }
-
     public FileStatus getFileStatus(Path f) throws IOException {
       synchronized (this) {
         return new InMemoryFileStatus(pathToFileAttribs.get(getPath(f)));
@@ -443,21 +405,6 @@ public class InMemoryFileSystem extends ChecksumFileSystem {
     super(new RawInMemoryFileSystem(uri, conf));
   }
     
-  /** copy/move operations are not supported */
-  public void copyFromLocalFile(boolean delSrc, Path src, Path dst)
-    throws IOException {}
-  public void copyToLocalFile(boolean delSrc, Path src, Path dst)
-    throws IOException {}
-    
-  public Path startLocalOutput(Path fsOutputFile, Path tmpLocalFile)
-    throws IOException {
-    return fsOutputFile;
-  }
-    
-  public void completeLocalOutput(Path fsOutputFile, Path tmpLocalFile)
-    throws IOException {
-  }
-    
   /**
    * Register a file with its size. This will also register a checksum for the
    * file that the user is trying to create. This is required since none of

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

@@ -54,24 +54,9 @@ public class RawLocalFileSystem extends FileSystem {
     return new File(path.toUri().getPath());
   }
 
-  /**
-   * Return 1x1 'localhost' cell if the file exists.
-   * Return null if otherwise.
-   */
-  public String[][] getFileCacheHints(Path f, long start, long len) throws IOException {
-    if (!exists(f)) {
-      return null;
-    } else {
-      String result[][] = new String[1][];
-      result[0] = new String[1];
-      result[0][0] = "localhost";
-      return result;
-    }
-  }
-  
   /** @deprecated */
   public String getName() { return "local"; }
-  
+
   public URI getUri() { return NAME; }
   
   public void initialize(URI uri, Configuration conf) {
@@ -190,16 +175,6 @@ public class RawLocalFileSystem extends FileSystem {
         new BufferedOutputStream(new LocalFSFileOutputStream(f), bufferSize));
   }
   
-  /**
-   * Replication is not supported for the local file system.
-   */
-  /** Set the replication of the given file */
-  public boolean setReplication(Path src,
-                                short replication
-                                ) throws IOException {
-    return true;
-  }
-  
   public boolean rename(Path src, Path dst) throws IOException {
     if (useCopyForRename) {
       return FileUtil.copy(this, src, this, dst, true, getConf());
@@ -321,18 +296,6 @@ public class RawLocalFileSystem extends FileSystem {
     rename(src, dst);
   }
   
-  @Override
-  public void copyFromLocalFile(boolean delSrc, Path src, Path dst)
-    throws IOException {
-    FileUtil.copy(this, src, this, dst, delSrc, getConf());
-  }
-  
-  @Override
-  public void copyToLocalFile(boolean delSrc, Path src, Path dst)
-    throws IOException {
-    FileUtil.copy(this, src, this, dst, delSrc, getConf());
-  }
-  
   // We can write output directly to the final location
   public Path startLocalOutput(Path fsOutputFile, Path tmpLocalFile)
     throws IOException {
@@ -356,10 +319,6 @@ public class RawLocalFileSystem extends FileSystem {
     return new RawLocalFileStatus(pathToFile(f));
   }
 
-  public short getDefaultReplication() {
-    return 1;
-  }
-  
   private class RawLocalFileStatus implements FileStatus {
     private long length;
     private boolean isDir;

src/java/org/apache/hadoop/fs/s3/S3FileSystem.java

@@ -279,15 +279,6 @@ public class S3FileSystem extends FileSystem {
     return true;
   }
 
-  /**
-   * Replication is not supported for S3 file systems since S3 handles it for
-   * us.
-   */
-  @Override
-  public short getDefaultReplication() {
-    return 1;
-  }
-
   /**
    * FileStatus for S3 file systems. 
    */
@@ -300,68 +291,6 @@ public class S3FileSystem extends FileSystem {
     return new S3FileStatus(inode);
   }
 
-  /**
-   * Replication is not supported for S3 file systems since S3 handles it for
-   * us.
-   */
-  @Override
-  public boolean setReplication(Path path, short replication)
-    throws IOException {
-    return true;
-  }
-
-  @Override
-  public long getDefaultBlockSize() {
-    return getConf().getLong("fs.s3.block.size", DEFAULT_BLOCK_SIZE);
-  }
-
-  /**
-   * Return 1x1 'localhost' cell if the file exists. Return null if otherwise.
-   */
-  @Override
-  public String[][] getFileCacheHints(Path f, long start, long len)
-    throws IOException {
-    // TODO: Check this is the correct behavior
-    if (!exists(f)) {
-      return null;
-    }
-    return new String[][] { { "localhost" } };
-  }
-
-  /** @deprecated */ @Deprecated
-    @Override
-    public void lock(Path path, boolean shared) throws IOException {
-    // TODO: Design and implement
-  }
-
-  /** @deprecated */ @Deprecated
-    @Override
-    public void release(Path path) throws IOException {
-    // TODO: Design and implement
-  }
-
-  @Override
-  public void copyFromLocalFile(boolean delSrc, Path src, Path dst) throws IOException {
-    FileUtil.copy(localFs, src, this, dst, delSrc, getConf());
-  }
-
-  @Override
-  public void copyToLocalFile(boolean delSrc, Path src, Path dst) throws IOException {
-    FileUtil.copy(this, src, localFs, dst, delSrc, getConf());
-  }
-
-  @Override
-  public Path startLocalOutput(Path fsOutputFile, Path tmpLocalFile)
-    throws IOException {
-    return tmpLocalFile;
-  }
-
-  @Override
-  public void completeLocalOutput(Path fsOutputFile, Path tmpLocalFile)
-    throws IOException {
-    moveFromLocalFile(tmpLocalFile, fsOutputFile);
-  }
-
   // diagnostic methods
 
   void dump() throws IOException {

src/test/org/apache/hadoop/mapred/TestMiniMRLocalFS.java

@@ -46,7 +46,7 @@ public class TestMiniMRLocalFS extends TestCase {
   public void testWithLocal() throws IOException {
     MiniMRCluster mr = null;
     try {
-      mr = new MiniMRCluster(2, "local", 3);
+      mr = new MiniMRCluster(2, "file:///", 3);
       double estimate = PiEstimator.launch(NUM_MAPS, NUM_SAMPLES, 
                                            mr.createJobConf());
       double error = Math.abs(Math.PI - estimate);