HADOOP-13737. Cleanup DiskChecker interface. Contributed by Arpit Agarwal.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/262827cf Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/262827cf Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/262827cf Branch: refs/heads/HDFS-7240 Commit: 262827cf75bf9c48cd95335eb04fd8ff1d64c538 Parents: 5e83a21 Author: Anu Engineer <aengin...@apache.org> Authored: Thu Oct 20 13:26:23 2016 -0700 Committer: Anu Engineer <aengin...@apache.org> Committed: Thu Oct 20 13:35:26 2016 -0700 ---------------------------------------------------------------------- .../org/apache/hadoop/util/DiskChecker.java | 178 +++++++------------ .../org/apache/hadoop/util/TestDiskChecker.java | 22 --- 2 files changed, 68 insertions(+), 132 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/262827cf/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java index a36a7a0..2c73af8 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java +++ b/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); + } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/262827cf/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java index 0d55d7e..e2e152a 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java +++ b/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(); - } - } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org