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

DelegationTokenIdentifier should cache the TokenIdentifier to UGI mapping

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: webhdfs
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      Webhdfs seeks involve closing the current connection, and reissuing a new open request with the new offset. The RPC layer caches connections so the DN keeps a lingering connection open to the NN. Connection caching is in part based on UGI. Although the client used the same token for the new offset request, the UGI is different which forces the DN to open another unnecessary connection to the NN.

      A job that performs many seeks will easily crash the NN due to fd exhaustion.

      1. HDFS-7597.patch
        4 kB
        Daryn Sharp
      2. HDFS-7597.patch
        5 kB
        Daryn Sharp
      3. HDFS-7597.patch
        5 kB
        Bob Hansen
      4. HDFS-7597.01.patch
        7 kB
        Xiao Chen

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #9998 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9998/)
          HDFS-7597. DelegationTokenIdentifier should cache the TokenIdentifier to (aajisaka: rev d433b16ce6d74f1a44bc29446c74b1cb5f8a10fa)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/DataNodeUGIProvider.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/security/TestDelegationToken.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/TestDataNodeUGIProvider.java
          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/security/token/delegation/DelegationTokenIdentifier.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #9998 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9998/ ) HDFS-7597 . DelegationTokenIdentifier should cache the TokenIdentifier to (aajisaka: rev d433b16ce6d74f1a44bc29446c74b1cb5f8a10fa) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/DataNodeUGIProvider.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/security/TestDelegationToken.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/TestDataNodeUGIProvider.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/security/token/delegation/DelegationTokenIdentifier.java
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Committed this to branch-2.8 and above. Thanks all who contributed to this issue!!

          Show
          ajisakaa Akira Ajisaka added a comment - Committed this to branch-2.8 and above. Thanks all who contributed to this issue!!
          Hide
          daryn Daryn Sharp added a comment -

          +1 Thanks for fixing the tests!

          Show
          daryn Daryn Sharp added a comment - +1 Thanks for fixing the tests!
          Hide
          ajisakaa Akira Ajisaka added a comment -

          LGTM, +1.

          Show
          ajisakaa Akira Ajisaka added a comment - LGTM, +1.
          Hide
          xiaochen Xiao Chen added a comment -

          Ping... Could anyone take a look?

          There were a +1 from Chris Nauroth and it seems to me very close to commit.
          Thanks!

          Show
          xiaochen Xiao Chen added a comment - Ping... Could anyone take a look? There were a +1 from Chris Nauroth and it seems to me very close to commit. Thanks!
          Hide
          xiaochen Xiao Chen added a comment -

          Latest failures seem unrelated. Could you or Chris Nauroth or Daryn Sharp review? Thank you.

          (BTW, thanks Kihwal Lee for launching the jenkins.)

          Show
          xiaochen Xiao Chen added a comment - Latest failures seem unrelated. Could you or Chris Nauroth or Daryn Sharp review? Thank you. (BTW, thanks Kihwal Lee for launching the jenkins.)
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 13m 4s Maven dependency ordering for branch
          +1 mvninstall 7m 49s trunk passed
          +1 compile 1m 59s trunk passed
          +1 checkstyle 0m 34s trunk passed
          +1 mvnsite 1m 39s trunk passed
          +1 mvneclipse 0m 26s trunk passed
          +1 findbugs 3m 38s trunk passed
          +1 javadoc 1m 28s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 1m 45s the patch passed
          +1 compile 1m 51s the patch passed
          +1 javac 1m 51s the patch passed
          +1 checkstyle 0m 28s the patch passed
          +1 mvnsite 1m 22s the patch passed
          +1 mvneclipse 0m 20s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 24s the patch passed
          +1 javadoc 1m 12s the patch passed
          +1 unit 0m 54s hadoop-hdfs-client in the patch passed.
          -1 unit 61m 46s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          105m 57s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.namenode.TestNamenodeCapacityReport
            hadoop.hdfs.server.datanode.TestDataNodeErasureCodingMetrics



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:e2f6409
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12810945/HDFS-7597.01.patch
          JIRA Issue HDFS-7597
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux cc0f47227f2c 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 / 5dfc38f
          Default Java 1.8.0_91
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15790/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15790/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15790/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15790/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 13m 4s Maven dependency ordering for branch +1 mvninstall 7m 49s trunk passed +1 compile 1m 59s trunk passed +1 checkstyle 0m 34s trunk passed +1 mvnsite 1m 39s trunk passed +1 mvneclipse 0m 26s trunk passed +1 findbugs 3m 38s trunk passed +1 javadoc 1m 28s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 45s the patch passed +1 compile 1m 51s the patch passed +1 javac 1m 51s the patch passed +1 checkstyle 0m 28s the patch passed +1 mvnsite 1m 22s the patch passed +1 mvneclipse 0m 20s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 24s the patch passed +1 javadoc 1m 12s the patch passed +1 unit 0m 54s hadoop-hdfs-client in the patch passed. -1 unit 61m 46s hadoop-hdfs in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 105m 57s Reason Tests Failed junit tests hadoop.hdfs.server.namenode.TestNamenodeCapacityReport   hadoop.hdfs.server.datanode.TestDataNodeErasureCodingMetrics Subsystem Report/Notes Docker Image:yetus/hadoop:e2f6409 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12810945/HDFS-7597.01.patch JIRA Issue HDFS-7597 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux cc0f47227f2c 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 / 5dfc38f Default Java 1.8.0_91 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/15790/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15790/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15790/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15790/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment - - edited

          Looks the test failures are related. I attached a patch to try to fix the test failures with minimum change, following the above discussions that we'll have a clean up jira after this.
          (Assigned to myself so I can attached a patch. Failed to assign back to Daryn because I can't seem to find him in the user dropdown list...)

          Show
          xiaochen Xiao Chen added a comment - - edited Looks the test failures are related. I attached a patch to try to fix the test failures with minimum change, following the above discussions that we'll have a clean up jira after this. (Assigned to myself so I can attached a patch. Failed to assign back to Daryn because I can't seem to find him in the user dropdown list...)
          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 2 new or modified test files.
          0 mvndep 0m 8s Maven dependency ordering for branch
          +1 mvninstall 7m 0s trunk passed
          +1 compile 1m 35s trunk passed
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 1m 30s trunk passed
          +1 mvneclipse 0m 23s trunk passed
          +1 findbugs 3m 17s trunk passed
          +1 javadoc 1m 16s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 1m 20s the patch passed
          +1 compile 1m 34s the patch passed
          +1 javac 1m 34s the patch passed
          -1 checkstyle 0m 27s hadoop-hdfs-project: The patch generated 2 new + 55 unchanged - 0 fixed = 57 total (was 55)
          +1 mvnsite 1m 24s the patch passed
          +1 mvneclipse 0m 19s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 28s the patch passed
          +1 javadoc 1m 11s the patch passed
          +1 unit 0m 54s hadoop-hdfs-client in the patch passed.
          -1 unit 65m 38s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 23s The patch does not generate ASF License warnings.
          94m 17s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider
            hadoop.hdfs.server.common.TestJspHelper
            hadoop.hdfs.server.blockmanagement.TestRBWBlockInvalidation
            hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery
            hadoop.hdfs.TestCrcCorruption



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:e2f6409
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12748885/HDFS-7597.patch
          JIRA Issue HDFS-7597
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 8ecc3dadbcdd 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 / 25064fb
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15781/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15781/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15781/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15781/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15781/console
          Powered by Apache Yetus 0.3.0 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 2 new or modified test files. 0 mvndep 0m 8s Maven dependency ordering for branch +1 mvninstall 7m 0s trunk passed +1 compile 1m 35s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 1m 30s trunk passed +1 mvneclipse 0m 23s trunk passed +1 findbugs 3m 17s trunk passed +1 javadoc 1m 16s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 20s the patch passed +1 compile 1m 34s the patch passed +1 javac 1m 34s the patch passed -1 checkstyle 0m 27s hadoop-hdfs-project: The patch generated 2 new + 55 unchanged - 0 fixed = 57 total (was 55) +1 mvnsite 1m 24s the patch passed +1 mvneclipse 0m 19s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 28s the patch passed +1 javadoc 1m 11s the patch passed +1 unit 0m 54s hadoop-hdfs-client in the patch passed. -1 unit 65m 38s hadoop-hdfs in the patch failed. +1 asflicense 0m 23s The patch does not generate ASF License warnings. 94m 17s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider   hadoop.hdfs.server.common.TestJspHelper   hadoop.hdfs.server.blockmanagement.TestRBWBlockInvalidation   hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery   hadoop.hdfs.TestCrcCorruption Subsystem Report/Notes Docker Image:yetus/hadoop:e2f6409 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12748885/HDFS-7597.patch JIRA Issue HDFS-7597 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 8ecc3dadbcdd 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 / 25064fb Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15781/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15781/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15781/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15781/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15781/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          kihwal Kihwal Lee added a comment -

          Should we trigger a new jenkins run and commit this patch?

          Yes, but the jenkins server is extremely slow. I could not kick the build manually. I will try it again later.

          Show
          kihwal Kihwal Lee added a comment - Should we trigger a new jenkins run and commit this patch? Yes, but the jenkins server is extremely slow. I could not kick the build manually. I will try it again later.
          Hide
          xiaochen Xiao Chen added a comment -

          I tried the patch on latest trunk, it still applies. Also tried to run the May 23 failed tests a couple of times, cannot reproduce. (Too old to see what exactly was failing in those tests)

          Should we trigger a new jenkins run and commit this patch? Thanks.

          Show
          xiaochen Xiao Chen added a comment - I tried the patch on latest trunk, it still applies. Also tried to run the May 23 failed tests a couple of times, cannot reproduce. (Too old to see what exactly was failing in those tests) Should we trigger a new jenkins run and commit this patch? Thanks.
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Yongjun Zhang / Daryn Sharp, Chris Nauroth and Xiaobing Zhou, please get this committed asap if you want this in the alpha 2.8.0 release. If not, I'll move it out within a week to the next release.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Yongjun Zhang / Daryn Sharp , Chris Nauroth and Xiaobing Zhou , please get this committed asap if you want this in the alpha 2.8.0 release. If not, I'll move it out within a week to the next release.
          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 2 new or modified test files.
          0 mvndep 0m 14s Maven dependency ordering for branch
          +1 mvninstall 7m 40s trunk passed
          +1 compile 1m 30s trunk passed
          +1 checkstyle 0m 31s trunk passed
          +1 mvnsite 1m 26s trunk passed
          +1 mvneclipse 0m 22s trunk passed
          +1 findbugs 3m 45s trunk passed
          +1 javadoc 1m 40s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 1m 43s the patch passed
          +1 compile 1m 42s the patch passed
          +1 javac 1m 42s the patch passed
          -1 checkstyle 0m 32s hadoop-hdfs-project: patch generated 2 new + 55 unchanged - 0 fixed = 57 total (was 55)
          +1 mvnsite 1m 40s the patch passed
          +1 mvneclipse 0m 20s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 3m 44s the patch passed
          +1 javadoc 1m 24s the patch passed
          +1 unit 0m 54s hadoop-hdfs-client in the patch passed.
          -1 unit 67m 12s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 23s Patch does not generate ASF License warnings.
          98m 31s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider
            hadoop.hdfs.server.common.TestJspHelper
            hadoop.hdfs.server.balancer.TestBalancer
          Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12748885/HDFS-7597.patch
          JIRA Issue HDFS-7597
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux df6da4756786 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 / 6d043aa
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15527/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15527/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15527/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15527/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15527/console
          Powered by Apache Yetus 0.2.0 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 2 new or modified test files. 0 mvndep 0m 14s Maven dependency ordering for branch +1 mvninstall 7m 40s trunk passed +1 compile 1m 30s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 1m 26s trunk passed +1 mvneclipse 0m 22s trunk passed +1 findbugs 3m 45s trunk passed +1 javadoc 1m 40s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 43s the patch passed +1 compile 1m 42s the patch passed +1 javac 1m 42s the patch passed -1 checkstyle 0m 32s hadoop-hdfs-project: patch generated 2 new + 55 unchanged - 0 fixed = 57 total (was 55) +1 mvnsite 1m 40s the patch passed +1 mvneclipse 0m 20s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 3m 44s the patch passed +1 javadoc 1m 24s the patch passed +1 unit 0m 54s hadoop-hdfs-client in the patch passed. -1 unit 67m 12s hadoop-hdfs in the patch failed. +1 asflicense 0m 23s Patch does not generate ASF License warnings. 98m 31s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider   hadoop.hdfs.server.common.TestJspHelper   hadoop.hdfs.server.balancer.TestBalancer Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2 Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12748885/HDFS-7597.patch JIRA Issue HDFS-7597 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux df6da4756786 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 / 6d043aa Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15527/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15527/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15527/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15527/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15527/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          yzhangal Yongjun Zhang added a comment -

          HI Daryn Sharp, Chris Nauroth and Xiaobing Zhou,

          Thanks a lot for your work here.

          Per my read of earlier discussion, it makes sense to me to replace the secure part fix of HDFS-8855 with the patch here (which is more general). However, I agree with Chris that we can commit the patch here and defer any further clean-up to a separate issue. I just kicked off a jenkins build to see if tests are clean.

          Thanks.

          Show
          yzhangal Yongjun Zhang added a comment - HI Daryn Sharp , Chris Nauroth and Xiaobing Zhou , Thanks a lot for your work here. Per my read of earlier discussion, it makes sense to me to replace the secure part fix of HDFS-8855 with the patch here (which is more general). However, I agree with Chris that we can commit the patch here and defer any further clean-up to a separate issue. I just kicked off a jenkins build to see if tests are clean. Thanks.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks all for the contribution and discussion.

          Chris Nauroth, do you think we can move forward to commit this patch? Please let me know if there's anything I can help with. Thanks!

          Show
          xiaochen Xiao Chen added a comment - Thanks all for the contribution and discussion. Chris Nauroth , do you think we can move forward to commit this patch? Please let me know if there's anything I can help with. Thanks!
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Daryn Sharp and Chris Nauroth At least for insecure case, HDFS-8855 patch is necessary because patch here doesn't address that. HDFS-8855 cached UGI based on users(e.g. real user and/or proxy user). For secure case, that part of HDFS-8855 could be pulled out unless DataNodeUGIProvider#tokenUGI can't guarantee different instances of DelegationTokenIdentifier returns same result of DelegationTokenIdentifier#equals (which is used by LRUMap to query entries) given the same token passed in by many requests.

          DataNodeUGIProvider#tokenUGI

              ByteArrayInputStream buf =
                new ByteArrayInputStream(token.getIdentifier());
              DataInputStream in = new DataInputStream(buf);
              DelegationTokenIdentifier id = new DelegationTokenIdentifier();
              id.readFields(in);
              UserGroupInformation ugi = id.getUser();
              ugi.addToken(token);
              return ugi;
          

          HDFS-7597 patch:

          70	  public UserGroupInformation getUser() {
          71	    UserGroupInformation ugi = ugiCache.get(this);
          72	    if (ugi == null) {
          73	      ugi = super.getUser();
          74	      ugiCache.put(this, ugi);
          75	    }
          76	    return ugi;
          77	  }
          

          HDFS-8855 patch:

          69	        final Token<DelegationTokenIdentifier> token = params.delegationToken();
          70	
          71	        ugi = ugiCache.get(buildTokenCacheKey(token),
          72	            new Callable<UserGroupInformation>() {
          73	              @Override
          74	              public UserGroupInformation call() throws Exception {
          75	                return tokenUGI(token);
          76	              }
          77	            });
          
          Show
          xiaobingo Xiaobing Zhou added a comment - Daryn Sharp and Chris Nauroth At least for insecure case, HDFS-8855 patch is necessary because patch here doesn't address that. HDFS-8855 cached UGI based on users(e.g. real user and/or proxy user). For secure case, that part of HDFS-8855 could be pulled out unless DataNodeUGIProvider#tokenUGI can't guarantee different instances of DelegationTokenIdentifier returns same result of DelegationTokenIdentifier#equals (which is used by LRUMap to query entries) given the same token passed in by many requests. DataNodeUGIProvider#tokenUGI ByteArrayInputStream buf = new ByteArrayInputStream(token.getIdentifier()); DataInputStream in = new DataInputStream(buf); DelegationTokenIdentifier id = new DelegationTokenIdentifier(); id.readFields(in); UserGroupInformation ugi = id.getUser(); ugi.addToken(token); return ugi; HDFS-7597 patch: 70 public UserGroupInformation getUser() { 71 UserGroupInformation ugi = ugiCache.get( this ); 72 if (ugi == null ) { 73 ugi = super .getUser(); 74 ugiCache.put( this , ugi); 75 } 76 return ugi; 77 } HDFS-8855 patch: 69 final Token<DelegationTokenIdentifier> token = params.delegationToken(); 70 71 ugi = ugiCache.get(buildTokenCacheKey(token), 72 new Callable<UserGroupInformation>() { 73 @Override 74 public UserGroupInformation call() throws Exception { 75 return tokenUGI(token); 76 } 77 });
          Hide
          cnauroth Chris Nauroth added a comment -

          I'm still not sure why HDFS-8855 is/was necessary because this internal patch solved the problem for us long ago.

          Yes, agreed. That's why it was a forehead-smacking moment when I realized the same issue essentially had been fixed twice mistakenly.

          I agree that this patch is a more general solution. We might consider pulling out HDFS-8855 later as a clean-up. As far as scope for this patch, do you want to do something to address the TestDataNodeUGIProvider failure, and we'll defer any further clean-up to a separate issue?

          Show
          cnauroth Chris Nauroth added a comment - I'm still not sure why HDFS-8855 is/was necessary because this internal patch solved the problem for us long ago. Yes, agreed. That's why it was a forehead-smacking moment when I realized the same issue essentially had been fixed twice mistakenly. I agree that this patch is a more general solution. We might consider pulling out HDFS-8855 later as a clean-up. As far as scope for this patch, do you want to do something to address the TestDataNodeUGIProvider failure, and we'll defer any further clean-up to a separate issue?
          Hide
          daryn Daryn Sharp added a comment -

          Chris Nauroth We can re-brand this as a more general improvement since it helps not only the DN but also the NN by reducing the per-connection UGI instances. I'm still not sure why HDFS-8855 is/was necessary because this internal patch solved the problem for us long ago.

          Show
          daryn Daryn Sharp added a comment - Chris Nauroth We can re-brand this as a more general improvement since it helps not only the DN but also the NN by reducing the per-connection UGI instances. I'm still not sure why HDFS-8855 is/was necessary because this internal patch solved the problem for us long ago.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 8s Maven dependency ordering for branch
          +1 mvninstall 6m 29s trunk passed
          +1 compile 1m 13s trunk passed with JDK v1.8.0_72
          +1 compile 1m 20s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 25s trunk passed
          +1 mvnsite 1m 24s trunk passed
          +1 mvneclipse 0m 26s trunk passed
          +1 findbugs 3m 28s trunk passed
          +1 javadoc 1m 23s trunk passed with JDK v1.8.0_72
          +1 javadoc 2m 12s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 17s the patch passed
          +1 compile 1m 9s the patch passed with JDK v1.8.0_72
          +1 javac 1m 9s the patch passed
          +1 compile 1m 19s the patch passed with JDK v1.7.0_95
          +1 javac 1m 19s the patch passed
          -1 checkstyle 0m 24s hadoop-hdfs-project: patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3)
          +1 mvnsite 1m 21s the patch passed
          +1 mvneclipse 0m 22s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 3m 54s the patch passed
          +1 javadoc 1m 22s the patch passed with JDK v1.8.0_72
          +1 javadoc 2m 8s the patch passed with JDK v1.7.0_95
          +1 unit 0m 49s hadoop-hdfs-client in the patch passed with JDK v1.8.0_72.
          -1 unit 66m 45s hadoop-hdfs in the patch failed with JDK v1.8.0_72.
          +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 79m 28s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 30s Patch does not generate ASF License warnings.
          183m 3s



          Reason Tests
          JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.common.TestJspHelper
            hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.datanode.TestBlockReplacement
            hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider
            hadoop.hdfs.server.namenode.TestEditLog



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12748885/HDFS-7597.patch
          JIRA Issue HDFS-7597
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux d929112951fd 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 / 5c465df
          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
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14674/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14674/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14674/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/14674/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14674/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/14674/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14674/console
          Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 8s Maven dependency ordering for branch +1 mvninstall 6m 29s trunk passed +1 compile 1m 13s trunk passed with JDK v1.8.0_72 +1 compile 1m 20s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 25s trunk passed +1 mvnsite 1m 24s trunk passed +1 mvneclipse 0m 26s trunk passed +1 findbugs 3m 28s trunk passed +1 javadoc 1m 23s trunk passed with JDK v1.8.0_72 +1 javadoc 2m 12s trunk passed with JDK v1.7.0_95 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 17s the patch passed +1 compile 1m 9s the patch passed with JDK v1.8.0_72 +1 javac 1m 9s the patch passed +1 compile 1m 19s the patch passed with JDK v1.7.0_95 +1 javac 1m 19s the patch passed -1 checkstyle 0m 24s hadoop-hdfs-project: patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3) +1 mvnsite 1m 21s the patch passed +1 mvneclipse 0m 22s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 3m 54s the patch passed +1 javadoc 1m 22s the patch passed with JDK v1.8.0_72 +1 javadoc 2m 8s the patch passed with JDK v1.7.0_95 +1 unit 0m 49s hadoop-hdfs-client in the patch passed with JDK v1.8.0_72. -1 unit 66m 45s hadoop-hdfs in the patch failed with JDK v1.8.0_72. +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 79m 28s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 30s Patch does not generate ASF License warnings. 183m 3s Reason Tests JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.common.TestJspHelper   hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.datanode.TestBlockReplacement   hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider   hadoop.hdfs.server.namenode.TestEditLog Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12748885/HDFS-7597.patch JIRA Issue HDFS-7597 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d929112951fd 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 / 5c465df 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 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14674/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14674/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14674/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/14674/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14674/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/14674/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14674/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          Oh right... in the meantime, there was HDFS-8855. Darn, I knew that issue sounded familiar, but I had forgotten it was this one.

          How shall we proceed?

          Show
          cnauroth Chris Nauroth added a comment - Oh right... in the meantime, there was HDFS-8855 . Darn, I knew that issue sounded familiar, but I had forgotten it was this one. How shall we proceed?
          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 2 new or modified test files.
          0 mvndep 0m 12s Maven dependency ordering for branch
          +1 mvninstall 7m 11s trunk passed
          +1 compile 1m 39s trunk passed with JDK v1.8.0_72
          +1 compile 1m 27s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 1m 29s trunk passed
          +1 mvneclipse 0m 24s trunk passed
          +1 findbugs 3m 33s trunk passed
          +1 javadoc 1m 35s trunk passed with JDK v1.8.0_72
          +1 javadoc 2m 16s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 9s Maven dependency ordering for patch
          +1 mvninstall 1m 18s the patch passed
          +1 compile 1m 32s the patch passed with JDK v1.8.0_72
          +1 javac 1m 32s the patch passed
          +1 compile 1m 25s the patch passed with JDK v1.7.0_95
          +1 javac 1m 25s the patch passed
          -1 checkstyle 0m 22s hadoop-hdfs-project: patch generated 1 new + 4 unchanged - 0 fixed = 5 total (was 4)
          +1 mvnsite 1m 24s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 4m 5s the patch passed
          +1 javadoc 1m 29s the patch passed with JDK v1.8.0_72
          +1 javadoc 2m 16s the patch passed with JDK v1.7.0_95
          +1 unit 0m 59s hadoop-hdfs-client in the patch passed with JDK v1.8.0_72.
          -1 unit 67m 7s hadoop-hdfs in the patch failed with JDK v1.8.0_72.
          +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 59m 14s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 23s Patch does not generate ASF License warnings.
          165m 47s



          Reason Tests
          JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.namenode.TestEditLog
            hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider
            hadoop.hdfs.server.common.TestJspHelper
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.namenode.TestEditLog
            hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12748885/HDFS-7597.patch
          JIRA Issue HDFS-7597
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c140c747da7f 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 / 5c465df
          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
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14676/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14676/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14676/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/14676/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14676/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/14676/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14676/console
          Powered by Apache Yetus 0.3.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 2 new or modified test files. 0 mvndep 0m 12s Maven dependency ordering for branch +1 mvninstall 7m 11s trunk passed +1 compile 1m 39s trunk passed with JDK v1.8.0_72 +1 compile 1m 27s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 26s trunk passed +1 mvnsite 1m 29s trunk passed +1 mvneclipse 0m 24s trunk passed +1 findbugs 3m 33s trunk passed +1 javadoc 1m 35s trunk passed with JDK v1.8.0_72 +1 javadoc 2m 16s trunk passed with JDK v1.7.0_95 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 18s the patch passed +1 compile 1m 32s the patch passed with JDK v1.8.0_72 +1 javac 1m 32s the patch passed +1 compile 1m 25s the patch passed with JDK v1.7.0_95 +1 javac 1m 25s the patch passed -1 checkstyle 0m 22s hadoop-hdfs-project: patch generated 1 new + 4 unchanged - 0 fixed = 5 total (was 4) +1 mvnsite 1m 24s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 4m 5s the patch passed +1 javadoc 1m 29s the patch passed with JDK v1.8.0_72 +1 javadoc 2m 16s the patch passed with JDK v1.7.0_95 +1 unit 0m 59s hadoop-hdfs-client in the patch passed with JDK v1.8.0_72. -1 unit 67m 7s hadoop-hdfs in the patch failed with JDK v1.8.0_72. +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 59m 14s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 23s Patch does not generate ASF License warnings. 165m 47s Reason Tests JDK v1.8.0_72 Failed junit tests hadoop.hdfs.server.namenode.TestEditLog   hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider   hadoop.hdfs.server.common.TestJspHelper JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.namenode.TestEditLog   hadoop.hdfs.server.datanode.web.webhdfs.TestDataNodeUGIProvider Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12748885/HDFS-7597.patch JIRA Issue HDFS-7597 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c140c747da7f 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 / 5c465df 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 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14676/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14676/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14676/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/14676/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_72.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14676/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/14676/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14676/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          I killed mine (14677). Thanks, Kihwal Lee.

          Show
          cnauroth Chris Nauroth added a comment - I killed mine (14677). Thanks, Kihwal Lee .
          Hide
          kihwal Kihwal Lee added a comment -

          I kicked the build already : https://builds.apache.org/job/PreCommit-HDFS-Build/14674
          I think this is what you started: https://builds.apache.org/job/PreCommit-HDFS-Build/14677
          Better kill one to avoid wasting resources.

          Show
          kihwal Kihwal Lee added a comment - I kicked the build already : https://builds.apache.org/job/PreCommit-HDFS-Build/14674 I think this is what you started: https://builds.apache.org/job/PreCommit-HDFS-Build/14677 Better kill one to avoid wasting resources.
          Hide
          cnauroth Chris Nauroth added a comment -

          Daryn Sharp, thank you for the reminder. It was a surprise, because I honestly thought we had already closed down on this one.

          I remain +1 for the patch, same as several months ago. I manually triggered a Jenkins run since it has been a while. I'll plan on committing this tomorrow barring any objections or surprises from pre-commit.

          Show
          cnauroth Chris Nauroth added a comment - Daryn Sharp , thank you for the reminder. It was a surprise, because I honestly thought we had already closed down on this one. I remain +1 for the patch, same as several months ago. I manually triggered a Jenkins run since it has been a while. I'll plan on committing this tomorrow barring any objections or surprises from pre-commit.
          Hide
          daryn Daryn Sharp added a comment -

          Patch still applies and is still relevant. It actually provides a performance boost to the NN due to reduced ugi instances. Instead of n-many ugis per task, it's a couple based on how quickly the lru recycles.

          Show
          daryn Daryn Sharp added a comment - Patch still applies and is still relevant. It actually provides a performance boost to the NN due to reduced ugi instances. Instead of n-many ugis per task, it's a couple based on how quickly the lru recycles.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 16m 52s Findbugs (version 3.0.0) appears to be broken on trunk.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 2 new or modified test files.
          +1 javac 7m 40s There were no new javac warning messages.
          +1 javadoc 9m 37s There were no new javadoc warning messages.
          +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 1m 59s The applied patch generated 1 new checkstyle issues (total was 11, now 12).
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 31s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 4m 22s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 native 3m 4s Pre-build of native portion
          -1 hdfs tests 162m 42s Tests failed in hadoop-hdfs.
          +1 hdfs tests 0m 26s Tests passed in hadoop-hdfs-client.
              209m 11s  



          Reason Tests
          Failed unit tests hadoop.hdfs.TestAppendSnapshotTruncate
            hadoop.hdfs.server.namenode.ha.TestStandbyIsHot



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

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 16m 52s Findbugs (version 3.0.0) appears to be broken on trunk. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 2 new or modified test files. +1 javac 7m 40s There were no new javac warning messages. +1 javadoc 9m 37s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 1m 59s The applied patch generated 1 new checkstyle issues (total was 11, now 12). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 31s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 4m 22s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 4s Pre-build of native portion -1 hdfs tests 162m 42s Tests failed in hadoop-hdfs. +1 hdfs tests 0m 26s Tests passed in hadoop-hdfs-client.     209m 11s   Reason Tests Failed unit tests hadoop.hdfs.TestAppendSnapshotTruncate   hadoop.hdfs.server.namenode.ha.TestStandbyIsHot Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12748885/HDFS-7597.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 4ab49a4 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11907/artifact/patchprocess/diffcheckstylehadoop-hdfs-client.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11907/artifact/patchprocess/testrun_hadoop-hdfs.txt hadoop-hdfs-client test log https://builds.apache.org/job/PreCommit-HDFS-Build/11907/artifact/patchprocess/testrun_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11907/testReport/ Java 1.7.0_55 uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11907/console This message was automatically generated.
          Hide
          bobhansen Bob Hansen added a comment -

          Refactored the diffs to move changes over to hadoop-client project

          Show
          bobhansen Bob Hansen added a comment - Refactored the diffs to move changes over to hadoop-client project
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 0s The patch command could not apply the patch during dryrun.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12692601/HDFS-7597.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 3cefc02
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10876/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 0s The patch command could not apply the patch during dryrun. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12692601/HDFS-7597.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 3cefc02 Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10876/console This message was automatically generated.
          Hide
          kihwal Kihwal Lee added a comment -

          DelegationTokenIdentifier.java has moved. Can you refresh the patch?

          Show
          kihwal Kihwal Lee added a comment - DelegationTokenIdentifier.java has moved. Can you refresh the patch?
          Hide
          wheat9 Haohui Mai added a comment -

          Daryn Sharp, can you please change the title and the description to match the intent of the patch?

          Show
          wheat9 Haohui Mai added a comment - Daryn Sharp , can you please change the title and the description to match the intent of the patch?
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12692601/HDFS-7597.patch
          against trunk revision e79be0e.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.tracing.TestTracing
          org.apache.hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA

          The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.blockmanagement.TestDatanodeManager

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9979//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9979//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12692601/HDFS-7597.patch against trunk revision e79be0e. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.tracing.TestTracing org.apache.hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.blockmanagement.TestDatanodeManager Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9979//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9979//console This message is automatically generated.
          Hide
          kihwal Kihwal Lee added a comment -

          precommit should work now. Kicked the build.

          Show
          kihwal Kihwal Lee added a comment - precommit should work now. Kicked the build.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Re-submitting to run Jenkins.

          Show
          ajisakaa Akira Ajisaka added a comment - Re-submitting to run Jenkins.
          Hide
          cnauroth Chris Nauroth added a comment - - edited

          +1 for the new patch, pending a fresh Jenkins run. Thanks, Daryn.

          Show
          cnauroth Chris Nauroth added a comment - - edited +1 for the new patch, pending a fresh Jenkins run. Thanks, Daryn.
          Hide
          daryn Daryn Sharp added a comment -

          The test failure was due to the test wanting to verify whether the NN would stomp the service addr in the token. The same token is used twice, once expecting the service to be changed, second not changed. Since the token ident is the same, the now cached ugi has associated the ident with two services. I simply made a new token for the "don't stomp the service" check since this condition was induced by the test doing something that won't happen in practice.

          Show
          daryn Daryn Sharp added a comment - The test failure was due to the test wanting to verify whether the NN would stomp the service addr in the token. The same token is used twice, once expecting the service to be changed, second not changed. Since the token ident is the same, the now cached ugi has associated the ident with two services. I simply made a new token for the "don't stomp the service" check since this condition was induced by the test doing something that won't happen in practice.
          Hide
          daryn Daryn Sharp added a comment -

          Did some quick and dirty tests. I tried to put the sync'ed LRUMap at a disadvantage by being reader heavy. A concurrent & bounded CacheBuilder map performs a bit slower but has wild variance whereas LRUMap is pretty stable. CB also increased the heap ~4-10X over the LRUMap.

          Show
          daryn Daryn Sharp added a comment - Did some quick and dirty tests. I tried to put the sync'ed LRUMap at a disadvantage by being reader heavy. A concurrent & bounded CacheBuilder map performs a bit slower but has wild variance whereas LRUMap is pretty stable. CB also increased the heap ~4-10X over the LRUMap .
          Hide
          daryn Daryn Sharp added a comment -

          I meant to say the map that the builder actually creates. The code for get/put is substantially more complicated than LRUMap so it can provide features we don't need in this use case.

          Maybe a CHM would be faster. I went with a simple solution to resolve recurring production issues. I'll try to run some quick benchmarks.

          The test failure is due to the way the test is written. I need to update it.

          Show
          daryn Daryn Sharp added a comment - I meant to say the map that the builder actually creates. The code for get/put is substantially more complicated than LRUMap so it can provide features we don't need in this use case. Maybe a CHM would be faster. I went with a simple solution to resolve recurring production issues. I'll try to run some quick benchmarks. The test failure is due to the way the test is written. I need to update it.
          Hide
          cmccabe Colin P. McCabe added a comment -

          The cache builder code is only used once at startup, though, to build the cache object. Being readable and developer-friendly is clearly the right thing to do for code that only runs once at startup. If there are examples of inefficiencies in the code that will actually be used at runtime, that would be more interesting.

          A CHM is neither useful nor performant unless you intend to cache many multiples of the number of accessing threads. Probably on the order of thousands which is overkill.

          Can you go into more detail about when the performance of a ConcurrentHashMap would be worse than a regular one? The last time I looked at it, CHM was just using lock striping. So basically each "get" or "put" takes a single lock, does its business, and then releases. This seems like the same level of overhead as a normal hash map. I don't think using multiple locks will be slower than one. By definition, interlocked instructions bypass CPU caches... that's what they're designed to do and must do.

          Like I said earlier, I am fine with this patch going in as-is (assuming the test failure is unrelated). But I'd like to get more understanding of the performance issues here so we can optimize in the future.

          Show
          cmccabe Colin P. McCabe added a comment - The cache builder code is only used once at startup, though, to build the cache object. Being readable and developer-friendly is clearly the right thing to do for code that only runs once at startup. If there are examples of inefficiencies in the code that will actually be used at runtime, that would be more interesting. A CHM is neither useful nor performant unless you intend to cache many multiples of the number of accessing threads. Probably on the order of thousands which is overkill. Can you go into more detail about when the performance of a ConcurrentHashMap would be worse than a regular one? The last time I looked at it, CHM was just using lock striping. So basically each "get" or "put" takes a single lock, does its business, and then releases. This seems like the same level of overhead as a normal hash map. I don't think using multiple locks will be slower than one. By definition, interlocked instructions bypass CPU caches... that's what they're designed to do and must do. Like I said earlier, I am fine with this patch going in as-is (assuming the test failure is unrelated). But I'd like to get more understanding of the performance issues here so we can optimize in the future.
          Hide
          daryn Daryn Sharp added a comment -

          Good discussion:

          • The commons LRUMap is no-frills, lightweight, and specifically tailored to this use case.
          • Guava in my experience is developer friendly at the expense of performance and excessive garbage. A quick glance at the cache builder code definitively confirms this.
          • A CHM is neither useful nor performant unless you intend to cache many multiples of the number of accessing threads. Probably on the order of thousands which is overkill.

          If we happen to rarely make 2 UGIs instead of 1, that's remarkably better than an unbounded N-many.

          Let me take a look at the test failure...

          Show
          daryn Daryn Sharp added a comment - Good discussion: The commons LRUMap is no-frills, lightweight, and specifically tailored to this use case. Guava in my experience is developer friendly at the expense of performance and excessive garbage. A quick glance at the cache builder code definitively confirms this. A CHM is neither useful nor performant unless you intend to cache many multiples of the number of accessing threads. Probably on the order of thousands which is overkill. If we happen to rarely make 2 UGIs instead of 1, that's remarkably better than an unbounded N-many. Let me take a look at the test failure...
          Hide
          cmccabe Colin P. McCabe added a comment -

          Good point, Chris. Actually, a Guava cache would be perfect here. I think it would be fine to do this in a follow-on change as well, if that's more convenient.

          Show
          cmccabe Colin P. McCabe added a comment - Good point, Chris. Actually, a Guava cache would be perfect here. I think it would be fine to do this in a follow-on change as well, if that's more convenient.
          Hide
          cnauroth Chris Nauroth added a comment -

          I don't believe ConcurrentHashMap alone provides the LRU semantics needed for this change. Another possibility is the Guava CacheBuilder, which advertises performance similar to ConcurrentHashMap with enforcement of a maximum size.

          http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/cache/CacheBuilder.html

          I'm still OK with the patch going in as is per my prior comment, but I would also +1 a version using CacheBuilder if others prefer.

          Show
          cnauroth Chris Nauroth added a comment - I don't believe ConcurrentHashMap alone provides the LRU semantics needed for this change. Another possibility is the Guava CacheBuilder , which advertises performance similar to ConcurrentHashMap with enforcement of a maximum size. http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/cache/CacheBuilder.html I'm still OK with the patch going in as is per my prior comment, but I would also +1 a version using CacheBuilder if others prefer.
          Hide
          cmccabe Colin P. McCabe added a comment - - edited

          Good find, Daryn. How about using ConcurrentHashMap#putIfAbsent or similar? Then the race condition could be totally eliminated, and also we would reduce the locking contention somewhat (because ConcurrentHashMap uses lock striping with multiple locks).

          Show
          cmccabe Colin P. McCabe added a comment - - edited Good find, Daryn. How about using ConcurrentHashMap#putIfAbsent or similar? Then the race condition could be totally eliminated, and also we would reduce the locking contention somewhat (because ConcurrentHashMap uses lock striping with multiple locks).
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12691375/HDFS-7597.patch
          against trunk revision ae91b13.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.common.TestJspHelper

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9167//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9167//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12691375/HDFS-7597.patch against trunk revision ae91b13. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.common.TestJspHelper Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9167//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9167//console This message is automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          I suppose another possibility could be a read-write lock, where the write half of the lock pair is only held for the put call. I think that's an optional thing though considering how small the race condition risk is here.

          +1 for the current patch pending Jenkins. Thanks again, Daryn.

          Show
          cnauroth Chris Nauroth added a comment - I suppose another possibility could be a read-write lock, where the write half of the lock pair is only held for the put call. I think that's an optional thing though considering how small the race condition risk is here. +1 for the current patch pending Jenkins. Thanks again, Daryn.
          Hide
          daryn Daryn Sharp added a comment -

          Good question, but synchronizing the whole operation will cause a cache-miss on one lookup to stall all other lookups including those that will be cache hits. While it might be tolerable on the the client-side with minimal impact, I don't think it's worth dragging down the performance of the server-side connection handling.

          Show
          daryn Daryn Sharp added a comment - Good question, but synchronizing the whole operation will cause a cache-miss on one lookup to stall all other lookups including those that will be cache hits. While it might be tolerable on the the client-side with minimal impact, I don't think it's worth dragging down the performance of the server-side connection handling.
          Hide
          cnauroth Chris Nauroth added a comment -

          Hi Daryn Sharp. Thanks for the patch. This looks good. Regarding the brief race condition, would it make sense to make the whole get-and-conditional-put operation synchronized on ugiCache?

          Show
          cnauroth Chris Nauroth added a comment - Hi Daryn Sharp . Thanks for the patch. This looks good. Regarding the brief race condition, would it make sense to make the whole get-and-conditional-put operation synchronized on ugiCache ?
          Hide
          daryn Daryn Sharp added a comment -

          Use a simple LRU cache on hdfs token identifiers. There's a possible race condition if two connections occur simultaneously when there's no cache entry. The result is 2 connections instead of 1, but the race is tiny and is preferable to an unbounded number of connections.

          Show
          daryn Daryn Sharp added a comment - Use a simple LRU cache on hdfs token identifiers. There's a possible race condition if two connections occur simultaneously when there's no cache entry. The result is 2 connections instead of 1, but the race is tiny and is preferable to an unbounded number of connections.
          Hide
          daryn Daryn Sharp added a comment - - edited

          The problem stems from the delegation token identifier generating a distinct UGI every time getUser() is called. The simple solution is caching the token identifier to ugi mapping. The webhdfs servlet extracts the token identifier's UGI so it can wrap the operation in a doAs. Caching will result in the same token identifier (from multiple connections) using a single UGI context on the client-side which then utilizes the possibly cached RPC connection.

          Caching will also benefit the server-side RPC layer. Multiple token-based connections will share a common UGI, thus reducing the garbage generated esp. for short-lived connections.

          This change should ideally be common, but I'm uncomfortable with whether other non-NN hadoop services (ex. yarn) are doing server-side manipulation of the ugi.

          Show
          daryn Daryn Sharp added a comment - - edited The problem stems from the delegation token identifier generating a distinct UGI every time getUser() is called. The simple solution is caching the token identifier to ugi mapping. The webhdfs servlet extracts the token identifier's UGI so it can wrap the operation in a doAs. Caching will result in the same token identifier (from multiple connections) using a single UGI context on the client-side which then utilizes the possibly cached RPC connection. Caching will also benefit the server-side RPC layer. Multiple token-based connections will share a common UGI, thus reducing the garbage generated esp. for short-lived connections. This change should ideally be common, but I'm uncomfortable with whether other non-NN hadoop services (ex. yarn) are doing server-side manipulation of the ugi.

            People

            • Assignee:
              daryn Daryn Sharp
              Reporter:
              daryn Daryn Sharp
            • Votes:
              0 Vote for this issue
              Watchers:
              23 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development