Hexiaoqiao commented on code in PR #5450:
URL: https://github.com/apache/hadoop/pull/5450#discussion_r1125839750
##########
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:
@saxenapranav Thanks for your pretty review comment. I am not sure if get
your points totally. IMO, this improvement is safe and self-contained, because
`synchronized` is reentrant and exclusive. So I am confused if it could involve
other consistency issues.
I would like to give my +1 if you were worried about perf-loss only for
`synchronized-synchronized`, for this case I think it could be acceptable,
anyway totally agree that both changes about performance we should given the
benchmark comparison.
Thanks @zhangshuyan0 and @saxenapranav , Please feel free to correct me if
something wrong.
--
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]
