wchevreuil commented on code in PR #5506:
URL: https://github.com/apache/hbase/pull/5506#discussion_r1386433669
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java:
##########
@@ -77,16 +81,49 @@ public void cacheBlock(BlockCacheKey cacheKey, Cacheable
buf) {
@Override
public Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, boolean
repeat,
boolean updateCacheMetrics) {
- // We are not in a position to exactly look at LRU cache or BC as
BlockType may not be getting
- // passed always.
+ Cacheable block = null;
+ // We don't know the block type. We should try to get it on one of the
caches only,
+ // but not both otherwise we'll over compute on misses. Here we check if
the key is on L1,
+ // if so, call getBlock on L1 and that will compute the hit. Otherwise,
we'll try to get it from
+ // L2 and whatever happens, we'll update the stats there.
boolean existInL1 = l1Cache.containsBlock(cacheKey);
- if (!existInL1 && updateCacheMetrics && !repeat) {
- // If the block does not exist in L1, the containsBlock should be
counted as one miss.
- l1Cache.getStats().miss(caching, cacheKey.isPrimary(),
cacheKey.getBlockType());
+ // if we know it's in L1, just delegate call to l1 and return it
+ if (existInL1) {
+ block = l1Cache.getBlock(cacheKey, caching, repeat, false);
+ } else {
+ block = l2Cache.getBlock(cacheKey, caching, repeat, false);
+ }
+ if (updateCacheMetrics) {
+ boolean metaBlock = isMetaBlock(cacheKey.getBlockType());
+ if (metaBlock) {
+ if (!existInL1 && block != null) {
+ LOG.warn("Cache key {} had block type {}, but was found in L2
cache.", cacheKey,
Review Comment:
Yes, I decided to put this as I saw a UT doing the opposite (creating a meta
block then wrongly passing block type data in the cachekey), so I wonder if
this mistake might be happening anywhere in the "real" code. I tried to some
review of all callees to this method and haven't found it, but I could had
missed something.
--
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]
