[
https://issues.apache.org/jira/browse/HDFS-17777?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18072841#comment-18072841
]
ASF GitHub Bot commented on HDFS-17777:
---------------------------------------
balodesecurity commented on code in PR #8325:
URL: https://github.com/apache/hadoop/pull/8325#discussion_r3067932030
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/ExcessRedundancyMap.java:
##########
@@ -40,6 +42,7 @@ class ExcessRedundancyMap {
private final Map<String, LightWeightHashSet<Block>> map = new HashMap<>();
Review Comment:
Thanks @ZanderXu for the review!
The ConcurrentHashMap + synchronized(set) approach breaks the atomicity in
some cases because ConcurrentHashMap only protects individual map operations
(like get and remove) atomically, while synchronized(set) only protects the set
operations — but the gap between them (after you get the set reference from the
map but before you lock it) is unprotected, which is exactly where a thread X
can sneak in, empty the set, and evict it from the map, leaving another thread
Y holding a reference to an orphaned set that no longer exists in the map.
> Improve ExcessRedundancyMap locking semantics
> ---------------------------------------------
>
> Key: HDFS-17777
> URL: https://issues.apache.org/jira/browse/HDFS-17777
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: namenode
> Affects Versions: 3.4.0, 3.3.6, 3.4.1
> Reporter: William Montaz
> Priority: Major
> Labels: pull-request-available
> Attachments: Capture d’écran 2025-04-30 à 14.32.41.png
>
>
> ExcessRedundancyMap introduce in HDFS-9838 uses synchronized keyword for
> threadsafety. However prior to introduce this class, the operations such as
> contains were made without a new lock, they were called inside BlockManager
> with verification that the fsNamesystem lock was held in read or write mode
> depending on the situation.
>
> ExcessRedundancyMap now forces all thread to grab the same exclusive lock,
> even if in general a lot more read are performed on the class.
>
> By using a ReentrantReadWriteLock for ExcessRedundancyMap we could improve
> throughput of the namenode. Another approach could be to introduce the same
> asserts on namesystem lock as it seems those methods are always called in the
> context of the FSNamesystem lock.
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
