HADOOP-18851. Performance improvement for DelegationTokenSecretManager #6803

vikaskr22 · 2024-05-07T10:42:48Z

Description of PR

AbstractDelegationTokenSecretManager's method all synchronized and are blocking each other, even multiple readers threads are blocking each other. This PR is an efforts towards optimising the synchronization contexts.
For detailed description, please go through the discussion on https://issues.apache.org/jira/browse/HADOOP-18851

Latest review I received where I was suggested not to remove synchronization entirely instead address the threads blocking issue using Lock API. This approach will help avoid any unknown concurrency issues.

How was this patch tested?

Build is working fine. And this is more about a logical change like where to acquire or release a lock. It requires careful review. There is not any functional logic change.

For code changes:

[ Y] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
[N ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

vikaskr22 · 2024-05-07T10:48:51Z

@ChenSammi , Can you review the changes and provide your input ? Thanks.

hadoop-yetus · 2024-05-07T13:03:13Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 19s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+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	32m 28s		trunk passed
+1 💚	compile	8m 49s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	8m 8s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	checkstyle	0m 42s		trunk passed
+1 💚	mvnsite	1m 0s		trunk passed
+1 💚	javadoc	0m 46s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 35s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 32s		trunk passed
+1 💚	shadedclient	21m 23s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 30s		the patch passed
+1 💚	compile	8m 32s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	8m 32s		the patch passed
+1 💚	compile	8m 10s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	javac	8m 10s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 39s	/results-checkstyle-hadoop-common-project_hadoop-common.txt	hadoop-common-project/hadoop-common: The patch generated 13 new + 23 unchanged - 1 fixed = 36 total (was 24)
+1 💚	mvnsite	0m 58s		the patch passed
+1 💚	javadoc	0m 43s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 37s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 32s		the patch passed
+1 💚	shadedclient	21m 28s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	16m 52s		hadoop-common in the patch passed.
+1 💚	asflicense	0m 42s		The patch does not generate ASF License warnings.
		139m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/1/artifact/out/Dockerfile
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 963820cfbb47 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `cd01f45`
Default Java	Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/1/testReport/
Max. process+thread count	3152 (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-6803/1/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

...n/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java

hadoop-yetus · 2024-05-08T05:32:54Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
			_ Prechecks _
+1 💚	dupname	0m 00s		No case conflicting files found.
+0 🆗	spotbugs	0m 01s		spotbugs executables are not available.
+0 🆗	codespell	0m 01s		codespell was not available.
+0 🆗	detsecrets	0m 01s		detect-secrets was not available.
+1 💚	@author	0m 00s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 00s		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	112m 27s		trunk passed
+1 💚	compile	48m 25s		trunk passed
+1 💚	checkstyle	5m 17s		trunk passed
-1 ❌	mvnsite	5m 00s	/branch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in trunk failed.
+1 💚	javadoc	5m 38s		trunk passed
+1 💚	shadedclient	172m 01s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	6m 06s		the patch passed
+1 💚	compile	44m 21s		the patch passed
+1 💚	javac	44m 21s		the patch passed
+1 💚	blanks	0m 00s		The patch has no blanks issues.
+1 💚	checkstyle	5m 27s		the patch passed
-1 ❌	mvnsite	5m 04s	/patch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in the patch failed.
+1 💚	javadoc	5m 25s		the patch passed
+1 💚	shadedclient	190m 40s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	asflicense	7m 16s		The patch does not generate ASF License warnings.
		591m 12s

Subsystem	Report/Notes
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	MINGW64_NT-10.0-17763 94506f5a0dcd 3.4.10-87d57229.x86_64 2024-02-14 20:17 UTC x86_64 Msys
Build tool	maven
Personality	/c/hadoop/dev-support/bin/hadoop.sh
git revision	trunk / `cd01f45`
Default Java	Azul Systems, Inc.-1.8.0_332-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/1/testReport/
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/1/console
versions	git=2.44.0.windows.1
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2024-05-08T10:24:37Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 20s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+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	32m 52s		trunk passed
+1 💚	compile	8m 54s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	8m 7s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	checkstyle	0m 40s		trunk passed
+1 💚	mvnsite	0m 54s		trunk passed
+1 💚	javadoc	0m 46s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 33s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 30s		trunk passed
+1 💚	shadedclient	20m 47s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 31s		the patch passed
+1 💚	compile	8m 42s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	8m 42s		the patch passed
+1 💚	compile	8m 17s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	javac	8m 17s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 44s	/results-checkstyle-hadoop-common-project_hadoop-common.txt	hadoop-common-project/hadoop-common: The patch generated 13 new + 23 unchanged - 1 fixed = 36 total (was 24)
+1 💚	mvnsite	0m 56s		the patch passed
+1 💚	javadoc	0m 43s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 36s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 34s		the patch passed
+1 💚	shadedclient	21m 28s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	16m 57s		hadoop-common in the patch passed.
+1 💚	asflicense	0m 41s		The patch does not generate ASF License warnings.
		139m 41s

Subsystem	Report/Notes
Docker	ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/2/artifact/out/Dockerfile
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux abe3780c86f4 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `4c3f83a`
Default Java	Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/2/testReport/
Max. process+thread count	1272 (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-6803/2/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

ChenSammi · 2024-05-09T01:19:01Z

...n/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java

      running = true;
      tokenRemoverThread = new Daemon(new ExpiredTokenRemover());
      tokenRemoverThread.start();
+    }finally {


There need be a blank between "}" and finally. Kindly check other "finally" statements too.

Also please check the checkstyle issues reported by the CI, https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/2/artifact/out/results-checkstyle-hadoop-common-project_hadoop-common.txt.

Hexiaoqiao · 2024-05-09T04:03:49Z

Hi @vikaskr22 @ChenSammi Thanks for your works here. One nit concerns, #6001 try to remove synchronization then revert and add RWLock here, any issues do you meet while remove synchronization? Thanks.

vikaskr22 · 2024-05-09T12:46:18Z

Hi @Hexiaoqiao , technically I didn't see or observe any issues till now but @ChenSammi has some concerns related to concurrency in other sub classes of AbstractDelegationTokenSecretManager.java .

I had done testing with RangerKMS and there ZKDelegationTokenSecretManager.java is used as implementing class. That means, our test suite only verifies ZK specific cases. There are other implementation that is for HDFS federation, YARN etc. I
don't have any test suite that ensures concurrency stability for other implementations as well.

@ChenSammi point is, as part of last commit, I removed "synchronization" from super class and took care of concurrency in ZK sub class. So I was breaking concurrency at API level and all the implementing classes needs to take care of that. and at the same time, I don't have test suite internally that verifies anything except Ranger KMS DT usage.

One more point she raised that, what about any other subclass outside this repo? They might have written their code in assumption that lock has been acquired at API level.

Although I went through the source code of other implementing classes, and for me it still seems good. But due to lack of automated test suite and concurrency nature, I agreed to implement using Lock API.

Using ReadWriteLock API, at least I am able to unblock multiple reader threads, like verifyToken(). Now verifyToken() can be invoked by multiple threads due to ReadLock nature. And write lock will ensure thread safety at API level.

@ChenSammi , Please add your input as well in case I missed anything. Thanks.

ChenSammi · 2024-05-10T03:15:15Z

@ChenSammi , Please add your input as well in case I missed anything. Thanks.

Yes, removing the synchronized directly have potential risks to sub classes of AbstractDelegationTokenSecretManager. Using LOCK is a safe way to replace synchronized and can improve the concurrency too. That's the motivation of this implementation. @Hexiaoqiao .

hadoop-yetus · 2024-05-10T07:56:12Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
			_ Prechecks _
+1 💚	dupname	0m 00s		No case conflicting files found.
+0 🆗	spotbugs	0m 01s		spotbugs executables are not available.
+0 🆗	codespell	0m 01s		codespell was not available.
+0 🆗	detsecrets	0m 01s		detect-secrets was not available.
+1 💚	@author	0m 00s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 00s		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	90m 41s		trunk passed
+1 💚	compile	40m 44s		trunk passed
+1 💚	checkstyle	4m 36s		trunk passed
-1 ❌	mvnsite	4m 23s	/branch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in trunk failed.
+1 💚	javadoc	4m 42s		trunk passed
+1 💚	shadedclient	150m 01s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	5m 07s		the patch passed
+1 💚	compile	39m 30s		the patch passed
+1 💚	javac	39m 30s		the patch passed
+1 💚	blanks	0m 01s		The patch has no blanks issues.
+1 💚	checkstyle	4m 54s		the patch passed
-1 ❌	mvnsite	4m 28s	/patch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in the patch failed.
+1 💚	javadoc	4m 49s		the patch passed
+1 💚	shadedclient	161m 51s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	asflicense	5m 44s		The patch does not generate ASF License warnings.
		501m 20s

Subsystem	Report/Notes
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	MINGW64_NT-10.0-17763 0f898775fb38 3.4.10-87d57229.x86_64 2024-02-14 20:17 UTC x86_64 Msys
Build tool	maven
Personality	/c/hadoop/dev-support/bin/hadoop.sh
git revision	trunk / `4c3f83a`
Default Java	Azul Systems, Inc.-1.8.0_332-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/2/testReport/
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/2/console
versions	git=2.44.0.windows.1
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2024-05-10T15:21:09Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 21s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+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	32m 47s		trunk passed
+1 💚	compile	8m 56s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	8m 12s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	checkstyle	0m 43s		trunk passed
+1 💚	mvnsite	0m 57s		trunk passed
+1 💚	javadoc	0m 43s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 34s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 30s		trunk passed
+1 💚	shadedclient	20m 58s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 30s		the patch passed
+1 💚	compile	8m 30s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	8m 30s		the patch passed
+1 💚	compile	8m 9s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	javac	8m 9s		the patch passed
+1 💚	blanks	0m 1s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 38s	/results-checkstyle-hadoop-common-project_hadoop-common.txt	hadoop-common-project/hadoop-common: The patch generated 9 new + 23 unchanged - 1 fixed = 32 total (was 24)
+1 💚	mvnsite	0m 54s		the patch passed
+1 💚	javadoc	0m 42s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 35s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 33s		the patch passed
+1 💚	shadedclient	21m 0s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	17m 1s		hadoop-common in the patch passed.
+1 💚	asflicense	0m 42s		The patch does not generate ASF License warnings.
		138m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/3/artifact/out/Dockerfile
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 91ff72ba8905 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `18c4f3a`
Default Java	Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/3/testReport/
Max. process+thread count	1275 (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-6803/3/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2024-05-11T06:46:50Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
			_ Prechecks _
+1 💚	dupname	0m 00s		No case conflicting files found.
+0 🆗	spotbugs	0m 01s		spotbugs executables are not available.
+0 🆗	codespell	0m 01s		codespell was not available.
+0 🆗	detsecrets	0m 01s		detect-secrets was not available.
+1 💚	@author	0m 00s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 00s		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	126m 36s		trunk passed
+1 💚	compile	59m 52s		trunk passed
+1 💚	checkstyle	7m 21s		trunk passed
-1 ❌	mvnsite	6m 47s	/branch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in trunk failed.
+1 💚	javadoc	7m 15s		trunk passed
+1 💚	shadedclient	206m 03s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	8m 00s		the patch passed
+1 💚	compile	56m 30s		the patch passed
+1 💚	javac	56m 30s		the patch passed
+1 💚	blanks	0m 01s		The patch has no blanks issues.
+1 💚	checkstyle	7m 08s		the patch passed
-1 ❌	mvnsite	6m 50s	/patch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in the patch failed.
+1 💚	javadoc	7m 28s		the patch passed
+1 💚	shadedclient	227m 44s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	asflicense	8m 54s		The patch does not generate ASF License warnings.
		705m 05s

Subsystem	Report/Notes
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	MINGW64_NT-10.0-17763 2000e04dde5c 3.4.10-87d57229.x86_64 2024-02-14 20:17 UTC x86_64 Msys
Build tool	maven
Personality	/c/hadoop/dev-support/bin/hadoop.sh
git revision	trunk / `18c4f3a`
Default Java	Azul Systems, Inc.-1.8.0_332-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/3/testReport/
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/3/console
versions	git=2.44.0.windows.1
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

vikaskr22 · 2024-05-13T06:39:22Z

@Hexiaoqiao , @ChenSammi , Anything else needed from my side? I had fixed all the review comments that's part of my commit.

Hexiaoqiao · 2024-05-13T07:16:12Z

@ChenSammi Got it. Make sense to me.

Yes, removing the synchronized directly have potential risks to sub classes of AbstractDelegationTokenSecretManager. Using LOCK is a safe way to replace synchronized and can improve the concurrency too. That's the motivation of this implementation.

@vikaskr22 The current changes LGTM. One nit comment about mvnsite build failed, not sure if related with this changes. Would you mind to double check? Thanks.

ChenSammi · 2024-05-13T07:26:42Z

@Hexiaoqiao , the mvnsite failure is, looks like not relevant, but other merged MR seems doesn't have this failure.

[INFO] --- exec-maven-plugin:1.3.1:exec (shelldocs) @ hadoop-common ---
tar: apache-yetus-0.14.0/lib/precommit/qbt.sh: Cannot create symlink to 'test-patch.sh': No such file or directory
tar: Exiting with failure status due to previous errors
/usr/bin/env: 'python3': No such file or directory

hadoop-yetus · 2024-05-13T10:57:38Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 31s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+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	44m 48s		trunk passed
+1 💚	compile	17m 33s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	15m 47s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	checkstyle	1m 13s		trunk passed
+1 💚	mvnsite	1m 40s		trunk passed
+1 💚	javadoc	1m 9s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 46s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	2m 26s		trunk passed
+1 💚	shadedclient	35m 54s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 54s		the patch passed
+1 💚	compile	16m 28s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	16m 28s		the patch passed
+1 💚	compile	15m 37s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	javac	15m 37s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	1m 10s	/results-checkstyle-hadoop-common-project_hadoop-common.txt	hadoop-common-project/hadoop-common: The patch generated 9 new + 23 unchanged - 1 fixed = 32 total (was 24)
+1 💚	mvnsite	1m 36s		the patch passed
+1 💚	javadoc	1m 7s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 50s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	2m 38s		the patch passed
+1 💚	shadedclient	35m 7s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	20m 25s		hadoop-common in the patch passed.
+1 💚	asflicense	1m 0s		The patch does not generate ASF License warnings.
		222m 33s

Subsystem	Report/Notes
Docker	ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/4/artifact/out/Dockerfile
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 476c6deadd75 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `62949a3`
Default Java	Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/4/testReport/
Max. process+thread count	1309 (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-6803/4/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2024-05-13T15:39:00Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
			_ Prechecks _
+1 💚	dupname	0m 01s		No case conflicting files found.
+0 🆗	spotbugs	0m 00s		spotbugs executables are not available.
+0 🆗	codespell	0m 00s		codespell was not available.
+0 🆗	detsecrets	0m 00s		detect-secrets was not available.
+1 💚	@author	0m 00s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 00s		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	91m 48s		trunk passed
+1 💚	compile	40m 52s		trunk passed
+1 💚	checkstyle	4m 43s		trunk passed
-1 ❌	mvnsite	4m 21s	/branch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in trunk failed.
+1 💚	javadoc	4m 40s		trunk passed
+1 💚	shadedclient	147m 22s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	5m 14s		the patch passed
+1 💚	compile	38m 12s		the patch passed
+1 💚	javac	38m 13s		the patch passed
+1 💚	blanks	0m 00s		The patch has no blanks issues.
+1 💚	checkstyle	4m 42s		the patch passed
-1 ❌	mvnsite	4m 33s	/patch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in the patch failed.
+1 💚	javadoc	5m 04s		the patch passed
+1 💚	shadedclient	161m 04s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	asflicense	6m 04s		The patch does not generate ASF License warnings.
		498m 23s

Subsystem	Report/Notes
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	MINGW64_NT-10.0-17763 908f425e64f3 3.4.10-87d57229.x86_64 2024-02-14 20:17 UTC x86_64 Msys
Build tool	maven
Personality	/c/hadoop/dev-support/bin/hadoop.sh
git revision	trunk / `62949a3`
Default Java	Azul Systems, Inc.-1.8.0_332-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/4/testReport/
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/4/console
versions	git=2.45.0.windows.1
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

Hexiaoqiao · 2024-05-14T03:38:33Z

the mvnsite failure is, looks like not relevant, but other merged MR seems doesn't have this failure.

Hi @ChenSammi, @vikaskr22 mvnsite failure is not related to this PR, it has been followed-up by another thread.

hadoop-yetus · 2024-05-14T10:55:47Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 20s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+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	32m 22s		trunk passed
+1 💚	compile	8m 45s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	8m 3s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	checkstyle	0m 38s		trunk passed
+1 💚	mvnsite	0m 52s		trunk passed
+1 💚	javadoc	0m 44s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 30s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 28s		trunk passed
+1 💚	shadedclient	20m 34s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 30s		the patch passed
+1 💚	compile	9m 51s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	9m 51s		the patch passed
+1 💚	compile	8m 35s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	javac	8m 35s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 39s		hadoop-common-project/hadoop-common: The patch generated 0 new + 23 unchanged - 1 fixed = 23 total (was 24)
+1 💚	mvnsite	0m 53s		the patch passed
+1 💚	javadoc	0m 39s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 31s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 26s		the patch passed
+1 💚	shadedclient	22m 15s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	17m 28s		hadoop-common in the patch passed.
+1 💚	asflicense	0m 35s		The patch does not generate ASF License warnings.
		140m 2s

Subsystem	Report/Notes
Docker	ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/5/artifact/out/Dockerfile
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux fa3112f0de77 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `b506761`
Default Java	Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6803/5/testReport/
Max. process+thread count	1775 (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-6803/5/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

ChenSammi · 2024-05-14T12:44:52Z

@Hexiaoqiao , thanks for the info about mvnsite. Currently, hadoop-yetus is passed. This "Apache Yetus", I never see it's all green. Should this github CI be all green before the PR can be committed?

hadoop-yetus · 2024-05-14T19:35:54Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
			_ Prechecks _
+1 💚	dupname	0m 00s		No case conflicting files found.
+0 🆗	spotbugs	0m 01s		spotbugs executables are not available.
+0 🆗	codespell	0m 01s		codespell was not available.
+0 🆗	detsecrets	0m 01s		detect-secrets was not available.
+1 💚	@author	0m 01s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 00s		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	119m 49s		trunk passed
+1 💚	compile	56m 02s		trunk passed
+1 💚	checkstyle	6m 47s		trunk passed
-1 ❌	mvnsite	6m 20s	/branch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in trunk failed.
+1 💚	javadoc	6m 38s		trunk passed
+1 💚	shadedclient	190m 46s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	6m 51s		the patch passed
+1 💚	compile	51m 03s		the patch passed
+1 💚	javac	51m 03s		the patch passed
+1 💚	blanks	0m 00s		The patch has no blanks issues.
+1 💚	checkstyle	7m 14s		the patch passed
-1 ❌	mvnsite	6m 18s	/patch-mvnsite-hadoop-common-project_hadoop-common.txt	hadoop-common in the patch failed.
+1 💚	javadoc	6m 44s		the patch passed
+1 💚	shadedclient	208m 05s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	asflicense	8m 54s		The patch does not generate ASF License warnings.
		652m 41s

Subsystem	Report/Notes
GITHUB PR	#6803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	MINGW64_NT-10.0-17763 2348886a150d 3.4.10-87d57229.x86_64 2024-02-14 20:17 UTC x86_64 Msys
Build tool	maven
Personality	/c/hadoop/dev-support/bin/hadoop.sh
git revision	trunk / `b506761`
Default Java	Azul Systems, Inc.-1.8.0_332-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/5/testReport/
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6803/5/console
versions	git=2.44.0.windows.1
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

ChenSammi · 2024-05-16T04:31:20Z

Thanks @vikaskr22 for the contribution, and @Hexiaoqiao for the review.

apache#6803)

HADOOP-18852: DT performance improvement latest review incorporation

cd01f45

github-actions bot added trunk Common labels May 7, 2024

ChenSammi changed the title ~~HADOOP-18851: DT performance improvement latest review incorporation~~ HADOOP-18851. DT performance improvement latest review incorporation May 8, 2024

ChenSammi reviewed May 8, 2024

View reviewed changes

...n/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java Outdated Show resolved Hide resolved

ChenSammi reviewed May 8, 2024

View reviewed changes

...n/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java Outdated Show resolved Hide resolved

ChenSammi reviewed May 8, 2024

View reviewed changes

...n/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java Outdated Show resolved Hide resolved

HADOOP-18852: DT performance improvement latest review incorporation

4c3f83a

ChenSammi reviewed May 9, 2024

View reviewed changes

ChenSammi requested a review from jojochuang May 9, 2024 01:19

HADOOP-18852: DT performance improvement latest review incorporation

18c4f3a

fix checkstyle

62949a3

fix line longer than 100, remove unused function

b506761

ChenSammi changed the title ~~HADOOP-18851. DT performance improvement latest review incorporation~~ HADOOP-18851. Performance improvement for DelegationTokenSecretManager May 16, 2024

ChenSammi merged commit f8dce6c into apache:trunk May 16, 2024
1 of 5 checks passed

K0K0V0K pushed a commit to K0K0V0K/hadoop that referenced this pull request May 17, 2024

HADOOP-18851. Performance improvement for DelegationTokenSecretManager (

72e2048

apache#6803)

K0K0V0K pushed a commit to K0K0V0K/hadoop that referenced this pull request May 17, 2024

HADOOP-18851. Performance improvement for DelegationTokenSecretManager (

5b9c899

apache#6803)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-18851. Performance improvement for DelegationTokenSecretManager #6803

HADOOP-18851. Performance improvement for DelegationTokenSecretManager #6803

vikaskr22 commented May 7, 2024 •

edited

vikaskr22 commented May 7, 2024

hadoop-yetus commented May 7, 2024

hadoop-yetus commented May 8, 2024

hadoop-yetus commented May 8, 2024

ChenSammi May 9, 2024 •

edited

Hexiaoqiao commented May 9, 2024

vikaskr22 commented May 9, 2024

ChenSammi commented May 10, 2024

hadoop-yetus commented May 10, 2024

hadoop-yetus commented May 10, 2024

hadoop-yetus commented May 11, 2024

vikaskr22 commented May 13, 2024

Hexiaoqiao commented May 13, 2024

ChenSammi commented May 13, 2024 •

edited

hadoop-yetus commented May 13, 2024

hadoop-yetus commented May 13, 2024

Hexiaoqiao commented May 14, 2024

hadoop-yetus commented May 14, 2024

ChenSammi commented May 14, 2024 •

edited

hadoop-yetus commented May 14, 2024

ChenSammi commented May 16, 2024

HADOOP-18851. Performance improvement for DelegationTokenSecretManager #6803

HADOOP-18851. Performance improvement for DelegationTokenSecretManager #6803

Conversation

vikaskr22 commented May 7, 2024 • edited

Description of PR

How was this patch tested?

For code changes:

vikaskr22 commented May 7, 2024

hadoop-yetus commented May 7, 2024

hadoop-yetus commented May 8, 2024

hadoop-yetus commented May 8, 2024

ChenSammi May 9, 2024 • edited

Choose a reason for hiding this comment

Hexiaoqiao commented May 9, 2024

vikaskr22 commented May 9, 2024

ChenSammi commented May 10, 2024

hadoop-yetus commented May 10, 2024

hadoop-yetus commented May 10, 2024

hadoop-yetus commented May 11, 2024

vikaskr22 commented May 13, 2024

Hexiaoqiao commented May 13, 2024

ChenSammi commented May 13, 2024 • edited

hadoop-yetus commented May 13, 2024

hadoop-yetus commented May 13, 2024

Hexiaoqiao commented May 14, 2024

hadoop-yetus commented May 14, 2024

ChenSammi commented May 14, 2024 • edited

hadoop-yetus commented May 14, 2024

ChenSammi commented May 16, 2024

vikaskr22 commented May 7, 2024 •

edited

ChenSammi May 9, 2024 •

edited

ChenSammi commented May 13, 2024 •

edited

ChenSammi commented May 14, 2024 •

edited