Details

    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      With an empty s3 bucket and run

      $ hadoop fs -D... -ls s3a://hdfs-s3a-test/
      
      15/05/04 15:21:34 WARN util.NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
      ls: `s3a://hdfs-s3a-test/': No such file or directory
      
      
      1. HADOOP-11918.004.patch
        2 kB
        Lei (Eddy) Xu
      2. HADOOP-11918-003.patch
        2 kB
        Thomas Demoor
      3. HADOOP-11918-002.patch
        3 kB
        Pieter Reuse
      4. HADOOP-11918.001.patch
        3 kB
        Lei (Eddy) Xu
      5. HADOOP-11918.000.patch
        2 kB
        Lei (Eddy) Xu

        Issue Links

          Activity

          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          Thanks a lot for the reviews, Chris Nauroth

          Show
          eddyxu Lei (Eddy) Xu added a comment - Thanks a lot for the reviews, Chris Nauroth
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2360 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2360/)
          HADOOP-11918. Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641)

          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2360 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2360/ ) HADOOP-11918 . Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2387 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2387/)
          HADOOP-11918. Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641)

          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2387 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2387/ ) HADOOP-11918 . Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #449 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/449/)
          HADOOP-11918. Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #449 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/449/ ) HADOOP-11918 . Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #1182 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1182/)
          HADOOP-11918. Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641)

          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #1182 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1182/ ) HADOOP-11918 . Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #420 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/420/)
          HADOOP-11918. Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641)

          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #420 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/420/ ) HADOOP-11918 . Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #443 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/443/)
          HADOOP-11918. Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #443 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/443/ ) HADOOP-11918 . Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #8522 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8522/)
          HADOOP-11918. Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641)

          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8522 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8522/ ) HADOOP-11918 . Listing an empty s3a root directory throws FileNotFound. Contributed by Lei (Eddy) Xu. (cnauroth: rev 7fe521b1dd49f81ae325f78cf531cfff15be6641) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for the patch. I have committed this to trunk and branch-2. Lei (Eddy) Xu, thank you for contributing the patch. Steve Loughran and Thomas Demoor, thank you for your code reviews.

          Show
          cnauroth Chris Nauroth added a comment - +1 for the patch. I have committed this to trunk and branch-2. Lei (Eddy) Xu , thank you for contributing the patch. Steve Loughran and Thomas Demoor , thank you for your code reviews.
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          The test failure is not relevant.

          Show
          eddyxu Lei (Eddy) Xu added a comment - The test failure is not relevant.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



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



          Reason Tests
          Failed unit tests hadoop.fs.sftp.TestSFTPFileSystem



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

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 18m 14s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 59s There were no new javac warning messages. +1 javadoc 10m 7s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 28s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 27s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 2m 41s The patch does not introduce any new Findbugs (version 3.0.0) warnings. -1 common tests 22m 37s Tests failed in hadoop-common. +1 tools/hadoop tests 0m 14s Tests passed in hadoop-aws.     65m 47s   Reason Tests Failed unit tests hadoop.fs.sftp.TestSFTPFileSystem Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12761985/HADOOP-11918.004.patch Optional Tests javac unit findbugs checkstyle javadoc git revision trunk / 1f707ec hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7694/artifact/patchprocess/testrun_hadoop-common.txt hadoop-aws test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7694/artifact/patchprocess/testrun_hadoop-aws.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7694/testReport/ Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7694/console This message was automatically generated.
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          Thanks a lot for helping out on this patch, Thomas Demoor.

          I updated the patch based on your changes, and addressed Chris Nauroth and Steve Loughran's comments.

          Please take another look. Thanks a lot, everyone.

          Show
          eddyxu Lei (Eddy) Xu added a comment - Thanks a lot for helping out on this patch, Thomas Demoor . I updated the patch based on your changes, and addressed Chris Nauroth and Steve Loughran 's comments. Please take another look. Thanks a lot, everyone.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          ..other than that though, I like it

          Show
          stevel@apache.org Steve Loughran added a comment - ..other than that though, I like it
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Better to use ContractTestUtils.assertDeleted() for the deletion operation; it verifies the output, that the file has actually gone away, and includes directory listings on failures so that you can post-mortem the failures

          Show
          stevel@apache.org Steve Loughran added a comment - Better to use ContractTestUtils.assertDeleted() for the deletion operation; it verifies the output, that the file has actually gone away, and includes directory listings on failures so that you can post-mortem the failures
          Hide
          cnauroth Chris Nauroth added a comment -

          Thank you, Lei (Eddy) Xu and Thomas Demoor. Patch v003 looks good to me aside from a few minor nit-picks.

              for (FileStatus status : statuses) {
               assertEquals("Could not remove files from root", true,
                   fs.delete(status.getPath(), true));
              }
          

          Indentation is slightly off for the assertEquals line inside the loop.

                  if (LOG.isDebugEnabled()) {
                    LOG.debug("Found root directory");
                  }
          

          There is no need to check isDebugEnabled, because there is no expensive string concatenation logic to build the log message. It's just a string literal.

          I ran TestS3AContractRootDir against my own testing S3 bucket, and it passed. It appears earlier feedback has been addressed too.

          I'll be +1 after the nit-picks are fixed. Steve Loughran, please let us know if you have any other thoughts on the patch.

          Show
          cnauroth Chris Nauroth added a comment - Thank you, Lei (Eddy) Xu and Thomas Demoor . Patch v003 looks good to me aside from a few minor nit-picks. for (FileStatus status : statuses) { assertEquals( "Could not remove files from root" , true , fs.delete(status.getPath(), true )); } Indentation is slightly off for the assertEquals line inside the loop. if (LOG.isDebugEnabled()) { LOG.debug( "Found root directory" ); } There is no need to check isDebugEnabled , because there is no expensive string concatenation logic to build the log message. It's just a string literal. I ran TestS3AContractRootDir against my own testing S3 bucket, and it passed. It appears earlier feedback has been addressed too. I'll be +1 after the nit-picks are fixed. Steve Loughran , please let us know if you have any other thoughts on the patch.
          Hide
          hadoopqa Hadoop QA added a comment -



          +1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 17m 39s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
          +1 javac 7m 44s There were no new javac warning messages.
          +1 javadoc 10m 3s There were no new javadoc warning messages.
          +1 release audit 0m 25s The applied patch does not increase the total number of release audit warnings.
          +1 checkstyle 1m 27s There were no new checkstyle issues.
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 29s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 2m 37s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 common tests 22m 57s Tests passed in hadoop-common.
          +1 tools/hadoop tests 0m 14s Tests passed in hadoop-aws.
              65m 11s  



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

          This message was automatically generated.

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

          I uploaded patch-003, which is what we are running. Feel free to rework as you see fit.

          In this patch Pieter Reuse has already addressed Steve's request to make the test apply to all filesystems by adding it to AbstractContractRootDirectoryTest. The test deletes all files in the root dir to assure it is empty before we test the listing code. Therefore, extra care was taken via skipIfUnsupported() similar to the other tests in this class.

          Show
          Thomas Demoor Thomas Demoor added a comment - I uploaded patch-003, which is what we are running. Feel free to rework as you see fit. In this patch Pieter Reuse has already addressed Steve's request to make the test apply to all filesystems by adding it to AbstractContractRootDirectoryTest . The test deletes all files in the root dir to assure it is empty before we test the listing code. Therefore, extra care was taken via skipIfUnsupported() similar to the other tests in this class.
          Hide
          cnauroth Chris Nauroth added a comment -

          Lei (Eddy) Xu, thank you very much!

          Show
          cnauroth Chris Nauroth added a comment - Lei (Eddy) Xu , thank you very much!
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          Chris Nauroth, Steve Loughran and Thomas Demoor. Sorry for late reply. This JIRA seems slipped from my inbox...

          I will pick up it this week and address Thomas and Steve's comments. Will post update soon..

          Show
          eddyxu Lei (Eddy) Xu added a comment - Chris Nauroth , Steve Loughran and Thomas Demoor . Sorry for late reply. This JIRA seems slipped from my inbox... I will pick up it this week and address Thomas and Steve's comments. Will post update soon..
          Hide
          cnauroth Chris Nauroth added a comment -

          I'd like to get this fix committed into the codebase. Lei (Eddy) Xu, are you able to respond to the last round of comments from Thomas and Steve? If you need to post a new patch, I'd be happy to help with code review and testing too. Thanks!

          Show
          cnauroth Chris Nauroth added a comment - I'd like to get this fix committed into the codebase. Lei (Eddy) Xu , are you able to respond to the last round of comments from Thomas and Steve? If you need to post a new patch, I'd be happy to help with code review and testing too. Thanks!
          Hide
          stevel@apache.org Steve Loughran added a comment -
          1. I like the solution in the source here
          2. I agree with thomas's comments about the tests. Specifically, we really need this same test to be run against all filesystems which allow operations against the root dir, otherwise we won't catch regressions in things like HDFS. Yes, I know an rm of / is traumatic, which is why we have a s AbstractContractRootDirectoryTest base class for only the brave to implement
          Show
          stevel@apache.org Steve Loughran added a comment - I like the solution in the source here I agree with thomas's comments about the tests. Specifically, we really need this same test to be run against all filesystems which allow operations against the root dir, otherwise we won't catch regressions in things like HDFS. Yes, I know an rm of / is traumatic, which is why we have a s AbstractContractRootDirectoryTest base class for only the brave to implement
          Hide
          hadoopqa Hadoop QA added a comment -



          +1 overall



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



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

          This message was automatically generated.

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

          We preferred the test approach of this patch over that of HADOOP-11742. However, as other tests sometimes leave artifacts behind, the test had transient problems and we propose to fix this by forcing an empty root in the test case for this patch , as Pieter Reuse pointed out. Lei (Eddy) Xu, did you see the same behaviour?

          Show
          Thomas Demoor Thomas Demoor added a comment - We preferred the test approach of this patch over that of HADOOP-11742 . However, as other tests sometimes leave artifacts behind, the test had transient problems and we propose to fix this by forcing an empty root in the test case for this patch , as Pieter Reuse pointed out. Lei (Eddy) Xu , did you see the same behaviour?
          Hide
          PieterReuse Pieter Reuse added a comment -

          I verified this patch with the test output below.

          However, running all the JUnit tests has the effect of spurious directories in the test bucket. This means that the bucket wasn't empty during the TestS3AFileSystem.testListRootDirectory, while the bug only surfaces on an empty directory.

          Because of this, I have added four lines to the test cleaning the entire bucket before doing the actual assert.

          -------------------------------------------------------
          T E S T S
          -------------------------------------------------------
          Running org.apache.hadoop.fs.contract.s3a.TestS3AContractSeek
          Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 22.706 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractSeek
          Running org.apache.hadoop.fs.contract.s3a.TestS3AContractDelete
          Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.564 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractDelete
          Running org.apache.hadoop.fs.contract.s3a.TestS3AContractOpen
          Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 10.598 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractOpen
          Running org.apache.hadoop.fs.contract.s3a.TestS3AContractRootDir
          Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 7.443 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractRootDir
          Running org.apache.hadoop.fs.contract.s3a.TestS3AContractCreate
          Tests run: 6, Failures: 0, Errors: 0, Skipped: 3, Time elapsed: 10.286 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractCreate
          Running org.apache.hadoop.fs.contract.s3a.TestS3AContractMkdir
          Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.942 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractMkdir
          Running org.apache.hadoop.fs.contract.s3a.TestS3AContractRename
          Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 17.752 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractRename
          Running org.apache.hadoop.fs.s3a.TestS3AFastOutputStream
          Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.564 sec - in org.apache.hadoop.fs.s3a.TestS3AFastOutputStream
          Running org.apache.hadoop.fs.s3a.TestS3ABlocksize
          Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 4.872 sec - in org.apache.hadoop.fs.s3a.TestS3ABlocksize
          Running org.apache.hadoop.fs.s3a.TestS3AFileSystemContract
          Tests run: 43, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 91.585 sec - in org.apache.hadoop.fs.s3a.TestS3AFileSystemContract
          Running org.apache.hadoop.fs.s3a.TestS3AConfiguration
          Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.948 sec - in org.apache.hadoop.fs.s3a.TestS3AConfiguration
          Running org.apache.hadoop.fs.s3a.scale.TestS3ADeleteManyFiles
          Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 533.853 sec - in org.apache.hadoop.fs.s3a.scale.TestS3ADeleteManyFiles
          Running org.apache.hadoop.fs.s3a.TestS3AFileSystem
          Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.476 sec - in org.apache.hadoop.fs.s3a.TestS3AFileSystem

          Results :

          Tests run: 101, Failures: 0, Errors: 0, Skipped: 3

          [INFO] ------------------------------------------------------------------------
          [INFO] BUILD SUCCESS
          [INFO] ------------------------------------------------------------------------
          [INFO] Total time: 12:34.020s
          [INFO] Finished at: Thu Jun 18 15:57:16 CEST 2015
          [INFO] Final Memory: 25M/419M
          [INFO] ------------------------------------------------------------------------

          Show
          PieterReuse Pieter Reuse added a comment - I verified this patch with the test output below. However , running all the JUnit tests has the effect of spurious directories in the test bucket. This means that the bucket wasn't empty during the TestS3AFileSystem.testListRootDirectory, while the bug only surfaces on an empty directory. Because of this, I have added four lines to the test cleaning the entire bucket before doing the actual assert. ------------------------------------------------------- T E S T S ------------------------------------------------------- Running org.apache.hadoop.fs.contract.s3a.TestS3AContractSeek Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 22.706 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractSeek Running org.apache.hadoop.fs.contract.s3a.TestS3AContractDelete Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.564 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractDelete Running org.apache.hadoop.fs.contract.s3a.TestS3AContractOpen Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 10.598 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractOpen Running org.apache.hadoop.fs.contract.s3a.TestS3AContractRootDir Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 7.443 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractRootDir Running org.apache.hadoop.fs.contract.s3a.TestS3AContractCreate Tests run: 6, Failures: 0, Errors: 0, Skipped: 3, Time elapsed: 10.286 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractCreate Running org.apache.hadoop.fs.contract.s3a.TestS3AContractMkdir Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.942 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractMkdir Running org.apache.hadoop.fs.contract.s3a.TestS3AContractRename Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 17.752 sec - in org.apache.hadoop.fs.contract.s3a.TestS3AContractRename Running org.apache.hadoop.fs.s3a.TestS3AFastOutputStream Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.564 sec - in org.apache.hadoop.fs.s3a.TestS3AFastOutputStream Running org.apache.hadoop.fs.s3a.TestS3ABlocksize Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 4.872 sec - in org.apache.hadoop.fs.s3a.TestS3ABlocksize Running org.apache.hadoop.fs.s3a.TestS3AFileSystemContract Tests run: 43, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 91.585 sec - in org.apache.hadoop.fs.s3a.TestS3AFileSystemContract Running org.apache.hadoop.fs.s3a.TestS3AConfiguration Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.948 sec - in org.apache.hadoop.fs.s3a.TestS3AConfiguration Running org.apache.hadoop.fs.s3a.scale.TestS3ADeleteManyFiles Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 533.853 sec - in org.apache.hadoop.fs.s3a.scale.TestS3ADeleteManyFiles Running org.apache.hadoop.fs.s3a.TestS3AFileSystem Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.476 sec - in org.apache.hadoop.fs.s3a.TestS3AFileSystem Results : Tests run: 101, Failures: 0, Errors: 0, Skipped: 3 [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 12:34.020s [INFO] Finished at: Thu Jun 18 15:57:16 CEST 2015 [INFO] Final Memory: 25M/419M [INFO] ------------------------------------------------------------------------
          Hide
          thodemoor Thomas Demoor added a comment -
          Show
          thodemoor Thomas Demoor added a comment - Sorry, here you go: https://issues.apache.org/jira/browse/HADOOP-11742
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          Hi, Thomas Demoor

          Are you pointing to another JIRA? HADOOP-11918 is this one.

          Thanks!

          Show
          eddyxu Lei (Eddy) Xu added a comment - Hi, Thomas Demoor Are you pointing to another JIRA? HADOOP-11918 is this one. Thanks!
          Hide
          thodemoor Thomas Demoor added a comment -

          Not yet, focusing on HADOOP-9565 first.

          Show
          thodemoor Thomas Demoor added a comment - Not yet, focusing on HADOOP-9565 first.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Thomas: have you tested it?

          Show
          stevel@apache.org Steve Loughran added a comment - Thomas: have you tested it?
          Hide
          thodemoor Thomas Demoor added a comment -

          Same bug, same fix conceptually but I prefer the code in HADOOP-11918

          Show
          thodemoor Thomas Demoor added a comment - Same bug, same fix conceptually but I prefer the code in HADOOP-11918
          Hide
          hadoopqa Hadoop QA added a comment -



          +1 overall



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



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

          This message was automatically generated.

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

          Hi, Steve Loughran Thanks a lot for the suggestions.

          I updated the patch to use the similar approach used in TestS3ABlocksize.

          Show
          eddyxu Lei (Eddy) Xu added a comment - Hi, Steve Loughran Thanks a lot for the suggestions. I updated the patch to use the similar approach used in TestS3ABlocksize .
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Could we have a patch that doesn't extend the Junit 3 FS contract test? its slow enough as it is. Add another Abstract contract test

          Show
          stevel@apache.org Steve Loughran added a comment - Could we have a patch that doesn't extend the Junit 3 FS contract test? its slow enough as it is. Add another Abstract contract test
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 15m 9s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
          -1 patch 0m 10s The patch command could not apply the patch.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12730309/HADOOP-11918.000.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / d701acc
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6481/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 15m 9s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. -1 patch 0m 10s The patch command could not apply the patch. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12730309/HADOOP-11918.000.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / d701acc Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6481/console This message was automatically generated.
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          If the input directory is root and there is no sub-files/directories found, returns a fake root directory FileStatus instead. Added a test to enforce it: the test would throw FileNotFoundException without this fix.

          Show
          eddyxu Lei (Eddy) Xu added a comment - If the input directory is root and there is no sub-files/directories found, returns a fake root directory FileStatus instead. Added a test to enforce it: the test would throw FileNotFoundException without this fix.

            People

            • Assignee:
              eddyxu Lei (Eddy) Xu
              Reporter:
              eddyxu Lei (Eddy) Xu
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development