[ 
https://issues.apache.org/jira/browse/HDFS-16939?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17696118#comment-17696118
 ] 

ASF GitHub Bot commented on HDFS-16939:
---------------------------------------

saxenapranav commented on code in PR #5450:
URL: https://github.com/apache/hadoop/pull/5450#discussion_r1124189602


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/LowRedundancyBlocks.java:
##########
@@ -369,7 +369,7 @@ synchronized boolean remove(BlockInfo block,
    * @return true if the block was found and removed from one of the priority
    *         queues
    */
-  boolean remove(BlockInfo block, int priLevel) {
+  synchronized boolean remove(BlockInfo block, int priLevel) {

Review Comment:
   This will just ensure only one thread getting into the remove method. But 
what if there is one thread trying to remove, other trying to add or size(). I 
feel following should help:
   ```
   private Object syncOnBlockInfoSet(int priority, BlockInfo blockInfo, String 
operation) {
       LightWeightLinkedSet<BlockInfo> blockInfos 
=priorityQueues.get(priority); 
       synchronized (blockInfos) {
         if("size".equalsIgnoreCase(operation)) {
           return blockInfos.size();
         }
         if("remove".equalsIgnoreCase(operation)) {
           return blockInfos.remove(blockInfo);
         }
         //implement other required methods.
         else {
           return null;
         }
       }
     }
   ```
   
   all code-pieces where we do operation on an element in `priorityQueues`, we 
call `syncOnBlockInfoSet` to do that operation. for ex: 
`size=syncOnBlockInfoSet(priority, null, "size")`





> Fix the thread safety bug in LowRedundancyBlocks
> ------------------------------------------------
>
>                 Key: HDFS-16939
>                 URL: https://issues.apache.org/jira/browse/HDFS-16939
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: namanode
>            Reporter: Shuyan Zhang
>            Assignee: Shuyan Zhang
>            Priority: Major
>              Labels: pull-request-available
>
> The remove method in LowRedundancyBlocks is not protected by synchronized. 
> This method is private and is called by BlockManager. As a result, 
> priorityQueues has the risk of being accessed concurrently by multiple 
> threads.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to