You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2020/10/01 06:56:35 UTC

[GitHub] [hadoop] mukund-thakur opened a new pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

mukund-thakur opened a new pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354


   Ran the new test using ap-south-1 bucket. 
   
   O/P- 
   `(ContractTestUtils.java:end(1847)) - Duration of listing 1000 files using listFiles() api with batch size of 10 including 10ms of processing time for each file: 12,223,848,028 nS
   2020-10-01 12:19:28,811 [JUnit-testMultiPagesListingPerformanceAndCorrectness] INFO  contract.ContractTestUtils (ContractTestUtils.java:end(1847)) - Duration of listing 1000 files using listStatus() api with batch size of 10 including 10ms of processing time for each file: 15,988,037,357 nS
   2020-10-01 12:19:41,050 [JUnit-testMultiPagesListingPerformanceAndCorrectness] INFO  contract.ContractTestUtils (ContractTestUtils.java:end(1847)) - Duration of listing 1000 files using listStatusIterator() api with batch size of 10 including 10ms of processing time for each file: 12,214,813,052 nS`
   
   From the logs we can see that time taken using listStatusIterator() and listFiles() matches and is less than listStatus().


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-703751138


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 48s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  1s |  |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |   |   0m  0s | [test4tests](test4tests) |  The patch appears to include 4 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   5m 42s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  29m 59s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  20m 40s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |  16m 57s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   2m 45s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m 24s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  20m  0s |  |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 16s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   2m 18s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m 17s |  |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 30s |  |  trunk passed  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 27s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 25s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  19m  0s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |  19m  0s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  17m  5s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |  17m  5s |  |  the patch passed  |
   | -0 :warning: |  checkstyle  |   2m 41s | [/diff-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/2/artifact/out/diff-checkstyle-root.txt) |  root: The patch generated 2 new + 19 unchanged - 0 fixed = 21 total (was 19)  |
   | +1 :green_heart: |  mvnsite  |   2m 20s |  |  the patch passed  |
   | -1 :x: |  whitespace  |   0m  0s | [/whitespace-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/2/artifact/out/whitespace-eol.txt) |  The patch has 6 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply  |
   | +1 :green_heart: |  shadedclient  |  15m  6s |  |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 14s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   2m 14s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   3m 44s |  |  the patch passed  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  |   9m 32s | [/patch-unit-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/2/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt) |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 33s |  |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 57s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 184m 19s |  |  |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.fs.contract.localfs.TestLocalFSContractGetFileStatus |
   |   | hadoop.fs.contract.rawlocal.TestRawlocalContractGetFileStatus |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2354 |
   | Optional Tests | dupname asflicense mvnsite markdownlint compile javac javadoc mvninstall unit shadedclient findbugs checkstyle |
   | uname | Linux e0a2860e5838 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 43b0c0b0546 |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/2/testReport/ |
   | Max. process+thread count | 2606 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/2/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on a change in pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on a change in pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#discussion_r500284497



##########
File path: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
##########
@@ -1107,6 +1107,7 @@ public Void next(final FileSystem fs, final Path p)
     }
 
     HdfsFileStatus[] partialListing = thisListing.getPartialListing();
+

Review comment:
       Please remove this before merging. Was just added to trigger hdfs tests by yetus.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-703639827


   > Looks good. Annoying about the return types which force you to do that wrapping/casting. Can't you just forcibly cast the return type of the inner iterator? after all, type erasure means all type info will be lost in the actual compiled binary. I'd prefer that as it will give you automatic passthrough of the IOStatistics stuff.
   
   This is not possible sadly. 
   
   > 
   > Add text to filesystem.md, something which:
   > 
   > * specifies the result is exactly the same a listStatus, provided no other caller updates the directory during the list
   > * declares that it's not atomic and performance implementations will page
   
   Done 
   
   > * and that if a path isn't there, that fact may not surface until next/hasNext...that is, we do lazy eval for all file IO
   >
   
   Actually this is not correct, we do throw FNFE if a path is not there, remember we have a check if the listing returns zeror results assuming the path as a directory, we fall back to file checks.
    
   > We need to similar new contract tests in AbstractContractGetFileStatusTest for all to use
   > 
   > * that in a dir with files and subdirectories, you get both returned in the listing
   > * that you can iterate through with next() to failure as well as hasNext/next, and get the same results
   > * listStatusIterator(file) returns the file
   > * listStatusIterator("/") gives you a listing of root (put that in AbstractContractRootDirectoryTest)
   > 
   Done
   > And two for changes partway through the iteration
   > 
   > * change the directory during a list to add/delete files
   > * deletes the actual path.
   > 
   > These tests can't assert on what will happen, and with paged IO aren't likely to pick up on changes...there just to show it can be done and pick up on any major issues with implementations.
   
   As discussed not adding these tests.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran merged pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
steveloughran merged pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-703751138






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-704489524


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   1m 30s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  1s |  |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |   |   0m  0s | [test4tests](test4tests) |  The patch appears to include 4 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   5m 50s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  37m 43s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  39m 37s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |  33m  4s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   4m 57s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   4m 48s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  31m 53s |  |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 22s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   3m 47s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m 56s |  |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   9m 40s |  |  trunk passed  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 36s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 33s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  37m 56s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |  37m 56s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  33m 25s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |  33m 25s |  |  the patch passed  |
   | -0 :warning: |  checkstyle  |   4m 50s | [/diff-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/4/artifact/out/diff-checkstyle-root.txt) |  root: The patch generated 2 new + 112 unchanged - 0 fixed = 114 total (was 112)  |
   | +1 :green_heart: |  mvnsite  |   4m 57s |  |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  22m 10s |  |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 41s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   5m 25s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |  12m 55s |  |  the patch passed  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  16m  0s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  |   3m 10s |  |  hadoop-hdfs-client in the patch passed.  |
   | +1 :green_heart: |  unit  |   2m 29s |  |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 34s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 323m 55s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2354 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux c0562df480cc 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 4347a5c9556 |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/4/testReport/ |
   | Max. process+thread count | 3117 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-aws U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/4/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-704228306


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 28s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |   |   0m  0s | [test4tests](test4tests) |  The patch appears to include 4 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   5m 37s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  23m 42s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  19m 48s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |  19m 34s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   2m 56s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m 24s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  19m 37s |  |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 15s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   2m 21s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m 17s |  |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 32s |  |  trunk passed  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 26s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 25s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  18m 53s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |  18m 53s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  17m  4s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |  17m  4s |  |  the patch passed  |
   | -0 :warning: |  checkstyle  |   3m 13s | [/diff-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/3/artifact/out/diff-checkstyle-root.txt) |  root: The patch generated 2 new + 19 unchanged - 0 fixed = 21 total (was 19)  |
   | +1 :green_heart: |  mvnsite  |   2m 25s |  |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  15m 39s |  |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 14s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   2m 17s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   3m 44s |  |  the patch passed  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   9m 35s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 35s |  |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 57s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 179m 55s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2354 |
   | Optional Tests | dupname asflicense mvnsite markdownlint compile javac javadoc mvninstall unit shadedclient findbugs checkstyle |
   | uname | Linux 1bb688443c0b 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 4347a5c9556 |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/3/testReport/ |
   | Max. process+thread count | 1459 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/3/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#discussion_r500254718



##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
##########
@@ -1520,7 +1521,8 @@ public static TreeScanResults treeWalk(FileSystem fs, Path path)
       while (true) {
         list.add(iterator.next());
       }
-    } catch (NoSuchElementException expected) {
+    } catch (NoSuchElementException | IllegalStateException expected) {

Review comment:
       as discussed: better to fix DirListingIterator to match RemoteIterator API; we can consider it's failure a bug

##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractGetFileStatusTest.java
##########
@@ -359,16 +357,39 @@ public void testListStatusFile() throws Throwable {
   public void testListStatusIteratorFile() throws Throwable {
     describe("test the listStatusIterator(path) on a file");
     Path f = touchf("listStItrFile");
+
     List<FileStatus> statusList = (List<FileStatus>) iteratorToList(
             getFileSystem().listStatusIterator(f));
-    assertEquals("size of file list returned", 1, statusList.size());
-    assertIsNamedFile(f, statusList.get(0));
+    validateListingForFile(f, statusList, false);
+
     List<FileStatus> statusList2 =
             (List<FileStatus>) iteratorToListThroughNextCallsAlone(
                     getFileSystem().listStatusIterator(f));
-    assertEquals("size of file list returned through next() calls",
-            1, statusList2.size());
-    assertIsNamedFile(f, statusList2.get(0));
+    validateListingForFile(f, statusList2, true);
+  }
+
+  /**
+   * Validate listing result for an input path which is file.
+   * @param f file.
+   * @param statusList list status of a file.
+   * @param nextCallAlone whether the listing generated just using
+   *                      next() calls.
+   */
+  private void validateListingForFile(Path f,
+                                      List<FileStatus> statusList,
+                                      boolean nextCallAlone) {
+    String msg = String.format("size of file list returned using %s should " +
+            "be 1", nextCallAlone ?
+            "next() calls alone" : "hasNext() and next() calls");
+    Assertions.assertThat(statusList)
+            .describedAs(msg)
+            .hasSize(1);
+    Assertions.assertThat(statusList.get(0).getPath().toString())
+            .describedAs("path returned should match with the input path")

Review comment:
       any reason not to leave both as Path and let Path.equals() to do the work? It compares URIs




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#discussion_r499720582



##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
##########
@@ -1453,6 +1453,52 @@ public static TreeScanResults treeWalk(FileSystem fs, Path path)
     return list;
   }
 
+  /**
+   * Convert a remote iterator over file status results into a list.
+   * The utility equivalents in commons collection and guava cannot be
+   * used here, as this is a different interface, one whose operators
+   * can throw IOEs.
+   * @param iterator input iterator
+   * @return the file status entries as a list.
+   * @throws IOException
+   */
+  public static List<? extends FileStatus> iteratorToList(
+          RemoteIterator<? extends FileStatus> iterator) throws IOException {
+    ArrayList<FileStatus> list = new ArrayList<>();
+    while (iterator.hasNext()) {
+      list.add(iterator.next());
+    }
+    return list;
+  }
+
+
+  /**
+   * Convert a remote iterator over file status results into a list.
+   * This uses {@link RemoteIterator#next()} calls only, expecting
+   * a raised {@link NoSuchElementException} exception to indicate that
+   * the end of the listing has been reached. This iteration strategy is
+   * designed to verify that the implementation of the remote iterator
+   * generates results and terminates consistently with the {@code hasNext/next}
+   * iteration. More succinctly "verifies that the {@code next()} operator
+   * isn't relying on {@code hasNext()} to always be called during an iteration.
+   * @param iterator input iterator
+   * @return the status entries as a list.
+   * @throws IOException IO problems
+   */
+  @SuppressWarnings("InfiniteLoopStatement")
+  public static List<? extends FileStatus> iteratorToListThroughNextCallsAlone(
+          RemoteIterator<? extends FileStatus> iterator) throws IOException {
+    ArrayList<FileStatus> list = new ArrayList<>();

Review comment:
       same: `List` as variable type
   
   Actually, it may make sense to type this method 
   ```
   RemoteIterator<T extends FileStatus>
   ```
   and have 
   ```
   List<T>
   ```
   and
   ```
   RemoteIterator<T>
   ```
    where appropriate

##########
File path: hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md
##########
@@ -294,6 +294,20 @@ any optimizations.
 The atomicity and consistency constraints are as for
 `listStatus(Path, PathFilter)`.
 
+### `RemoteIterator<FileStatus> listStatusIterator(Path p)`
+Return an iterator enumerating the `FileStatus` entries under
+a path. This is similar to `listStatus(Path)` except the fact that
+rather than returning an entire list, an iterator is returned.
+The result is exactly the same as listStatus, provided no other caller 

Review comment:
       use `listStatus()` with backticks

##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
##########
@@ -242,6 +243,13 @@ public void testSimpleRootListing() throws IOException {
             + "listStatus = " + listStatusResult
             + "listFiles = " + listFilesResult,
         fileList.size() <= statuses.length);
+    List<FileStatus> statusList = (List<FileStatus>) iteratorToList(
+            fs.listStatusIterator(root));
+    String listStatusItrRes = join(statusList, "\n");

Review comment:
       if you can have both sets of listing as collections, use Assertions.assertThat() as you've done earlier

##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractGetFileStatusTest.java
##########
@@ -322,6 +355,22 @@ public void testListStatusFile() throws Throwable {
     verifyStatusArrayMatchesFile(f, getFileSystem().listStatus(f));
   }
 
+  @Test
+  public void testListStatusIteratorFile() throws Throwable {
+    describe("test the listStatusIterator(path) on a file");
+    Path f = touchf("listStItrFile");
+    List<FileStatus> statusList = (List<FileStatus>) iteratorToList(
+            getFileSystem().listStatusIterator(f));
+    assertEquals("size of file list returned", 1, statusList.size());
+    assertIsNamedFile(f, statusList.get(0));
+    List<FileStatus> statusList2 =
+            (List<FileStatus>) iteratorToListThroughNextCallsAlone(
+                    getFileSystem().listStatusIterator(f));
+    assertEquals("size of file list returned through next() calls",

Review comment:
       AssertionsAssertThat to assert that the list size == 1




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on a change in pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on a change in pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#discussion_r500284497



##########
File path: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
##########
@@ -1107,6 +1107,7 @@ public Void next(final FileSystem fs, final Path p)
     }
 
     HdfsFileStatus[] partialListing = thisListing.getPartialListing();
+

Review comment:
       Please remove this before changing. Was just added to trigger hdfs tests by yetus.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-705034766


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   1m 11s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  1s |  |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |   |   0m  0s | [test4tests](test4tests) |  The patch appears to include 4 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   6m 16s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  36m 12s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  34m 39s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |  25m 17s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   3m 48s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   4m 14s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  29m 54s |  |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 43s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   3m 22s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m 46s |  |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   8m 45s |  |  trunk passed  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 42s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  1s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  34m 17s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |  34m 17s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  28m 38s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |  28m 38s |  |  the patch passed  |
   | -0 :warning: |  checkstyle  |   4m 11s | [/diff-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/artifact/out/diff-checkstyle-root.txt) |  root: The patch generated 1 new + 112 unchanged - 0 fixed = 113 total (was 112)  |
   | +1 :green_heart: |  mvnsite  |   4m 31s |  |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  18m 42s |  |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 26s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   3m 54s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   9m 35s |  |  the patch passed  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  |  14m 51s | [/patch-unit-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt) |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  |   3m 10s |  |  hadoop-hdfs-client in the patch passed.  |
   | +1 :green_heart: |  unit  |   2m  7s |  |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m  8s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 286m 52s |  |  |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.ipc.TestProtoBufRpc |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2354 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux 424cbd754c37 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 16aea11c945 |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/testReport/ |
   | Max. process+thread count | 3262 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-aws U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-703732717


   _stevel_ and that if a path isn't there, that fact may not surface until next/hasNext...that is, we do lazy eval for all file IO
   
   _mukund_ Actually this is not correct, we do throw FNFE if a path is not there, remember we have a check if the listing returns zeror results assuming the path as a directory, we fall back to file checks.
   
   I'd like the document to leave the option open of a fully async initial listing operation. so the people coding against it *now* are lined up for future filesystems that do that. Same for any other APIs we add, "late reporting of existence of paths/permissions" is something callers MUST expect


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-704828798


   yay! all the HDFS client tests passed first time! happy!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-704829250


   OK, +1 on this. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran removed a comment on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
steveloughran removed a comment on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-702371522


   Looks good. Annoying about the return types which force you to do that wrapping/casting. Can't you just forcibly cast the return type of the inner iterator? after all, type erasure means all type info will be lost in the actual compiled binary. I'd prefer that as it will give you automatic passthrough of the IOStatistics stuff.
   
   Add text to filesystem.md, something which: 
   
   * specifies the result is exactly the same a listStatus, provided no other caller updates the directory during the list
   * declares that it's not atomic and performance implementations will page
   * and that if a path isn't there, that fact may not surface until next/hasNext...that is, we do lazy eval for all file IO
   
   
   We need to similar new contract tests in AbstractContractGetFileStatusTest for all to use
   
   * that in a dir with files and subdirectories, you get both returned in the listing
   * that you can iterate through with next() to failure as well as hasNext/next, and get the same results
   * listStatusIterator(file) returns the file
   * listStatusIterator("/") gives you a listing of root (put that in AbstractContractRootDirectoryTest)
   
   And two for changes partway through the iteration
   
   * change the directory during a list to add/delete files
   * deletes the actual path.
   
   These tests can't assert on what will happen, and with paged IO aren't likely to pick up on changes...there just to show it can be done and pick up on any major issues with implementations.
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-704954857


   Thanks @steveloughran 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-705034766


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   1m 11s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  1s |  |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |   |   0m  0s | [test4tests](test4tests) |  The patch appears to include 4 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   6m 16s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  36m 12s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  34m 39s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |  25m 17s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   3m 48s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   4m 14s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  29m 54s |  |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 43s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   3m 22s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m 46s |  |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   8m 45s |  |  trunk passed  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 42s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  1s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  34m 17s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |  34m 17s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  28m 38s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |  28m 38s |  |  the patch passed  |
   | -0 :warning: |  checkstyle  |   4m 11s | [/diff-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/artifact/out/diff-checkstyle-root.txt) |  root: The patch generated 1 new + 112 unchanged - 0 fixed = 113 total (was 112)  |
   | +1 :green_heart: |  mvnsite  |   4m 31s |  |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  18m 42s |  |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 26s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   3m 54s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |   9m 35s |  |  the patch passed  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  |  14m 51s | [/patch-unit-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt) |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  |   3m 10s |  |  hadoop-hdfs-client in the patch passed.  |
   | +1 :green_heart: |  unit  |   2m  7s |  |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m  8s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 286m 52s |  |  |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.ipc.TestProtoBufRpc |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2354 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux 424cbd754c37 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 16aea11c945 |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/testReport/ |
   | Max. process+thread count | 3262 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-aws U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/5/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-704489524


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   1m 30s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  1s |  |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |   |   0m  0s | [test4tests](test4tests) |  The patch appears to include 4 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   5m 50s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  37m 43s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  39m 37s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  compile  |  33m  4s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  checkstyle  |   4m 57s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   4m 48s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  31m 53s |  |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 22s |  |  trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   3m 47s |  |  trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +0 :ok: |  spotbugs  |   1m 56s |  |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   9m 40s |  |  trunk passed  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 36s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 33s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  37m 56s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javac  |  37m 56s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  33m 25s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  javac  |  33m 25s |  |  the patch passed  |
   | -0 :warning: |  checkstyle  |   4m 50s | [/diff-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/4/artifact/out/diff-checkstyle-root.txt) |  root: The patch generated 2 new + 112 unchanged - 0 fixed = 114 total (was 112)  |
   | +1 :green_heart: |  mvnsite  |   4m 57s |  |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  22m 10s |  |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 41s |  |  the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1  |
   | +1 :green_heart: |  javadoc  |   5m 25s |  |  the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01  |
   | +1 :green_heart: |  findbugs  |  12m 55s |  |  the patch passed  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  16m  0s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  |   3m 10s |  |  hadoop-hdfs-client in the patch passed.  |
   | +1 :green_heart: |  unit  |   2m 29s |  |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 34s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 323m 55s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2354 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux c0562df480cc 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 4347a5c9556 |
   | Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/4/testReport/ |
   | Max. process+thread count | 3117 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-tools/hadoop-aws U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2354/4/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
   | Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on pull request #2354: HADOOP-17281 Implement FileSystem.listStatusIterator() in S3AFileSystem

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #2354:
URL: https://github.com/apache/hadoop/pull/2354#issuecomment-702371522


   Looks good. Annoying about the return types which force you to do that wrapping/casting. Can't you just forcibly cast the return type of the inner iterator? after all, type erasure means all type info will be lost in the actual compiled binary. I'd prefer that as it will give you automatic passthrough of the IOStatistics stuff.
   
   Add text to filesystem.md, something which: 
   
   * specifies the result is exactly the same a listStatus, provided no other caller updates the directory during the list
   * declares that it's not atomic and performance implementations will page
   * and that if a path isn't there, that fact may not surface until next/hasNext...that is, we do lazy eval for all file IO
   
   
   We need to similar new contract tests in AbstractContractGetFileStatusTest for all to use
   
   * that in a dir with files and subdirectories, you get both returned in the listing
   * that you can iterate through with next() to failure as well as hasNext/next, and get the same results
   * listStatusIterator(file) returns the file
   * listStatusIterator("/") gives you a listing of root (put that in AbstractContractRootDirectoryTest)
   
   And two for changes partway through the iteration
   
   * change the directory during a list to add/delete files
   * deletes the actual path.
   
   These tests can't assert on what will happen, and with paged IO aren't likely to pick up on changes...there just to show it can be done and pick up on any major issues with implementations.
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org