wchevreuil commented on PR #5530: URL: https://github.com/apache/hbase/pull/5530#issuecomment-1821288528
> To be honest I can not fully understand why the description on jira will lead to code change in this PR... Especially you mentioned we may still fail to cache the entry so the entry in blocksByHFile will be there forever, If you look at the original code, we were adding the block [here](https://github.com/apache/hbase/blob/master/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java#L1167), which is before we have actually cached the block. If you end up getting a BucketAllocatorException, we then never remove the block from blocksByHFile. > but then we moved 'blocksByHFile.add' to the very beginning? Why? We moved it to the `putIntoBackingMap` method, [which is only called after we made sure the block was cached](https://github.com/wchevreuil/hbase/blob/HBASE-28211/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java#L1231). That way we don't end up with a `blocksByHFile` that's inconsistent with the `backingMap` and the cache itself. -- 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]
