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

FileSystem#listStatus should throw IOE upon access error

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.6.0
    • Fix Version/s: 3.0.0-alpha2
    • Component/s: fs
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Change FileSystem#listStatus contract to never return null. Local filesystems prior to 3.0.0 returned null upon access error. It is considered erroneous. We should expect FileSystem#listStatus to throw IOException upon access error.

      Description

      In HADOOP-6201 and HDFS-538 it was agreed that FileSystem::listStatus should throw FileNotFoundException instead of returning null, when the target directory did not exist.

      However, in LocalFileSystem implementation today, FileSystem::listStatus still may return null, when the target directory exists but does not grant read permission. This causes NPE in many callers, for all the reasons cited in HADOOP-6201 and HDFS-538. See HADOOP-7327 and its linked issues for examples.

      1. HADOOP-7352.001.patch
        8 kB
        John Zhuge
      2. HADOOP-7352.002.patch
        10 kB
        John Zhuge
      3. HADOOP-7352.003.patch
        10 kB
        John Zhuge
      4. HADOOP-7352.004.patch
        10 kB
        John Zhuge
      5. HADOOP-7352.005.patch
        10 kB
        John Zhuge

        Issue Links

          Activity

          Hide
          mattf Matt Foley added a comment -

          This behavior, of returning null on access error, is following the pattern of File::list and File::listFiles, which are defined by the JDK. However, those cause NPE in a lot of Hadoop code too, which is why they've recently been fixed by HADOOP-7342, HADOOP-7322, HDFS-1934, and HDFS-2019. Those patches provided FileUtil alternatives to the JDK methods. That approach isn't applicable to FileSystem::listStatus because it is an abstract method in an important contract class defined by Hadoop. It has a massive number of callers in Common and Mapred (although I found none in HDFS).

          I believe this change in semantics, to throw IOException instead of returning null, would have no negative impact. I've examined the 36 non-trivial callers in Common, and only 2 checked for null result. All the others would definitely get NPE on null result, and the two that did check had faulty logic! In Mapred there are far more callers, but almost all of them also will throw NPE on null result. In going through about half of them, I found one correct and one incorrect null check in about 20 callers.

          Therefore, I believe there is no downside to changing the semantics of the base method in this way, and we'll get rid of some mystery NPEs. We will need to scan for any callers that check for null and use it as a non-error condition; there won't be many but there will be a couple.

          Please give feedback. If this is acceptable I will provide a patch for each of the underlying FileSystem.listStatus implementations, and do the careful scan for callers that actually expect null as an allowed result.

          Show
          mattf Matt Foley added a comment - This behavior, of returning null on access error, is following the pattern of File::list and File::listFiles, which are defined by the JDK. However, those cause NPE in a lot of Hadoop code too, which is why they've recently been fixed by HADOOP-7342 , HADOOP-7322 , HDFS-1934 , and HDFS-2019 . Those patches provided FileUtil alternatives to the JDK methods. That approach isn't applicable to FileSystem::listStatus because it is an abstract method in an important contract class defined by Hadoop. It has a massive number of callers in Common and Mapred (although I found none in HDFS). I believe this change in semantics, to throw IOException instead of returning null, would have no negative impact. I've examined the 36 non-trivial callers in Common, and only 2 checked for null result. All the others would definitely get NPE on null result, and the two that did check had faulty logic! In Mapred there are far more callers, but almost all of them also will throw NPE on null result. In going through about half of them, I found one correct and one incorrect null check in about 20 callers. Therefore, I believe there is no downside to changing the semantics of the base method in this way, and we'll get rid of some mystery NPEs. We will need to scan for any callers that check for null and use it as a non-error condition; there won't be many but there will be a couple. Please give feedback. If this is acceptable I will provide a patch for each of the underlying FileSystem.listStatus implementations, and do the careful scan for callers that actually expect null as an allowed result.
          Hide
          mattf Matt Foley added a comment -

          Correction: there were calls to FileSystem::listStatus in HDFS, but they were all in Test classes, except for two pass-through calls in HftpFileSystem.listStatus() and HftpFileSystem.LsParser.listStatus().

          Show
          mattf Matt Foley added a comment - Correction: there were calls to FileSystem::listStatus in HDFS, but they were all in Test classes, except for two pass-through calls in HftpFileSystem.listStatus() and HftpFileSystem.LsParser.listStatus().
          Hide
          mattf Matt Foley added a comment -

          In a comment in another bug (<a href="https://issues.apache.org/jira/browse/HADOOP-7327?focusedCommentId=13042494&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13042494">here</a>), Daryn points out that if the exceptions are thrown from a lower level with knowledge of the cause of the problem, it would be good to throw something informative such as AccessControlException, rather than just a generic IOException.

          I agree in principle, but AccessControlException is a subclass of SecurityException, which is a RuntimeException and therefore no better than NPE. All of the callers of listStatus() are prepared to get an IOException, but none of the standard sub-classes of IOException include access failures. Is there a Hadoop or Apache defined sub-class of IOException that would be appropriate?

          Show
          mattf Matt Foley added a comment - In a comment in another bug (<a href="https://issues.apache.org/jira/browse/HADOOP-7327?focusedCommentId=13042494&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13042494">here</a>), Daryn points out that if the exceptions are thrown from a lower level with knowledge of the cause of the problem, it would be good to throw something informative such as AccessControlException, rather than just a generic IOException. I agree in principle, but AccessControlException is a subclass of SecurityException, which is a RuntimeException and therefore no better than NPE. All of the callers of listStatus() are prepared to get an IOException, but none of the standard sub-classes of IOException include access failures. Is there a Hadoop or Apache defined sub-class of IOException that would be appropriate?
          Hide
          mattf Matt Foley added a comment -

          Added "relates to" link for the bugs that fixed "FileSystem::ListStatus should throw FileNotFoundException instead of returning null".

          Show
          mattf Matt Foley added a comment - Added "relates to" link for the bugs that fixed "FileSystem::ListStatus should throw FileNotFoundException instead of returning null".
          Hide
          daryn Daryn Sharp added a comment -

          Hadoop common has a org.apache.hadoop.fs.permission.AccessControlException derives from IOException.

          Show
          daryn Daryn Sharp added a comment - Hadoop common has a org.apache.hadoop.fs.permission.AccessControlException derives from IOException .
          Hide
          jghoman Jakob Homan added a comment -

          It's correct to make the contract consistent. Trying to make IOExceptions useful at this point is probably a lost cause. Otherwise, the changes proposed sound reasonable.

          Show
          jghoman Jakob Homan added a comment - It's correct to make the contract consistent. Trying to make IOExceptions useful at this point is probably a lost cause. Otherwise, the changes proposed sound reasonable.
          Hide
          bharathm Bharath Mundlapudi added a comment -

          Throwing an IOException is reasonable when listStatus returns null. The reason for this null case can be multiple things like file may not be present, permission issue, disk is bad, OS decided to make file system readonly etc etc. Since at FileSystem level we may not know all the causes for this so we should throw an IOException. Do you agree?

          Show
          bharathm Bharath Mundlapudi added a comment - Throwing an IOException is reasonable when listStatus returns null. The reason for this null case can be multiple things like file may not be present, permission issue, disk is bad, OS decided to make file system readonly etc etc. Since at FileSystem level we may not know all the causes for this so we should throw an IOException. Do you agree?
          Hide
          mattf Matt Foley added a comment -

          @Daryn - thanks, I'll use that where appropriate.

          @Bharath - agreed, the exception will always be IOException or one of its subclasses; IOException where there is no detailed info available or no better subclass defined.

          Show
          mattf Matt Foley added a comment - @Daryn - thanks, I'll use that where appropriate. @Bharath - agreed, the exception will always be IOException or one of its subclasses; IOException where there is no detailed info available or no better subclass defined.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I want to revisit this as part of the FS contract spec/changes. Does throwing an IOE still seem the right approach. And is there any test for this yet?

          Show
          stevel@apache.org Steve Loughran added a comment - I want to revisit this as part of the FS contract spec/changes. Does throwing an IOE still seem the right approach. And is there any test for this yet?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          HDFS & the FS spec do effectively require an IOE of some form to be raised here.

          Localfs does return null if a list operation fails due to an invalid path or other access problem. This is a bug: it must be escalated to an IOE for consistency with everything else.

          If someone were to write a patch I'd review it.

          Show
          stevel@apache.org Steve Loughran added a comment - HDFS & the FS spec do effectively require an IOE of some form to be raised here. Localfs does return null if a list operation fails due to an invalid path or other access problem. This is a bug: it must be escalated to an IOE for consistency with everything else. If someone were to write a patch I'd review it.
          Hide
          cmccabe Colin P. McCabe added a comment -

          This should be easier with the new jdk7 changes. We now have access to directory listing APIs like DirectoryStream that throw IOEs on problems instead of returning null.

          Show
          cmccabe Colin P. McCabe added a comment - This should be easier with the new jdk7 changes. We now have access to directory listing APIs like DirectoryStream that throw IOEs on problems instead of returning null.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Looking at the codepath fpr {{FileStatus[] listStatus(Path f, PathFilter filter)

          ., it invokes abstract FileStatus[] listStatus(Path f) }} via {{listStatus(ArrayList<FileStatus> results, Path f, PathFilter filter), and includes a check for the return value being null.

          That is, if you go LocalFileSystem.listStatus(path) , in some circumstances null can be returned, but in listStatus(path, filter), you'll get an IOE back.

          It should be an IOE everywhere.

          Show
          stevel@apache.org Steve Loughran added a comment - Looking at the codepath fpr {{FileStatus[] listStatus(Path f, PathFilter filter) ., it invokes abstract FileStatus[] listStatus(Path f) }} via {{listStatus(ArrayList<FileStatus> results, Path f, PathFilter filter) , and includes a check for the return value being null. That is, if you go LocalFileSystem.listStatus(path) , in some circumstances null can be returned, but in listStatus(path, filter), you'll get an IOE back. It should be an IOE everywhere.
          Hide
          jzhuge John Zhuge added a comment -

          Hi Matt Foley,

          I have been working on patches for HADOOP-13191 which seems like a duplicate of this JIRA. Do you mind if I take over this JIRA and merge the two together?

          Cc. Xiao Chen.

          Thanks,
          John Zhuge

          Show
          jzhuge John Zhuge added a comment - Hi Matt Foley , I have been working on patches for HADOOP-13191 which seems like a duplicate of this JIRA. Do you mind if I take over this JIRA and merge the two together? Cc. Xiao Chen . Thanks, John Zhuge
          Hide
          jzhuge John Zhuge added a comment -

          Patch 001:

          • RawLocalFileSystem#listStatus throws IOE upon access error. Leverage FileUtil#list.
          • Update filesystem.md
          • Pass unit test TestFSMainOperationsLocalFileSystem and TestFSMainOperationsWebHdfs

          Xiao Chen This patches follows HADOOP-13191.004.patch, addressing all your review comments.

          Show
          jzhuge John Zhuge added a comment - Patch 001: RawLocalFileSystem#listStatus throws IOE upon access error. Leverage FileUtil#list . Update filesystem.md Pass unit test TestFSMainOperationsLocalFileSystem and TestFSMainOperationsWebHdfs Xiao Chen This patches follows HADOOP-13191 .004.patch , addressing all your review comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 4 new or modified test files.
          0 mvndep 0m 15s Maven dependency ordering for branch
          +1 mvninstall 6m 37s trunk passed
          +1 compile 6m 49s trunk passed
          +1 checkstyle 1m 28s trunk passed
          +1 mvnsite 1m 51s trunk passed
          +1 mvneclipse 0m 40s trunk passed
          +1 findbugs 3m 13s trunk passed
          +1 javadoc 1m 24s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 31s the patch passed
          +1 compile 6m 47s the patch passed
          +1 javac 6m 47s the patch passed
          -0 checkstyle 1m 32s root: The patch generated 1 new + 294 unchanged - 0 fixed = 295 total (was 294)
          +1 mvnsite 2m 0s the patch passed
          +1 mvneclipse 0m 49s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 48s the patch passed
          +1 javadoc 1m 34s the patch passed
          -1 unit 17m 8s hadoop-common in the patch failed.
          +1 unit 1m 6s hadoop-hdfs-client in the patch passed.
          +1 unit 9m 27s hadoop-distcp in the patch passed.
          +1 asflicense 0m 26s The patch does not generate ASF License warnings.
          92m 39s



          Reason Tests
          Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-7352
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827438/HADOOP-7352.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b636fdcfb894 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f414d5e
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10458/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10458/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10458/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10458/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 4 new or modified test files. 0 mvndep 0m 15s Maven dependency ordering for branch +1 mvninstall 6m 37s trunk passed +1 compile 6m 49s trunk passed +1 checkstyle 1m 28s trunk passed +1 mvnsite 1m 51s trunk passed +1 mvneclipse 0m 40s trunk passed +1 findbugs 3m 13s trunk passed +1 javadoc 1m 24s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 31s the patch passed +1 compile 6m 47s the patch passed +1 javac 6m 47s the patch passed -0 checkstyle 1m 32s root: The patch generated 1 new + 294 unchanged - 0 fixed = 295 total (was 294) +1 mvnsite 2m 0s the patch passed +1 mvneclipse 0m 49s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 48s the patch passed +1 javadoc 1m 34s the patch passed -1 unit 17m 8s hadoop-common in the patch failed. +1 unit 1m 6s hadoop-hdfs-client in the patch passed. +1 unit 9m 27s hadoop-distcp in the patch passed. +1 asflicense 0m 26s The patch does not generate ASF License warnings. 92m 39s Reason Tests Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-7352 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827438/HADOOP-7352.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b636fdcfb894 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f414d5e Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10458/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10458/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10458/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10458/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jzhuge John Zhuge added a comment -

          Move review comments from HADOOP-13191 to here.

          Steve commented:

          Be aware that the FS shell expects globStatus to return null in certain conditions: someone needs to look at all uses of this call and make sure that it isn't being used

          John commented:

          Went through 99 usages of globStatus(Path) in the following components: hadoop-common, hadoop-distcp, hadoop-rumen, hadoop-streaming, hadoop-yarn-*, hadoop-hdfs (test only).

          Most globStatus callers check both listFileStatus not null and listFileStatus.length > 0, or expect listFileStatus to be not null, except:

          fs.shell.PathData
            public static PathData[] expandAsGlob(String pattern, Configuration conf)
            throws IOException {
              Path globPath = new Path(pattern);
              FileSystem fs = globPath.getFileSystem(conf);    
              FileStatus[] stats = fs.globStatus(globPath);
              PathData[] items = null;
              
              if (stats == null) {
                // remove any quoting in the glob pattern
                pattern = pattern.replaceAll("\\\\(.)", "$1");
                // not a glob & file not found, so add the path with a null stat
                items = new PathData[]{ new PathData(fs, pattern, null) };
              } else {
          

          I will include the fix for expandAsGlob in the next patch.

          Show
          jzhuge John Zhuge added a comment - Move review comments from HADOOP-13191 to here. Steve commented: Be aware that the FS shell expects globStatus to return null in certain conditions: someone needs to look at all uses of this call and make sure that it isn't being used John commented: Went through 99 usages of globStatus(Path) in the following components: hadoop-common, hadoop-distcp, hadoop-rumen, hadoop-streaming, hadoop-yarn-*, hadoop-hdfs (test only). Most globStatus callers check both listFileStatus not null and listFileStatus.length > 0 , or expect listFileStatus to be not null , except: fs.shell.PathData public static PathData[] expandAsGlob( String pattern, Configuration conf) throws IOException { Path globPath = new Path(pattern); FileSystem fs = globPath.getFileSystem(conf); FileStatus[] stats = fs.globStatus(globPath); PathData[] items = null ; if (stats == null ) { // remove any quoting in the glob pattern pattern = pattern.replaceAll( "\\\\(.)" , "$1" ); // not a glob & file not found, so add the path with a null stat items = new PathData[]{ new PathData(fs, pattern, null ) }; } else { I will include the fix for expandAsGlob in the next patch.
          Hide
          jzhuge John Zhuge added a comment -

          Question: do we expect empty array or IOE when globbing a dir without permission? e.g., globStatus('/root/d1/unreadable_dir/d3/*').

          Globber.doGlob
                  for (FileStatus candidate : candidates) {
                    if (globFilter.hasPattern()) {
                      FileStatus[] children = listStatus(candidate.getPath());
                      if (children.length == 1) {
          
          Show
          jzhuge John Zhuge added a comment - Question: do we expect empty array or IOE when globbing a dir without permission? e.g., globStatus('/root/d1/unreadable_dir/d3/*') . Globber.doGlob for (FileStatus candidate : candidates) { if (globFilter.hasPattern()) { FileStatus[] children = listStatus(candidate.getPath()); if (children.length == 1) {
          Hide
          jzhuge John Zhuge added a comment - - edited

          Globber has its own version of listStatus which converts FileNotFoundException to empty array:

            private FileStatus[] listStatus(Path path) throws IOException {
              try {
                if (fs != null) {
                  return fs.listStatus(path);
                } else {
                  return fc.util().listStatus(path);
                }
              } catch (FileNotFoundException e) {
                return new FileStatus[0];
              }
            }
          
          Show
          jzhuge John Zhuge added a comment - - edited Globber has its own version of listStatus which converts FileNotFoundException to empty array: private FileStatus[] listStatus(Path path) throws IOException { try { if (fs != null ) { return fs.listStatus(path); } else { return fc.util().listStatus(path); } } catch (FileNotFoundException e) { return new FileStatus[0]; } }
          Hide
          jzhuge John Zhuge added a comment -

          Patch 002:

          • For directory, RawLocalFileSystem#listStatus calls FileUtil#list that throws IOE upon access error
          • Add unit test FSMainOperationsBaseTest#testGlobStatusThrowsExceptionForUnreadableDir
          • Add unit test TestPathData#testGlobThrowsExceptionForUnreadableDir
          • Update filesystem.md
          • Pass tests: TestFSMainOperationsLocalFileSystem, TestFSMainOperationsWebHdfs, TestFSMainOperationsWebHdfs, and TestPathData.
          Show
          jzhuge John Zhuge added a comment - Patch 002: For directory, RawLocalFileSystem#listStatus calls FileUtil#list that throws IOE upon access error Add unit test FSMainOperationsBaseTest#testGlobStatusThrowsExceptionForUnreadableDir Add unit test TestPathData#testGlobThrowsExceptionForUnreadableDir Update filesystem.md Pass tests: TestFSMainOperationsLocalFileSystem, TestFSMainOperationsWebHdfs, TestFSMainOperationsWebHdfs, and TestPathData.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          0 mvndep 0m 13s Maven dependency ordering for branch
          +1 mvninstall 6m 59s trunk passed
          +1 compile 7m 1s trunk passed
          +1 checkstyle 1m 27s trunk passed
          +1 mvnsite 1m 54s trunk passed
          +1 mvneclipse 0m 40s trunk passed
          +1 findbugs 3m 16s trunk passed
          +1 javadoc 1m 24s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 31s the patch passed
          +1 compile 7m 0s the patch passed
          +1 javac 7m 0s the patch passed
          -0 checkstyle 1m 36s root: The patch generated 1 new + 288 unchanged - 0 fixed = 289 total (was 288)
          +1 mvnsite 2m 0s the patch passed
          +1 mvneclipse 0m 49s the patch passed
          -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 findbugs 3m 50s the patch passed
          +1 javadoc 1m 31s the patch passed
          +1 unit 8m 15s hadoop-common in the patch passed.
          +1 unit 1m 1s hadoop-hdfs-client in the patch passed.
          +1 unit 9m 12s hadoop-distcp in the patch passed.
          +1 asflicense 0m 28s The patch does not generate ASF License warnings.
          83m 57s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-7352
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827658/HADOOP-7352.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux fdf79d05f74a 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b6d839a
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10466/artifact/patchprocess/diff-checkstyle-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/10466/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10466/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10466/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 10s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. 0 mvndep 0m 13s Maven dependency ordering for branch +1 mvninstall 6m 59s trunk passed +1 compile 7m 1s trunk passed +1 checkstyle 1m 27s trunk passed +1 mvnsite 1m 54s trunk passed +1 mvneclipse 0m 40s trunk passed +1 findbugs 3m 16s trunk passed +1 javadoc 1m 24s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 31s the patch passed +1 compile 7m 0s the patch passed +1 javac 7m 0s the patch passed -0 checkstyle 1m 36s root: The patch generated 1 new + 288 unchanged - 0 fixed = 289 total (was 288) +1 mvnsite 2m 0s the patch passed +1 mvneclipse 0m 49s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 3m 50s the patch passed +1 javadoc 1m 31s the patch passed +1 unit 8m 15s hadoop-common in the patch passed. +1 unit 1m 1s hadoop-hdfs-client in the patch passed. +1 unit 9m 12s hadoop-distcp in the patch passed. +1 asflicense 0m 28s The patch does not generate ASF License warnings. 83m 57s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-7352 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827658/HADOOP-7352.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux fdf79d05f74a 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b6d839a Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10466/artifact/patchprocess/diff-checkstyle-root.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/10466/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10466/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10466/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jzhuge John Zhuge added a comment -

          Patch 003:

          • Fix checkstyle and whilespace
          Show
          jzhuge John Zhuge added a comment - Patch 003: Fix checkstyle and whilespace
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          0 mvndep 1m 20s Maven dependency ordering for branch
          +1 mvninstall 6m 38s trunk passed
          +1 compile 6m 48s trunk passed
          +1 checkstyle 1m 28s trunk passed
          +1 mvnsite 1m 51s trunk passed
          +1 mvneclipse 0m 40s trunk passed
          +1 findbugs 3m 18s trunk passed
          +1 javadoc 1m 24s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 31s the patch passed
          +1 compile 6m 49s the patch passed
          +1 javac 6m 49s the patch passed
          +1 checkstyle 1m 32s the patch passed
          +1 mvnsite 1m 58s the patch passed
          +1 mvneclipse 0m 50s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 47s the patch passed
          +1 javadoc 1m 33s the patch passed
          -1 unit 17m 6s hadoop-common in the patch failed.
          +1 unit 1m 6s hadoop-hdfs-client in the patch passed.
          +1 unit 9m 24s hadoop-distcp in the patch passed.
          +1 asflicense 0m 25s The patch does not generate ASF License warnings.
          93m 41s



          Reason Tests
          Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-7352
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827697/HADOOP-7352.003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c7d8482dbfa0 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / baab489
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10468/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10468/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10468/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. 0 mvndep 1m 20s Maven dependency ordering for branch +1 mvninstall 6m 38s trunk passed +1 compile 6m 48s trunk passed +1 checkstyle 1m 28s trunk passed +1 mvnsite 1m 51s trunk passed +1 mvneclipse 0m 40s trunk passed +1 findbugs 3m 18s trunk passed +1 javadoc 1m 24s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 31s the patch passed +1 compile 6m 49s the patch passed +1 javac 6m 49s the patch passed +1 checkstyle 1m 32s the patch passed +1 mvnsite 1m 58s the patch passed +1 mvneclipse 0m 50s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 47s the patch passed +1 javadoc 1m 33s the patch passed -1 unit 17m 6s hadoop-common in the patch failed. +1 unit 1m 6s hadoop-hdfs-client in the patch passed. +1 unit 9m 24s hadoop-distcp in the patch passed. +1 asflicense 0m 25s The patch does not generate ASF License warnings. 93m 41s Reason Tests Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-7352 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827697/HADOOP-7352.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c7d8482dbfa0 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / baab489 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10468/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10468/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10468/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          it doesn't catch permissions though: those get relayed

          Show
          stevel@apache.org Steve Loughran added a comment - it doesn't catch permissions though: those get relayed
          Hide
          jzhuge John Zhuge added a comment - - edited

          You are right.

          Patch 003 ensures listStatus, globStatus, and expandAsGlob to throw IOE upon access error. However, it is just 1 of the 3 possible choices:

          • All throw IOE
          • listStatus throws IOE, globStatus, and expandAsGlob return empty array
          • All 3 return empty array
          Show
          jzhuge John Zhuge added a comment - - edited You are right. Patch 003 ensures listStatus , globStatus , and expandAsGlob to throw IOE upon access error. However, it is just 1 of the 3 possible choices: All throw IOE listStatus throws IOE, globStatus , and expandAsGlob return empty array All 3 return empty array
          Hide
          jzhuge John Zhuge added a comment -

          Steve Loughran, Matt Foley, Colin P. McCabe, Daryn Sharp, could you please review Patch 003? It changes the filesystem contract and could have quite an impact.

          Please consider 3 possible behaviors:

          • Patch 003: listStatus, {{globStatus, and expandAsGlob all throw IOE
          • listStatus throws IOE, globStatus and expandAsGlob return empty array
          • All 3 return empty array
          Show
          jzhuge John Zhuge added a comment - Steve Loughran , Matt Foley , Colin P. McCabe , Daryn Sharp , could you please review Patch 003? It changes the filesystem contract and could have quite an impact. Please consider 3 possible behaviors: Patch 003: listStatus, {{globStatus , and expandAsGlob all throw IOE listStatus throws IOE, globStatus and expandAsGlob return empty array All 3 return empty array
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks John for continuing the work here!
          Looks like my comments in HADOOP-13191 are addressed, and I think the patch looks fine in general. Would like Steve Loughran and others' review too.

          Show
          xiaochen Xiao Chen added a comment - Thanks John for continuing the work here! Looks like my comments in HADOOP-13191 are addressed, and I think the patch looks fine in general. Would like Steve Loughran and others' review too.
          Hide
          jzhuge John Zhuge added a comment -

          Steve Loughran Do you have any comment on Patch 003?

          Show
          jzhuge John Zhuge added a comment - Steve Loughran Do you have any comment on Patch 003?
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks John for the work again. I think the patch makes sense and addresses the initial concerns. Some nits and a question:
          Nits:
          FileSystem.java

          -    if (listing == null) {
          -      throw new IOException("Error accessing " + f);
          -    }
          -
          

          I think we can put a preconditions check here, to make it more explicit if some implementation returns a null.

          filesystem.md
          Let's specify 'prior to 3.0.0', instead of 'in the past'.

          Question:
          Regarding Globber#doGlob, it seems to be a separated case?

              /*
               * When the input pattern "looks" like just a simple filename, and we
               * can't find it, we return null rather than an empty array.
               * This is a special case which the shell relies on.
               *
               * To be more precise: if there were no results, AND there were no
               * groupings (aka brackets), and no wildcards in the input (aka stars),
               * we return null.
               */
              if ((!sawWildcard) && results.isEmpty() &&
                  (flattenedPatterns.size() <= 1)) {
                return null;
              }
          
          Show
          xiaochen Xiao Chen added a comment - Thanks John for the work again. I think the patch makes sense and addresses the initial concerns. Some nits and a question: Nits: FileSystem.java - if (listing == null ) { - throw new IOException( "Error accessing " + f); - } - I think we can put a preconditions check here, to make it more explicit if some implementation returns a null. filesystem.md Let's specify 'prior to 3.0.0', instead of 'in the past'. Question: Regarding Globber#doGlob , it seems to be a separated case? /* * When the input pattern "looks" like just a simple filename, and we * can't find it, we return null rather than an empty array. * This is a special case which the shell relies on. * * To be more precise: if there were no results, AND there were no * groupings (aka brackets), and no wildcards in the input (aka stars), * we return null . */ if ((!sawWildcard) && results.isEmpty() && (flattenedPatterns.size() <= 1)) { return null ; }
          Hide
          jzhuge John Zhuge added a comment -

          Thanks Xiao Chen for the review! I will fix the nits in the new patch.

          Good question. I will investigate in a separate JIRA.

          Show
          jzhuge John Zhuge added a comment - Thanks Xiao Chen for the review! I will fix the nits in the new patch. Good question. I will investigate in a separate JIRA.
          Hide
          daryn Daryn Sharp added a comment -

          It changes the filesystem contract and could have quite an impact.

          I'll take a look this afternoon, but you seem to be saying this may be a major compatibility break?

          Show
          daryn Daryn Sharp added a comment - It changes the filesystem contract and could have quite an impact. I'll take a look this afternoon, but you seem to be saying this may be a major compatibility break?
          Hide
          jzhuge John Zhuge added a comment -

          Thanks Daryn Sharp. It depends how many callers expect null return from FileSystem#listStatus. So far we only found very few so it is probably not major. Really appreciate more eyeballs.

          Show
          jzhuge John Zhuge added a comment - Thanks Daryn Sharp . It depends how many callers expect null return from FileSystem#listStatus . So far we only found very few so it is probably not major. Really appreciate more eyeballs.
          Hide
          jzhuge John Zhuge added a comment -

          Patch 004:

          • Rebase
          • Fix 2 nits
          Show
          jzhuge John Zhuge added a comment - Patch 004: Rebase Fix 2 nits
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          0 mvndep 0m 17s Maven dependency ordering for branch
          +1 mvninstall 7m 57s trunk passed
          +1 compile 8m 27s trunk passed
          +1 checkstyle 1m 37s trunk passed
          +1 mvnsite 3m 3s trunk passed
          +1 mvneclipse 1m 50s trunk passed
          +1 findbugs 3m 44s trunk passed
          +1 javadoc 1m 31s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 49s the patch passed
          +1 compile 7m 17s the patch passed
          +1 javac 7m 17s the patch passed
          +1 checkstyle 1m 33s the patch passed
          +1 mvnsite 2m 3s the patch passed
          +1 mvneclipse 0m 49s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 4m 28s the patch passed
          +1 javadoc 2m 7s the patch passed
          +1 unit 8m 54s hadoop-common in the patch passed.
          +1 unit 1m 10s hadoop-hdfs-client in the patch passed.
          +1 unit 10m 18s hadoop-distcp in the patch passed.
          +1 asflicense 0m 32s The patch does not generate ASF License warnings.
          93m 38s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-7352
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830644/HADOOP-7352.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux bda1fe7c095e 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 03f519a
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10623/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10623/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. 0 mvndep 0m 17s Maven dependency ordering for branch +1 mvninstall 7m 57s trunk passed +1 compile 8m 27s trunk passed +1 checkstyle 1m 37s trunk passed +1 mvnsite 3m 3s trunk passed +1 mvneclipse 1m 50s trunk passed +1 findbugs 3m 44s trunk passed +1 javadoc 1m 31s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 49s the patch passed +1 compile 7m 17s the patch passed +1 javac 7m 17s the patch passed +1 checkstyle 1m 33s the patch passed +1 mvnsite 2m 3s the patch passed +1 mvneclipse 0m 49s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 4m 28s the patch passed +1 javadoc 2m 7s the patch passed +1 unit 8m 54s hadoop-common in the patch passed. +1 unit 1m 10s hadoop-hdfs-client in the patch passed. +1 unit 10m 18s hadoop-distcp in the patch passed. +1 asflicense 0m 32s The patch does not generate ASF License warnings. 93m 38s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-7352 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830644/HADOOP-7352.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux bda1fe7c095e 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 03f519a Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10623/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10623/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks John for the new patch. Sorry I missed this earlier: we should also update FileSystem.java's listStatus javadoc to say it no longer returns null. +1 after that though.

          Will let this float until next Tuesday to let Steve Loughran, Daryn Sharp and other watchers take a look.

          Show
          xiaochen Xiao Chen added a comment - Thanks John for the new patch. Sorry I missed this earlier: we should also update FileSystem.java's listStatus javadoc to say it no longer returns null. +1 after that though. Will let this float until next Tuesday to let Steve Loughran , Daryn Sharp and other watchers take a look.
          Hide
          jzhuge John Zhuge added a comment -

          Patch 005:

          • Fix javadoc for FileSystem#listStatus
          Show
          jzhuge John Zhuge added a comment - Patch 005: Fix javadoc for FileSystem#listStatus
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 12m 25s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          0 mvndep 1m 37s Maven dependency ordering for branch
          +1 mvninstall 7m 21s trunk passed
          +1 compile 7m 4s trunk passed
          +1 checkstyle 1m 31s trunk passed
          +1 mvnsite 1m 53s trunk passed
          +1 mvneclipse 0m 49s trunk passed
          +1 findbugs 3m 20s trunk passed
          +1 javadoc 1m 22s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 38s the patch passed
          +1 compile 7m 14s the patch passed
          +1 javac 7m 14s the patch passed
          +1 checkstyle 1m 33s the patch passed
          +1 mvnsite 1m 59s the patch passed
          +1 mvneclipse 0m 47s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 53s the patch passed
          +1 javadoc 1m 29s the patch passed
          +1 unit 7m 27s hadoop-common in the patch passed.
          +1 unit 1m 5s hadoop-hdfs-client in the patch passed.
          +1 unit 9m 53s hadoop-distcp in the patch passed.
          +1 asflicense 0m 28s The patch does not generate ASF License warnings.
          98m 31s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-7352
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833224/HADOOP-7352.005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 3441bbb5a1e1 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 0a85d07
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10776/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10776/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 12m 25s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. 0 mvndep 1m 37s Maven dependency ordering for branch +1 mvninstall 7m 21s trunk passed +1 compile 7m 4s trunk passed +1 checkstyle 1m 31s trunk passed +1 mvnsite 1m 53s trunk passed +1 mvneclipse 0m 49s trunk passed +1 findbugs 3m 20s trunk passed +1 javadoc 1m 22s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 38s the patch passed +1 compile 7m 14s the patch passed +1 javac 7m 14s the patch passed +1 checkstyle 1m 33s the patch passed +1 mvnsite 1m 59s the patch passed +1 mvneclipse 0m 47s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 53s the patch passed +1 javadoc 1m 29s the patch passed +1 unit 7m 27s hadoop-common in the patch passed. +1 unit 1m 5s hadoop-hdfs-client in the patch passed. +1 unit 9m 53s hadoop-distcp in the patch passed. +1 asflicense 0m 28s The patch does not generate ASF License warnings. 98m 31s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-7352 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833224/HADOOP-7352.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 3441bbb5a1e1 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 0a85d07 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10776/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-distcp U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10776/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          LGTM.

          FWIW the code coming in HADOOP-13716 is designed to make those tests for raised exceptions in specific parts of the code way easier to write in Java 8

          IOException ioe = intercept(IOException.class,
            () -> { return PathData.expandAsGlob("foo/*", conf); });
          
          Show
          stevel@apache.org Steve Loughran added a comment - LGTM. FWIW the code coming in HADOOP-13716 is designed to make those tests for raised exceptions in specific parts of the code way easier to write in Java 8 IOException ioe = intercept(IOException.class, () -> { return PathData.expandAsGlob( "foo/*" , conf); });
          Hide
          jzhuge John Zhuge added a comment -

          Thanks Steve Loughran. That'd be great. How do you expect multiple types of exceptions?

          Show
          jzhuge John Zhuge added a comment - Thanks Steve Loughran . That'd be great. How do you expect multiple types of exceptions?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          good q. you don't, but you can do that afterwards if you want to.

          IOException ioe = intercept(IOException.class,
            () -> { return PathData.expandAsGlob("foo/*", conf); });
          
          if (ioe instanceOf FileNotFoundException) 
            handleStrictException(ioe);
          else
           handleLaxException();
          
          Show
          stevel@apache.org Steve Loughran added a comment - good q. you don't, but you can do that afterwards if you want to. IOException ioe = intercept(IOException.class, () -> { return PathData.expandAsGlob( "foo/*" , conf); }); if (ioe instanceOf FileNotFoundException) handleStrictException(ioe); else handleLaxException();
          Hide
          xiaochen Xiao Chen added a comment -

          +1 to patch 5, committing.

          Show
          xiaochen Xiao Chen added a comment - +1 to patch 5, committing.
          Hide
          xiaochen Xiao Chen added a comment -

          Committed this to trunk.
          Thanks John Zhuge for the patch, Steve Loughran and all for the review and discussions!

          Show
          xiaochen Xiao Chen added a comment - Committed this to trunk. Thanks John Zhuge for the patch, Steve Loughran and all for the review and discussions!
          Hide
          xiaochen Xiao Chen added a comment -

          Hi John Zhuge,
          Could you please put up a short release note? Thanks.

          Show
          xiaochen Xiao Chen added a comment - Hi John Zhuge , Could you please put up a short release note? Thanks.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10635 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10635/)
          HADOOP-7352. FileSystem#listStatus should throw IOE upon access error. (xiao: rev efdf810cf9f72d78e97e860576c64a382ece437c)

          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/web/TestTokenAspect.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
          • (edit) hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithAcls.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java
          • (edit) hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithXAttrs.java
          • (edit) hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10635 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10635/ ) HADOOP-7352 . FileSystem#listStatus should throw IOE upon access error. (xiao: rev efdf810cf9f72d78e97e860576c64a382ece437c) (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/web/TestTokenAspect.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java (edit) hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithAcls.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java (edit) hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithXAttrs.java (edit) hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
          Hide
          jzhuge John Zhuge added a comment -

          Thanks Xiao Chen for the review and commit! Thanks Steve Loughran for the review and many others for the discussions.

          Show
          jzhuge John Zhuge added a comment - Thanks Xiao Chen for the review and commit! Thanks Steve Loughran for the review and many others for the discussions.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Hi Xiao Chen/John Zhuge/Steve Loughran, this commit broke TestLogsCLI. I filed YARN-5777 and uploaded a patch to fix the test. Would you review it?

          Show
          ajisakaa Akira Ajisaka added a comment - Hi Xiao Chen / John Zhuge / Steve Loughran , this commit broke TestLogsCLI. I filed YARN-5777 and uploaded a patch to fix the test. Would you review it?
          Hide
          jzhuge John Zhuge added a comment -

          Sure Akira Ajisaka. Thanks for reporting the issue.

          Show
          jzhuge John Zhuge added a comment - Sure Akira Ajisaka . Thanks for reporting the issue.
          Hide
          liuml07 Mingliang Liu added a comment -

          Is testGlobStatusThrowsExceptionForUnreadableDir failing in trunk? Thanks,

          I got:

          -------------------------------------------------------
          T E S T S
          -------------------------------------------------------

          -------------------------------------------------------
          T E S T S
          -------------------------------------------------------
          Running org.apache.hadoop.fs.azure.TestNativeAzureFileSystemOperationsMocked
          Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.182 sec <<< FAILURE! - in org.apache.hadoop.fs.azure.TestNativeAzureFileSystemOperationsMocked
          testGlobStatusThrowsExceptionForUnreadableDir(org.apache.hadoop.fs.azure.TestNativeAzureFileSystemOperationsMocked) Time elapsed: 1.111 sec <<< FAILURE!
          java.lang.AssertionError: Should throw IOException
          at org.junit.Assert.fail(Assert.java:88)
          at org.apache.hadoop.fs.FSMainOperationsBaseTest.testGlobStatusThrowsExceptionForUnreadableDir(FSMainOperationsBaseTest.java:643)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
          at java.lang.reflect.Method.invoke(Method.java:497)
          at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
          at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
          at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
          at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
          at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
          at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
          at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
          at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
          at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
          at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
          at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
          at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
          at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
          at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
          at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
          at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:254)
          at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:149)
          at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:124)
          at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200)
          at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153)
          at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)

          Results :

          Failed tests:
          TestNativeAzureFileSystemOperationsMocked>FSMainOperationsBaseTest.testGlobStatusThrowsExceptionForUnreadableDir:643 Should throw IOException

          Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

          Show
          liuml07 Mingliang Liu added a comment - Is testGlobStatusThrowsExceptionForUnreadableDir failing in trunk ? Thanks, I got: ------------------------------------------------------- T E S T S ------------------------------------------------------- ------------------------------------------------------- T E S T S ------------------------------------------------------- Running org.apache.hadoop.fs.azure.TestNativeAzureFileSystemOperationsMocked Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.182 sec <<< FAILURE! - in org.apache.hadoop.fs.azure.TestNativeAzureFileSystemOperationsMocked testGlobStatusThrowsExceptionForUnreadableDir(org.apache.hadoop.fs.azure.TestNativeAzureFileSystemOperationsMocked) Time elapsed: 1.111 sec <<< FAILURE! java.lang.AssertionError: Should throw IOException at org.junit.Assert.fail(Assert.java:88) at org.apache.hadoop.fs.FSMainOperationsBaseTest.testGlobStatusThrowsExceptionForUnreadableDir(FSMainOperationsBaseTest.java:643) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:497) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27) at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50) at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236) at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229) at org.junit.runners.ParentRunner.run(ParentRunner.java:309) at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:254) at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:149) at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:124) at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200) at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153) at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103) Results : Failed tests: TestNativeAzureFileSystemOperationsMocked>FSMainOperationsBaseTest.testGlobStatusThrowsExceptionForUnreadableDir:643 Should throw IOException Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
          Hide
          jzhuge John Zhuge added a comment -

          Thanks Mingliang Liu for reporting the issue. Filed HADOOP-13787. Probably Azure listStatus does not follow the new contract introduced in this JIRA.

          Show
          jzhuge John Zhuge added a comment - Thanks Mingliang Liu for reporting the issue. Filed HADOOP-13787 . Probably Azure listStatus does not follow the new contract introduced in this JIRA.

            People

            • Assignee:
              jzhuge John Zhuge
              Reporter:
              mattf Matt Foley
            • Votes:
              0 Vote for this issue
              Watchers:
              16 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development