Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0-alpha1
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Hide
      <!-- markdown -->

      Audit logs will now only be generated in the following two cases:
      * When an operation results in an `AccessControlException`
      * When an operation is successful

      Notably, this means audit log events will not be generated for exceptions besides AccessControlException.
      Show
      <!-- markdown --> Audit logs will now only be generated in the following two cases: * When an operation results in an `AccessControlException` * When an operation is successful Notably, this means audit log events will not be generated for exceptions besides AccessControlException.

      Description

      So, the big question here is what should go in the audit log? All failures, or just "permission denied" failures? Or, to put it a different way, if someone attempts to do something and it fails because a file doesn't exist, is that worth an audit log entry?

      We are currently inconsistent on this point. For example, concat, getContentSummary, addCacheDirective, and setErasureEncodingPolicy create an audit log entry for all failures, but setOwner, delete, and setAclEntries attempt to only create an entry for AccessControlException-based failures. There are a few operations, like allowSnapshot, disallowSnapshot, and startRollingUpgrade that never create audit log failure entries at all. They simply log nothing for any failure, and log success for a successful operation.

      So to summarize, different HDFS operations currently fall into 3 categories:
      1. audit-log all failures
      2. audit-log only AccessControlException failures
      3. never audit-log failures

      Which category is right? And how can we fix the inconsistency

      1. HDFS-9395.001.patch
        20 kB
        Kuhu Shukla
      2. HDFS-9395.002.patch
        19 kB
        Kuhu Shukla
      3. HDFS-9395.003.patch
        30 kB
        Kuhu Shukla
      4. HDFS-9395.004.patch
        30 kB
        Kuhu Shukla
      5. HDFS-9395.005.patch
        36 kB
        Kuhu Shukla
      6. HDFS-9395.006.patch
        41 kB
        Kuhu Shukla
      7. HDFS-9395.007.patch
        42 kB
        Kuhu Shukla
      8. HDFS-9395-branch-2.7.001.patch
        41 kB
        Kuhu Shukla
      9. HDFS-9395-branch-2.7.002.patch
        41 kB
        Kuhu Shukla

        Issue Links

          Activity

          Hide
          kshukla Kuhu Shukla added a comment -

          Thank you Andrew Wang for updating the release note. I was under the assumption that there was something else to be updated during the commit as well, along with this row. Sorry about that. The message looks good. Appreciate the help.

          Show
          kshukla Kuhu Shukla added a comment - Thank you Andrew Wang for updating the release note. I was under the assumption that there was something else to be updated during the commit as well, along with this row. Sorry about that. The message looks good. Appreciate the help.
          Hide
          andrew.wang Andrew Wang added a comment -

          I tried my hand at a release note, please correct if inaccurate.

          Show
          andrew.wang Andrew Wang added a comment - I tried my hand at a release note, please correct if inaccurate.
          Hide
          rchiang Ray Chiang added a comment -

          Also, can the new behavior be documented in the "Release Notes" section for Hadoop 3? This is only one of a few incompatible changes missing such documentation.

          Show
          rchiang Ray Chiang added a comment - Also, can the new behavior be documented in the "Release Notes" section for Hadoop 3? This is only one of a few incompatible changes missing such documentation.
          Hide
          kihwal Kihwal Lee added a comment -

          Vinod Kumar Vavilapalli, I also reverted it from branch-2.7.

          Show
          kihwal Kihwal Lee added a comment - Vinod Kumar Vavilapalli , I also reverted it from branch-2.7.
          Hide
          kshukla Kuhu Shukla added a comment -

          Vinod Kumar Vavilapalli, This bug fix was originally requested by our grid operations team. It could introduce an incompatibility to certain users, but was considered critical enough to be pushed to branch-2* at that time. I have opened HDFS-10776 for continuing further discussion and appreciate any suggestions/ideas you may have on this. Thanks a lot!

          Show
          kshukla Kuhu Shukla added a comment - Vinod Kumar Vavilapalli , This bug fix was originally requested by our grid operations team. It could introduce an incompatibility to certain users, but was considered critical enough to be pushed to branch-2* at that time. I have opened HDFS-10776 for continuing further discussion and appreciate any suggestions/ideas you may have on this. Thanks a lot!
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Just reverted this incompatible change from branch-2, branch-2.8 and branch-2.7.3 after Allen Wittenauer pointed this out on 2.7.3 RC1 voting thread.

          Kuhu Shukla / Kihwal Lee, can you comment on why this incompatible change is pushed into branch-2.*?

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Just reverted this incompatible change from branch-2, branch-2.8 and branch-2.7.3 after Allen Wittenauer pointed this out on 2.7.3 RC1 voting thread. Kuhu Shukla / Kihwal Lee , can you comment on why this incompatible change is pushed into branch-2.*?
          Hide
          kihwal Kihwal Lee added a comment -

          The two tests failed due to OOM and timeout. It must be the environment. They all pass with the patch when I ran.

          -------------------------------------------------------
           T E S T S
          -------------------------------------------------------
          OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=768m; support was removed in 8.0
          Running org.apache.hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
          Tests run: 36, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 217.27 sec - in org.apache.hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
          OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=768m; support was removed in 8.0
          Running org.apache.hadoop.tools.TestJMXGet
          Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 26.792 sec - in org.apache.hadoop.tools.TestJMXGet
          
          Results :
          
          Tests run: 38, Failures: 0, Errors: 0, Skipped: 0
          

          +1 for the branch-2.7 patch. It looks like a correct port.

          Show
          kihwal Kihwal Lee added a comment - The two tests failed due to OOM and timeout. It must be the environment. They all pass with the patch when I ran. ------------------------------------------------------- T E S T S ------------------------------------------------------- OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=768m; support was removed in 8.0 Running org.apache.hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots Tests run: 36, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 217.27 sec - in org.apache.hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=768m; support was removed in 8.0 Running org.apache.hadoop.tools.TestJMXGet Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 26.792 sec - in org.apache.hadoop.tools.TestJMXGet Results : Tests run: 38, Failures: 0, Errors: 0, Skipped: 0 +1 for the branch-2.7 patch. It looks like a correct port.
          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 7m 18s branch-2.7 passed
          +1 compile 1m 15s branch-2.7 passed with JDK v1.8.0_72
          +1 compile 1m 4s branch-2.7 passed with JDK v1.7.0_95
          +1 checkstyle 0m 30s branch-2.7 passed
          +1 mvnsite 1m 2s branch-2.7 passed
          +1 mvneclipse 0m 14s branch-2.7 passed
          -1 findbugs 3m 3s hadoop-hdfs-project/hadoop-hdfs in branch-2.7 has 1 extant Findbugs warnings.
          +1 javadoc 1m 22s branch-2.7 passed with JDK v1.8.0_72
          +1 javadoc 2m 9s branch-2.7 passed with JDK v1.7.0_95
          +1 mvninstall 0m 56s the patch passed
          +1 compile 1m 7s the patch passed with JDK v1.8.0_72
          +1 javac 1m 7s the patch passed
          +1 compile 0m 57s the patch passed with JDK v1.7.0_95
          +1 javac 0m 57s the patch passed
          -1 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: patch generated 1 new + 360 unchanged - 3 fixed = 361 total (was 363)
          +1 mvnsite 0m 53s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          -1 whitespace 0m 0s The patch has 4123 line(s) that end in whitespace. Use git apply --whitespace=fix.
          -1 whitespace 1m 41s The patch has 110 line(s) with tabs.
          +1 findbugs 3m 8s the patch passed
          +1 javadoc 1m 10s the patch passed with JDK v1.8.0_72
          +1 javadoc 1m 52s the patch passed with JDK v1.7.0_95
          -1 unit 45m 2s hadoop-hdfs in the patch failed with JDK v1.8.0_72.
          -1 unit 41m 51s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          -1 asflicense 43m 9s Patch generated 80 ASF License warnings.
          163m 6s



          Reason Tests
          JDK v1.8.0_72 Failed junit tests hadoop.hdfs.TestCrcCorruption
            hadoop.hdfs.TestAppendSnapshotTruncate
            hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
            hadoop.tools.TestJMXGet
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
            hadoop.tools.TestJMXGet



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:c420dfe
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12790014/HDFS-9395-branch-2.7.002.patch
          JIRA Issue HDFS-9395
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux aa86d92a26d8 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2.7 / 8ceb9f3
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/whitespace-eol.txt
          whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/whitespace-tabs.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14617/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/14617/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/14617/console
          Powered by Apache Yetus 0.2.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 7m 18s branch-2.7 passed +1 compile 1m 15s branch-2.7 passed with JDK v1.8.0_72 +1 compile 1m 4s branch-2.7 passed with JDK v1.7.0_95 +1 checkstyle 0m 30s branch-2.7 passed +1 mvnsite 1m 2s branch-2.7 passed +1 mvneclipse 0m 14s branch-2.7 passed -1 findbugs 3m 3s hadoop-hdfs-project/hadoop-hdfs in branch-2.7 has 1 extant Findbugs warnings. +1 javadoc 1m 22s branch-2.7 passed with JDK v1.8.0_72 +1 javadoc 2m 9s branch-2.7 passed with JDK v1.7.0_95 +1 mvninstall 0m 56s the patch passed +1 compile 1m 7s the patch passed with JDK v1.8.0_72 +1 javac 1m 7s the patch passed +1 compile 0m 57s the patch passed with JDK v1.7.0_95 +1 javac 0m 57s the patch passed -1 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: patch generated 1 new + 360 unchanged - 3 fixed = 361 total (was 363) +1 mvnsite 0m 53s the patch passed +1 mvneclipse 0m 11s the patch passed -1 whitespace 0m 0s The patch has 4123 line(s) that end in whitespace. Use git apply --whitespace=fix. -1 whitespace 1m 41s The patch has 110 line(s) with tabs. +1 findbugs 3m 8s the patch passed +1 javadoc 1m 10s the patch passed with JDK v1.8.0_72 +1 javadoc 1m 52s the patch passed with JDK v1.7.0_95 -1 unit 45m 2s hadoop-hdfs in the patch failed with JDK v1.8.0_72. -1 unit 41m 51s hadoop-hdfs in the patch failed with JDK v1.7.0_95. -1 asflicense 43m 9s Patch generated 80 ASF License warnings. 163m 6s Reason Tests JDK v1.8.0_72 Failed junit tests hadoop.hdfs.TestCrcCorruption   hadoop.hdfs.TestAppendSnapshotTruncate   hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots   hadoop.tools.TestJMXGet JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots   hadoop.tools.TestJMXGet Subsystem Report/Notes Docker Image:yetus/hadoop:c420dfe JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12790014/HDFS-9395-branch-2.7.002.patch JIRA Issue HDFS-9395 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux aa86d92a26d8 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2.7 / 8ceb9f3 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/whitespace-eol.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/whitespace-tabs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14617/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14617/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/14617/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/14617/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          kshukla Kuhu Shukla added a comment -

          Re-opening to submit 2.7 patch.

          Show
          kshukla Kuhu Shukla added a comment - Re-opening to submit 2.7 patch.
          Hide
          kshukla Kuhu Shukla added a comment -

          Adding v2 of the branch-2.7 patch with the modified test which mitigates the impact of HDFS-8332's absence in branch-2.7.

          Show
          kshukla Kuhu Shukla added a comment - Adding v2 of the branch-2.7 patch with the modified test which mitigates the impact of HDFS-8332 's absence in branch-2.7.
          Hide
          kshukla Kuhu Shukla added a comment -

          Adding patch for branch-2.7, I have added @Ignore to testSetQuota which is failing exactly like in branch-2 per HDFS-9855. Seeking comments on that. Thanks a lot!

          Show
          kshukla Kuhu Shukla added a comment - Adding patch for branch-2.7, I have added @Ignore to testSetQuota which is failing exactly like in branch-2 per HDFS-9855 . Seeking comments on that. Thanks a lot!
          Hide
          kihwal Kihwal Lee added a comment -

          I've committed this to trunk, branch-2 and branch-2.8.
          A test case in the newly added tests is failing in branch-2/2.8, but Kuhu and I think it is due to a bug in branch-2. We will file a new jira and investigate.

          Show
          kihwal Kihwal Lee added a comment - I've committed this to trunk, branch-2 and branch-2.8. A test case in the newly added tests is failing in branch-2/2.8, but Kuhu and I think it is due to a bug in branch-2. We will file a new jira and investigate.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9361 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9361/)
          HDFS-9395. Make HDFS audit logging consistant. Contributed by Kuhu (kihwal: rev d27d7fc72e279614212c1eae52a84675073e89fb)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLoggerWithCommands.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9361 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9361/ ) HDFS-9395 . Make HDFS audit logging consistant. Contributed by Kuhu (kihwal: rev d27d7fc72e279614212c1eae52a84675073e89fb) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLoggerWithCommands.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          kihwal Kihwal Lee added a comment -

          Thanks for the clarification and reviews. I will commit it soon.

          Show
          kihwal Kihwal Lee added a comment - Thanks for the clarification and reviews. I will commit it soon.
          Hide
          kshukla Kuhu Shukla added a comment -

          Thanks Vinayakumar. I will go ahead and assign HDFS-5040 to myself. The test failures were locally validated to be unrelated, and most of them are timeouts.

          Show
          kshukla Kuhu Shukla added a comment - Thanks Vinayakumar. I will go ahead and assign HDFS-5040 to myself. The test failures were locally validated to be unrelated, and most of them are timeouts.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 9m 13s trunk passed
          +1 compile 1m 3s trunk passed with JDK v1.8.0_72
          +1 compile 0m 53s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 27s trunk passed
          +1 mvnsite 1m 8s trunk passed
          +1 mvneclipse 0m 18s trunk passed
          +1 findbugs 2m 15s trunk passed
          +1 javadoc 1m 36s trunk passed with JDK v1.8.0_72
          +1 javadoc 2m 22s trunk passed with JDK v1.7.0_95
          +1 mvninstall 1m 5s the patch passed
          +1 compile 1m 13s the patch passed with JDK v1.8.0_72
          +1 javac 1m 13s the patch passed
          +1 compile 0m 54s the patch passed with JDK v1.7.0_95
          +1 javac 0m 54s the patch passed
          +1 checkstyle 0m 26s the patch passed
          +1 mvnsite 1m 8s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 39s the patch passed
          +1 javadoc 1m 37s the patch passed with JDK v1.8.0_72
          +1 javadoc 2m 18s the patch passed with JDK v1.7.0_95
          -1 unit 120m 12s hadoop-hdfs in the patch failed with JDK v1.8.0_72.
          -1 unit 113m 47s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 32s Patch does not generate ASF License warnings.
          268m 55s



          Reason Tests
          JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.namenode.ha.TestEditLogTailer
            hadoop.hdfs.shortcircuit.TestShortCircuitCache
            hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
            hadoop.hdfs.security.TestDelegationTokenForProxyUser
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.server.datanode.TestBlockReplacement
            hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot
            hadoop.hdfs.server.datanode.TestDirectoryScanner
          JDK v1.8.0_72 Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.TestDFSUpgradeFromImage
            hadoop.hdfs.TestPread
            hadoop.hdfs.server.namenode.ha.TestEditLogTailer
            hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
            hadoop.hdfs.TestErasureCodeBenchmarkThroughput
            hadoop.hdfs.TestLocalDFS
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.TestSafeMode
            hadoop.hdfs.server.datanode.TestBlockReplacement
            hadoop.hdfs.server.namenode.ha.TestDFSUpgradeWithHA
            hadoop.hdfs.server.datanode.TestDirectoryScanner



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12788704/HDFS-9395.007.patch
          JIRA Issue HDFS-9395
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 3dbbce7f5faa 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 342c957
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14556/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14556/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14556/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14556/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14556/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14556/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 9m 13s trunk passed +1 compile 1m 3s trunk passed with JDK v1.8.0_72 +1 compile 0m 53s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 27s trunk passed +1 mvnsite 1m 8s trunk passed +1 mvneclipse 0m 18s trunk passed +1 findbugs 2m 15s trunk passed +1 javadoc 1m 36s trunk passed with JDK v1.8.0_72 +1 javadoc 2m 22s trunk passed with JDK v1.7.0_95 +1 mvninstall 1m 5s the patch passed +1 compile 1m 13s the patch passed with JDK v1.8.0_72 +1 javac 1m 13s the patch passed +1 compile 0m 54s the patch passed with JDK v1.7.0_95 +1 javac 0m 54s the patch passed +1 checkstyle 0m 26s the patch passed +1 mvnsite 1m 8s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 39s the patch passed +1 javadoc 1m 37s the patch passed with JDK v1.8.0_72 +1 javadoc 2m 18s the patch passed with JDK v1.7.0_95 -1 unit 120m 12s hadoop-hdfs in the patch failed with JDK v1.8.0_72. -1 unit 113m 47s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 32s Patch does not generate ASF License warnings. 268m 55s Reason Tests JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.namenode.ha.TestEditLogTailer   hadoop.hdfs.shortcircuit.TestShortCircuitCache   hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl   hadoop.hdfs.security.TestDelegationTokenForProxyUser   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.server.datanode.TestBlockReplacement   hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot   hadoop.hdfs.server.datanode.TestDirectoryScanner JDK v1.8.0_72 Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2 JDK v1.7.0_95 Failed junit tests hadoop.hdfs.TestDFSUpgradeFromImage   hadoop.hdfs.TestPread   hadoop.hdfs.server.namenode.ha.TestEditLogTailer   hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl   hadoop.hdfs.TestErasureCodeBenchmarkThroughput   hadoop.hdfs.TestLocalDFS   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.TestSafeMode   hadoop.hdfs.server.datanode.TestBlockReplacement   hadoop.hdfs.server.namenode.ha.TestDFSUpgradeWithHA   hadoop.hdfs.server.datanode.TestDirectoryScanner Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12788704/HDFS-9395.007.patch JIRA Issue HDFS-9395 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 3dbbce7f5faa 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 342c957 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/14556/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14556/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14556/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14556/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14556/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14556/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vinayrpet Vinayakumar B added a comment -

          I noticed that ACE is thrown before the try block so no audit logs are logged as expected, test not included in the patch. Seeking some comments on whether we can have scenarios valid for this change.

          I think, HDFS-5040, is more suitable for bringing all admin commands into audit-log.
          I prefer to keep the current change in v007 patch and discuss moving checkSuperuserPrivilege() inside try-block for all admin commands to catch ACE and log it in HDFS-5040.

          Given that, +1 for the latest patch.

          Show
          vinayrpet Vinayakumar B added a comment - I noticed that ACE is thrown before the try block so no audit logs are logged as expected, test not included in the patch. Seeking some comments on whether we can have scenarios valid for this change. I think, HDFS-5040 , is more suitable for bringing all admin commands into audit-log. I prefer to keep the current change in v007 patch and discuss moving checkSuperuserPrivilege() inside try-block for all admin commands to catch ACE and log it in HDFS-5040 . Given that, +1 for the latest patch.
          Hide
          cnauroth Chris Nauroth added a comment -

          getAclStatus looks correct in patch v007. Thanks!

          Show
          cnauroth Chris Nauroth added a comment - getAclStatus looks correct in patch v007. Thanks!
          Hide
          kshukla Kuhu Shukla added a comment -

          I have updated the patch per comments from Vinayakumar which fixes the related test failures in TestAuditLogger. I did add the catch block for ACE in setErasureCodingPolicy but while writing the test I noticed that ACE is thrown before the try block so no audit logs are logged as expected, test not included in the patch. Seeking some comments on whether we can have scenarios valid for this change.

          @Test
            public void testSetErasureCodingPolicy() throws Exception {
              Path path = new Path("/test");
              fs.mkdirs(path, new FsPermission((short)0));
              fileSys = DFSTestUtil.getFileSystemAs(user1, conf);
              try {
                ((DistributedFileSystem) fileSys).setErasureCodingPolicy(path, null);
                fail("The operation should have failed with AccessControlException");
              } catch (AccessControlException ace) {
                
              }
            }
          
          Show
          kshukla Kuhu Shukla added a comment - I have updated the patch per comments from Vinayakumar which fixes the related test failures in TestAuditLogger. I did add the catch block for ACE in setErasureCodingPolicy but while writing the test I noticed that ACE is thrown before the try block so no audit logs are logged as expected, test not included in the patch. Seeking some comments on whether we can have scenarios valid for this change. @Test public void testSetErasureCodingPolicy() throws Exception { Path path = new Path( "/test" ); fs.mkdirs(path, new FsPermission(( short )0)); fileSys = DFSTestUtil.getFileSystemAs(user1, conf); try { ((DistributedFileSystem) fileSys).setErasureCodingPolicy(path, null ); fail( "The operation should have failed with AccessControlException" ); } catch (AccessControlException ace) { } }
          Hide
          kshukla Kuhu Shukla added a comment -

          Thanks Vinayakumar B for the comments.

          setErasureCodingPolicy does a checkSuperuserPrivilege before the actual call. As per my understanding, in a case where ACE can occur, the audit log would still not be logged through the finally block but would be thrown when the check is made. So this is allowing only successful logging like it was previously. While going through methods that do checkSuperuserPrivilege, I saw most of them don't log ACEs( eg. finalizeRollingUpgrade) but some do like createEncryptionZone. Should we be fixing those as well, and if yes, do we log the ACE?
          For example , in allowSnapshot and disallowSnapshot only successful ones are logged.

          Show
          kshukla Kuhu Shukla added a comment - Thanks Vinayakumar B for the comments. setErasureCodingPolicy does a checkSuperuserPrivilege before the actual call. As per my understanding, in a case where ACE can occur, the audit log would still not be logged through the finally block but would be thrown when the check is made. So this is allowing only successful logging like it was previously. While going through methods that do checkSuperuserPrivilege, I saw most of them don't log ACEs( eg. finalizeRollingUpgrade) but some do like createEncryptionZone . Should we be fixing those as well, and if yes, do we log the ACE? For example , in allowSnapshot and disallowSnapshot only successful ones are logged.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 26s 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 40s trunk passed
          +1 compile 0m 40s trunk passed with JDK v1.8.0_72
          +1 compile 0m 41s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 24s trunk passed
          +1 mvnsite 0m 51s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 1m 56s trunk passed
          +1 javadoc 1m 6s trunk passed with JDK v1.8.0_72
          +1 javadoc 1m 46s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 44s the patch passed
          +1 compile 0m 36s the patch passed with JDK v1.8.0_72
          +1 javac 0m 36s the patch passed
          +1 compile 0m 38s the patch passed with JDK v1.7.0_95
          +1 javac 0m 38s the patch passed
          +1 checkstyle 0m 21s the patch passed
          +1 mvnsite 0m 49s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 5s the patch passed
          +1 javadoc 1m 4s the patch passed with JDK v1.8.0_72
          +1 javadoc 1m 42s the patch passed with JDK v1.7.0_95
          -1 unit 76m 14s hadoop-hdfs in the patch failed with JDK v1.8.0_72.
          -1 unit 80m 16s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 51s Patch does not generate ASF License warnings.
          183m 43s



          Reason Tests
          JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.namenode.TestAuditLogger
            hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot
            hadoop.hdfs.server.namenode.TestAuditLoggerWithCommands
            hadoop.hdfs.TestSafeModeWithStripedFile
          JDK v1.8.0_72 Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.blockmanagement.TestBlockManager
            hadoop.hdfs.server.namenode.TestAuditLogger
            hadoop.hdfs.server.namenode.ha.TestHAAppend
            hadoop.hdfs.server.namenode.TestAuditLoggerWithCommands
            hadoop.hdfs.shortcircuit.TestShortCircuitCache
            hadoop.hdfs.server.namenode.snapshot.TestSnapshotFileLength



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12788601/HDFS-9395.006.patch
          JIRA Issue HDFS-9395
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 4bf5ca125dba 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 748b6c0
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14542/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14542/console
          Powered by Apache Yetus 0.2.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 26s 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 40s trunk passed +1 compile 0m 40s trunk passed with JDK v1.8.0_72 +1 compile 0m 41s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 24s trunk passed +1 mvnsite 0m 51s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 56s trunk passed +1 javadoc 1m 6s trunk passed with JDK v1.8.0_72 +1 javadoc 1m 46s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 44s the patch passed +1 compile 0m 36s the patch passed with JDK v1.8.0_72 +1 javac 0m 36s the patch passed +1 compile 0m 38s the patch passed with JDK v1.7.0_95 +1 javac 0m 38s the patch passed +1 checkstyle 0m 21s the patch passed +1 mvnsite 0m 49s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 5s the patch passed +1 javadoc 1m 4s the patch passed with JDK v1.8.0_72 +1 javadoc 1m 42s the patch passed with JDK v1.7.0_95 -1 unit 76m 14s hadoop-hdfs in the patch failed with JDK v1.8.0_72. -1 unit 80m 16s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 51s Patch does not generate ASF License warnings. 183m 43s Reason Tests JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.namenode.TestAuditLogger   hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot   hadoop.hdfs.server.namenode.TestAuditLoggerWithCommands   hadoop.hdfs.TestSafeModeWithStripedFile JDK v1.8.0_72 Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2 JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.blockmanagement.TestBlockManager   hadoop.hdfs.server.namenode.TestAuditLogger   hadoop.hdfs.server.namenode.ha.TestHAAppend   hadoop.hdfs.server.namenode.TestAuditLoggerWithCommands   hadoop.hdfs.shortcircuit.TestShortCircuitCache   hadoop.hdfs.server.namenode.snapshot.TestSnapshotFileLength Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12788601/HDFS-9395.006.patch JIRA Issue HDFS-9395 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4bf5ca125dba 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 748b6c0 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/14542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14542/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14542/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vinayrpet Vinayakumar B added a comment -

          Thanks Kuhu Shukla for the update.

          2 comments for latest patch.
          1. getAclStatus, from Chris' suggestion above, need to preserve auditlogging for successfull operation. But its also necessary to log Access failed operations, similar to checkAccess.

          2. setErasureCodingPolicy now missing access failed audit log.

          Show
          vinayrpet Vinayakumar B added a comment - Thanks Kuhu Shukla for the update. 2 comments for latest patch. 1. getAclStatus , from Chris' suggestion above, need to preserve auditlogging for successfull operation. But its also necessary to log Access failed operations, similar to checkAccess . 2. setErasureCodingPolicy now missing access failed audit log.
          Hide
          kshukla Kuhu Shukla added a comment -

          Updating patch based on latest comments. Removed the null check for addCacheDirective method's return value based on findbugs warning. getXAttrs and listXAttrs now log successful calls too.

          Show
          kshukla Kuhu Shukla added a comment - Updating patch based on latest comments. Removed the null check for addCacheDirective method's return value based on findbugs warning. getXAttrs and listXAttrs now log successful calls too.
          Hide
          kshukla Kuhu Shukla added a comment -

          Thanks Chris Nauroth! One follow up question,

          Currently getAclStatus logs even failed attempts through the logAuditEvent call in the finally block.

           } finally {
                readUnlock();
                logAuditEvent(success, "getAclStatus", src);
              }
          

          I am making it so that only successful ones are logged. Does that make sense or am I missing something here? Appreciate your inputs.

          Show
          kshukla Kuhu Shukla added a comment - Thanks Chris Nauroth ! One follow up question, Currently getAclStatus logs even failed attempts through the logAuditEvent call in the finally block. } finally { readUnlock(); logAuditEvent(success, "getAclStatus" , src); } I am making it so that only successful ones are logged. Does that make sense or am I missing something here? Appreciate your inputs.
          Hide
          cnauroth Chris Nauroth added a comment -

          We can skip success auditlog for checkAccess and getAclStatus.

          getAclStatus currently logs if successful, and we should preserve that. From reading earlier comments, it sounds like we're going for a scheme of not logging if the operation as a whole fails due to reasons unrelated to permissions or ACLs. I think that makes sense, but please do preserve logging if successful.

          checkAccess is somewhat unique in that the point of the call is to determine if the caller has access. That led to the decision not to bother logging for the success path.

          Show
          cnauroth Chris Nauroth added a comment - We can skip success auditlog for checkAccess and getAclStatus. getAclStatus currently logs if successful, and we should preserve that. From reading earlier comments, it sounds like we're going for a scheme of not logging if the operation as a whole fails due to reasons unrelated to permissions or ACLs. I think that makes sense, but please do preserve logging if successful. checkAccess is somewhat unique in that the point of the call is to determine if the caller has access. That led to the decision not to bother logging for the success path.
          Hide
          vinayrpet Vinayakumar B added a comment -

          Thanks Kuhu Shukla.
          I got the Chris` point.
          We can skip success auditlog for checkAccess and getAclStatus.
          Thanks,

          Show
          vinayrpet Vinayakumar B added a comment - Thanks Kuhu Shukla . I got the Chris` point. We can skip success auditlog for checkAccess and getAclStatus. Thanks,
          Hide
          kshukla Kuhu Shukla added a comment -

          Kihwal Lee, Chris Nauroth, Vinayakumar B, Requesting for comments on whether checkAccess and getAclStatus should follow the same scheme. Thanks a lot for all the help!

          Show
          kshukla Kuhu Shukla added a comment - Kihwal Lee , Chris Nauroth , Vinayakumar B , Requesting for comments on whether checkAccess and getAclStatus should follow the same scheme. Thanks a lot for all the help!
          Hide
          kshukla Kuhu Shukla added a comment -

          Thank you Vinayakumar. While making this change I also noticed the checkAccess method which logs only when there is an ACE and not for successful attempts. My concern is from an older JIRA, HDFS-6570.

          getAclStatus is probably the simplest method to look at for an example that does everything. Do you think we need to write to the audit log for this method? I'm thinking that we shouldn't, because the purpose of this method is to query whether or not the user has access.

          Chris Nauroth, Request for some comments on the methods that dont log successful attempts like checkAccess and getAclStatus. Thanks a lot!

          Show
          kshukla Kuhu Shukla added a comment - Thank you Vinayakumar. While making this change I also noticed the checkAccess method which logs only when there is an ACE and not for successful attempts. My concern is from an older JIRA, HDFS-6570 . getAclStatus is probably the simplest method to look at for an example that does everything. Do you think we need to write to the audit log for this method? I'm thinking that we shouldn't, because the purpose of this method is to query whether or not the user has access. Chris Nauroth , Request for some comments on the methods that dont log successful attempts like checkAccess and getAclStatus. Thanks a lot!
          Hide
          vinayrpet Vinayakumar B added a comment -

          Thanks Kuhu Shukla for the updated patch.

          1. Following can be moved out of finally block.

                logAuditEvent(success, "setErasureCodingPolicy", srcArg, null,
                    resultingStat);
          	  

          2. Not related to patch, but can be included, if (auditLog.isInfoEnabled() && isExternalInvocation()) can be removed in startRollingUpgrade and finalizeRollingUpgrade, as its checked inside logAuditEvent(..) too.

              if (auditLog.isInfoEnabled() && isExternalInvocation()) {
                logAuditEvent(true, "startRollingUpgrade", null, null, null);
              }
              

          3.
          getAclStatus(), getXAttrs(..), listXAttrs missing, successfull auditlog.

          Show
          vinayrpet Vinayakumar B added a comment - Thanks Kuhu Shukla for the updated patch. 1. Following can be moved out of finally block. logAuditEvent(success, "setErasureCodingPolicy" , srcArg, null , resultingStat); 2. Not related to patch, but can be included, if (auditLog.isInfoEnabled() && isExternalInvocation()) can be removed in startRollingUpgrade and finalizeRollingUpgrade , as its checked inside logAuditEvent(..) too. if (auditLog.isInfoEnabled() && isExternalInvocation()) { logAuditEvent( true , "startRollingUpgrade" , null , null , null ); } 3. getAclStatus() , getXAttrs(..) , listXAttrs missing, successfull auditlog.
          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.
          0 mvndep 0m 18s Maven dependency ordering for branch
          +1 mvninstall 8m 37s trunk passed
          +1 compile 1m 7s trunk passed with JDK v1.8.0_66
          +1 compile 0m 56s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 28s trunk passed
          +1 mvnsite 1m 0s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 2m 1s trunk passed
          +1 javadoc 1m 10s trunk passed with JDK v1.8.0_66
          +1 javadoc 1m 53s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 9s Maven dependency ordering for patch
          +1 mvninstall 0m 48s the patch passed
          +1 compile 0m 41s the patch passed with JDK v1.8.0_66
          +1 javac 0m 41s the patch passed
          +1 compile 0m 42s the patch passed with JDK v1.7.0_91
          +1 javac 0m 43s the patch passed
          +1 checkstyle 0m 21s the patch passed
          +1 mvnsite 0m 53s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          -1 findbugs 2m 23s hadoop-hdfs-project/hadoop-hdfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 1m 15s the patch passed with JDK v1.8.0_66
          +1 javadoc 1m 54s the patch passed with JDK v1.7.0_91
          -1 unit 60m 0s hadoop-hdfs in the patch failed with JDK v1.8.0_66.
          +1 unit 55m 21s hadoop-hdfs in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 22s Patch does not generate ASF License warnings.
          145m 20s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs
            Nullcheck of effectiveDirective at line 6232 of value previously dereferenced in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean) At FSNamesystem.java:6232 of value previously dereferenced in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean) At FSNamesystem.java:[line 6232]
          JDK v1.8.0_66 Failed junit tests hadoop.hdfs.server.datanode.TestBlockScanner
            hadoop.hdfs.shortcircuit.TestShortCircuitCache



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786852/HDFS-9395.005.patch
          JIRA Issue HDFS-9395
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 1719f8ddb23e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 193d27d
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/14427/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14427/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14427/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14427/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Max memory used 77MB
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14427/console
          Powered by Apache Yetus 0.2.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. 0 mvndep 0m 18s Maven dependency ordering for branch +1 mvninstall 8m 37s trunk passed +1 compile 1m 7s trunk passed with JDK v1.8.0_66 +1 compile 0m 56s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 28s trunk passed +1 mvnsite 1m 0s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 2m 1s trunk passed +1 javadoc 1m 10s trunk passed with JDK v1.8.0_66 +1 javadoc 1m 53s trunk passed with JDK v1.7.0_91 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 0m 48s the patch passed +1 compile 0m 41s the patch passed with JDK v1.8.0_66 +1 javac 0m 41s the patch passed +1 compile 0m 42s the patch passed with JDK v1.7.0_91 +1 javac 0m 43s the patch passed +1 checkstyle 0m 21s the patch passed +1 mvnsite 0m 53s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 findbugs 2m 23s hadoop-hdfs-project/hadoop-hdfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 1m 15s the patch passed with JDK v1.8.0_66 +1 javadoc 1m 54s the patch passed with JDK v1.7.0_91 -1 unit 60m 0s hadoop-hdfs in the patch failed with JDK v1.8.0_66. +1 unit 55m 21s hadoop-hdfs in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 22s Patch does not generate ASF License warnings. 145m 20s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs   Nullcheck of effectiveDirective at line 6232 of value previously dereferenced in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean) At FSNamesystem.java:6232 of value previously dereferenced in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean) At FSNamesystem.java: [line 6232] JDK v1.8.0_66 Failed junit tests hadoop.hdfs.server.datanode.TestBlockScanner   hadoop.hdfs.shortcircuit.TestShortCircuitCache Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786852/HDFS-9395.005.patch JIRA Issue HDFS-9395 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1719f8ddb23e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 193d27d Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/14427/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/14427/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14427/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14427/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Max memory used 77MB Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14427/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          kshukla Kuhu Shukla added a comment -

          I have updated the patch. The patch does not change audit logging for :
          allowSnapshot, setErasureCodingPolicy, disallowSnapshot, finalizeRollingUpgrade since they use superUserPrivilege check and they should be addressed separately.
          Additionally added renameTo and getEZPath to the methods that need this change. renameTo now logs when there an ACE or the return value of success is true.
          I have not changed the return value of addCacheDirective since that (may) alter behavior and is not related audit logs. Please let me know if that sounds right Vinayakumar B. Thanks again.

          Changed the test to have @BeforeClass and made appropriate changes to fileSys re-initialization as it is closed on several occasions to instigate IOEs.

          Show
          kshukla Kuhu Shukla added a comment - I have updated the patch. The patch does not change audit logging for : allowSnapshot , setErasureCodingPolicy , disallowSnapshot , finalizeRollingUpgrade since they use superUserPrivilege check and they should be addressed separately. Additionally added renameTo and getEZPath to the methods that need this change. renameTo now logs when there an ACE or the return value of success is true. I have not changed the return value of addCacheDirective since that (may) alter behavior and is not related audit logs. Please let me know if that sounds right Vinayakumar B . Thanks again. Changed the test to have @BeforeClass and made appropriate changes to fileSys re-initialization as it is closed on several occasions to instigate IOEs.
          Hide
          cnauroth Chris Nauroth added a comment -

          Kuhu Shukla, thank you!

          Show
          cnauroth Chris Nauroth added a comment - Kuhu Shukla , thank you!
          Hide
          kshukla Kuhu Shukla added a comment -

          Thank you Vinayakumar B for the detailed comments. Will update patch shortly.

          Show
          kshukla Kuhu Shukla added a comment - Thank you Vinayakumar B for the detailed comments. Will update patch shortly.
          Hide
          kshukla Kuhu Shukla added a comment -

          Failing tests from last pre-commit were mostly due to timeouts and irreproducible locally. Requoting my comments from patch upload.

          Updated patch with changes to cachepool and other methods that need to follow the decided scheme with test. I have a few questions:
          1. for methods like getSnapshottableDirListing and listCachePools I am not sure how to make it throw an AccessControlException since it lists contents for the filesystem user itself.
          2. for isFileClosed() we never logged the successful operation, should we do that now, or will it be too noisy?
          3. for finalizeRollingUpgrade, the superuser privilege is already checked so I did not add the ACE try-catch block. Is that correct?

          Requesting comments from Kihwal Lee,Daryn Sharp, Colin P. McCabe. Thanks a lot!

          Show
          kshukla Kuhu Shukla added a comment - Failing tests from last pre-commit were mostly due to timeouts and irreproducible locally. Requoting my comments from patch upload. Updated patch with changes to cachepool and other methods that need to follow the decided scheme with test. I have a few questions: 1. for methods like getSnapshottableDirListing and listCachePools I am not sure how to make it throw an AccessControlException since it lists contents for the filesystem user itself. 2. for isFileClosed() we never logged the successful operation, should we do that now, or will it be too noisy? 3. for finalizeRollingUpgrade, the superuser privilege is already checked so I did not add the ACE try-catch block. Is that correct? Requesting comments from Kihwal Lee , Daryn Sharp , Colin P. McCabe . Thanks a lot!
          Hide
          vinayrpet Vinayakumar B added a comment -

          Below are the places need fix

          Audit Logging In Finally block
          ------------------

          addCacheDirective
          addCachePool
          getEZForPath
          listCacheDirectives
          listCachePools
          modifyCacheDirective
          modifyCachePool
          removeCacheDirective
          removeCachePool
          setErasureCodingPolicy
          

          Only Success audit logs ( Successfull Authorization )
          --------------------

          allowSnapshot
          concat
          createSnapshot
          deleteSnapshot
          disallowSnapshot
          finalizeRollingUpgrade
          computeSnapshotDiff
          listSnapshottableDirectory
          renameSnapshot
          startRollingUpgrade
          

          There might be some cases, where ACE will be thrown for Superuser check (admin commands).
          Logging failure audit log for this can be discussed in other Jira (May be HDFS-5040, since it is intended for that).
          Other places needs to be fixed.

          In Addition,
          FSN#renameTo(..), is mixing rename result with permission check to log at last. This will conflict with above scheme. So, instead only consider permission check.

          Based on above changes, Some audit logs ( for non-ACE failures ) will go missing.
          So this change needs to be marked as Incompatible, for heads-up.

          Patch should cover all above methods. I see some are not covered in current patch.

          Current changes in the patch looks good.
          Following are some nits.
          1.

          +    } catch (AccessControlException ace) {
          +      success = false;
          +      logAuditEvent(success, "deleteSnapshot", rootPath, null, null);
          +      throw ace;
          
          +    } catch (AccessControlException ace) {
          +      success = false;
          +      logAuditEvent(success, "modifyCacheDirective", idStr,
          +          directive.toString(), null);
          +      throw ace;
               } finally {
          

          success=false is not required. Its already false.

          2.

                 effectiveDirective = FSNDNCacheOp.addCacheDirective(this, cacheManager,
                     directive, flags, logRetryCache);
          +    } catch (AccessControlException ace) {
          +      logAuditEvent(success, "addCacheDirective", null,
          +          null, null);
          +      throw ace;

          In FSNamesystem#addCacheDirective(..), above code will not return null, it will either return non-null value, or throw exception.
          So, similar to other methods, success = true; can be inside try-block itself. And other null checks may not be required for effectiveDirective

          3. How about making the one cluster for all tests in @BeforeClass to speedup test?

          Show
          vinayrpet Vinayakumar B added a comment - Below are the places need fix Audit Logging In Finally block ------------------ addCacheDirective addCachePool getEZForPath listCacheDirectives listCachePools modifyCacheDirective modifyCachePool removeCacheDirective removeCachePool setErasureCodingPolicy Only Success audit logs ( Successfull Authorization ) -------------------- allowSnapshot concat createSnapshot deleteSnapshot disallowSnapshot finalizeRollingUpgrade computeSnapshotDiff listSnapshottableDirectory renameSnapshot startRollingUpgrade There might be some cases, where ACE will be thrown for Superuser check (admin commands). Logging failure audit log for this can be discussed in other Jira (May be HDFS-5040 , since it is intended for that). Other places needs to be fixed. In Addition, FSN#renameTo(..) , is mixing rename result with permission check to log at last. This will conflict with above scheme. So, instead only consider permission check. Based on above changes, Some audit logs ( for non-ACE failures ) will go missing. So this change needs to be marked as Incompatible, for heads-up. Patch should cover all above methods. I see some are not covered in current patch. Current changes in the patch looks good. Following are some nits. 1. + } catch (AccessControlException ace) { + success = false ; + logAuditEvent(success, "deleteSnapshot" , rootPath, null , null ); + throw ace; + } catch (AccessControlException ace) { + success = false ; + logAuditEvent(success, "modifyCacheDirective" , idStr, + directive.toString(), null ); + throw ace; } finally { success=false is not required. Its already false. 2. effectiveDirective = FSNDNCacheOp.addCacheDirective( this , cacheManager, directive, flags, logRetryCache); + } catch (AccessControlException ace) { + logAuditEvent(success, "addCacheDirective" , null , + null , null ); + throw ace; In FSNamesystem#addCacheDirective(..) , above code will not return null, it will either return non-null value, or throw exception. So, similar to other methods, success = true; can be inside try-block itself. And other null checks may not be required for effectiveDirective 3. How about making the one cluster for all tests in @BeforeClass to speedup test?
          Hide
          kshukla Kuhu Shukla added a comment -

          Thank Chris! I have followed up on HDFS-5730 and will also update HDFS-5040 accordingly today.

          Show
          kshukla Kuhu Shukla added a comment - Thank Chris! I have followed up on HDFS-5730 and will also update HDFS-5040 accordingly today.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 18m 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.
          0 mvndep 0m 19s Maven dependency ordering for branch
          +1 mvninstall 10m 58s trunk passed
          +1 compile 1m 47s trunk passed with JDK v1.8.0_72
          +1 compile 1m 15s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 38s trunk passed
          +1 mvnsite 1m 33s trunk passed
          +1 mvneclipse 0m 24s trunk passed
          +1 findbugs 2m 52s trunk passed
          +1 javadoc 1m 31s trunk passed with JDK v1.8.0_72
          +1 javadoc 2m 3s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 0m 55s the patch passed
          +1 compile 0m 54s the patch passed with JDK v1.8.0_72
          +1 javac 0m 54s the patch passed
          +1 compile 0m 44s the patch passed with JDK v1.7.0_95
          +1 javac 0m 44s the patch passed
          +1 checkstyle 0m 22s the patch passed
          +1 mvnsite 0m 57s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 17s the patch passed
          +1 javadoc 1m 15s the patch passed with JDK v1.8.0_72
          +1 javadoc 1m 59s the patch passed with JDK v1.7.0_95
          -1 unit 84m 45s hadoop-hdfs in the patch failed with JDK v1.8.0_72.
          -1 unit 100m 14s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 31s Patch does not generate ASF License warnings.
          239m 17s



          Reason Tests
          JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.datanode.TestBlockScanner
            hadoop.hdfs.server.datanode.TestDataNodeMetrics
            hadoop.hdfs.security.TestDelegationTokenForProxyUser
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.server.datanode.TestBlockReplacement
            hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks
            hadoop.hdfs.server.datanode.TestDirectoryScanner
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.datanode.TestBlockScanner
            hadoop.hdfs.TestHFlush
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.server.datanode.TestBlockReplacement
            hadoop.hdfs.server.namenode.ha.TestDFSUpgradeWithHA
            hadoop.hdfs.server.namenode.TestFileTruncate
            hadoop.hdfs.server.datanode.TestDirectoryScanner
          JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786751/HDFS-9395.004.patch
          JIRA Issue HDFS-9395
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 0ecb6b658178 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 5b59a0e
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14423/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14423/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14423/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14423/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14423/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Max memory used 77MB
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14423/console
          Powered by Apache Yetus 0.2.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 18m 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. 0 mvndep 0m 19s Maven dependency ordering for branch +1 mvninstall 10m 58s trunk passed +1 compile 1m 47s trunk passed with JDK v1.8.0_72 +1 compile 1m 15s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 38s trunk passed +1 mvnsite 1m 33s trunk passed +1 mvneclipse 0m 24s trunk passed +1 findbugs 2m 52s trunk passed +1 javadoc 1m 31s trunk passed with JDK v1.8.0_72 +1 javadoc 2m 3s trunk passed with JDK v1.7.0_95 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 55s the patch passed +1 compile 0m 54s the patch passed with JDK v1.8.0_72 +1 javac 0m 54s the patch passed +1 compile 0m 44s the patch passed with JDK v1.7.0_95 +1 javac 0m 44s the patch passed +1 checkstyle 0m 22s the patch passed +1 mvnsite 0m 57s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 17s the patch passed +1 javadoc 1m 15s the patch passed with JDK v1.8.0_72 +1 javadoc 1m 59s the patch passed with JDK v1.7.0_95 -1 unit 84m 45s hadoop-hdfs in the patch failed with JDK v1.8.0_72. -1 unit 100m 14s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 31s Patch does not generate ASF License warnings. 239m 17s Reason Tests JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.datanode.TestBlockScanner   hadoop.hdfs.server.datanode.TestDataNodeMetrics   hadoop.hdfs.security.TestDelegationTokenForProxyUser   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.server.datanode.TestBlockReplacement   hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks   hadoop.hdfs.server.datanode.TestDirectoryScanner JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.datanode.TestBlockScanner   hadoop.hdfs.TestHFlush   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.server.datanode.TestBlockReplacement   hadoop.hdfs.server.namenode.ha.TestDFSUpgradeWithHA   hadoop.hdfs.server.namenode.TestFileTruncate   hadoop.hdfs.server.datanode.TestDirectoryScanner JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2 Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786751/HDFS-9395.004.patch JIRA Issue HDFS-9395 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0ecb6b658178 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5b59a0e Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/14423/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14423/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14423/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14423/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14423/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Max memory used 77MB Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14423/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          kshukla Kuhu Shukla added a comment -

          Updating patch with corrected checkstyle warnings.

          Show
          kshukla Kuhu Shukla added a comment - Updating patch with corrected checkstyle warnings.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 11s Maven dependency ordering for branch
          +1 mvninstall 6m 39s trunk passed
          +1 compile 0m 37s trunk passed with JDK v1.8.0_72
          +1 compile 0m 41s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 23s trunk passed
          +1 mvnsite 0m 52s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 54s trunk passed
          +1 javadoc 1m 9s trunk passed with JDK v1.8.0_72
          +1 javadoc 1m 47s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 0m 47s the patch passed
          +1 compile 0m 37s the patch passed with JDK v1.8.0_72
          -1 javac 1m 32s hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72 with JDK v1.8.0_72 generated 2 new + 33 unchanged - 0 fixed = 35 total (was 33)
          +1 javac 0m 37s the patch passed
          +1 compile 0m 38s the patch passed with JDK v1.7.0_95
          -1 javac 2m 10s hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95 with JDK v1.7.0_95 generated 2 new + 35 unchanged - 0 fixed = 37 total (was 35)
          +1 javac 0m 38s the patch passed
          +1 checkstyle 0m 20s the patch passed
          +1 mvnsite 0m 48s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 7s the patch passed
          +1 javadoc 1m 5s the patch passed with JDK v1.8.0_72
          +1 javadoc 1m 42s the patch passed with JDK v1.7.0_95
          -1 unit 66m 35s hadoop-hdfs in the patch failed with JDK v1.8.0_72.
          -1 unit 65m 16s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 21s Patch does not generate ASF License warnings.
          157m 23s



          Reason Tests
          JDK v1.8.0_72 Failed junit tests hadoop.hdfs.TestRenameWhileOpen
            hadoop.hdfs.server.balancer.TestBalancer
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.shortcircuit.TestShortCircuitCache



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786593/HDFS-9395.003.patch
          JIRA Issue HDFS-9395
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9418b54c9868 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / cfa8513
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72: https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt
          javac hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14413/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Max memory used 76MB
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14413/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 11s Maven dependency ordering for branch +1 mvninstall 6m 39s trunk passed +1 compile 0m 37s trunk passed with JDK v1.8.0_72 +1 compile 0m 41s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 23s trunk passed +1 mvnsite 0m 52s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 54s trunk passed +1 javadoc 1m 9s trunk passed with JDK v1.8.0_72 +1 javadoc 1m 47s trunk passed with JDK v1.7.0_95 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 47s the patch passed +1 compile 0m 37s the patch passed with JDK v1.8.0_72 -1 javac 1m 32s hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72 with JDK v1.8.0_72 generated 2 new + 33 unchanged - 0 fixed = 35 total (was 33) +1 javac 0m 37s the patch passed +1 compile 0m 38s the patch passed with JDK v1.7.0_95 -1 javac 2m 10s hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95 with JDK v1.7.0_95 generated 2 new + 35 unchanged - 0 fixed = 37 total (was 35) +1 javac 0m 38s the patch passed +1 checkstyle 0m 20s the patch passed +1 mvnsite 0m 48s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 7s the patch passed +1 javadoc 1m 5s the patch passed with JDK v1.8.0_72 +1 javadoc 1m 42s the patch passed with JDK v1.7.0_95 -1 unit 66m 35s hadoop-hdfs in the patch failed with JDK v1.8.0_72. -1 unit 65m 16s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 21s Patch does not generate ASF License warnings. 157m 23s Reason Tests JDK v1.8.0_72 Failed junit tests hadoop.hdfs.TestRenameWhileOpen   hadoop.hdfs.server.balancer.TestBalancer JDK v1.7.0_95 Failed junit tests hadoop.hdfs.shortcircuit.TestShortCircuitCache Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786593/HDFS-9395.003.patch JIRA Issue HDFS-9395 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9418b54c9868 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / cfa8513 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72: https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt javac hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14413/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14413/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Max memory used 76MB Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14413/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          This effort likely supersedes prior unsubmitted patches attached to HDFS-5040 and HDFS-5730. It would be good to reconcile the activity there with the changes proposed here, and possibly close out those older issues.

          Show
          cnauroth Chris Nauroth added a comment - This effort likely supersedes prior unsubmitted patches attached to HDFS-5040 and HDFS-5730 . It would be good to reconcile the activity there with the changes proposed here, and possibly close out those older issues.
          Hide
          kshukla Kuhu Shukla added a comment -

          Updated patch with changes to cachepool and other methods that need to follow the decided scheme with test. I have a few questions:
          1. for methods like getSnapshottableDirListing and listCachePools I am not sure how to make it throw an AccessControlException since it lists contents for the filesystem user itself.
          2. for isFileClosed() we never logged the successful operation, should we do that now, or will it be too noisy?
          3. for finalizeRollingUpgrade, the superuser privilege is already checked so I did not add the ACE try-catch block. Is that correct?

          Show
          kshukla Kuhu Shukla added a comment - Updated patch with changes to cachepool and other methods that need to follow the decided scheme with test. I have a few questions: 1. for methods like getSnapshottableDirListing and listCachePools I am not sure how to make it throw an AccessControlException since it lists contents for the filesystem user itself. 2. for isFileClosed() we never logged the successful operation, should we do that now, or will it be too noisy? 3. for finalizeRollingUpgrade , the superuser privilege is already checked so I did not add the ACE try-catch block. Is that correct?
          Hide
          kshukla Kuhu Shukla added a comment -

          I will update the patch shortly to also cover cachePool operations. Thanks a lot for the helpful feedback.

          Show
          kshukla Kuhu Shukla added a comment - I will update the patch shortly to also cover cachePool operations. Thanks a lot for the helpful feedback.
          Hide
          cmccabe Colin P. McCabe added a comment -

          To make it simple, logAuditEvent(false, <cmd>, src); should be inside catch block of AccessControlException, and logAuditEvent(true, <cmd>, src); just before returning. No auditlogging in finally blocks. Right?

          +1

          Show
          cmccabe Colin P. McCabe added a comment - To make it simple, logAuditEvent(false, <cmd>, src); should be inside catch block of AccessControlException, and logAuditEvent(true, <cmd>, src); just before returning. No auditlogging in finally blocks. Right? +1
          Hide
          vinayrpet Vinayakumar B added a comment -

          If everyone agrees, then we should create a patch which fixes all the operations to log allowed=false only when there is a permission denied error, allowed=true only when the operation succeeds, and nothing otherwise.

          To make it simple, logAuditEvent(false, <cmd>, src); should be inside catch block of AccessControlException, and logAuditEvent(true, <cmd>, src); just before returning. No auditlogging in finally blocks. Right?

          Show
          vinayrpet Vinayakumar B added a comment - If everyone agrees, then we should create a patch which fixes all the operations to log allowed=false only when there is a permission denied error, allowed=true only when the operation succeeds, and nothing otherwise. To make it simple, logAuditEvent(false, <cmd>, src); should be inside catch block of AccessControlException , and logAuditEvent(true, <cmd>, src); just before returning. No auditlogging in finally blocks. Right?
          Hide
          kihwal Kihwal Lee added a comment -

          ... a patch which fixes all the operations to log allowed=false only when there is a permission denied error, allowed=true only when the operation succeeds, and nothing otherwise.

          +1 for this.

          Show
          kihwal Kihwal Lee added a comment - ... a patch which fixes all the operations to log allowed=false only when there is a permission denied error, allowed=true only when the operation succeeds, and nothing otherwise. +1 for this.
          Hide
          cmccabe Colin P. McCabe added a comment -

          My position is #2. The audit log should be a record of successful or unauthorized access to data - not a debug record of every attempted operation. The reduced SNR of already monstrous logs and the performance penalty (it's already extremely high) of logging unsuccessful operations generated by polling, globbing, file not found, rename targets existing, etc is of no value to me.

          Fair enough. The fact that the audit log line contains allowed=false on failure is also a strong suggestion that #2 is the original intention of the audit log. Like I said earlier, I haven't managed to find any documentation anywhere about this, though.

          I see your concern. I'm not aware of any existing issues involving non-ACE and I've been in the code a lot lately for performance work. Hypothetical future bugs is not compelling enough to cripple performance. If anyone feels strongly, it must be a config option that imposes no penalty when off.

          One thing that helps a lot is that most (all?) places use FSPermissionChecker to check permissions, rather than rolling their own permission check code.

          If everyone agrees, then we should create a patch which fixes all the operations to log allowed=false only when there is a permission denied error, allowed=true only when the operation succeeds, and nothing otherwise.

          Show
          cmccabe Colin P. McCabe added a comment - My position is #2. The audit log should be a record of successful or unauthorized access to data - not a debug record of every attempted operation. The reduced SNR of already monstrous logs and the performance penalty (it's already extremely high) of logging unsuccessful operations generated by polling, globbing, file not found, rename targets existing, etc is of no value to me. Fair enough. The fact that the audit log line contains allowed=false on failure is also a strong suggestion that #2 is the original intention of the audit log. Like I said earlier, I haven't managed to find any documentation anywhere about this, though. I see your concern. I'm not aware of any existing issues involving non-ACE and I've been in the code a lot lately for performance work. Hypothetical future bugs is not compelling enough to cripple performance. If anyone feels strongly, it must be a config option that imposes no penalty when off. One thing that helps a lot is that most (all?) places use FSPermissionChecker to check permissions, rather than rolling their own permission check code. If everyone agrees, then we should create a patch which fixes all the operations to log allowed=false only when there is a permission denied error, allowed=true only when the operation succeeds, and nothing otherwise.
          Hide
          daryn Daryn Sharp added a comment -

          My position is #2. The audit log should be a record of successful or unauthorized access to data - not a debug record of every attempted operation. The reduced SNR of already monstrous logs and the performance penalty (it's already extremely high) of logging unsuccessful operations generated by polling, globbing, file not found, rename targets existing, etc is of no value to me.

          One potential issue I see with category #2 is that if there is some failure that ultimately is permissions-related, but which fails to generate the specific AccessControlException subclass of exception, we will miss it.

          I see your concern. I'm not aware of any existing issues involving non-ACE and I've been in the code a lot lately for performance work. Hypothetical future bugs is not compelling enough to cripple performance. If anyone feels strongly, it must be a config option that imposes no penalty when off.

          Show
          daryn Daryn Sharp added a comment - My position is #2. The audit log should be a record of successful or unauthorized access to data - not a debug record of every attempted operation. The reduced SNR of already monstrous logs and the performance penalty (it's already extremely high) of logging unsuccessful operations generated by polling, globbing, file not found, rename targets existing, etc is of no value to me. One potential issue I see with category #2 is that if there is some failure that ultimately is permissions-related, but which fails to generate the specific AccessControlException subclass of exception, we will miss it. I see your concern. I'm not aware of any existing issues involving non-ACE and I've been in the code a lot lately for performance work. Hypothetical future bugs is not compelling enough to cripple performance. If anyone feels strongly, it must be a config option that imposes no penalty when off.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 9s Maven dependency ordering for branch
          +1 mvninstall 6m 59s trunk passed
          +1 compile 0m 44s trunk passed with JDK v1.8.0_66
          +1 compile 0m 41s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 24s trunk passed
          +1 mvnsite 0m 52s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 1m 58s trunk passed
          +1 javadoc 1m 4s trunk passed with JDK v1.8.0_66
          +1 javadoc 1m 49s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 0m 46s the patch passed
          +1 compile 0m 41s the patch passed with JDK v1.8.0_66
          +1 javac 0m 41s the patch passed
          +1 compile 0m 40s the patch passed with JDK v1.7.0_91
          +1 javac 0m 40s the patch passed
          +1 checkstyle 0m 20s the patch passed
          +1 mvnsite 0m 49s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 9s the patch passed
          +1 javadoc 1m 9s the patch passed with JDK v1.8.0_66
          +1 javadoc 1m 51s the patch passed with JDK v1.7.0_91
          -1 unit 66m 8s hadoop-hdfs in the patch failed with JDK v1.8.0_66.
          -1 unit 64m 4s hadoop-hdfs in the patch failed with JDK v1.7.0_91.
          +1 asflicense 0m 27s Patch does not generate ASF License warnings.
          156m 51s



          Reason Tests
          JDK v1.8.0_66 Failed junit tests hadoop.hdfs.security.TestDelegationTokenForProxyUser
            hadoop.hdfs.server.datanode.TestBlockScanner
            hadoop.hdfs.TestFileAppend
            hadoop.hdfs.TestReconstructStripedFile
          JDK v1.7.0_91 Failed junit tests hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
            hadoop.hdfs.server.namenode.ha.TestRequestHedgingProxyProvider
            hadoop.hdfs.server.datanode.TestBlockScanner
            hadoop.hdfs.server.blockmanagement.TestRBWBlockInvalidation
            hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFSStriped
            hadoop.hdfs.server.namenode.ha.TestStandbyCheckpoints



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12785853/HDFS-9395.002.patch
          JIRA Issue HDFS-9395
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 2879de1ae378 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 4ae543f
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14347/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14347/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14347/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14347/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14347/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Max memory used 77MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14347/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 6m 59s trunk passed +1 compile 0m 44s trunk passed with JDK v1.8.0_66 +1 compile 0m 41s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 24s trunk passed +1 mvnsite 0m 52s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 58s trunk passed +1 javadoc 1m 4s trunk passed with JDK v1.8.0_66 +1 javadoc 1m 49s trunk passed with JDK v1.7.0_91 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 0m 46s the patch passed +1 compile 0m 41s the patch passed with JDK v1.8.0_66 +1 javac 0m 41s the patch passed +1 compile 0m 40s the patch passed with JDK v1.7.0_91 +1 javac 0m 40s the patch passed +1 checkstyle 0m 20s the patch passed +1 mvnsite 0m 49s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 9s the patch passed +1 javadoc 1m 9s the patch passed with JDK v1.8.0_66 +1 javadoc 1m 51s the patch passed with JDK v1.7.0_91 -1 unit 66m 8s hadoop-hdfs in the patch failed with JDK v1.8.0_66. -1 unit 64m 4s hadoop-hdfs in the patch failed with JDK v1.7.0_91. +1 asflicense 0m 27s Patch does not generate ASF License warnings. 156m 51s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.hdfs.security.TestDelegationTokenForProxyUser   hadoop.hdfs.server.datanode.TestBlockScanner   hadoop.hdfs.TestFileAppend   hadoop.hdfs.TestReconstructStripedFile JDK v1.7.0_91 Failed junit tests hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl   hadoop.hdfs.server.namenode.ha.TestRequestHedgingProxyProvider   hadoop.hdfs.server.datanode.TestBlockScanner   hadoop.hdfs.server.blockmanagement.TestRBWBlockInvalidation   hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFSStriped   hadoop.hdfs.server.namenode.ha.TestStandbyCheckpoints Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12785853/HDFS-9395.002.patch JIRA Issue HDFS-9395 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2879de1ae378 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 4ae543f Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/14347/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14347/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14347/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14347/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14347/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Max memory used 77MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14347/console This message was automatically generated.
          Hide
          cmccabe Colin P. McCabe added a comment - - edited

          So, the big question here is what should go in the audit log? All failures, or just "permission denied" failures? Or, to put it a different way, if someone attempts to do something and it fails because a file doesn't exist, is that worth an audit log entry?

          We are currently inconsistent on this point. For example, concat, getContentSummary, addCacheDirective, and setErasureEncodingPolicy create an audit log entry for all failures, but setOwner, delete, and setAclEntries attempt to only create an entry for AccessControlException-based failures. There are a few operations, like allowSnapshot, disallowSnapshot, and startRollingUpgrade that never create audit log failure entries at all. They simply log nothing for any failure, and log success for a successful operation.

          So to summarize, different HDFS operations currently fall into 3 categories:
          1. audit-log all failures
          2. audit-log only AccessControlException failures
          3. never audit-log failures

          Category #3 seems like a clear violation of what people expect out of the audit log, since it will leave out all the unsuccessful attempts to do some privileged operation. So perhaps the category #3 operations are clearly buggy. The question then becomes, is the category #1 or #2 interpretation correct? One potential issue I see with category #2 is that if there is some failure that ultimately is permissions-related, but which fails to generate the specific AccessControlException subclass of exception, we will miss it. So category #1 operations are more robust against changes in the exception handling.

          Show
          cmccabe Colin P. McCabe added a comment - - edited So, the big question here is what should go in the audit log? All failures, or just "permission denied" failures? Or, to put it a different way, if someone attempts to do something and it fails because a file doesn't exist, is that worth an audit log entry? We are currently inconsistent on this point. For example, concat , getContentSummary , addCacheDirective , and setErasureEncodingPolicy create an audit log entry for all failures, but setOwner , delete , and setAclEntries attempt to only create an entry for AccessControlException -based failures. There are a few operations, like allowSnapshot , disallowSnapshot , and startRollingUpgrade that never create audit log failure entries at all. They simply log nothing for any failure, and log success for a successful operation. So to summarize, different HDFS operations currently fall into 3 categories: 1. audit-log all failures 2. audit-log only AccessControlException failures 3. never audit-log failures Category #3 seems like a clear violation of what people expect out of the audit log, since it will leave out all the unsuccessful attempts to do some privileged operation. So perhaps the category #3 operations are clearly buggy. The question then becomes, is the category #1 or #2 interpretation correct? One potential issue I see with category #2 is that if there is some failure that ultimately is permissions-related, but which fails to generate the specific AccessControlException subclass of exception, we will miss it. So category #1 operations are more robust against changes in the exception handling.
          Hide
          kshukla Kuhu Shukla added a comment -

          Updated patch that addresses findbugs and checkstyle issues. The test failures are irreproducible on local machine and are also unrelated to this change.

          Show
          kshukla Kuhu Shukla added a comment - Updated patch that addresses findbugs and checkstyle issues. The test failures are irreproducible on local machine and are also unrelated to this change.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 14s Maven dependency ordering for branch
          +1 mvninstall 6m 34s trunk passed
          +1 compile 0m 40s trunk passed with JDK v1.8.0_66
          +1 compile 0m 41s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 52s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 53s trunk passed
          +1 javadoc 1m 6s trunk passed with JDK v1.8.0_66
          +1 javadoc 1m 45s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 0m 45s the patch passed
          +1 compile 0m 38s the patch passed with JDK v1.8.0_66
          +1 javac 0m 38s the patch passed
          +1 compile 0m 38s the patch passed with JDK v1.7.0_91
          +1 javac 0m 38s the patch passed
          -1 checkstyle 0m 19s hadoop-hdfs-project/hadoop-hdfs: patch generated 1 new + 186 unchanged - 0 fixed = 187 total (was 186)
          +1 mvnsite 0m 50s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          -1 findbugs 2m 9s hadoop-hdfs-project/hadoop-hdfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 1m 3s the patch passed with JDK v1.8.0_66
          +1 javadoc 1m 43s the patch passed with JDK v1.7.0_91
          -1 unit 51m 35s hadoop-hdfs in the patch failed with JDK v1.8.0_66.
          +1 unit 50m 49s hadoop-hdfs in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 20s Patch does not generate ASF License warnings.
          127m 51s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs
            Redundant nullcheck of effectiveDirective which is known to be null in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean) Redundant null check at FSNamesystem.java:is known to be null in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean) Redundant null check at FSNamesystem.java:[line 6211]
          JDK v1.8.0_66 Failed junit tests hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency
            hadoop.hdfs.server.datanode.TestBlockScanner



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12785714/HDFS-9395.001.patch
          JIRA Issue HDFS-9395
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e5cda7ea54d2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 700a176
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14344/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/14344/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14344/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14344/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14344/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Max memory used 76MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14344/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 14s Maven dependency ordering for branch +1 mvninstall 6m 34s trunk passed +1 compile 0m 40s trunk passed with JDK v1.8.0_66 +1 compile 0m 41s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 52s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 53s trunk passed +1 javadoc 1m 6s trunk passed with JDK v1.8.0_66 +1 javadoc 1m 45s trunk passed with JDK v1.7.0_91 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 45s the patch passed +1 compile 0m 38s the patch passed with JDK v1.8.0_66 +1 javac 0m 38s the patch passed +1 compile 0m 38s the patch passed with JDK v1.7.0_91 +1 javac 0m 38s the patch passed -1 checkstyle 0m 19s hadoop-hdfs-project/hadoop-hdfs: patch generated 1 new + 186 unchanged - 0 fixed = 187 total (was 186) +1 mvnsite 0m 50s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 findbugs 2m 9s hadoop-hdfs-project/hadoop-hdfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 1m 3s the patch passed with JDK v1.8.0_66 +1 javadoc 1m 43s the patch passed with JDK v1.7.0_91 -1 unit 51m 35s hadoop-hdfs in the patch failed with JDK v1.8.0_66. +1 unit 50m 49s hadoop-hdfs in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 20s Patch does not generate ASF License warnings. 127m 51s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs   Redundant nullcheck of effectiveDirective which is known to be null in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean) Redundant null check at FSNamesystem.java:is known to be null in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean) Redundant null check at FSNamesystem.java: [line 6211] JDK v1.8.0_66 Failed junit tests hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency   hadoop.hdfs.server.datanode.TestBlockScanner Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12785714/HDFS-9395.001.patch JIRA Issue HDFS-9395 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e5cda7ea54d2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 700a176 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14344/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/14344/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/14344/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14344/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_66.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14344/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Max memory used 76MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14344/console This message was automatically generated.
          Hide
          kshukla Kuhu Shukla added a comment -

          Thank you Colin P. McCabe for your comments. I am uploading a preliminary patch which in fact:

          • logs when AccessControlException(s) occur for methods like getContentSummary, setQuota etc. with allowed=false
          • logs for successful execution with allowed=true as expected.
          • not log in the case of IOExceptions.

          since we want allowed=true/false stand for access and not success/failure of the operation alone.

          Show
          kshukla Kuhu Shukla added a comment - Thank you Colin P. McCabe for your comments. I am uploading a preliminary patch which in fact: logs when AccessControlException(s) occur for methods like getContentSummary, setQuota etc. with allowed=false logs for successful execution with allowed=true as expected. not log in the case of IOExceptions. since we want allowed=true/false stand for access and not success/failure of the operation alone.
          Hide
          cmccabe Colin P. McCabe added a comment - - edited

          [edit: see my later comments about this. I was writing this assuming that audit log strategy #1 was correct, but without considering the other possibilities]

          If I understand correctly, the relevant code block is here:

            ContentSummary getContentSummary(final String src) throws IOException {
              checkOperation(OperationCategory.READ);
              readLock();
              boolean success = true;
              try {
                checkOperation(OperationCategory.READ);
                return FSDirStatAndListingOp.getContentSummary(dir, src);
              } catch (AccessControlException ace) {
                success = false;
                throw ace;
              } finally {
                readUnlock();
                logAuditEvent(success, "contentSummary", src);
              }
            }
          

          The code appears to be making the assumption that the only IOE that can be thrown is AccessControlException. I don't think this is correct. It would be better to change this to something like this, similar to our other audit log use-cases:

            ContentSummary getContentSummary(final String src) throws IOException {
              checkOperation(OperationCategory.READ);
              readLock();
              boolean success = false;
              try {
                checkOperation(OperationCategory.READ);
                ContentSummary csum = FSDirStatAndListingOp.getContentSummary(dir, src);
                success = true;
                return csum;
              } catch (AccessControlException ace) {
                throw ace;
              } finally {
                readUnlock();
                logAuditEvent(success, "contentSummary", src);
              }
            }
          

          It's by design? HDFS-5163

          No, it's a bug. Also, I looked at the code prior to the HDFS-4949 branch merge, and the bug existed prior to HDFS-5163 or any of the other HDFS-4949 JIRAs.

          Hope this helps.

          Show
          cmccabe Colin P. McCabe added a comment - - edited [edit: see my later comments about this. I was writing this assuming that audit log strategy #1 was correct, but without considering the other possibilities] If I understand correctly, the relevant code block is here: ContentSummary getContentSummary( final String src) throws IOException { checkOperation(OperationCategory.READ); readLock(); boolean success = true ; try { checkOperation(OperationCategory.READ); return FSDirStatAndListingOp.getContentSummary(dir, src); } catch (AccessControlException ace) { success = false ; throw ace; } finally { readUnlock(); logAuditEvent(success, "contentSummary" , src); } } The code appears to be making the assumption that the only IOE that can be thrown is AccessControlException . I don't think this is correct. It would be better to change this to something like this, similar to our other audit log use-cases: ContentSummary getContentSummary( final String src) throws IOException { checkOperation(OperationCategory.READ); readLock(); boolean success = false ; try { checkOperation(OperationCategory.READ); ContentSummary csum = FSDirStatAndListingOp.getContentSummary(dir, src); success = true ; return csum; } catch (AccessControlException ace) { throw ace; } finally { readUnlock(); logAuditEvent(success, "contentSummary" , src); } } It's by design? HDFS-5163 No, it's a bug. Also, I looked at the code prior to the HDFS-4949 branch merge, and the bug existed prior to HDFS-5163 or any of the other HDFS-4949 JIRAs. Hope this helps.
          Hide
          kshukla Kuhu Shukla added a comment -

          Colin P. McCabe, could you share any inputs you might have on this? Thanks a lot.

          Show
          kshukla Kuhu Shukla added a comment - Colin P. McCabe , could you share any inputs you might have on this? Thanks a lot.
          Hide
          kihwal Kihwal Lee added a comment -

          It's by design? HDFS-5163

          Show
          kihwal Kihwal Lee added a comment - It's by design? HDFS-5163

            People

            • Assignee:
              kshukla Kuhu Shukla
              Reporter:
              kihwal Kihwal Lee
            • Votes:
              0 Vote for this issue
              Watchers:
              16 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development