anoopsjohn commented on a change in pull request #3474:
URL: https://github.com/apache/hbase/pull/3474#discussion_r670181887



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java
##########
@@ -79,7 +79,13 @@ public Cacheable getBlock(BlockCacheKey cacheKey, boolean 
caching,
       boolean repeat, boolean updateCacheMetrics) {
     // TODO: is there a hole here, or just awkwardness since in the lruCache 
getBlock
     // we end up calling l2Cache.getBlock.
-    return lruCache.containsBlock(cacheKey)?
+    boolean existInL1 = lruCache.containsBlock(cacheKey);
+    if (!existInL1 && updateCacheMetrics) {

Review comment:
       Within the LRUBlock cache we do stats.miss when its repeat = false
   if (!repeat && updateCacheMetrics) {
           stats.miss(caching, cacheKey.isPrimary(), cacheKey.getBlockType());
         }
   Checked 2.x based my version and I believe in 1.x also same.  Pls check and 
adjust. 
   Else patch looks good. And simple. :-)
   Very very nice catch (again)




-- 
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]


Reply via email to