hfutatzhanghb commented on PR #6717:
URL: https://github.com/apache/hadoop/pull/6717#issuecomment-2049700613

   > @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to