HDFS-17458. Remove unnecessary BP lock in ReplicaMap. #6717

hfutatzhanghb · 2024-04-10T09:55:11Z

Description of PR

In HDFS-16429 we make LightWeightResizableGSet to be thread safe, and in HDFS-16511 we change some methods in ReplicaMap to acquire read lock instead of acquiring write lock.

This PR try to remove unnecessary Block_Pool read lock further.

Recently, I performed stress tests on datanodes to measure their read/write operations/second.

Before we removing some lock（createRbw、finalizeBlock etc.）, it can only achieve ~2K write ops. After optimizing, it can achieve more than 5K write ops.

I have summarize all caller methods of ReplicaMap#get, ReplicaMap#remove, ReplicaMap#replicas, ReplicaMap#add.
It is safe to remove.

Below table is ReplicaMap#add's invocation point.

method name	notes
append	VOLUME writeLock
createRbw	BLOCK_POOL readLock with inner VOLUME writeLock
convertTemporaryToRbw	VOLUME writeLock
finalizeReplica	VOLUME writeLock
checkAndUpdate	VOLUME writeLock
initReplicaRecoveryImpl	VOLUME writeLock（initReplicaRecovery）
updateReplicaUnderRecovery	VOLUME writeLock（在updateReplicaUnderRecovery）
evictBlocks	VOLUME writeLock
resolveDuplicateReplicas	VOLUME writeLock(checkAndUpdate)
readReplicasFromCache	not in dataset lock，dataset initialize。

hadoop-yetus · 2024-04-10T14:57:23Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	6m 41s		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 30s		trunk passed
+1 💚	compile	0m 42s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	0m 40s		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 45s		trunk passed
+1 💚	javadoc	0m 44s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	1m 7s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 42s		trunk passed
+1 💚	shadedclient	21m 31s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 38s		the patch passed
+1 💚	compile	0m 41s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	0m 41s		the patch passed
+1 💚	compile	0m 35s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	javac	0m 35s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 28s		the patch passed
+1 💚	mvnsite	0m 39s		the patch passed
+1 💚	javadoc	0m 31s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	1m 0s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
-1 ❌	spotbugs	1m 46s	/new-spotbugs-hadoop-hdfs-project_hadoop-hdfs.html	hadoop-hdfs-project/hadoop-hdfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
+1 💚	shadedclient	22m 9s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	204m 2s	/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt	hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 32s		The patch does not generate ASF License warnings.
		301m 12s

Reason	Tests
SpotBugs	module:hadoop-hdfs-project/hadoop-hdfs
	Return value of putIfAbsent is ignored, but curSet is reused in org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.ReplicaMap.mergeAll(ReplicaMap) At ReplicaMap.java:ignored, but curSet is reused in org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.ReplicaMap.mergeAll(ReplicaMap) At ReplicaMap.java:[line 178]
Failed junit tests	hadoop.hdfs.tools.TestDFSAdmin

Subsystem	Report/Notes
Docker	ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6717/1/artifact/out/Dockerfile
GITHUB PR	#6717
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux bf96324ae525 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 / `4746843`
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-6717/1/testReport/
Max. process+thread count	4322 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6717/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.

hfutatzhanghb · 2024-04-11T06:31:05Z

@Hexiaoqiao @zhangshuyan0 @tomscut Sir, could you please take a look at this PR when you have free time? Thanks a lot.

hadoop-yetus · 2024-04-11T06:53:24Z

💔 -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 45s		trunk passed
+1 💚	compile	0m 41s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	0m 40s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	checkstyle	0m 42s		trunk passed
+1 💚	mvnsite	0m 46s		trunk passed
+1 💚	javadoc	0m 42s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	1m 9s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 44s		trunk passed
+1 💚	shadedclient	21m 46s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 37s		the patch passed
+1 💚	compile	0m 41s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	0m 41s		the patch passed
+1 💚	compile	0m 37s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	javac	0m 37s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 30s		the patch passed
+1 💚	mvnsite	0m 38s		the patch passed
+1 💚	javadoc	0m 32s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	1m 4s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
-1 ❌	spotbugs	1m 48s	/new-spotbugs-hadoop-hdfs-project_hadoop-hdfs.html	hadoop-hdfs-project/hadoop-hdfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
+1 💚	shadedclient	22m 0s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	202m 55s	/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt	hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 27s		The patch does not generate ASF License warnings.
		294m 17s

Reason	Tests
SpotBugs	module:hadoop-hdfs-project/hadoop-hdfs
	Return value of putIfAbsent is ignored, but curSet is reused in org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.ReplicaMap.mergeAll(ReplicaMap) At ReplicaMap.java:ignored, but curSet is reused in org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.ReplicaMap.mergeAll(ReplicaMap) At ReplicaMap.java:[line 178]
Failed junit tests	hadoop.hdfs.server.datanode.TestDirectoryScanner

Subsystem	Report/Notes
Docker	ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6717/2/artifact/out/Dockerfile
GITHUB PR	#6717
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux fbf778c6fa4c 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 / `4288502`
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-6717/2/testReport/
Max. process+thread count	3976 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6717/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.

Hexiaoqiao · 2024-04-11T10:32:09Z

@hfutatzhanghb Thanks for your works. We should be careful to remove BP lock here. List one of the changes as example, it will return one definite value before this PR because hold RW lock here, but uncertain after this PR, such as another thread invoke map.put between map.get and return it will return null, but if invoke map.put before them it will return one ReplicaInfo object.

  ReplicaInfo get(String bpid, long blockId) {
    checkBlockPool(bpid);
    - try (AutoCloseDataSetLock l = lockManager.readLock(LockLevel.BLOCK_POOl, bpid)) {
    -   LightWeightResizableGSet<Block, ReplicaInfo> m = map.get(bpid);
    -   return m != null ? m.get(new Block(blockId)) : null;
    - }
    + LightWeightResizableGSet<Block, ReplicaInfo> m = map.get(bpid);
    + return m != null ? m.get(new Block(blockId)) : null;
  }

I didn't traverse all invoker here, and not sure if it will involve some potential risk. FYI.

hadoop-yetus · 2024-04-11T11:18:59Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 23s		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	34m 9s		trunk passed
+1 💚	compile	0m 47s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	0m 40s		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 45s		trunk passed
+1 💚	javadoc	0m 41s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	1m 8s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 44s		trunk passed
+1 💚	shadedclient	22m 9s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 37s		the patch passed
+1 💚	compile	0m 38s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	0m 38s		the patch passed
+1 💚	compile	0m 37s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	javac	0m 37s		the patch passed
+1 💚	blanks	0m 1s		The patch has no blanks issues.
+1 💚	checkstyle	0m 29s		the patch passed
+1 💚	mvnsite	0m 40s		the patch passed
+1 💚	javadoc	0m 31s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	1m 2s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	1m 42s		the patch passed
+1 💚	shadedclient	24m 23s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	197m 0s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 26s		The patch does not generate ASF License warnings.
		292m 52s

Subsystem	Report/Notes
Docker	ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6717/3/artifact/out/Dockerfile
GITHUB PR	#6717
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 159c60de77f8 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 / `4acc5b4`
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-6717/3/testReport/
Max. process+thread count	4388 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6717/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.

hfutatzhanghb · 2024-04-11T13:30:30Z

@hfutatzhanghb Thanks for your works. We should be careful to remove BP lock here. List one of the changes as example, it will return one definite value before this PR because hold RW lock here, but uncertain after this PR, such as another thread invoke map.put between map.get and return it will return null, but if invoke map.put before them it will return one ReplicaInfo object.
  ReplicaInfo get(String bpid, long blockId) {
    checkBlockPool(bpid);
    - try (AutoCloseDataSetLock l = lockManager.readLock(LockLevel.BLOCK_POOl, bpid)) {
    -   LightWeightResizableGSet<Block, ReplicaInfo> m = map.get(bpid);
    -   return m != null ? m.get(new Block(blockId)) : null;
    - }
    + LightWeightResizableGSet<Block, ReplicaInfo> m = map.get(bpid);
    + return m != null ? m.get(new Block(blockId)) : null;
  }
I didn't traverse all invoker here, and not sure if it will involve some potential risk. FYI.

Sir, Thanks for your replying. Yes, we need to be very careful to modify class ReplicaMap. In fact, i have check the methods one by one and I think we can push this PR forward after it runs stablely on our product for a long time.

hadoop-yetus · 2024-04-11T13:52:11Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	17m 46s		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	49m 38s		trunk passed
+1 💚	compile	1m 23s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	1m 15s		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 24s		trunk passed
+1 💚	javadoc	1m 8s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	1m 42s		trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	3m 18s		trunk passed
+1 💚	shadedclient	42m 45s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 12s		the patch passed
+1 💚	compile	1m 16s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	1m 16s		the patch passed
+1 💚	compile	1m 9s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	javac	1m 9s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	1m 2s		the patch passed
+1 💚	mvnsite	1m 14s		the patch passed
+1 💚	javadoc	0m 56s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	1m 34s		the patch passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06
+1 💚	spotbugs	3m 21s		the patch passed
+1 💚	shadedclient	41m 52s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	265m 27s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 48s		The patch does not generate ASF License warnings.
		442m 25s

Subsystem	Report/Notes
Docker	ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6717/4/artifact/out/Dockerfile
GITHUB PR	#6717
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux e0f27f718e00 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 / `4acc5b4`
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-6717/4/testReport/
Max. process+thread count	2635 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6717/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.

zhangshuyan0 · 2024-04-15T11:57:25Z

...oop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/ReplicaMap.java

    }
+    return m.put(replicaInfo);


It's not safe here. If there is somebody changing the map after line125 but before line127, the replicaInfo may not be able to added to map.

hadoop-yetus · 2024-04-24T06:40:14Z

💔 -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	110m 52s		trunk passed
+1 💚	compile	7m 26s		trunk passed
+1 💚	checkstyle	5m 54s		trunk passed
+1 💚	mvnsite	8m 10s		trunk passed
+1 💚	javadoc	7m 12s		trunk passed
+1 💚	shadedclient	179m 58s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	6m 11s		the patch passed
+1 💚	compile	4m 15s		the patch passed
+1 💚	javac	4m 15s		the patch passed
+1 💚	blanks	0m 00s		The patch has no blanks issues.
+1 💚	checkstyle	2m 52s		the patch passed
+1 💚	mvnsite	5m 06s		the patch passed
+1 💚	javadoc	4m 25s		the patch passed
+1 💚	shadedclient	191m 20s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	asflicense	6m 18s		The patch does not generate ASF License warnings.
		515m 24s

Subsystem	Report/Notes
GITHUB PR	#6717
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	MINGW64_NT-10.0-17763 8b1fb94b8141 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 / `4acc5b4`
Default Java	Azul Systems, Inc.-1.8.0_332-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6717/1/testReport/
modules	C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6717/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.

Hexiaoqiao · 2024-04-29T04:01:16Z

@hfutatzhanghb As discussed above (which is also confirmed by @zhangshuyan0), It is unsafe to remove BP lock here, just close this PR. Please feel free to reopen it if there is one more graceful improvement. Thanks again.

hfutatzhanghb · 2024-04-29T04:03:30Z

@hfutatzhanghb As discussed above (which is also confirmed by @zhangshuyan0), It is unsafe to remove BP lock here, just close this PR. Please feel free to reopen it if there is one more graceful improvement. Thanks again.

@Hexiaoqiao Ok, Sir. I have push another PR to optimize this. please check #6764

HDFS-17458. Remove unnecessary BP lock in ReplicaMap.

4746843

github-actions bot added HDFS trunk labels Apr 10, 2024

hfutatzhanghb added 3 commits April 11, 2024 09:57

ReplicaMap#replicas also need not BP read lock.

4288502

do not need to modify mergeAll.

375fab3

initBlockPool do not need modify

4acc5b4

zhangshuyan0 reviewed Apr 15, 2024

View reviewed changes

Hexiaoqiao closed this Apr 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDFS-17458. Remove unnecessary BP lock in ReplicaMap. #6717

HDFS-17458. Remove unnecessary BP lock in ReplicaMap. #6717

hfutatzhanghb commented Apr 10, 2024 •

edited

Loading

hadoop-yetus commented Apr 10, 2024

hfutatzhanghb commented Apr 11, 2024

hadoop-yetus commented Apr 11, 2024

Hexiaoqiao commented Apr 11, 2024

hadoop-yetus commented Apr 11, 2024

hfutatzhanghb commented Apr 11, 2024

hadoop-yetus commented Apr 11, 2024

zhangshuyan0 Apr 15, 2024

hadoop-yetus commented Apr 24, 2024

Hexiaoqiao commented Apr 29, 2024

hfutatzhanghb commented Apr 29, 2024

HDFS-17458. Remove unnecessary BP lock in ReplicaMap. #6717

HDFS-17458. Remove unnecessary BP lock in ReplicaMap. #6717

Conversation

hfutatzhanghb commented Apr 10, 2024 • edited Loading

Description of PR

hadoop-yetus commented Apr 10, 2024

hfutatzhanghb commented Apr 11, 2024

hadoop-yetus commented Apr 11, 2024

Hexiaoqiao commented Apr 11, 2024

hadoop-yetus commented Apr 11, 2024

hfutatzhanghb commented Apr 11, 2024

hadoop-yetus commented Apr 11, 2024

zhangshuyan0 Apr 15, 2024

Choose a reason for hiding this comment

hadoop-yetus commented Apr 24, 2024

Hexiaoqiao commented Apr 29, 2024

hfutatzhanghb commented Apr 29, 2024

hfutatzhanghb commented Apr 10, 2024 •

edited

Loading