Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-12056

Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.7.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: util
    • Labels:
      None

      Description

      DiskChecker#checkDirs should check null pointer for the return value from File#listFiles. Based on the document for File#listFiles at https://docs.oracle.com/javase/7/docs/api/java/io/File.html#listFiles() :

      Returns null if an I/O error occurs.
      

      So it will be good to check null pointer and throw DiskErrorException if it is null.

      1. HADOOP-12056.000.patch
        2 kB
        zhihai xu
      2. HADOOP-12056.001.patch
        2 kB
        zhihai xu
      3. HADOOP-12056.002.patch
        3 kB
        zhihai xu
      4. HADOOP-12056.003.patch
        3 kB
        zhihai xu

        Activity

        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2166 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2166/)
        HADOOP-12056. Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb)

        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2166 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2166/ ) HADOOP-12056 . Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #218 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/218/)
        HADOOP-12056. Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb)

        • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #218 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/218/ ) HADOOP-12056 . Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #209 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/209/)
        HADOOP-12056. Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #209 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/209/ ) HADOOP-12056 . Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2148 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2148/)
        HADOOP-12056. Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2148 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2148/ ) HADOOP-12056 . Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #950 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/950/)
        HADOOP-12056. Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb)

        • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #950 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/950/ ) HADOOP-12056 . Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #220 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/220/)
        HADOOP-12056. Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb)

        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #220 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/220/ ) HADOOP-12056 . Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        Hide
        zxu zhihai xu added a comment -

        thanks Lei (Eddy) Xu for the review, thanks Andrew Wang for the review and committing the patch.

        Show
        zxu zhihai xu added a comment - thanks Lei (Eddy) Xu for the review, thanks Andrew Wang for the review and committing the patch.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-trunk-Commit #7979 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7979/)
        HADOOP-12056. Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb)

        • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #7979 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7979/ ) HADOOP-12056 . Use DirectoryStream in DiskChecker#checkDirs to detect errors when listing a directory. Contributed by Zhihai Xu. (wang: rev bc11e158b1726174fae2c7d2e8aa1f5005bf42eb) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestDiskChecker.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DiskChecker.java
        Hide
        andrew.wang Andrew Wang added a comment -

        Committed to trunk and branch-2. Thanks for the patch Zhihai, and Eddy for also reviewing.

        Show
        andrew.wang Andrew Wang added a comment - Committed to trunk and branch-2. Thanks for the patch Zhihai, and Eddy for also reviewing.
        Hide
        eddyxu Lei (Eddy) Xu added a comment -

        Hi, zhihai xu Thanks much for updating the patch. +1 (non-binding).

        Show
        eddyxu Lei (Eddy) Xu added a comment - Hi, zhihai xu Thanks much for updating the patch. +1 (non-binding).
        Hide
        andrew.wang Andrew Wang added a comment -

        +1 LGTM, thanks Zhihai. Will commit shortly.

        Show
        andrew.wang Andrew Wang added a comment - +1 LGTM, thanks Zhihai. Will commit shortly.
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 16m 20s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 7m 34s There were no new javac warning messages.
        +1 javadoc 9m 40s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 1m 8s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 35s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 1m 50s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 common tests 22m 33s Tests passed in hadoop-common.
            61m 38s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12737995/HADOOP-12056.003.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 7588585
        hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6930/artifact/patchprocess/testrun_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6930/testReport/
        Java 1.7.0_55
        uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6930/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 16m 20s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 34s There were no new javac warning messages. +1 javadoc 9m 40s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 8s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 35s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 50s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 common tests 22m 33s Tests passed in hadoop-common.     61m 38s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12737995/HADOOP-12056.003.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 7588585 hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6930/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6930/testReport/ Java 1.7.0_55 uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6930/console This message was automatically generated.
        Hide
        zxu zhihai xu added a comment -

        Hi Andrew Wang, I attached a new patch HADOOP-12056.003.patch, which use DirectoryStream instead of listFiles. Please review it. Many thanks.

        Show
        zxu zhihai xu added a comment - Hi Andrew Wang , I attached a new patch HADOOP-12056 .003.patch, which use DirectoryStream instead of listFiles . Please review it. Many thanks.
        Hide
        zxu zhihai xu added a comment -

        Hi Andrew Wang, that is good information, I thought no one use DirectoryStream in hadoop code base yet. Yes, IOUtils used it already. I will use DirectoryStream to fix this issue, which will give more detail message from the IOException as Lei (Eddy) Xu suggested in previous comment.

        Show
        zxu zhihai xu added a comment - Hi Andrew Wang , that is good information, I thought no one use DirectoryStream in hadoop code base yet. Yes, IOUtils used it already. I will use DirectoryStream to fix this issue, which will give more detail message from the IOException as Lei (Eddy) Xu suggested in previous comment.
        Hide
        andrew.wang Andrew Wang added a comment -

        Hmm, what's the potential issue with using DirectoryStream? I see we already use it in IOUtils, and it seems to have a Windows implementation.

        Show
        andrew.wang Andrew Wang added a comment - Hmm, what's the potential issue with using DirectoryStream? I see we already use it in IOUtils, and it seems to have a Windows implementation.
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 16m 29s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 7m 34s There were no new javac warning messages.
        +1 javadoc 9m 38s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 1m 4s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 33s mvn install still works.
        +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
        +1 findbugs 1m 52s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 common tests 23m 15s Tests passed in hadoop-common.
            62m 25s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12737707/HADOOP-12056.002.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 18dd01d
        hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6927/artifact/patchprocess/testrun_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6927/testReport/
        Java 1.7.0_55
        uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6927/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 16m 29s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 34s There were no new javac warning messages. +1 javadoc 9m 38s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 4s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 33s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. +1 findbugs 1m 52s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 common tests 23m 15s Tests passed in hadoop-common.     62m 25s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12737707/HADOOP-12056.002.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 18dd01d hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6927/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6927/testReport/ Java 1.7.0_55 uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6927/console This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 16m 18s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 7m 33s There were no new javac warning messages.
        +1 javadoc 9m 37s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 1m 6s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 34s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 1m 51s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 common tests 23m 30s Tests passed in hadoop-common.
            62m 28s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12737704/HADOOP-12056.002.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 18dd01d
        hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6926/artifact/patchprocess/testrun_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6926/testReport/
        Java 1.7.0_55
        uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6926/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 16m 18s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 33s There were no new javac warning messages. +1 javadoc 9m 37s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 6s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 34s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 51s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 common tests 23m 30s Tests passed in hadoop-common.     62m 28s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12737704/HADOOP-12056.002.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 18dd01d hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6926/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6926/testReport/ Java 1.7.0_55 uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6926/console This message was automatically generated.
        Hide
        zxu zhihai xu added a comment -

        Discuss with Lei (Eddy) Xu offline, DirectoryStream need java.nio.file.Files support, which may have compatibility issue at other platforms.
        We agreed to add a log for this IO error and rename testCheckDirs to testCheckDirsFailedWhenListDirs. thanks Lei (Eddy) Xu for the thorough review. I uploaded a new patch HADOOP-12056.002.patch which addressed these issues. Please review it.

        Show
        zxu zhihai xu added a comment - Discuss with Lei (Eddy) Xu offline, DirectoryStream need java.nio.file.Files support, which may have compatibility issue at other platforms. We agreed to add a log for this IO error and rename testCheckDirs to testCheckDirsFailedWhenListDirs. thanks Lei (Eddy) Xu for the thorough review. I uploaded a new patch HADOOP-12056 .002.patch which addressed these issues. Please review it.
        Hide
        eddyxu Lei (Eddy) Xu added a comment -

        Hi, zhihai xu Thanks for updating the patch.

        I tried, but it is failed with "Not a directory" exception at DiskChecker#checkDirAccess

        What would be the real problem to fix in this patch? Is the Null due to the input is a file or an IO error?

        If the later is the case, would it be OK to use DirectoryStream instead of listFiles(), as it can report IOException. Also we should test against this case.

        Show
        eddyxu Lei (Eddy) Xu added a comment - Hi, zhihai xu Thanks for updating the patch. I tried, but it is failed with "Not a directory" exception at DiskChecker#checkDirAccess What would be the real problem to fix in this patch? Is the Null due to the input is a file or an IO error? If the later is the case, would it be OK to use DirectoryStream instead of listFiles() , as it can report IOException . Also we should test against this case.
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 16m 58s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 7m 42s There were no new javac warning messages.
        +1 javadoc 9m 48s There were no new javadoc warning messages.
        +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 1m 5s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 37s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 1m 49s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 common tests 23m 14s Tests passed in hadoop-common.
            63m 14s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12737409/HADOOP-12056.001.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / bc85959
        hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6910/artifact/patchprocess/testrun_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6910/testReport/
        Java 1.7.0_55
        uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6910/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 16m 58s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 42s There were no new javac warning messages. +1 javadoc 9m 48s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 5s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 37s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 49s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 common tests 23m 14s Tests passed in hadoop-common.     63m 14s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12737409/HADOOP-12056.001.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / bc85959 hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6910/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6910/testReport/ Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6910/console This message was automatically generated.
        Hide
        zxu zhihai xu added a comment -

        Hi Lei (Eddy) Xu, thanks for the review.
        It looks like PathUtils is located at HDFS project, which is not accessible by TestDiskChecker.
        So instead of using PathUtils, I use directory "target/TestDiskChecker" new Path("target", TestDiskChecker.class.getSimpleName())

        Why not pass a file directly?

        I tried, but it is failed with "Not a directory" exception at DiskChecker#checkDirAccess

            if (!dir.isDirectory()) {
              throw new DiskErrorException("Not a directory: "
                                           + dir.toString());
            }
        

        I uploaded a new patch HADOOP-12056.001.patch which addressed all the other comments. Please review it.

        Show
        zxu zhihai xu added a comment - Hi Lei (Eddy) Xu , thanks for the review. It looks like PathUtils is located at HDFS project, which is not accessible by TestDiskChecker. So instead of using PathUtils, I use directory "target/TestDiskChecker" new Path("target", TestDiskChecker.class.getSimpleName()) Why not pass a file directly? I tried, but it is failed with "Not a directory" exception at DiskChecker#checkDirAccess if (!dir.isDirectory()) { throw new DiskErrorException( "Not a directory: " + dir.toString()); } I uploaded a new patch HADOOP-12056 .001.patch which addressed all the other comments. Please review it.
        Hide
        eddyxu Lei (Eddy) Xu added a comment -

        Hi, zhihai xu Nice find.

        This patch looks good in general. It'd be nice to address a few small comments, mostly regarding the unit tests.

        • The test name should be testCheckDirsOnFile().
        • Can you create the test file under test dirs, which can be obtained from PathUtils#getTestDir():
           File localDir = File.createTempFile("test", "tmp");
          
        • I have a question in the following code:
          File spyLocalDir = spy(localDir);
          doReturn(null).when(spyLocalDir).listFiles();
          

        Why not pass a file directly? Would the case, which the input is a file, be handled by checkDir(dir) directly? I think you can pass a regular file to see what happens.

        • We can use GenericTestUtils#assertExceptionContains to verify the DiskErrorException is correct.

        +1 non-binding once these are addressed. Thanks again for working on this.

        Show
        eddyxu Lei (Eddy) Xu added a comment - Hi, zhihai xu Nice find. This patch looks good in general. It'd be nice to address a few small comments, mostly regarding the unit tests. The test name should be testCheckDirsOnFile() . Can you create the test file under test dirs, which can be obtained from PathUtils#getTestDir() : File localDir = File.createTempFile( "test" , "tmp" ); I have a question in the following code: File spyLocalDir = spy(localDir); doReturn( null ).when(spyLocalDir).listFiles(); Why not pass a file directly? Would the case, which the input is a file, be handled by checkDir(dir) directly? I think you can pass a regular file to see what happens. We can use GenericTestUtils#assertExceptionContains to verify the DiskErrorException is correct. +1 non-binding once these are addressed. Thanks again for working on this.
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 16m 13s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 7m 29s There were no new javac warning messages.
        +1 javadoc 9m 37s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 1m 3s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 32s mvn install still works.
        +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse.
        +1 findbugs 1m 50s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 common tests 22m 46s Tests passed in hadoop-common.
            61m 30s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12737026/HADOOP-12056.000.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 03fb5c6
        hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6896/artifact/patchprocess/testrun_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6896/testReport/
        Java 1.7.0_55
        uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6896/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 16m 13s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 29s There were no new javac warning messages. +1 javadoc 9m 37s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 3s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 32s mvn install still works. +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse. +1 findbugs 1m 50s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 common tests 22m 46s Tests passed in hadoop-common.     61m 30s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12737026/HADOOP-12056.000.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 03fb5c6 hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6896/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6896/testReport/ Java 1.7.0_55 uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6896/console This message was automatically generated.

          People

          • Assignee:
            zxu zhihai xu
            Reporter:
            zxu zhihai xu
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development