HADOOP-17208. LoadBalanceKMSClientProvider#deleteKey should invalidat… #2259

xiaoyuyao · 2020-08-28T23:36:42Z

https://issues.apache.org/jira/browse/HADOOP-17208

Proposed Fix:
Send invalidCache to all KMS provider instances upon delete to ensure the server side key cache is drained upon deletion.
Also fix a NPE when invalidCache for a key that has been deleted.

How is it tested:
Tested with RangerKMS with 3 instances.

…eCache via all KMSClientProvider instances.

hadoop-yetus · 2020-08-29T02:25:44Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 30s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+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.
		_ trunk Compile Tests _
+1 💚	mvninstall	30m 46s	trunk passed
+1 💚	compile	21m 52s	trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	compile	18m 17s	trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+1 💚	checkstyle	0m 46s	trunk passed
+1 💚	mvnsite	1m 27s	trunk passed
+1 💚	shadedclient	16m 4s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 30s	trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	javadoc	1m 26s	trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+0 🆗	spotbugs	2m 31s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	2m 28s	trunk passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 54s	the patch passed
+1 💚	compile	21m 18s	the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	javac	21m 18s	the patch passed
+1 💚	compile	18m 19s	the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+1 💚	javac	18m 19s	the patch passed
+1 💚	checkstyle	0m 43s	the patch passed
+1 💚	mvnsite	1m 23s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedclient	14m 0s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 27s	the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	javadoc	1m 28s	the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+1 💚	findbugs	2m 39s	the patch passed
		_ Other Tests _
+1 💚	unit	9m 45s	hadoop-common in the patch passed.
-1 ❌	asflicense	0m 44s	The patch generated 3 ASF License warnings.
		167m 46s

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2259/1/artifact/out/Dockerfile
GITHUB PR	#2259
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux fb302841850a 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `4454286`
Default Java	Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2259/1/testReport/
asflicense	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2259/1/artifact/out/patch-asflicense-problems.txt
Max. process+thread count	1669 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2259/1/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

Hexiaoqiao

Thanks @xiaoyuyao for your works. LGTM.
BTW, add new UT to cover invalidate cache when delete key should be better?

jiwq

Thanks for @xiaoyuyao contribution. I agree with @Hexiaoqiao comment that should add new UT which was lost before to cover the deleteKey method in this patch.

xiaoyuyao · 2020-09-12T00:51:06Z

Thanks @Hexiaoqiao and @jiwq for the review. The LBKMSCP is tricky to unit test within MiniCluster environment. The scenario here fits better for an end-to-end test that requires multiple KMS server instances to work with JCEKS, which does not have consistency guarantee like Ranger KMS. I would suggest open a separate ticket to add end-to-end test for KMS-HA, some of this can be implemented with docker based tests under junit.

Hexiaoqiao · 2020-09-12T02:40:12Z

Thanks @xiaoyuyao for your reply, it makes sense to me. asflicense warning seems not related, trigger ci manually and wait another building.

hadoop-yetus · 2020-09-12T05:17:57Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 31s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+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.
		_ trunk Compile Tests _
+1 💚	mvninstall	28m 47s	trunk passed
+1 💚	compile	19m 34s	trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	compile	16m 44s	trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+1 💚	checkstyle	0m 53s	trunk passed
+1 💚	mvnsite	1m 26s	trunk passed
+1 💚	shadedclient	16m 44s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 39s	trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	javadoc	1m 36s	trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+0 🆗	spotbugs	2m 19s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	2m 16s	trunk passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 50s	the patch passed
+1 💚	compile	18m 46s	the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	javac	18m 46s	the patch passed
+1 💚	compile	16m 51s	the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+1 💚	javac	16m 51s	the patch passed
+1 💚	checkstyle	0m 51s	the patch passed
+1 💚	mvnsite	1m 27s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedclient	14m 24s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 37s	the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	javadoc	1m 34s	the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+1 💚	findbugs	2m 21s	the patch passed
		_ Other Tests _
+1 💚	unit	9m 25s	hadoop-common in the patch passed.
+1 💚	asflicense	0m 56s	The patch does not generate ASF License warnings.
		159m 31s

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2259/2/artifact/out/Dockerfile
GITHUB PR	#2259
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 68b90656a559 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `958cab8`
Default Java	Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2259/2/testReport/
Max. process+thread count	1668 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2259/2/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

...-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java

xiaoyuyao

Thanks @Hexiaoqiao for the additional comments. I've posted my response inline.

Hexiaoqiao

Thanks @xiaoyuyao for your comments. +1 from my side.

…eCache via all KMSClientProvider instances. (apache#2259)

…eCache via all KMSClientProvider instances. (apache#2259) (cherry picked from commit 6adf846) Change-Id: I63e9c36fcba70bfa948553fbf4b7e3ec3e82d208

HADOOP-17208. LoadBalanceKMSClientProvider#deleteKey should invalidat…

609357f

…eCache via all KMSClientProvider instances.

Hexiaoqiao reviewed Aug 29, 2020

View reviewed changes

jiwq reviewed Aug 31, 2020

View reviewed changes

Hexiaoqiao reviewed Sep 15, 2020

View reviewed changes

...-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java Show resolved Hide resolved

xiaoyuyao commented Sep 16, 2020

View reviewed changes

Hexiaoqiao approved these changes Sep 17, 2020

View reviewed changes

xiaoyuyao merged commit 6adf846 into apache:trunk Sep 17, 2020

bilaharith pushed a commit to bilaharith/hadoop that referenced this pull request Sep 27, 2020

HADOOP-17208. LoadBalanceKMSClientProvider#deleteKey should invalidat…

c606f2e

…eCache via all KMSClientProvider instances. (apache#2259)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-17208. LoadBalanceKMSClientProvider#deleteKey should invalidat… #2259

HADOOP-17208. LoadBalanceKMSClientProvider#deleteKey should invalidat… #2259

xiaoyuyao commented Aug 28, 2020

hadoop-yetus commented Aug 29, 2020

Hexiaoqiao left a comment

jiwq left a comment

xiaoyuyao commented Sep 12, 2020

Hexiaoqiao commented Sep 12, 2020

hadoop-yetus commented Sep 12, 2020

xiaoyuyao left a comment

Hexiaoqiao left a comment

HADOOP-17208. LoadBalanceKMSClientProvider#deleteKey should invalidat… #2259

HADOOP-17208. LoadBalanceKMSClientProvider#deleteKey should invalidat… #2259

Conversation

xiaoyuyao commented Aug 28, 2020

hadoop-yetus commented Aug 29, 2020

Hexiaoqiao left a comment

Choose a reason for hiding this comment

jiwq left a comment

Choose a reason for hiding this comment

xiaoyuyao commented Sep 12, 2020

Hexiaoqiao commented Sep 12, 2020

hadoop-yetus commented Sep 12, 2020

xiaoyuyao left a comment

Choose a reason for hiding this comment

Hexiaoqiao left a comment

Choose a reason for hiding this comment