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

Listing encryption zones fails when deleting a EZ that is on a snapshotted directory

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.6.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: hdfs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      If a EZ directory is under a snapshotable directory, and a snapshot has been taking, then if this EZ is permanently deleted, it causes hdfs crypto listZones command to fail without showing any of the still available zones.

      This happens only after the EZ is removed from Trash folder. For example, considering /test-snap folder is snapshotable and there is already an snapshot for it:

      $ hdfs crypto -listZones
      /user/systest           my-key
      /test-snap/EZ-1       my-key
      
      $ hdfs dfs -rmr /test-snap/EZ-1
      INFO fs.TrashPolicyDefault: Moved: 'hdfs://ns1/test-snap/EZ-1' to trash at: hdfs://ns1/user/hdfs/.Trash/Current/test-snap/EZ-1
      
      $ hdfs crypto -listZones
      /user/systest           my-key
      /user/hdfs/.Trash/Current/test-snap/EZ-1  my-key 
      
      $ hdfs dfs -rmr /user/hdfs/.Trash/Current/test-snap/EZ-1
      Deleted /user/hdfs/.Trash/Current/test-snap/EZ-1
      
      $ hdfs crypto -listZones
      RemoteException: Absolute path required
      

      Once this error happens, hdfs crypto -listZones only works again if we remove the snapshot:

      $ hdfs dfs -deleteSnapshot /test-snap snap1
      $ hdfs crypto -listZones
      /user/systest           my-key
      

      If we instead delete the EZ using skipTrash option, hdfs crypto -listZones does not break:

      $ hdfs crypto -listZones
      /user/systest           my-key
      /test-snap/EZ-2  my-key
      
      $ hdfs dfs -rmr -skipTrash /test-snap/EZ-2
      Deleted /test-snap/EZ-2
      
      $ hdfs crypto -listZones
      /user/systest           my-key
      

      The different behaviour seems to be because when removing the EZ trash folder, it's related INode is left with no parent INode. This causes EncryptionZoneManager.listEncryptionZones to throw the seen error, when trying to resolve the inodes in the given path.

      Am proposing a patch that fixes this issue by simply performing an additional check on EncryptionZoneManager.listEncryptionZones for the case an inode has no parent, so that it would be skipped on the list without trying to resolve it. Feedback on the proposal is appreciated.

      1. HDFS-11197-1.patch
        6 kB
        Wellington Chevreuil
      2. HDFS-11197-2.patch
        7 kB
        Wellington Chevreuil
      3. HDFS-11197-3.patch
        8 kB
        Wellington Chevreuil
      4. HDFS-11197-4.patch
        8 kB
        Wellington Chevreuil
      5. HDFS-11197-5.patch
        7 kB
        Wellington Chevreuil
      6. HDFS-11197-6.patch
        7 kB
        Wellington Chevreuil
      7. HDFS-11197-7.patch
        14 kB
        Wellington Chevreuil

        Activity

        Hide
        wchevreuil Wellington Chevreuil added a comment -

        First patch proposal.

        Show
        wchevreuil Wellington Chevreuil added a comment - First patch proposal.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 11s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 6m 45s trunk passed
        +1 compile 0m 45s trunk passed
        +1 checkstyle 0m 25s trunk passed
        +1 mvnsite 0m 50s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        -1 findbugs 1m 40s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings.
        +1 javadoc 0m 39s trunk passed
        +1 mvninstall 0m 45s the patch passed
        +1 compile 0m 42s the patch passed
        +1 javac 0m 42s the patch passed
        -0 checkstyle 0m 22s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 5 unchanged - 0 fixed = 9 total (was 5)
        +1 mvnsite 0m 50s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 46s the patch passed
        +1 javadoc 0m 36s the patch passed
        -1 unit 61m 45s hadoop-hdfs in the patch failed.
        -1 asflicense 0m 18s The patch generated 1 ASF License warnings.
        79m 54s



        Reason Tests
        Failed junit tests hadoop.hdfs.TestEncryptionZonesWithKMS
          hadoop.hdfs.TestEncryptionZones



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11197
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841314/HDFS-11197-1.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 241260205bf3 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / e0fa492
        Default Java 1.8.0_111
        findbugs v3.0.0
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17732/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17732/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/17732/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17732/testReport/
        asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/17732/artifact/patchprocess/patch-asflicense-problems.txt
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17732/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 45s trunk passed +1 compile 0m 45s trunk passed +1 checkstyle 0m 25s trunk passed +1 mvnsite 0m 50s trunk passed +1 mvneclipse 0m 12s trunk passed -1 findbugs 1m 40s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings. +1 javadoc 0m 39s trunk passed +1 mvninstall 0m 45s the patch passed +1 compile 0m 42s the patch passed +1 javac 0m 42s the patch passed -0 checkstyle 0m 22s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 5 unchanged - 0 fixed = 9 total (was 5) +1 mvnsite 0m 50s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 46s the patch passed +1 javadoc 0m 36s the patch passed -1 unit 61m 45s hadoop-hdfs in the patch failed. -1 asflicense 0m 18s The patch generated 1 ASF License warnings. 79m 54s Reason Tests Failed junit tests hadoop.hdfs.TestEncryptionZonesWithKMS   hadoop.hdfs.TestEncryptionZones Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11197 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841314/HDFS-11197-1.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 241260205bf3 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / e0fa492 Default Java 1.8.0_111 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17732/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17732/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/17732/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17732/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/17732/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17732/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        wchevreuil Wellington Chevreuil added a comment -

        Second patch with the checkstyle issues fixed for TestEncryptionZoneManager class. Apologies for not had ran checkstyles locally previously, had succesfully ran it now.

        Also changed to comply the situation from the failed tests org.apache.hadoop.hdfs.TestEncryptionZones and org.apache.hadoop.hdfs.TestEncryptionZonesWithKMS, where the EZ is actually the root folder.

        Added a 3rd unit test to cover this case. Tests are passing locally, now.

        Show
        wchevreuil Wellington Chevreuil added a comment - Second patch with the checkstyle issues fixed for TestEncryptionZoneManager class. Apologies for not had ran checkstyles locally previously, had succesfully ran it now. Also changed to comply the situation from the failed tests org.apache.hadoop.hdfs.TestEncryptionZones and org.apache.hadoop.hdfs.TestEncryptionZonesWithKMS , where the EZ is actually the root folder. Added a 3rd unit test to cover this case. Tests are passing locally, now.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 13s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 8m 40s trunk passed
        +1 compile 0m 56s trunk passed
        +1 checkstyle 0m 31s trunk passed
        +1 mvnsite 1m 5s trunk passed
        +1 mvneclipse 0m 15s trunk passed
        -1 findbugs 1m 56s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings.
        +1 javadoc 0m 41s trunk passed
        +1 mvninstall 0m 53s the patch passed
        +1 compile 0m 51s the patch passed
        +1 javac 0m 51s the patch passed
        +1 checkstyle 0m 25s the patch passed
        +1 mvnsite 0m 58s the patch passed
        +1 mvneclipse 0m 12s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 2m 0s the patch passed
        +1 javadoc 0m 42s the patch passed
        -1 unit 64m 56s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 20s The patch does not generate ASF License warnings.
        86m 55s



        Reason Tests
        Failed junit tests hadoop.hdfs.TestEncryptionZonesWithKMS
          hadoop.hdfs.TestEncryptionZones



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11197
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841369/HDFS-11197-2.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux c6180276ac43 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 19f373a
        Default Java 1.8.0_111
        findbugs v3.0.0
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17735/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/17735/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17735/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17735/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 8m 40s trunk passed +1 compile 0m 56s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 1m 5s trunk passed +1 mvneclipse 0m 15s trunk passed -1 findbugs 1m 56s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings. +1 javadoc 0m 41s trunk passed +1 mvninstall 0m 53s the patch passed +1 compile 0m 51s the patch passed +1 javac 0m 51s the patch passed +1 checkstyle 0m 25s the patch passed +1 mvnsite 0m 58s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 0s the patch passed +1 javadoc 0m 42s the patch passed -1 unit 64m 56s hadoop-hdfs in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 86m 55s Reason Tests Failed junit tests hadoop.hdfs.TestEncryptionZonesWithKMS   hadoop.hdfs.TestEncryptionZones Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11197 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841369/HDFS-11197-2.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c6180276ac43 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 19f373a Default Java 1.8.0_111 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17735/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/17735/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17735/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17735/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        wchevreuil Wellington Chevreuil added a comment -

        For some reason, the changes I mentioned on my previous comment were not included in the last patch. Including a 3rd patch with the fix for the test and the additional unit test for checking this condition on TestEncryptionZoneManager.

        The findbugs warning does not seem related to any of the changes from this patch.

        Show
        wchevreuil Wellington Chevreuil added a comment - For some reason, the changes I mentioned on my previous comment were not included in the last patch. Including a 3rd patch with the fix for the test and the additional unit test for checking this condition on TestEncryptionZoneManager. The findbugs warning does not seem related to any of the changes from this patch.
        Hide
        wchevreuil Wellington Chevreuil added a comment - - edited

        Just removing the extra logging message from EncryptionZoneManager.listEncryptionZones I had left on the previous patch.

        Show
        wchevreuil Wellington Chevreuil added a comment - - edited Just removing the extra logging message from EncryptionZoneManager.listEncryptionZones I had left on the previous patch.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 11s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 7m 12s trunk passed
        +1 compile 0m 45s trunk passed
        +1 checkstyle 0m 25s trunk passed
        +1 mvnsite 0m 51s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        -1 findbugs 1m 41s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings.
        +1 javadoc 0m 39s trunk passed
        +1 mvninstall 0m 54s the patch passed
        +1 compile 0m 53s the patch passed
        +1 javac 0m 53s the patch passed
        +1 checkstyle 0m 24s the patch passed
        +1 mvnsite 0m 57s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
        +1 findbugs 1m 55s the patch passed
        +1 javadoc 0m 40s the patch passed
        +1 unit 61m 40s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 18s The patch does not generate ASF License warnings.
        81m 7s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11197
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841447/HDFS-11197-4.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 837aca05ca18 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / c87b3a4
        Default Java 1.8.0_111
        findbugs v3.0.0
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17741/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/17741/artifact/patchprocess/whitespace-eol.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17741/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17741/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 12s trunk passed +1 compile 0m 45s trunk passed +1 checkstyle 0m 25s trunk passed +1 mvnsite 0m 51s trunk passed +1 mvneclipse 0m 12s trunk passed -1 findbugs 1m 41s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings. +1 javadoc 0m 39s trunk passed +1 mvninstall 0m 54s the patch passed +1 compile 0m 53s the patch passed +1 javac 0m 53s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 0m 57s the patch passed +1 mvneclipse 0m 10s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 1m 55s the patch passed +1 javadoc 0m 40s the patch passed +1 unit 61m 40s hadoop-hdfs in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 81m 7s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11197 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841447/HDFS-11197-4.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 837aca05ca18 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c87b3a4 Default Java 1.8.0_111 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17741/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/17741/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17741/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17741/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        xiaochen Xiao Chen added a comment -

        Thanks a lot Wellington Chevreuil for reporting the issue and posting the fix!

        Idea looks good, some minor comments:

        • When possible, assertEquals is preferred over assertTrue, since they clearly says what's expected and what's actual when failing
        • No need to catch and exception and Assert.fail - just let it throw and junit will fail the tests with this exception
        • The mocks are good to validate the change. But also feeling worth having the scenario you mentioned in the description added as a unit test. Maybe some additional steps at the end of TestEncryptionZones#testSnapshotsOnEncryptionZones, or a new test case. Your call.
        • Trivially comment you could use static imports to save the explicit class names (e.g. Assert., Mockito.) on each call.
        • please fix the whitespace
        • findbugs is unrelated to this change. Seems to be HDFS-10930
        Show
        xiaochen Xiao Chen added a comment - Thanks a lot Wellington Chevreuil for reporting the issue and posting the fix! Idea looks good, some minor comments: When possible, assertEquals is preferred over assertTrue, since they clearly says what's expected and what's actual when failing No need to catch and exception and Assert.fail - just let it throw and junit will fail the tests with this exception The mocks are good to validate the change. But also feeling worth having the scenario you mentioned in the description added as a unit test. Maybe some additional steps at the end of TestEncryptionZones#testSnapshotsOnEncryptionZones , or a new test case. Your call. Trivially comment you could use static imports to save the explicit class names (e.g. Assert. , Mockito. ) on each call. please fix the whitespace findbugs is unrelated to this change. Seems to be HDFS-10930
        Hide
        wchevreuil Wellington Chevreuil added a comment - - edited

        Hi Xiao Chen. Thanks for the review! I'll apply the suggested, and generate another patch. For the the scenario from TestEncryptionZones#testSnapshotsOnEncryptionZones, I had already added a new test case TestEncryptionZoneManager#testListEncryptionZonesForRoot to cover similar situation at TestEncryptionZoneManager level.

        Do you think it's worth add these other tests to TestEncryptionZones also?

        Show
        wchevreuil Wellington Chevreuil added a comment - - edited Hi Xiao Chen . Thanks for the review! I'll apply the suggested, and generate another patch. For the the scenario from TestEncryptionZones#testSnapshotsOnEncryptionZones , I had already added a new test case TestEncryptionZoneManager#testListEncryptionZonesForRoot to cover similar situation at TestEncryptionZoneManager level. Do you think it's worth add these other tests to TestEncryptionZones also?
        Hide
        xiaochen Xiao Chen added a comment -

        Thanks for the prompt response Welllington.
        I was thinking about the whole scenarios in the description of this jira: create zone -> create snapshot -> remove dir -> list zone. Do you think it makes sense to add that?

        Show
        xiaochen Xiao Chen added a comment - Thanks for the prompt response Welllington. I was thinking about the whole scenarios in the description of this jira: create zone -> create snapshot -> remove dir -> list zone. Do you think it makes sense to add that?
        Hide
        xiaochen Xiao Chen added a comment -

        Seeing the edit comment, I think it's fine since the new test verifies what's failing before to be passing. Thanks.

        Show
        xiaochen Xiao Chen added a comment - Seeing the edit comment, I think it's fine since the new test verifies what's failing before to be passing. Thanks.
        Hide
        wchevreuil Wellington Chevreuil added a comment -

        Thanks for confirming on this, Xiao Chen! I'm submitting another patch with the whitespace fix, plus the code conventions changes on the test class.

        Please let me know if you have any additional comments on this.

        Show
        wchevreuil Wellington Chevreuil added a comment - Thanks for confirming on this, Xiao Chen ! I'm submitting another patch with the whitespace fix, plus the code conventions changes on the test class. Please let me know if you have any additional comments on this.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 22s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 9m 52s trunk passed
        +1 compile 1m 0s trunk passed
        +1 checkstyle 0m 35s trunk passed
        +1 mvnsite 1m 5s trunk passed
        +1 mvneclipse 0m 17s trunk passed
        -1 findbugs 2m 8s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings.
        +1 javadoc 0m 49s trunk passed
        +1 mvninstall 0m 58s the patch passed
        +1 compile 0m 58s the patch passed
        +1 javac 0m 58s the patch passed
        +1 checkstyle 0m 29s the patch passed
        +1 mvnsite 1m 3s the patch passed
        +1 mvneclipse 0m 13s the patch passed
        -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
        +1 findbugs 2m 23s the patch passed
        +1 javadoc 0m 44s the patch passed
        +1 unit 90m 47s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 19s The patch does not generate ASF License warnings.
        115m 39s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11197
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841576/HDFS-11197-5.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux cc31b9b0c7ce 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 51211a7
        Default Java 1.8.0_111
        findbugs v3.0.0
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17746/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/17746/artifact/patchprocess/whitespace-eol.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17746/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17746/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 9m 52s trunk passed +1 compile 1m 0s trunk passed +1 checkstyle 0m 35s trunk passed +1 mvnsite 1m 5s trunk passed +1 mvneclipse 0m 17s trunk passed -1 findbugs 2m 8s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings. +1 javadoc 0m 49s trunk passed +1 mvninstall 0m 58s the patch passed +1 compile 0m 58s the patch passed +1 javac 0m 58s the patch passed +1 checkstyle 0m 29s the patch passed +1 mvnsite 1m 3s the patch passed +1 mvneclipse 0m 13s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 2m 23s the patch passed +1 javadoc 0m 44s the patch passed +1 unit 90m 47s hadoop-hdfs in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 115m 39s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11197 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841576/HDFS-11197-5.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux cc31b9b0c7ce 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 51211a7 Default Java 1.8.0_111 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17746/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/17746/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17746/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17746/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        xiaochen Xiao Chen added a comment -

        Thanks for the new rev Wellington Chevreuil.

        Please fix the left whitespace. Also a nit: assertEquals takes the parameter as expected, actual. This will be helpful when test fails.

        +1 after the above fixes.

        Show
        xiaochen Xiao Chen added a comment - Thanks for the new rev Wellington Chevreuil . Please fix the left whitespace. Also a nit: assertEquals takes the parameter as expected , actual . This will be helpful when test fails. +1 after the above fixes.
        Hide
        wchevreuil Wellington Chevreuil added a comment -

        Thanks again, Xiao Chen! Sending another patch attempt with your last suggestions.

        Apologies for the white space previously. I actually tried to run the git apply --whitespace=fix <<patch_file>> on that patch to fix white spaces, but I guess it didn't work (or maybe I'm doing something wrong). Anyway, I believe it has no white spaces now. Let me know if you find any additional enhancements.

        Show
        wchevreuil Wellington Chevreuil added a comment - Thanks again, Xiao Chen ! Sending another patch attempt with your last suggestions. Apologies for the white space previously. I actually tried to run the git apply --whitespace=fix <<patch_file>> on that patch to fix white spaces, but I guess it didn't work (or maybe I'm doing something wrong). Anyway, I believe it has no white spaces now. Let me know if you find any additional enhancements.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 21s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 7m 27s trunk passed
        +1 compile 0m 49s trunk passed
        +1 checkstyle 0m 25s trunk passed
        +1 mvnsite 1m 1s trunk passed
        +1 mvneclipse 0m 14s trunk passed
        -1 findbugs 1m 50s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings.
        +1 javadoc 0m 41s trunk passed
        +1 mvninstall 0m 49s the patch passed
        +1 compile 0m 45s the patch passed
        +1 javac 0m 45s the patch passed
        +1 checkstyle 0m 24s the patch passed
        +1 mvnsite 0m 52s the patch passed
        +1 mvneclipse 0m 13s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 2m 5s the patch passed
        +1 javadoc 0m 42s the patch passed
        -1 unit 91m 43s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 19s The patch does not generate ASF License warnings.
        112m 0s



        Reason Tests
        Failed junit tests hadoop.fs.viewfs.TestViewFsAtHdfsRoot



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11197
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841653/HDFS-11197-6.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux cdb5be01dea0 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / f885160
        Default Java 1.8.0_111
        findbugs v3.0.0
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17752/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/17752/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17752/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17752/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 27s trunk passed +1 compile 0m 49s trunk passed +1 checkstyle 0m 25s trunk passed +1 mvnsite 1m 1s trunk passed +1 mvneclipse 0m 14s trunk passed -1 findbugs 1m 50s hadoop-hdfs-project/hadoop-hdfs in trunk has 1 extant Findbugs warnings. +1 javadoc 0m 41s trunk passed +1 mvninstall 0m 49s the patch passed +1 compile 0m 45s the patch passed +1 javac 0m 45s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 5s the patch passed +1 javadoc 0m 42s the patch passed -1 unit 91m 43s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 112m 0s Reason Tests Failed junit tests hadoop.fs.viewfs.TestViewFsAtHdfsRoot Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11197 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841653/HDFS-11197-6.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux cdb5be01dea0 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f885160 Default Java 1.8.0_111 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/17752/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/17752/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17752/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17752/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        wchevreuil Wellington Chevreuil added a comment -

        Just noticed build failed for last submitted patch. This failed unit test ViewFsBaseTest.testGetBlockLocations does not seem related to any of this patch code changes, though.

        Show
        wchevreuil Wellington Chevreuil added a comment - Just noticed build failed for last submitted patch. This failed unit test ViewFsBaseTest.testGetBlockLocations does not seem related to any of this patch code changes, though.
        Hide
        jojochuang Wei-Chiu Chuang added a comment - - edited

        Hi Wellington Chevreuil thanks very much for your patch. The fix itself looks good to me.
        On the other hand I think the test need some change. Some points below:

        1. If I remove the fix, I expected all three new tests would fail. But actually the second and the third one passed. Would you explain?
        2. I would have expected that without the fix, the first test would throw an "Absolute path required" exception, but instead it threw a NPE.
        3. I would recommend adding an end-to-end test. Take a look at hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml and TestCryptoAdminCLI test for example.

        Thanks!

        Show
        jojochuang Wei-Chiu Chuang added a comment - - edited Hi Wellington Chevreuil thanks very much for your patch. The fix itself looks good to me. On the other hand I think the test need some change. Some points below: If I remove the fix, I expected all three new tests would fail. But actually the second and the third one passed. Would you explain? I would have expected that without the fix, the first test would throw an "Absolute path required" exception, but instead it threw a NPE. I would recommend adding an end-to-end test. Take a look at hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml and TestCryptoAdminCLI test for example. Thanks!
        Hide
        wchevreuil Wellington Chevreuil added a comment -

        Hi Wei-Chiu Chuang. Thanks for the comments. Regarding the tests behaviour, please see my comments below:

        1) Only the 1st test is actually testing the condition that caused hdfs crypto -listZones to fail, without the fix. The other two test the conditions that would not cause hdfs crypto -listZones to fail, but were added to ensure the proposed change would not cause the normal scenario to fail.

        2) The reason for the NPE on the 1st test without the fix is because it's mocking FSDirectory, but it's not changing behaviour for FSDirectory.getINodesInPath when an invalid path is informed. Without the if that checks if the inode has a parent or is the root inode, this 1st test will cause EncryptionZoneManager.listEncryptionZones to call FSDirectory.getINodesInPath passing second as value, but the mocked FSDirectory does not mock such condition. To emulate this, it would need to add when(mockedDir.getINodesInPath("second", DirOp.READ_LINK)).thenThrow(new AssertionError("Absolute path required, but got 'second'"));. Would you think it would be relevant?

        3) Sure, I will create some end-to-end tests. Do you think it's ok if I just add new tests on hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml and TestCryptoAdminCLI, or do you it's better create a separate class/config file pair?

        Show
        wchevreuil Wellington Chevreuil added a comment - Hi Wei-Chiu Chuang . Thanks for the comments. Regarding the tests behaviour, please see my comments below: 1) Only the 1st test is actually testing the condition that caused hdfs crypto -listZones to fail, without the fix. The other two test the conditions that would not cause hdfs crypto -listZones to fail, but were added to ensure the proposed change would not cause the normal scenario to fail. 2) The reason for the NPE on the 1st test without the fix is because it's mocking FSDirectory , but it's not changing behaviour for FSDirectory.getINodesInPath when an invalid path is informed. Without the if that checks if the inode has a parent or is the root inode, this 1st test will cause EncryptionZoneManager.listEncryptionZones to call FSDirectory.getINodesInPath passing second as value, but the mocked FSDirectory does not mock such condition. To emulate this, it would need to add when(mockedDir.getINodesInPath("second", DirOp.READ_LINK)).thenThrow(new AssertionError("Absolute path required, but got 'second'")); . Would you think it would be relevant? 3) Sure, I will create some end-to-end tests. Do you think it's ok if I just add new tests on hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml and TestCryptoAdminCLI, or do you it's better create a separate class/config file pair?
        Hide
        wchevreuil Wellington Chevreuil added a comment -

        Hi Wei-Chiu Chuang, am proposing a new patch, adding end-to-end tests for this specific issue on hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml and TestCryptoAdminCLI.

        Few changes performed on TestCryptoAdminCLI for these tests:
        1) Since the issue involved snapshots, I had to make TestConfigFileParserCryptoAdmin to extend CLITestHelperDFS.TestConfigFileParserDFS, instead of CLITestHelper.TestConfigFileParser directly, so that it could parse dfs-admin-command commands from testCryptoConf.xml. This was needed because the test had to run a dfsadmin -allowSnapshot command;

        2) The issue also involved files being moved to trash, prior to permanent deletion. For that purpose, I had to enable "move to Trash" behaviour on TestCryptoAdminCLI.setup method, by adding conf.setLong(CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY, 10);

        The additional tests check for listZones output under the following cases:
        1) Creates a single EZ then call listZones and check if only the created EZ was returned;
        2) Creates two EZs, then delete one of those, the call listZones and check if the two are listed. The deleted EZ should be listed "/user/*/.Trash/Current" directory, since Trash is enabled;
        3) Creates two EZs, then remove one of the EZ, then remove the deleted one from Trash, then call listZones and checks if only the non-deleted EZ was returned;
        4) Creates two EZs under a snapshottable directory, takes a snapshot for the directory, remove one of the EZs, then remove this EZ from Trash, calls listZones and check if only the non-deleted EZ was returned.

        Please let me know on your thoughts.

        Show
        wchevreuil Wellington Chevreuil added a comment - Hi Wei-Chiu Chuang , am proposing a new patch, adding end-to-end tests for this specific issue on hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml and TestCryptoAdminCLI . Few changes performed on TestCryptoAdminCLI for these tests: 1) Since the issue involved snapshots, I had to make TestConfigFileParserCryptoAdmin to extend CLITestHelperDFS.TestConfigFileParserDFS , instead of CLITestHelper.TestConfigFileParser directly, so that it could parse dfs-admin-command commands from testCryptoConf.xml . This was needed because the test had to run a dfsadmin -allowSnapshot command; 2) The issue also involved files being moved to trash, prior to permanent deletion. For that purpose, I had to enable "move to Trash" behaviour on TestCryptoAdminCLI.setup method, by adding conf.setLong(CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY, 10); The additional tests check for listZones output under the following cases: 1) Creates a single EZ then call listZones and check if only the created EZ was returned; 2) Creates two EZs, then delete one of those, the call listZones and check if the two are listed. The deleted EZ should be listed "/user/*/.Trash/Current" directory, since Trash is enabled; 3) Creates two EZs, then remove one of the EZ, then remove the deleted one from Trash, then call listZones and checks if only the non-deleted EZ was returned; 4) Creates two EZs under a snapshottable directory, takes a snapshot for the directory, remove one of the EZs, then remove this EZ from Trash, calls listZones and check if only the non-deleted EZ was returned. Please let me know on your thoughts.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 7m 30s trunk passed
        +1 compile 0m 47s trunk passed
        +1 checkstyle 0m 27s trunk passed
        +1 mvnsite 0m 56s trunk passed
        +1 mvneclipse 0m 13s trunk passed
        +1 findbugs 1m 58s trunk passed
        +1 javadoc 0m 42s trunk passed
        +1 mvninstall 0m 57s the patch passed
        +1 compile 0m 55s the patch passed
        +1 javac 0m 55s the patch passed
        +1 checkstyle 0m 26s the patch passed
        +1 mvnsite 1m 1s the patch passed
        +1 mvneclipse 0m 12s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 2m 5s the patch passed
        +1 javadoc 0m 40s the patch passed
        -1 unit 66m 17s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 20s The patch does not generate ASF License warnings.
        87m 5s



        Reason Tests
        Failed junit tests hadoop.hdfs.TestEncryptionZones



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue HDFS-11197
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841653/HDFS-11197-6.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 65b359ae32d8 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / a7288da
        Default Java 1.8.0_111
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/17782/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17782/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17782/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 30s trunk passed +1 compile 0m 47s trunk passed +1 checkstyle 0m 27s trunk passed +1 mvnsite 0m 56s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 58s trunk passed +1 javadoc 0m 42s trunk passed +1 mvninstall 0m 57s the patch passed +1 compile 0m 55s the patch passed +1 javac 0m 55s the patch passed +1 checkstyle 0m 26s the patch passed +1 mvnsite 1m 1s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 5s the patch passed +1 javadoc 0m 40s the patch passed -1 unit 66m 17s hadoop-hdfs in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 87m 5s Reason Tests Failed junit tests hadoop.hdfs.TestEncryptionZones Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11197 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841653/HDFS-11197-6.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 65b359ae32d8 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a7288da Default Java 1.8.0_111 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/17782/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17782/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17782/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        wchevreuil Wellington Chevreuil added a comment -

        I'm not sure this org.apache.hadoop.hdfs.TestEncryptionZones.testStartFileRetry test failure is actually related. There were no changes to the actual fix between the last two patches, and the test is passing on my local build:

        -------------------------------------------------------------------------------
        Test set: org.apache.hadoop.hdfs.TestEncryptionZones
        -------------------------------------------------------------------------------
        Tests run: 26, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 46.25 sec - in org.apache.hadoop.hdfs.TestEncryptionZones
        
        Show
        wchevreuil Wellington Chevreuil added a comment - I'm not sure this org.apache.hadoop.hdfs.TestEncryptionZones.testStartFileRetry test failure is actually related. There were no changes to the actual fix between the last two patches, and the test is passing on my local build: ------------------------------------------------------------------------------- Test set: org.apache.hadoop.hdfs.TestEncryptionZones ------------------------------------------------------------------------------- Tests run: 26, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 46.25 sec - in org.apache.hadoop.hdfs.TestEncryptionZones
        Hide
        jojochuang Wei-Chiu Chuang added a comment - - edited

        That fails because of this error:

        2016-12-07 00:51:50,706 [IPC Server handler 6 on 59757] INFO  ipc.Server (Server.java:logException(2697)) - IPC Server handler 6 on 59757, call Call#819 Retry#0 org.apache.hadoop.hdfs.protocol.ClientProtocol.create from 127.0.0.1:42394
        org.apache.hadoop.hdfs.server.namenode.RetryStartFileException: Preconditions for creating a file failed because of a transient error, retry create later.
        	at org.apache.hadoop.hdfs.server.namenode.FSDirEncryptionZoneOp.getFileEncryptionInfo(FSDirEncryptionZoneOp.java:330)
        	at org.apache.hadoop.hdfs.server.namenode.FSNamesystem.startFileInt(FSNamesystem.java:2249)
        	at org.apache.hadoop.hdfs.server.namenode.FSNamesystem.startFile(FSNamesystem.java:2175)
        	at org.apache.hadoop.hdfs.server.namenode.NameNodeRpcServer.create(NameNodeRpcServer.java:742)
        	at org.apache.hadoop.hdfs.protocolPB.ClientNamenodeProtocolServerSideTranslatorPB.create(ClientNamenodeProtocolServerSideTranslatorPB.java:420)
        	at org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos$ClientNamenodeProtocol$2.callBlockingMethod(ClientNamenodeProtocolProtos.java)
        	at org.apache.hadoop.ipc.ProtobufRpcEngine$Server$ProtoBufRpcInvoker.call(ProtobufRpcEngine.java:522)
        	at org.apache.hadoop.ipc.RPC$Server.call(RPC.java:991)
        	at org.apache.hadoop.ipc.Server$RpcCall.run(Server.java:867)
        	at org.apache.hadoop.ipc.Server$RpcCall.run(Server.java:813)
        	at java.security.AccessController.doPrivileged(Native Method)
        	at javax.security.auth.Subject.doAs(Subject.java:422)
        	at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1857)
        	at org.apache.hadoop.ipc.Server$Handler.run(Server.java:2653)
        

        I have seen the exact same test error in the past, so most likely it's not your patch that caused it. Xiao Chen filed HDFS-11093 for this previously.

        Show
        jojochuang Wei-Chiu Chuang added a comment - - edited That fails because of this error: 2016-12-07 00:51:50,706 [IPC Server handler 6 on 59757] INFO ipc.Server (Server.java:logException(2697)) - IPC Server handler 6 on 59757, call Call#819 Retry#0 org.apache.hadoop.hdfs.protocol.ClientProtocol.create from 127.0.0.1:42394 org.apache.hadoop.hdfs.server.namenode.RetryStartFileException: Preconditions for creating a file failed because of a transient error, retry create later. at org.apache.hadoop.hdfs.server.namenode.FSDirEncryptionZoneOp.getFileEncryptionInfo(FSDirEncryptionZoneOp.java:330) at org.apache.hadoop.hdfs.server.namenode.FSNamesystem.startFileInt(FSNamesystem.java:2249) at org.apache.hadoop.hdfs.server.namenode.FSNamesystem.startFile(FSNamesystem.java:2175) at org.apache.hadoop.hdfs.server.namenode.NameNodeRpcServer.create(NameNodeRpcServer.java:742) at org.apache.hadoop.hdfs.protocolPB.ClientNamenodeProtocolServerSideTranslatorPB.create(ClientNamenodeProtocolServerSideTranslatorPB.java:420) at org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos$ClientNamenodeProtocol$2.callBlockingMethod(ClientNamenodeProtocolProtos.java) at org.apache.hadoop.ipc.ProtobufRpcEngine$Server$ProtoBufRpcInvoker.call(ProtobufRpcEngine.java:522) at org.apache.hadoop.ipc.RPC$Server.call(RPC.java:991) at org.apache.hadoop.ipc.Server$RpcCall.run(Server.java:867) at org.apache.hadoop.ipc.Server$RpcCall.run(Server.java:813) at java.security.AccessController.doPrivileged(Native Method) at javax.security.auth.Subject.doAs(Subject.java:422) at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1857) at org.apache.hadoop.ipc.Server$Handler.run(Server.java:2653) I have seen the exact same test error in the past, so most likely it's not your patch that caused it. Xiao Chen filed HDFS-11093 for this previously.
        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        +1 from me. Xiao Chen would you also like to comment on the latest patch?
        Thanks.

        Show
        jojochuang Wei-Chiu Chuang added a comment - +1 from me. Xiao Chen would you also like to comment on the latest patch? Thanks.
        Hide
        xiaochen Xiao Chen added a comment -

        +1, committing this.

        Show
        xiaochen Xiao Chen added a comment - +1, committing this.
        Hide
        xiaochen Xiao Chen added a comment -

        Committed to trunk, branch-2 and branch-2.8.

        Thanks a lot Wellington Chevreuil for reporting and fixing the issue, and Wei-Chiu Chuang for reviewing!

        Show
        xiaochen Xiao Chen added a comment - Committed to trunk, branch-2 and branch-2.8. Thanks a lot Wellington Chevreuil for reporting and fixing the issue, and Wei-Chiu Chuang for reviewing!
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10972 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10972/)
        HDFS-11197. Listing encryption zones fails when deleting a EZ that is on (xiao: rev 401c7318723d8d62c7fc29728f7f4e8d336b4d2f)

        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/cli/TestCryptoAdminCLI.java
        • (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEncryptionZoneManager.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10972 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10972/ ) HDFS-11197 . Listing encryption zones fails when deleting a EZ that is on (xiao: rev 401c7318723d8d62c7fc29728f7f4e8d336b4d2f) (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/cli/TestCryptoAdminCLI.java (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEncryptionZoneManager.java

          People

          • Assignee:
            wchevreuil Wellington Chevreuil
            Reporter:
            wchevreuil Wellington Chevreuil
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development