HADOOP-13737. Cleanup DiskChecker interface. Contributed by Arpit Agarwal.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java

@@ -20,9 +20,6 @@ package org.apache.hadoop.util;
 
 import java.io.File;
 import java.io.IOException;
-import java.nio.file.DirectoryStream;
-import java.nio.file.DirectoryIteratorException;
-import java.nio.file.Files;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
@@ -53,62 +50,6 @@ public class DiskChecker {
     }
   }
       
-  /** 
-   * The semantics of mkdirsWithExistsCheck method is different from the mkdirs
-   * method provided in the Sun's java.io.File class in the following way:
-   * While creating the non-existent parent directories, this method checks for
-   * the existence of those directories if the mkdir fails at any point (since
-   * that directory might have just been created by some other process).
-   * If both mkdir() and the exists() check fails for any seemingly 
-   * non-existent directory, then we signal an error; Sun's mkdir would signal
-   * an error (return false) if a directory it is attempting to create already
-   * exists or the mkdir fails.
-   * @param dir
-   * @return true on success, false on failure
-   */
-  public static boolean mkdirsWithExistsCheck(File dir) {
-    if (dir.mkdir() || dir.exists()) {
-      return true;
-    }
-    File canonDir = null;
-    try {
-      canonDir = dir.getCanonicalFile();
-    } catch (IOException e) {
-      return false;
-    }
-    String parent = canonDir.getParent();
-    return (parent != null) && 
-           (mkdirsWithExistsCheck(new File(parent)) &&
-                                      (canonDir.mkdir() || canonDir.exists()));
-  }
-
-  /**
-   * Recurse down a directory tree, checking all child directories.
-   * @param dir
-   * @throws DiskErrorException
-   */
-  public static void checkDirs(File dir) throws DiskErrorException {
-    checkDir(dir);
-    IOException ex = null;
-    try (DirectoryStream<java.nio.file.Path> stream =
-        Files.newDirectoryStream(dir.toPath())) {
-      for (java.nio.file.Path entry: stream) {
-        File child = entry.toFile();
-        if (child.isDirectory()) {
-          checkDirs(child);
-        }
-      }
-    } catch (DirectoryIteratorException de) {
-      ex = de.getCause();
-    } catch (IOException ie) {
-      ex = ie;
-    }
-    if (ex != null) {
-      throw new DiskErrorException("I/O error when open a directory: "
-          + dir.toString(), ex);
-    }
-  }
-
   /**
    * Create the directory if it doesn't exist and check that dir is readable,
    * writable and executable
@@ -121,39 +62,7 @@ public class DiskChecker {
       throw new DiskErrorException("Cannot create directory: "
                                    + dir.toString());
     }
-    checkDirAccess(dir);
-  }
-
-  /**
-   * Create the directory or check permissions if it already exists.
-   *
-   * The semantics of mkdirsWithExistsAndPermissionCheck method is different
-   * from the mkdirs method provided in the Sun's java.io.File class in the
-   * following way:
-   * While creating the non-existent parent directories, this method checks for
-   * the existence of those directories if the mkdir fails at any point (since
-   * that directory might have just been created by some other process).
-   * If both mkdir() and the exists() check fails for any seemingly
-   * non-existent directory, then we signal an error; Sun's mkdir would signal
-   * an error (return false) if a directory it is attempting to create already
-   * exists or the mkdir fails.
-   *
-   * @param localFS local filesystem
-   * @param dir directory to be created or checked
-   * @param expected expected permission
-   * @throws IOException
-   */
-  public static void mkdirsWithExistsAndPermissionCheck(
-      LocalFileSystem localFS, Path dir, FsPermission expected)
-      throws IOException {
-    File directory = localFS.pathToFile(dir);
-    boolean created = false;
-
-    if (!directory.exists())
-      created = mkdirsWithExistsCheck(directory);
-
-    if (created || !localFS.getFileStatus(dir).getPermission().equals(expected))
-        localFS.setPermission(dir, expected);
+    checkAccessByFileMethods(dir);
   }
 
   /**
@@ -170,24 +79,7 @@ public class DiskChecker {
                               FsPermission expected)
   throws DiskErrorException, IOException {
     mkdirsWithExistsAndPermissionCheck(localFS, dir, expected);
-    checkDirAccess(localFS.pathToFile(dir));
-  }
-
-  /**
-   * Checks that the given file is a directory and that the current running
-   * process can read, write, and execute it.
-   * 
-   * @param dir File to check
-   * @throws DiskErrorException if dir is not a directory, not readable, not
-   *   writable, or not executable
-   */
-  private static void checkDirAccess(File dir) throws DiskErrorException {
-    if (!dir.isDirectory()) {
-      throw new DiskErrorException("Not a directory: "
-                                   + dir.toString());
-    }
-
-    checkAccessByFileMethods(dir);
+    checkAccessByFileMethods(localFS.pathToFile(dir));
   }
 
   /**
@@ -200,6 +92,11 @@ public class DiskChecker {
    */
   private static void checkAccessByFileMethods(File dir)
       throws DiskErrorException {
+    if (!dir.isDirectory()) {
+      throw new DiskErrorException("Not a directory: "
+          + dir.toString());
+    }
+
     if (!FileUtil.canRead(dir)) {
       throw new DiskErrorException("Directory is not readable: "
                                    + dir.toString());
@@ -215,4 +112,65 @@ public class DiskChecker {
                                    + dir.toString());
     }
   }
+
+  /**
+   * The semantics of mkdirsWithExistsCheck method is different from the mkdirs
+   * method provided in the Sun's java.io.File class in the following way:
+   * While creating the non-existent parent directories, this method checks for
+   * the existence of those directories if the mkdir fails at any point (since
+   * that directory might have just been created by some other process).
+   * If both mkdir() and the exists() check fails for any seemingly
+   * non-existent directory, then we signal an error; Sun's mkdir would signal
+   * an error (return false) if a directory it is attempting to create already
+   * exists or the mkdir fails.
+   * @param dir
+   * @return true on success, false on failure
+   */
+  private static boolean mkdirsWithExistsCheck(File dir) {
+    if (dir.mkdir() || dir.exists()) {
+      return true;
+    }
+    File canonDir;
+    try {
+      canonDir = dir.getCanonicalFile();
+    } catch (IOException e) {
+      return false;
+    }
+    String parent = canonDir.getParent();
+    return (parent != null) &&
+        (mkdirsWithExistsCheck(new File(parent)) &&
+            (canonDir.mkdir() || canonDir.exists()));
+  }
+
+  /**
+   * Create the directory or check permissions if it already exists.
+   *
+   * The semantics of mkdirsWithExistsAndPermissionCheck method is different
+   * from the mkdirs method provided in the Sun's java.io.File class in the
+   * following way:
+   * While creating the non-existent parent directories, this method checks for
+   * the existence of those directories if the mkdir fails at any point (since
+   * that directory might have just been created by some other process).
+   * If both mkdir() and the exists() check fails for any seemingly
+   * non-existent directory, then we signal an error; Sun's mkdir would signal
+   * an error (return false) if a directory it is attempting to create already
+   * exists or the mkdir fails.
+   *
+   * @param localFS local filesystem
+   * @param dir directory to be created or checked
+   * @param expected expected permission
+   * @throws IOException
+   */
+  static void mkdirsWithExistsAndPermissionCheck(
+      LocalFileSystem localFS, Path dir, FsPermission expected)
+      throws IOException {
+    File directory = localFS.pathToFile(dir);
+    boolean created = false;
+
+    if (!directory.exists())
+      created = mkdirsWithExistsCheck(directory);
+
+    if (created || !localFS.getFileStatus(dir).getPermission().equals(expected))
+      localFS.setPermission(dir, expected);
+  }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java

@@ -32,7 +32,6 @@ import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.LocalFileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsPermission;
-import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.hadoop.util.DiskChecker.DiskErrorException;
 
 public class TestDiskChecker {
@@ -192,25 +191,4 @@ public class TestDiskChecker {
     System.out.println("checkDir success: " + success);
 
   }
-
-  @Test (timeout = 30000)
-  public void testCheckDirsIOException() throws Throwable {
-    Path path = new Path("target", TestDiskChecker.class.getSimpleName());
-    File localDir = new File(path.toUri().getRawPath());
-    localDir.mkdir();
-    File localFile = new File(localDir, "test");
-    localFile.createNewFile();
-    File spyLocalDir = spy(localDir);
-    doReturn(localFile.toPath()).when(spyLocalDir).toPath();
-    try {
-      DiskChecker.checkDirs(spyLocalDir);
-      fail("Expected exception for I/O error");
-    } catch (DiskErrorException e) {
-      GenericTestUtils.assertExceptionContains("I/O error", e);
-      assertTrue(e.getCause() instanceof IOException);
-    } finally {
-      localFile.delete();
-      localDir.delete();
-    }
-  }
 }