Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-10823

Implement HttpFSFileSystem#listStatusIterator

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.6.4
    • Fix Version/s: 2.9.0, 3.0.0-alpha2
    • Component/s: httpfs
    • Labels:
      None
    • Target Version/s:

      Description

      Let's expose the same functionality added in HDFS-10784 for WebHDFS in HttpFS too.

      1. HDFS-10823.001.patch
        21 kB
        Andrew Wang
      2. HDFS-10823.002.patch
        27 kB
        Andrew Wang
      3. HDFS-10823.003.patch
        26 kB
        Andrew Wang
      4. HDFS-10823.004.patch
        28 kB
        Andrew Wang
      5. HDFS-10823.005.patch
        28 kB
        Andrew Wang
      6. HDFS-10823.006.patch
        28 kB
        Andrew Wang

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10450 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10450/)
          HDFS-10823. Implement HttpFSFileSystem#listStatusIterator. (wang: rev 8a40953058d50d421d62b71067a13b626b3cba1f)

          • (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystem.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/hadoop/FileSystemAccessService.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/client/HttpFSUtils.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSParametersProvider.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/client/BaseTestHttpFSWith.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10450 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10450/ ) HDFS-10823 . Implement HttpFSFileSystem#listStatusIterator. (wang: rev 8a40953058d50d421d62b71067a13b626b3cba1f) (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystem.java (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/hadoop/FileSystemAccessService.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/client/HttpFSUtils.java (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSParametersProvider.java (edit) hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/client/BaseTestHttpFSWith.java
          Hide
          andrew.wang Andrew Wang added a comment -

          Committed to trunk and branch-2. Thank you Xiao for the prompt and detailed reviews!

          Show
          andrew.wang Andrew Wang added a comment - Committed to trunk and branch-2. Thank you Xiao for the prompt and detailed reviews!
          Hide
          andrew.wang Andrew Wang added a comment -

          Test failure looks unrelated, already tracked by HADOOP-13101. Will commit shortly.

          Show
          andrew.wang Andrew Wang added a comment - Test failure looks unrelated, already tracked by HADOOP-13101 . Will commit shortly.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 16s Maven dependency ordering for branch
          +1 mvninstall 8m 49s trunk passed
          +1 compile 9m 1s trunk passed
          +1 checkstyle 1m 47s trunk passed
          +1 mvnsite 2m 15s trunk passed
          +1 mvneclipse 0m 42s trunk passed
          +1 findbugs 3m 48s trunk passed
          +1 javadoc 1m 35s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 58s the patch passed
          +1 compile 7m 55s the patch passed
          +1 javac 7m 55s the patch passed
          -0 checkstyle 1m 35s root: The patch generated 7 new + 909 unchanged - 1 fixed = 916 total (was 910)
          +1 mvnsite 2m 4s the patch passed
          +1 mvneclipse 0m 47s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 4m 6s the patch passed
          +1 javadoc 1m 22s the patch passed
          -1 unit 7m 29s hadoop-common in the patch failed.
          +1 unit 0m 55s hadoop-hdfs-client in the patch passed.
          +1 unit 3m 28s hadoop-hdfs-httpfs in the patch passed.
          +1 asflicense 0m 23s The patch does not generate ASF License warnings.
          61m 55s



          Reason Tests
          Failed junit tests hadoop.net.TestDNS



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10823
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828902/HDFS-10823.006.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 27225a2280f1 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 / b09a03c
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16777/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16777/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16777/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16777/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 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 16s Maven dependency ordering for branch +1 mvninstall 8m 49s trunk passed +1 compile 9m 1s trunk passed +1 checkstyle 1m 47s trunk passed +1 mvnsite 2m 15s trunk passed +1 mvneclipse 0m 42s trunk passed +1 findbugs 3m 48s trunk passed +1 javadoc 1m 35s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 58s the patch passed +1 compile 7m 55s the patch passed +1 javac 7m 55s the patch passed -0 checkstyle 1m 35s root: The patch generated 7 new + 909 unchanged - 1 fixed = 916 total (was 910) +1 mvnsite 2m 4s the patch passed +1 mvneclipse 0m 47s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 4m 6s the patch passed +1 javadoc 1m 22s the patch passed -1 unit 7m 29s hadoop-common in the patch failed. +1 unit 0m 55s hadoop-hdfs-client in the patch passed. +1 unit 3m 28s hadoop-hdfs-httpfs in the patch passed. +1 asflicense 0m 23s The patch does not generate ASF License warnings. 61m 55s Reason Tests Failed junit tests hadoop.net.TestDNS Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828902/HDFS-10823.006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 27225a2280f1 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 / b09a03c Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16777/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/16777/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16777/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16777/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 -

          Makes sense, +1 pending jenkins. Thanks for the great work, Andrew!

          Show
          xiaochen Xiao Chen added a comment - Makes sense, +1 pending jenkins. Thanks for the great work, Andrew!
          Hide
          andrew.wang Andrew Wang added a comment -

          One more rev. Fixed the missing break and used the new constants.

          For the null checks, the rest of the json parsing logic in HttpFSServer doesn't do any for required parameters. So I think this behavior is inline with the rest of the code.

          For the suppression, I copy-pasted this style from code blocks right above, so would prefer to leave as is.

          Show
          andrew.wang Andrew Wang added a comment - One more rev. Fixed the missing break and used the new constants. For the null checks, the rest of the json parsing logic in HttpFSServer doesn't do any for required parameters. So I think this behavior is inline with the rest of the code. For the suppression, I copy-pasted this style from code blocks right above, so would prefer to leave as is.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks for the quick rev, Andrew. I think we're pretty close.
          Nits:

          • HttpFSFileSystem Sorry I wasn't clear in my previous comments.

            I see we check null on remainingEntries but not on DirectoryListing or partialListing, is this intentional?

            I meant to ask this: do we need to check the case where json doesn't contain those elements, which could lead to NPE?

            Suggest to add DirectoryListing as a predefined constant too, and use the constants.

            I meant to say, 'xxx too. And use the constants, including 'partialListing' and 'remainingEntries'.

          • BaseTestHttpFSWith missing break after line 994. (not from this patch though).
          • HttpFSServer line 335 I think we usually put this on a separate line.
          Show
          xiaochen Xiao Chen added a comment - Thanks for the quick rev, Andrew. I think we're pretty close. Nits: HttpFSFileSystem Sorry I wasn't clear in my previous comments. I see we check null on remainingEntries but not on DirectoryListing or partialListing, is this intentional? I meant to ask this: do we need to check the case where json doesn't contain those elements, which could lead to NPE? Suggest to add DirectoryListing as a predefined constant too, and use the constants. I meant to say, 'xxx too. And use the constants, including 'partialListing' and 'remainingEntries'. BaseTestHttpFSWith missing break after line 994. (not from this patch though). HttpFSServer line 335 I think we usually put this on a separate line.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 9s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 1m 20s Maven dependency ordering for branch
          +1 mvninstall 7m 14s trunk passed
          +1 compile 7m 26s trunk passed
          +1 checkstyle 1m 40s trunk passed
          +1 mvnsite 2m 1s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          +1 findbugs 3m 22s trunk passed
          +1 javadoc 1m 21s trunk passed
          0 mvndep 0m 15s Maven dependency ordering for patch
          +1 mvninstall 1m 53s the patch passed
          +1 compile 7m 42s the patch passed
          +1 javac 7m 42s the patch passed
          -0 checkstyle 1m 42s root: The patch generated 6 new + 909 unchanged - 1 fixed = 915 total (was 910)
          +1 mvnsite 1m 53s the patch passed
          +1 mvneclipse 0m 38s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 38s the patch passed
          +1 javadoc 1m 17s the patch passed
          +1 unit 7m 21s hadoop-common in the patch passed.
          +1 unit 0m 56s hadoop-hdfs-client in the patch passed.
          +1 unit 3m 32s hadoop-hdfs-httpfs in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          57m 23s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10823
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828766/HDFS-10823.005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ffdb9ba9e5bd 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 / ec3ea18
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16762/artifact/patchprocess/diff-checkstyle-root.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16762/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16762/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 9s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 1m 20s Maven dependency ordering for branch +1 mvninstall 7m 14s trunk passed +1 compile 7m 26s trunk passed +1 checkstyle 1m 40s trunk passed +1 mvnsite 2m 1s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 3m 22s trunk passed +1 javadoc 1m 21s trunk passed 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 1m 53s the patch passed +1 compile 7m 42s the patch passed +1 javac 7m 42s the patch passed -0 checkstyle 1m 42s root: The patch generated 6 new + 909 unchanged - 1 fixed = 915 total (was 910) +1 mvnsite 1m 53s the patch passed +1 mvneclipse 0m 38s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 38s the patch passed +1 javadoc 1m 17s the patch passed +1 unit 7m 21s hadoop-common in the patch passed. +1 unit 0m 56s hadoop-hdfs-client in the patch passed. +1 unit 3m 32s hadoop-hdfs-httpfs in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 57m 23s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828766/HDFS-10823.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ffdb9ba9e5bd 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 / ec3ea18 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16762/artifact/patchprocess/diff-checkstyle-root.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16762/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16762/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          One more rev, thanks again for reviewing. Hit your comments and fixed the one valid checkstyle.

          Show
          andrew.wang Andrew Wang added a comment - One more rev, thanks again for reviewing. Hit your comments and fixed the one valid checkstyle.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks for explaining, Andrew. Yeah what you said makes sense, and yep, javadoc coverage is great.
          Some more comments:

          This default implementation lists the entire directory as a single batch, so there is no next batch.

          Thanks for explaining, sgtm. Is it possible to clarify this for the default implementation from javadoc or log?

          Any recommendations on additional docs

          Sorry I forgot HttpFs didn't have it's own doc.... And the new changes to FS aren't supposed to be used by user, so we're good here.

          Somre more cosmetic things:

          • FSOperations
            • FSListStatusBatch's member vars could be final
          • HttpFSFileSystem
            • I see we check null on remainingEntries but not on DirectoryListing or partialListing, is this intentional?
            • Suggest to add DirectoryListing as a predefined constant too, and use the constants.

          Test failure seems related, but generally looks pretty good to me.

          Show
          xiaochen Xiao Chen added a comment - Thanks for explaining, Andrew. Yeah what you said makes sense, and yep, javadoc coverage is great. Some more comments: This default implementation lists the entire directory as a single batch, so there is no next batch. Thanks for explaining, sgtm. Is it possible to clarify this for the default implementation from javadoc or log? Any recommendations on additional docs Sorry I forgot HttpFs didn't have it's own doc.... And the new changes to FS aren't supposed to be used by user, so we're good here. Somre more cosmetic things: FSOperations FSListStatusBatch 's member vars could be final HttpFSFileSystem I see we check null on remainingEntries but not on DirectoryListing or partialListing , is this intentional? Suggest to add DirectoryListing as a predefined constant too, and use the constants. Test failure seems related, but generally looks pretty good to me.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 1m 29s Maven dependency ordering for branch
          +1 mvninstall 7m 48s trunk passed
          +1 compile 8m 51s trunk passed
          +1 checkstyle 1m 38s trunk passed
          +1 mvnsite 2m 28s trunk passed
          +1 mvneclipse 0m 41s trunk passed
          +1 findbugs 3m 53s trunk passed
          +1 javadoc 1m 20s trunk passed
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 44s the patch passed
          +1 compile 7m 27s the patch passed
          +1 javac 7m 27s the patch passed
          -0 checkstyle 1m 42s root: The patch generated 7 new + 908 unchanged - 1 fixed = 915 total (was 909)
          +1 mvnsite 1m 58s the patch passed
          +1 mvneclipse 0m 43s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 47s the patch passed
          +1 javadoc 1m 21s the patch passed
          -1 unit 7m 45s hadoop-common in the patch failed.
          +1 unit 0m 59s hadoop-hdfs-client in the patch passed.
          +1 unit 4m 7s hadoop-hdfs-httpfs in the patch passed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          61m 37s



          Reason Tests
          Failed junit tests hadoop.fs.TestHarFileSystem
            hadoop.fs.TestFilterFileSystem



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10823
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828700/HDFS-10823.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux a55a131bbb91 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 / fcbac00
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16753/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16753/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16753/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16753/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 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 1m 29s Maven dependency ordering for branch +1 mvninstall 7m 48s trunk passed +1 compile 8m 51s trunk passed +1 checkstyle 1m 38s trunk passed +1 mvnsite 2m 28s trunk passed +1 mvneclipse 0m 41s trunk passed +1 findbugs 3m 53s trunk passed +1 javadoc 1m 20s trunk passed 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 44s the patch passed +1 compile 7m 27s the patch passed +1 javac 7m 27s the patch passed -0 checkstyle 1m 42s root: The patch generated 7 new + 908 unchanged - 1 fixed = 915 total (was 909) +1 mvnsite 1m 58s the patch passed +1 mvneclipse 0m 43s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 47s the patch passed +1 javadoc 1m 21s the patch passed -1 unit 7m 45s hadoop-common in the patch failed. +1 unit 0m 59s hadoop-hdfs-client in the patch passed. +1 unit 4m 7s hadoop-hdfs-httpfs in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 61m 37s Reason Tests Failed junit tests hadoop.fs.TestHarFileSystem   hadoop.fs.TestFilterFileSystem Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828700/HDFS-10823.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a55a131bbb91 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 / fcbac00 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16753/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/16753/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16753/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16753/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          Thanks for reviewing Xiao, new rev attached. Addressed except as per below:

          – The new listStatus should return new DirectoryEntries(listing, token, false);

          I think it's correct as is. This default implementation lists the entire directory as a single batch, so there is no next batch.

          We could do some more work to truncate the listing to start at the passed token and set the returned token to the last element, but I think this falls outside of the spec since hasMore is always false and the token is supposed to come from a previous call and be opaque.

          Maybe the helper methods can go to HttpFSUtils?

          I think you're talking about toFileStatus here? That method is unfortunately not static since makeQualified is not static, so we can't move it.

          Maybe we could reorganize the code in listStatus to parse remainingEntries first, then newToken, so that we don't need 2 duplicate comments.

          I just deleted the dup comment. I like ordering the parsing in the order of the arguments to DirectoryEntries.

          We'll need (a bunch of) documentation.

          Any recommendations on additional docs? The batched listing API is already covered in WebHDFS.md, and I think the javadoc coverage is pretty good.

          Show
          andrew.wang Andrew Wang added a comment - Thanks for reviewing Xiao, new rev attached. Addressed except as per below: – The new listStatus should return new DirectoryEntries(listing, token, false); I think it's correct as is. This default implementation lists the entire directory as a single batch, so there is no next batch. We could do some more work to truncate the listing to start at the passed token and set the returned token to the last element, but I think this falls outside of the spec since hasMore is always false and the token is supposed to come from a previous call and be opaque. Maybe the helper methods can go to HttpFSUtils ? I think you're talking about toFileStatus here? That method is unfortunately not static since makeQualified is not static, so we can't move it. Maybe we could reorganize the code in listStatus to parse remainingEntries first, then newToken , so that we don't need 2 duplicate comments. I just deleted the dup comment. I like ordering the parsing in the order of the arguments to DirectoryEntries. We'll need (a bunch of) documentation. Any recommendations on additional docs? The batched listing API is already covered in WebHDFS.md, and I think the javadoc coverage is pretty good.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks for the patch Andrew! I didn't find a better alternative, and think this makes sense.

          Some review comments so far, I will have a more thorough review.

          • FileSystem#DirectoryEntries:
            • member variables could be final.
            • The new listStatus should return new DirectoryEntries(listing, token, false);
            • How do you feel just calling this new listStatus sth like listStatusBatch? This is more consistent with the webhdfs/httpfs params, and removes the confusion for users on the method overload.
          • WebHdfsFileSystem
            • seems we can remove the DirListingIterator there.
          • HttpFSFileSystem
            • Maybe the helper methods can go to HttpFSUtils?
            • Maybe we could reorganize the code in listStatus to parse remainingEntries first, then newToken, so that we don't need 2 duplicate comments.
          • We'll need (a bunch of) documentation.
          Show
          xiaochen Xiao Chen added a comment - Thanks for the patch Andrew! I didn't find a better alternative, and think this makes sense. Some review comments so far, I will have a more thorough review. FileSystem#DirectoryEntries : member variables could be final. The new listStatus should return new DirectoryEntries(listing, token, false); How do you feel just calling this new listStatus sth like listStatusBatch ? This is more consistent with the webhdfs/httpfs params, and removes the confusion for users on the method overload. WebHdfsFileSystem seems we can remove the DirListingIterator there. HttpFSFileSystem Maybe the helper methods can go to HttpFSUtils ? Maybe we could reorganize the code in listStatus to parse remainingEntries first, then newToken , so that we don't need 2 duplicate comments. We'll need (a bunch of) documentation.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 1m 28s Maven dependency ordering for branch
          +1 mvninstall 10m 6s trunk passed
          +1 compile 9m 37s trunk passed
          +1 checkstyle 1m 50s trunk passed
          +1 mvnsite 2m 27s trunk passed
          +1 mvneclipse 0m 44s trunk passed
          +1 findbugs 3m 58s trunk passed
          +1 javadoc 1m 49s trunk passed
          0 mvndep 0m 19s Maven dependency ordering for patch
          +1 mvninstall 2m 37s the patch passed
          +1 compile 10m 50s the patch passed
          +1 javac 10m 50s the patch passed
          -0 checkstyle 2m 18s root: The patch generated 6 new + 909 unchanged - 1 fixed = 915 total (was 910)
          +1 mvnsite 2m 37s the patch passed
          +1 mvneclipse 0m 46s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 0s hadoop-hdfs-project/hadoop-hdfs-httpfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 1m 50s the patch passed
          -1 unit 10m 2s hadoop-common in the patch failed.
          +1 unit 1m 10s hadoop-hdfs-client in the patch passed.
          +1 unit 4m 22s hadoop-hdfs-httpfs in the patch passed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          76m 36s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs-httpfs
            Nullcheck of token at line 336 of value previously dereferenced in org.apache.hadoop.fs.http.server.HttpFSServer.get(String, HttpFSParametersProvider$OperationParam, Parameters, HttpServletRequest) At HttpFSServer.java:336 of value previously dereferenced in org.apache.hadoop.fs.http.server.HttpFSServer.get(String, HttpFSParametersProvider$OperationParam, Parameters, HttpServletRequest) At HttpFSServer.java:[line 333]
          Failed junit tests hadoop.ipc.TestIPC



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10823
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828562/HDFS-10823.003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b89d3e6836f0 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 / 2a8f55a
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16749/artifact/patchprocess/diff-checkstyle-root.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16749/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-httpfs.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16749/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16749/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16749/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 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 1m 28s Maven dependency ordering for branch +1 mvninstall 10m 6s trunk passed +1 compile 9m 37s trunk passed +1 checkstyle 1m 50s trunk passed +1 mvnsite 2m 27s trunk passed +1 mvneclipse 0m 44s trunk passed +1 findbugs 3m 58s trunk passed +1 javadoc 1m 49s trunk passed 0 mvndep 0m 19s Maven dependency ordering for patch +1 mvninstall 2m 37s the patch passed +1 compile 10m 50s the patch passed +1 javac 10m 50s the patch passed -0 checkstyle 2m 18s root: The patch generated 6 new + 909 unchanged - 1 fixed = 915 total (was 910) +1 mvnsite 2m 37s the patch passed +1 mvneclipse 0m 46s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 0s hadoop-hdfs-project/hadoop-hdfs-httpfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 1m 50s the patch passed -1 unit 10m 2s hadoop-common in the patch failed. +1 unit 1m 10s hadoop-hdfs-client in the patch passed. +1 unit 4m 22s hadoop-hdfs-httpfs in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 76m 36s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs-httpfs   Nullcheck of token at line 336 of value previously dereferenced in org.apache.hadoop.fs.http.server.HttpFSServer.get(String, HttpFSParametersProvider$OperationParam, Parameters, HttpServletRequest) At HttpFSServer.java:336 of value previously dereferenced in org.apache.hadoop.fs.http.server.HttpFSServer.get(String, HttpFSParametersProvider$OperationParam, Parameters, HttpServletRequest) At HttpFSServer.java: [line 333] Failed junit tests hadoop.ipc.TestIPC Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828562/HDFS-10823.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b89d3e6836f0 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 / 2a8f55a Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16749/artifact/patchprocess/diff-checkstyle-root.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16749/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-httpfs.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/16749/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16749/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16749/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          Woops, consolidated patch attached.

          Show
          andrew.wang Andrew Wang added a comment - Woops, consolidated patch attached.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 8s HDFS-10823 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Issue HDFS-10823
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828560/HDFS-10823.003.patch
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16748/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 0s Docker mode activated. -1 patch 0m 8s HDFS-10823 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue HDFS-10823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828560/HDFS-10823.003.patch Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16748/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          One more pass over precommit.

          Show
          andrew.wang Andrew Wang added a comment - One more pass over precommit.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 1m 23s Maven dependency ordering for branch
          +1 mvninstall 8m 46s trunk passed
          +1 compile 8m 38s trunk passed
          +1 checkstyle 1m 40s trunk passed
          +1 mvnsite 2m 14s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          +1 findbugs 3m 27s trunk passed
          +1 javadoc 1m 26s trunk passed
          0 mvndep 0m 15s Maven dependency ordering for patch
          +1 mvninstall 1m 55s the patch passed
          +1 compile 8m 21s the patch passed
          +1 javac 8m 21s the patch passed
          -0 checkstyle 1m 35s root: The patch generated 8 new + 909 unchanged - 1 fixed = 917 total (was 910)
          +1 mvnsite 1m 49s the patch passed
          +1 mvneclipse 0m 36s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 0m 32s hadoop-hdfs-project/hadoop-hdfs-httpfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 1m 17s the patch passed
          +1 unit 7m 30s hadoop-common in the patch passed.
          +1 unit 0m 54s hadoop-hdfs-client in the patch passed.
          +1 unit 3m 30s hadoop-hdfs-httpfs in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          61m 2s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs-httpfs
            new org.apache.hadoop.fs.http.server.FSOperations$FSListStatusBatch(String, byte[]) may expose internal representation by storing an externally mutable object into FSOperations$FSListStatusBatch.token At FSOperations.java:internal representation by storing an externally mutable object into FSOperations$FSListStatusBatch.token At FSOperations.java:[line 661]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10823
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828538/HDFS-10823.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b52af3e5424b 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 / 2a8f55a
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16745/artifact/patchprocess/diff-checkstyle-root.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16745/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-httpfs.html
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16745/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16745/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 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 1m 23s Maven dependency ordering for branch +1 mvninstall 8m 46s trunk passed +1 compile 8m 38s trunk passed +1 checkstyle 1m 40s trunk passed +1 mvnsite 2m 14s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 3m 27s trunk passed +1 javadoc 1m 26s trunk passed 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 1m 55s the patch passed +1 compile 8m 21s the patch passed +1 javac 8m 21s the patch passed -0 checkstyle 1m 35s root: The patch generated 8 new + 909 unchanged - 1 fixed = 917 total (was 910) +1 mvnsite 1m 49s the patch passed +1 mvneclipse 0m 36s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 0m 32s hadoop-hdfs-project/hadoop-hdfs-httpfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 1m 17s the patch passed +1 unit 7m 30s hadoop-common in the patch passed. +1 unit 0m 54s hadoop-hdfs-client in the patch passed. +1 unit 3m 30s hadoop-hdfs-httpfs in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 61m 2s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs-httpfs   new org.apache.hadoop.fs.http.server.FSOperations$FSListStatusBatch(String, byte[]) may expose internal representation by storing an externally mutable object into FSOperations$FSListStatusBatch.token At FSOperations.java:internal representation by storing an externally mutable object into FSOperations$FSListStatusBatch.token At FSOperations.java: [line 661] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828538/HDFS-10823.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b52af3e5424b 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 / 2a8f55a Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16745/artifact/patchprocess/diff-checkstyle-root.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16745/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-httpfs.html Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16745/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16745/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          New patch attached. Fixed up the precommit checks. Also downgraded the new FileSystem method to protected, and use a wrapper FS to access the batched listing API where we need it in HttpFS.

          Show
          andrew.wang Andrew Wang added a comment - New patch attached. Fixed up the precommit checks. Also downgraded the new FileSystem method to protected, and use a wrapper FS to access the batched listing API where we need it in HttpFS.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 23s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 16s Maven dependency ordering for branch
          +1 mvninstall 7m 17s trunk passed
          +1 compile 7m 22s trunk passed
          +1 checkstyle 1m 40s trunk passed
          +1 mvnsite 2m 12s trunk passed
          +1 mvneclipse 0m 40s trunk passed
          +1 findbugs 3m 17s trunk passed
          +1 javadoc 1m 31s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 2m 12s the patch passed
          +1 compile 8m 52s the patch passed
          +1 javac 8m 52s the patch passed
          -0 checkstyle 1m 44s root: The patch generated 6 new + 682 unchanged - 1 fixed = 688 total (was 683)
          +1 mvnsite 2m 24s the patch passed
          +1 mvneclipse 0m 42s the patch passed
          +1 whitespace 0m 1s The patch has no whitespace issues.
          -1 findbugs 1m 53s hadoop-common-project/hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          -1 findbugs 0m 36s hadoop-hdfs-project/hadoop-hdfs-httpfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 1m 26s the patch passed
          -1 unit 7m 57s hadoop-common in the patch failed.
          +1 unit 1m 1s hadoop-hdfs-client in the patch passed.
          -1 unit 4m 10s hadoop-hdfs-httpfs in the patch failed.
          +1 asflicense 0m 26s The patch does not generate ASF License warnings.
          60m 59s



          Reason Tests
          FindBugs module:hadoop-common-project/hadoop-common
            Should org.apache.hadoop.fs.FileSystem$DirectoryEntries be a static inner class? At FileSystem.java:inner class? At FileSystem.java:[lines 1549-1564]
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs-httpfs
            new org.apache.hadoop.fs.http.server.FSOperations$FSListStatusBatch(String, byte[]) may expose internal representation by storing an externally mutable object into FSOperations$FSListStatusBatch.token At FSOperations.java:internal representation by storing an externally mutable object into FSOperations$FSListStatusBatch.token At FSOperations.java:[line 656]
          Failed junit tests hadoop.fs.TestHarFileSystem
            hadoop.fs.TestFilterFileSystem
            hadoop.fs.http.client.TestHttpFSFileSystemLocalFileSystem



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10823
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827858/HDFS-10823.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b5d5400ef631 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 / db6d243
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/diff-checkstyle-root.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-httpfs.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-httpfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16733/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16733/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 23s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 16s Maven dependency ordering for branch +1 mvninstall 7m 17s trunk passed +1 compile 7m 22s trunk passed +1 checkstyle 1m 40s trunk passed +1 mvnsite 2m 12s trunk passed +1 mvneclipse 0m 40s trunk passed +1 findbugs 3m 17s trunk passed +1 javadoc 1m 31s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 2m 12s the patch passed +1 compile 8m 52s the patch passed +1 javac 8m 52s the patch passed -0 checkstyle 1m 44s root: The patch generated 6 new + 682 unchanged - 1 fixed = 688 total (was 683) +1 mvnsite 2m 24s the patch passed +1 mvneclipse 0m 42s the patch passed +1 whitespace 0m 1s The patch has no whitespace issues. -1 findbugs 1m 53s hadoop-common-project/hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) -1 findbugs 0m 36s hadoop-hdfs-project/hadoop-hdfs-httpfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 1m 26s the patch passed -1 unit 7m 57s hadoop-common in the patch failed. +1 unit 1m 1s hadoop-hdfs-client in the patch passed. -1 unit 4m 10s hadoop-hdfs-httpfs in the patch failed. +1 asflicense 0m 26s The patch does not generate ASF License warnings. 60m 59s Reason Tests FindBugs module:hadoop-common-project/hadoop-common   Should org.apache.hadoop.fs.FileSystem$DirectoryEntries be a static inner class? At FileSystem.java:inner class? At FileSystem.java: [lines 1549-1564] FindBugs module:hadoop-hdfs-project/hadoop-hdfs-httpfs   new org.apache.hadoop.fs.http.server.FSOperations$FSListStatusBatch(String, byte[]) may expose internal representation by storing an externally mutable object into FSOperations$FSListStatusBatch.token At FSOperations.java:internal representation by storing an externally mutable object into FSOperations$FSListStatusBatch.token At FSOperations.java: [line 656] Failed junit tests hadoop.fs.TestHarFileSystem   hadoop.fs.TestFilterFileSystem   hadoop.fs.http.client.TestHttpFSFileSystemLocalFileSystem Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827858/HDFS-10823.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b5d5400ef631 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 / db6d243 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/diff-checkstyle-root.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-httpfs.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/16733/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-httpfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16733/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs-httpfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16733/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          WIP Patch attached. This depends on HDFS-10837 which isn't committed yet, so not marking this Patch Available.

          As described above, we have an unfortunate mismatch between the FS API and WebHDFS for implementing a generic iterator. This is why we have both a DirectoryListing and DirectoryEntries class that are slightly different. We could discard DL in favor of the more generic DE, though then the WebHDFS protocol would diverge a bit from the HDFS protobuf protocol. As it stands, if you are a downstream who wants to be generic to WebHDFS and HTTPFS, you need to treat remainingEntries like a boolean. This is kind of true anyway, since there could always be a concurrent writer adding more files to a listing which throws off the count.

          I also didn't implement the subclass tricks that would let us make the FileSystem additions protected. I annotated them Private with strong warnings in javadoc, which I hope is sufficient.

          Show
          andrew.wang Andrew Wang added a comment - WIP Patch attached. This depends on HDFS-10837 which isn't committed yet, so not marking this Patch Available. As described above, we have an unfortunate mismatch between the FS API and WebHDFS for implementing a generic iterator. This is why we have both a DirectoryListing and DirectoryEntries class that are slightly different. We could discard DL in favor of the more generic DE, though then the WebHDFS protocol would diverge a bit from the HDFS protobuf protocol. As it stands, if you are a downstream who wants to be generic to WebHDFS and HTTPFS, you need to treat remainingEntries like a boolean. This is kind of true anyway, since there could always be a concurrent writer adding more files to a listing which throws off the count. I also didn't implement the subclass tricks that would let us make the FileSystem additions protected. I annotated them Private with strong warnings in javadoc, which I hope is sufficient.
          Hide
          andrew.wang Andrew Wang added a comment -

          I asked Tucu about this offline, and he told me it's because HttpFS was designed to front any arbitrary FileSystem (but exposing WebHDFS). I haven't heard of anyone using it except as an HDFS gateway though, so I think this is an uncommon usecase. There's also an API impedance mismatch, since WebHDFS was designed for REST access to HDFS, not as a REST API for generic FileSystem access.

          But, without a large overhaul of the HttpFS codebase, I don't think it's something we're going to address here. I'll try the subclassing hack to at least make the new FileSystem method protected rather than public. Better alternatives welcomed.

          Show
          andrew.wang Andrew Wang added a comment - I asked Tucu about this offline, and he told me it's because HttpFS was designed to front any arbitrary FileSystem (but exposing WebHDFS). I haven't heard of anyone using it except as an HDFS gateway though, so I think this is an uncommon usecase. There's also an API impedance mismatch, since WebHDFS was designed for REST access to HDFS, not as a REST API for generic FileSystem access. But, without a large overhaul of the HttpFS codebase, I don't think it's something we're going to address here. I'll try the subclassing hack to at least make the new FileSystem method protected rather than public. Better alternatives welcomed.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Andrew for the nice investigation and thoughts here. I've only done HDFS-10481 with httpfs, which is too trivial a jira to understand deeply...

          If I understand this right, the main difference is for webhdfs in HDFS-10784, we can conveniently batch via NamenodeWebHdfsMethods#getDirectoryListing. But without such method for httpfs, we have to find our own way to batch. Sounds big....

          Could you explain why HttpFSServer is essentially doing webhdfs -> fs -> webhdfs? I hope we can find a place to hook up on webhdfs and batch from there.

          Sorry if I got it wrong somewhere, please feel free to correct me.

          Show
          xiaochen Xiao Chen added a comment - Thanks Andrew for the nice investigation and thoughts here. I've only done HDFS-10481 with httpfs, which is too trivial a jira to understand deeply... If I understand this right, the main difference is for webhdfs in HDFS-10784 , we can conveniently batch via NamenodeWebHdfsMethods#getDirectoryListing . But without such method for httpfs, we have to find our own way to batch. Sounds big.... Could you explain why HttpFSServer is essentially doing webhdfs -> fs -> webhdfs? I hope we can find a place to hook up on webhdfs and batch from there. Sorry if I got it wrong somewhere, please feel free to correct me.
          Hide
          andrew.wang Andrew Wang added a comment -

          Made a little progress breaking out the iterator from HDFS-10784. However, HttpFS uses the public FileSystem APIs, meaning we'd need to add a public API to FileSystem too (or pull some hack with subclassing). This is because HttpFSServer essentially does WebHDFS -> FileSystem -> WebHDFS. Not happy with this.

          Show
          andrew.wang Andrew Wang added a comment - Made a little progress breaking out the iterator from HDFS-10784 . However, HttpFS uses the public FileSystem APIs, meaning we'd need to add a public API to FileSystem too (or pull some hack with subclassing). This is because HttpFSServer essentially does WebHDFS -> FileSystem -> WebHDFS. Not happy with this.
          Hide
          andrew.wang Andrew Wang added a comment -

          Thinking about it a little more, what we'd like is a generic way of implementing an iterator over any Filesystem. Basically there are two concerns:

          Fetching the next batch:

          • We know that HDFS returns results in sorted order, so the "startAfter" parameter serves as a cursor.
          • S3 only supports startAfter on the first request, and subsequent requests need to pass an opaque "continuation token" (http://docs.aws.amazon.com/AmazonS3/latest/API/v2-RESTBucketGET.html). Takeaway: use an opaque, FS-specific token.
          • POSIX readdir doesn't give any provision for "next batch", since it just gives a stream of results. In this case, maybe we don't try to batch and return the whole listing in one shot.

          Knowing when we're out of entries:

          • HDFS has a "remainingEntries" field returned in the DirectoryListing.
          • S3 has a boolean "IsTruncated" field that does something similar.
          • POSIX readdir makes you call it until it returns null. Again kind of annoying, since we need to issue one more call to know we're done.

          Ignoring POSIX, I think we can implement a generic class like HDFS's DirectoryListing with a cursor and a boolean that will work for at least two important FileSystems. I'll poke around with this.

          Show
          andrew.wang Andrew Wang added a comment - Thinking about it a little more, what we'd like is a generic way of implementing an iterator over any Filesystem. Basically there are two concerns: Fetching the next batch: We know that HDFS returns results in sorted order, so the "startAfter" parameter serves as a cursor. S3 only supports startAfter on the first request, and subsequent requests need to pass an opaque "continuation token" ( http://docs.aws.amazon.com/AmazonS3/latest/API/v2-RESTBucketGET.html ). Takeaway: use an opaque, FS-specific token. POSIX readdir doesn't give any provision for "next batch", since it just gives a stream of results. In this case, maybe we don't try to batch and return the whole listing in one shot. Knowing when we're out of entries: HDFS has a "remainingEntries" field returned in the DirectoryListing. S3 has a boolean "IsTruncated" field that does something similar. POSIX readdir makes you call it until it returns null. Again kind of annoying, since we need to issue one more call to know we're done. Ignoring POSIX, I think we can implement a generic class like HDFS's DirectoryListing with a cursor and a boolean that will work for at least two important FileSystems. I'll poke around with this.
          Hide
          andrew.wang Andrew Wang added a comment -

          Looking at the HttpFS code, it's a bit tricky to do this. HttpFS uses the FileSystem interface, which doesn't have an appropriate listStatus method for implementing a cursor. So, we need an API like the one HDFS-9366 added to FileSystem, and then overriding implementations in the FileSystem subclasses we care about.

          Xiao Chen any thoughts here? This is turning out to be more work than I expected.

          Show
          andrew.wang Andrew Wang added a comment - Looking at the HttpFS code, it's a bit tricky to do this. HttpFS uses the FileSystem interface, which doesn't have an appropriate listStatus method for implementing a cursor. So, we need an API like the one HDFS-9366 added to FileSystem, and then overriding implementations in the FileSystem subclasses we care about. Xiao Chen any thoughts here? This is turning out to be more work than I expected.

            People

            • Assignee:
              andrew.wang Andrew Wang
              Reporter:
              andrew.wang Andrew Wang
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development