HADOOP-19346. ViewFileSystem.InnerCache: Replaced ReentrantReadWriteLock with ConcurrentHashMap/putIfAbsent() #7187

xinglin · 2024-11-26T04:55:46Z

Description of PR

HADOOP-19346. ViewFileSystem.InnerCache: Replaced ReentrantReadWriteLock with ConcurrentHashMap/putIfAbsent()

How was this patch tested?

mvn package -Pdist -PskipTests

For code changes:

Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
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?

…ck with ConcurrentHashMap/putIfAbsent()

xinglin · 2024-11-26T04:59:25Z

@ctrezzo: Could you take a look? thanks,

hadoop-yetus · 2024-11-26T07:13:42Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 18s		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	31m 53s		trunk passed
+1 💚	compile	9m 2s		trunk passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	compile	8m 8s		trunk passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	checkstyle	0m 45s		trunk passed
+1 💚	mvnsite	0m 59s		trunk passed
+1 💚	javadoc	0m 47s		trunk passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 35s		trunk passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	spotbugs	1m 32s		trunk passed
+1 💚	shadedclient	21m 9s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 30s		the patch passed
+1 💚	compile	8m 39s		the patch passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javac	8m 39s		the patch passed
+1 💚	compile	8m 15s		the patch passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	javac	8m 15s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 35s	/results-checkstyle-hadoop-common-project_hadoop-common.txt	hadoop-common-project/hadoop-common: The patch generated 1 new + 36 unchanged - 0 fixed = 37 total (was 36)
+1 💚	mvnsite	0m 55s		the patch passed
+1 💚	javadoc	0m 43s		the patch passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 36s		the patch passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
-1 ❌	spotbugs	1m 36s	/new-spotbugs-hadoop-common-project_hadoop-common.html	hadoop-common-project/hadoop-common generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
+1 💚	shadedclient	20m 58s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	17m 34s	/patch-unit-hadoop-common-project_hadoop-common.txt	hadoop-common in the patch passed.
+1 💚	asflicense	0m 41s		The patch does not generate ASF License warnings.
		136m 50s

Reason	Tests
SpotBugs	module:hadoop-common-project/hadoop-common
	ViewFileSystem$InnerCache$Key is incompatible with expected argument type org.apache.hadoop.fs.FileSystem in org.apache.hadoop.fs.viewfs.ViewFileSystem$InnerCache.get(URI, Configuration) At ViewFileSystem.java:argument type org.apache.hadoop.fs.FileSystem in org.apache.hadoop.fs.viewfs.ViewFileSystem$InnerCache.get(URI, Configuration) At ViewFileSystem.java:[line 130]
	Return value of putIfAbsent is ignored, but fs is reused in org.apache.hadoop.fs.viewfs.ViewFileSystem$InnerCache.get(URI, Configuration) At ViewFileSystem.java:ignored, but fs is reused in org.apache.hadoop.fs.viewfs.ViewFileSystem$InnerCache.get(URI, Configuration) At ViewFileSystem.java:[line 135]
Failed junit tests	hadoop.fs.viewfs.TestViewFileSystemLocalFileSystem
	hadoop.fs.viewfs.TestViewFileSystemWithAuthorityLocalFileSystem
	hadoop.fs.viewfs.TestViewFileSystemDelegationTokenSupport

Subsystem	Report/Notes
Docker	ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7187/1/artifact/out/Dockerfile
GITHUB PR	#7187
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux a5004d1939ea 5.15.0-124-generic #134-Ubuntu SMP Fri Sep 27 20:20:17 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `ccacc00`
Default Java	Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7187/1/testReport/
Max. process+thread count	3155 (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-7187/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.

...p-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java

sjlee · 2024-11-26T15:41:56Z

+1 to @pjfanning's comments. In fact, in principle all you need is just one call to computeIfAbsent() which takes care of thread safety, concurrency, and at-most-once cost for the computation. No need to call contains() or get() in addition.

The original commit is not correct in a concurrent situation FYI.

...p-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java

sjlee · 2024-11-26T16:00:18Z

Can you share some rationale or tabletop concurrent benchmark tests to show the improvements with the new approach from the old? I do think the ConcurrentHashMap is a fine implementation, but I'm curious what led you to look into the ReadWriteLock implementation.

xinglin · 2024-11-26T16:11:00Z

Can you share some rationale or tabletop concurrent benchmark tests to show the improvements with the new approach from the old? I do think the ConcurrentHashMap is a fine implementation, but I'm curious what led you to look into the ReadWriteLock implementation.

We are running a hadoop version without a lock protecting the InnerCache.get() and basically rediscovered this bug, after two months' investigation.

I was about to backport the ReadWriteLock approach from trunk. Then, a team member suggested that we could just use ConcurrentHashMap here, which provides a more clean approach than the current ReadWriteLock approach.

Performance-wise, I think they should be similar? Only one thread will be allowed to create the new FileSystem instance for each missing key in both cases.

hadoop-yetus · 2024-11-26T17:15:44Z

💔 -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 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	31m 44s		trunk passed
+1 💚	compile	8m 58s		trunk passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	compile	8m 11s		trunk passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	checkstyle	0m 42s		trunk passed
+1 💚	mvnsite	1m 1s		trunk passed
+1 💚	javadoc	0m 48s		trunk passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 31s		trunk passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	spotbugs	1m 32s		trunk passed
+1 💚	shadedclient	22m 17s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 35s		the patch passed
+1 💚	compile	8m 52s		the patch passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javac	8m 52s		the patch passed
+1 💚	compile	8m 23s		the patch passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	javac	8m 23s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 35s	/results-checkstyle-hadoop-common-project_hadoop-common.txt	hadoop-common-project/hadoop-common: The patch generated 2 new + 36 unchanged - 0 fixed = 38 total (was 36)
+1 💚	mvnsite	0m 56s		the patch passed
+1 💚	javadoc	0m 43s		the patch passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 29s		the patch passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
-1 ❌	spotbugs	1m 33s	/new-spotbugs-hadoop-common-project_hadoop-common.html	hadoop-common-project/hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
+1 💚	shadedclient	23m 46s		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 39s		The patch does not generate ASF License warnings.
		140m 22s

Reason	Tests
SpotBugs	module:hadoop-common-project/hadoop-common
	ViewFileSystem$InnerCache$Key is incompatible with expected argument type org.apache.hadoop.fs.FileSystem in org.apache.hadoop.fs.viewfs.ViewFileSystem$InnerCache.get(URI, Configuration) At ViewFileSystem.java:argument type org.apache.hadoop.fs.FileSystem in org.apache.hadoop.fs.viewfs.ViewFileSystem$InnerCache.get(URI, Configuration) At ViewFileSystem.java:[line 141]

Subsystem	Report/Notes
Docker	ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7187/2/artifact/out/Dockerfile
GITHUB PR	#7187
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 7f6f22115ba5 5.15.0-124-generic #134-Ubuntu SMP Fri Sep 27 20:20:17 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `3f3f145`
Default Java	Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7187/2/testReport/
Max. process+thread count	1310 (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-7187/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.

hadoop-yetus · 2024-11-26T18:19:26Z

💔 -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	34m 10s		trunk passed
+1 💚	compile	9m 53s		trunk passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	compile	8m 19s		trunk passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	checkstyle	0m 40s		trunk passed
+1 💚	mvnsite	0m 54s		trunk passed
+1 💚	javadoc	0m 43s		trunk passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 30s		trunk passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	spotbugs	1m 25s		trunk passed
+1 💚	shadedclient	24m 58s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 32s		the patch passed
+1 💚	compile	8m 34s		the patch passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javac	8m 34s		the patch passed
+1 💚	compile	8m 12s		the patch passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	javac	8m 12s		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 2 new + 36 unchanged - 0 fixed = 38 total (was 36)
+1 💚	mvnsite	0m 58s		the patch passed
+1 💚	javadoc	0m 44s		the patch passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 36s		the patch passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	spotbugs	1m 35s		the patch passed
+1 💚	shadedclient	21m 22s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	17m 38s		hadoop-common in the patch passed.
+1 💚	asflicense	0m 41s		The patch does not generate ASF License warnings.
		144m 31s

Subsystem	Report/Notes
Docker	ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7187/3/artifact/out/Dockerfile
GITHUB PR	#7187
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 29151b5fdcec 5.15.0-124-generic #134-Ubuntu SMP Fri Sep 27 20:20:17 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `1e17311`
Default Java	Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7187/3/testReport/
Max. process+thread count	3154 (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-7187/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.

sjlee · 2024-11-26T19:52:15Z

We are running a hadoop version without a lock protecting the InnerCache.get() and basically rediscovered this bug, after two months' investigation.

Oh so was this due to a correctness bug not performance that led you to this? It wasn't very clear from the JIRA description.

Performance-wise, I think they should be similar? Only one thread will be allowed to create the new FileSystem instance for each missing key in both cases.

ConcurrentHashMap is better because RWL locks globally whenever there is a mutation whereas ConcurrentHashMap can be more granular.

xinglin · 2024-11-26T21:14:30Z

Oh so was this due to a correctness bug not performance that led you to this? It wasn't very clear from the JIRA description.

yes, correctness issue: we are getting stale/incorrect reads from observer nodes (fileLen=0 for non-zero bytes files which has been closed by the same FileSystem object). We finally narrowed it down the RC: we are using two different dfs clients/ObserverReadProxyProviders, though the key (scheme/authority) to InnerCache.map.get() is the same.

Then, we compared trunk and our internal version. We found in our internal version (based off 2.10.0), we don't have the lock to prevent concurrent updates to InnerCache.map.

sjlee · 2024-11-26T21:28:21Z

I see. What you're saying is, your internal version has the RWL code removed and you are seeing a correctness issue. You're NOT saying the upstream version has a correctness issue, correct? From my cursory look, I think the existing code is correct in a concurrent setting.

If that's the case, my original comment basically stands. From the community standpoint, it would not be fixing a correctness issue but rather improving implementations and/or performance. While I think the new version is probably a better one, it would be good to see how both versions behave in a pretty high-concurrency situation. It shouldn't be too difficult to test quickly. WDYT?

xinglin · 2024-11-26T21:54:57Z

What you're saying is, your internal version has the RWL code removed and you are seeing a correctness issue. . You're NOT saying the upstream version has a correctness issue, correct?

Correct, our internal version simply did not have that PR, which added the lock. and it is our internal version which has the correctness issue. Trunk version is good.

We are not trying to improve the performance here. The concurrentHashMap is a cleaner approach than the old lock based approach. You already mentioned concurrentHashMap provides finer-grain concurrency and should improve performance. So, it is a win-win in both code-readable/maintainability and performance.

tbh, I did not want to spend time benchmarking RWLock vs concurrentHashMap. Both are standard constructs that we are supposed to use. This is client-side code: most of the overhead will be from communicating with NN/DN, vs competing for locks here.

hadoop-yetus · 2024-11-26T23:35:33Z

💔 -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	31m 59s		trunk passed
+1 💚	compile	8m 57s		trunk passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	compile	8m 3s		trunk passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	checkstyle	0m 43s		trunk passed
+1 💚	mvnsite	1m 2s		trunk passed
+1 💚	javadoc	0m 49s		trunk passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 35s		trunk passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	spotbugs	1m 32s		trunk passed
+1 💚	shadedclient	21m 17s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 33s		the patch passed
+1 💚	compile	8m 37s		the patch passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javac	8m 37s		the patch passed
+1 💚	compile	8m 9s		the patch passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	javac	8m 9s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 40s		the patch passed
+1 💚	mvnsite	0m 53s		the patch passed
+1 💚	javadoc	0m 43s		the patch passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 36s		the patch passed with JDK Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
+1 💚	spotbugs	1m 34s		the patch passed
+1 💚	shadedclient	21m 10s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	17m 43s		hadoop-common in the patch passed.
+1 💚	asflicense	0m 41s		The patch does not generate ASF License warnings.
		137m 20s

Subsystem	Report/Notes
Docker	ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7187/4/artifact/out/Dockerfile
GITHUB PR	#7187
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux f7df674b2134 5.15.0-124-generic #134-Ubuntu SMP Fri Sep 27 20:20:17 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `bbfa1e1`
Default Java	Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_432-8u432-ga~~us1-0ubuntu2~~20.04-ga
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7187/4/testReport/
Max. process+thread count	3154 (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-7187/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.

sjlee · 2024-11-26T23:54:01Z

+1 from me. Can you also update the JIRA description to match what it is?

Since I haven't committed PRs in a long while, it would take me a long time to get familiar with the steps. @ctrezzo, can you please review and commit if you approve? Thanks.

xinglin · 2024-11-27T01:06:42Z

Added more details to jira ticket.

ctrezzo

+1 It looks good to me as well. Thank you for the patch @xinglin !

HADOOP-19346 ViewFileSystem.InnerCache: Replaced ReentrantReadWriteLo…

ccacc00

…ck with ConcurrentHashMap/putIfAbsent()

github-actions bot added trunk Common labels Nov 26, 2024

pjfanning reviewed Nov 26, 2024

View reviewed changes

...p-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java Outdated Show resolved Hide resolved

Use computeIfAbsent()

3f3f145

pjfanning reviewed Nov 26, 2024

View reviewed changes

...p-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java Outdated Show resolved Hide resolved

sjlee reviewed Nov 26, 2024

View reviewed changes

...p-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java Outdated Show resolved Hide resolved

Removed unneeded contains check.

1e17311

fix checkstyle

bbfa1e1

ctrezzo approved these changes Nov 27, 2024

View reviewed changes

ctrezzo merged commit 964e089 into apache:trunk Nov 27, 2024
1 of 3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-19346. ViewFileSystem.InnerCache: Replaced ReentrantReadWriteLock with ConcurrentHashMap/putIfAbsent() #7187

HADOOP-19346. ViewFileSystem.InnerCache: Replaced ReentrantReadWriteLock with ConcurrentHashMap/putIfAbsent() #7187

xinglin commented Nov 26, 2024

xinglin commented Nov 26, 2024

hadoop-yetus commented Nov 26, 2024

sjlee commented Nov 26, 2024

sjlee commented Nov 26, 2024

xinglin commented Nov 26, 2024

hadoop-yetus commented Nov 26, 2024

hadoop-yetus commented Nov 26, 2024

sjlee commented Nov 26, 2024

xinglin commented Nov 26, 2024

sjlee commented Nov 26, 2024

xinglin commented Nov 26, 2024 •

edited

Loading

hadoop-yetus commented Nov 26, 2024

sjlee commented Nov 26, 2024 •

edited

Loading

xinglin commented Nov 27, 2024

ctrezzo left a comment

HADOOP-19346. ViewFileSystem.InnerCache: Replaced ReentrantReadWriteLock with ConcurrentHashMap/putIfAbsent() #7187

HADOOP-19346. ViewFileSystem.InnerCache: Replaced ReentrantReadWriteLock with ConcurrentHashMap/putIfAbsent() #7187

Conversation

xinglin commented Nov 26, 2024

Description of PR

How was this patch tested?

For code changes:

xinglin commented Nov 26, 2024

hadoop-yetus commented Nov 26, 2024

sjlee commented Nov 26, 2024

sjlee commented Nov 26, 2024

xinglin commented Nov 26, 2024

hadoop-yetus commented Nov 26, 2024

hadoop-yetus commented Nov 26, 2024

sjlee commented Nov 26, 2024

xinglin commented Nov 26, 2024

sjlee commented Nov 26, 2024

xinglin commented Nov 26, 2024 • edited Loading

hadoop-yetus commented Nov 26, 2024

sjlee commented Nov 26, 2024 • edited Loading

xinglin commented Nov 27, 2024

ctrezzo left a comment

Choose a reason for hiding this comment

xinglin commented Nov 26, 2024 •

edited

Loading

sjlee commented Nov 26, 2024 •

edited

Loading