Hudson added a comment - 29/Nov/16 02:40 SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10902 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10902/ ) HADOOP-13838 . KMSTokenRenewer should close providers (xiaochen via (rkanter: rev 47ca9e26fba4a639e43bee5bfc001ffc4b42330d) (edit) hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java

Xiao Chen added a comment - 29/Nov/16 02:24 Thank you Robert Kanter ! Cherry picked to branch-2.8 as well to match HADOOP-13155 .

Robert Kanter added a comment - 29/Nov/16 02:15 Thanks Xiao Chen . Committed to trunk and branch-2!

Robert Kanter added a comment - 29/Nov/16 02:04 +1

Xiao Chen added a comment - 29/Nov/16 02:02 Test failure look unrelated, and the checkstyle is about the test method being too long, which was already around the border before this change.

Hadoop QA added a comment - 29/Nov/16 01:58 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s 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 20s Maven dependency ordering for branch +1 mvninstall 6m 55s trunk passed +1 compile 9m 31s trunk passed +1 checkstyle 1m 37s trunk passed +1 mvnsite 2m 25s trunk passed +1 mvneclipse 0m 57s trunk passed +1 findbugs 3m 46s trunk passed +1 javadoc 1m 57s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 47s the patch passed +1 compile 9m 17s the patch passed +1 javac 9m 17s the patch passed -0 checkstyle 2m 56s root: The patch generated 1 new + 309 unchanged - 0 fixed = 310 total (was 309) +1 mvnsite 2m 40s the patch passed +1 mvneclipse 1m 6s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 4m 34s the patch passed +1 javadoc 2m 18s the patch passed -1 unit 9m 9s hadoop-common in the patch failed. +1 unit 2m 26s hadoop-kms in the patch passed. +1 unit 70m 49s hadoop-hdfs in the patch passed. +1 asflicense 0m 42s The patch does not generate ASF License warnings. 159m 40s Reason Tests Failed junit tests hadoop.net.TestDNS Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13838 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12840746/HADOOP-13838.02.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux fbc18b9b543d 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 / a2b1ff0 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11151/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/11151/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11151/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11151/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Xiao Chen added a comment - 28/Nov/16 23:04 Thanks a lot for the reviews, Robert and Andrew. Patch adds the unit test for KMS, and also closes the instance in FSN.

Hadoop QA added a comment - 28/Nov/16 22:19 -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 doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 8m 46s trunk passed +1 compile 10m 1s trunk passed +1 checkstyle 0m 28s trunk passed +1 mvnsite 1m 4s trunk passed +1 mvneclipse 0m 19s trunk passed +1 findbugs 1m 34s trunk passed +1 javadoc 0m 48s trunk passed +1 mvninstall 0m 39s the patch passed +1 compile 9m 56s the patch passed +1 javac 9m 56s the patch passed +1 checkstyle 0m 30s the patch passed +1 mvnsite 1m 3s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 56s the patch passed +1 javadoc 0m 55s the patch passed +1 unit 9m 22s hadoop-common in the patch passed. +1 asflicense 0m 34s The patch does not generate ASF License warnings. 50m 32s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13838 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12840723/HADOOP-13838.01.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c4bbf32e888a 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 / a2b1ff0 Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11150/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11150/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Andrew Wang added a comment - 28/Nov/16 21:38 Good catch here Xiao. Do you want to also close the key provider in FSNamesystem? I think this only really affects unit tests, but would be good code hygiene.

Robert Kanter added a comment - 28/Nov/16 21:38 The fix seems good to me. Can you add a unit test?

Xiao Chen added a comment - 28/Nov/16 21:24 This only applies to the KMSTokenRenwer added by HADOOP-13155 . Fixing in patch 1. HDFS clients caches the provider in clientcontext, which closes the provider on cache removal.