HBASE-27229 BucketCache statistics should not count evictions by hfile (#4639)

Signed-off-by: Andrew Purtell <apurtell@apache.org>

hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java

@@ -566,14 +566,17 @@ public class BucketCache implements BlockCache, HeapSize {
   /**
    * This method is invoked after the bucketEntry is removed from {@link BucketCache#backingMap}
    */
-  void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean decrementBlockNumber) {
+  void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean decrementBlockNumber,
+    boolean evictedByEvictionProcess) {
     bucketEntry.markAsEvicted();
     blocksByHFile.remove(cacheKey);
     if (decrementBlockNumber) {
       this.blockNumber.decrement();
     }
+    if (evictedByEvictionProcess) {
       cacheStats.evicted(bucketEntry.getCachedTime(), cacheKey.isPrimary());
     }
+  }
 
   /**
    * Free the {{@link BucketEntry} actually,which could only be invoked when the
@@ -602,7 +605,7 @@ public class BucketCache implements BlockCache, HeapSize {
    */
   @Override
   public boolean evictBlock(BlockCacheKey cacheKey) {
-    return doEvictBlock(cacheKey, null);
+    return doEvictBlock(cacheKey, null, false);
   }
 
   /**
@@ -614,7 +617,8 @@ public class BucketCache implements BlockCache, HeapSize {
    * @param bucketEntry {@link BucketEntry} matched {@link BlockCacheKey} to evict.
    * @return true to indicate whether we've evicted successfully or not.
    */
-  private boolean doEvictBlock(BlockCacheKey cacheKey, BucketEntry bucketEntry) {
+  private boolean doEvictBlock(BlockCacheKey cacheKey, BucketEntry bucketEntry,
+    boolean evictedByEvictionProcess) {
     if (!cacheEnabled) {
       return false;
     }
@@ -625,14 +629,14 @@ public class BucketCache implements BlockCache, HeapSize {
     final BucketEntry bucketEntryToUse = bucketEntry;
 
     if (bucketEntryToUse == null) {
-      if (existedInRamCache) {
+      if (existedInRamCache && evictedByEvictionProcess) {
         cacheStats.evicted(0, cacheKey.isPrimary());
       }
       return existedInRamCache;
     } else {
       return bucketEntryToUse.withWriteLock(offsetLock, () -> {
         if (backingMap.remove(cacheKey, bucketEntryToUse)) {
-          blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache);
+          blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache, evictedByEvictionProcess);
           return true;
         }
         return false;
@@ -682,7 +686,7 @@ public class BucketCache implements BlockCache, HeapSize {
    */
   boolean evictBucketEntryIfNoRpcReferenced(BlockCacheKey blockCacheKey, BucketEntry bucketEntry) {
     if (!bucketEntry.isRpcRef()) {
-      return doEvictBlock(blockCacheKey, bucketEntry);
+      return doEvictBlock(blockCacheKey, bucketEntry, true);
     }
     return false;
   }
@@ -803,7 +807,7 @@ public class BucketCache implements BlockCache, HeapSize {
    * blocks evicted
    * @param why Why we are being called
    */
-  private void freeSpace(final String why) {
+  void freeSpace(final String why) {
     // Ensure only one freeSpace progress at a time
     if (!freeSpaceLock.tryLock()) {
       return;
@@ -997,7 +1001,7 @@ public class BucketCache implements BlockCache, HeapSize {
     BucketEntry previousEntry = backingMap.put(key, bucketEntry);
     if (previousEntry != null && previousEntry != bucketEntry) {
       previousEntry.withWriteLock(offsetLock, () -> {
-        blockEvicted(key, previousEntry, false);
+        blockEvicted(key, previousEntry, false, false);
         return null;
       });
     }
@@ -1148,7 +1152,7 @@ public class BucketCache implements BlockCache, HeapSize {
         final BucketEntry bucketEntry = bucketEntries[i];
         bucketEntry.withWriteLock(offsetLock, () -> {
           if (backingMap.remove(key, bucketEntry)) {
-            blockEvicted(key, bucketEntry, false);
+            blockEvicted(key, bucketEntry, false, false);
           }
           return null;
         });

hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java

@@ -262,7 +262,7 @@ public class TestBucketCache {
     };
     evictThread.start();
     cache.offsetLock.waitForWaiters(lockId, 1);
-    cache.blockEvicted(cacheKey, cache.backingMap.remove(cacheKey), true);
+    cache.blockEvicted(cacheKey, cache.backingMap.remove(cacheKey), true, true);
     assertEquals(0, cache.getBlockCount());
     cacheAndWaitUntilFlushedToBucket(cache, cacheKey,
       new CacheTestUtils.ByteArrayCacheable(new byte[10]));
@@ -576,6 +576,56 @@ public class TestBucketCache {
     assertEquals(testValue, bucketEntry.offset());
   }
 
+  @Test
+  public void testEvictionCount() throws InterruptedException {
+    int size = 100;
+    int length = HConstants.HFILEBLOCK_HEADER_SIZE + size;
+    ByteBuffer buf1 = ByteBuffer.allocate(size), buf2 = ByteBuffer.allocate(size);
+    HFileContext meta = new HFileContextBuilder().build();
+    ByteBuffAllocator allocator = ByteBuffAllocator.HEAP;
+    HFileBlock blockWithNextBlockMetadata = new HFileBlock(BlockType.DATA, size, size, -1,
+      ByteBuff.wrap(buf1), HFileBlock.FILL_HEADER, -1, 52, -1, meta, allocator);
+    HFileBlock blockWithoutNextBlockMetadata = new HFileBlock(BlockType.DATA, size, size, -1,
+      ByteBuff.wrap(buf2), HFileBlock.FILL_HEADER, -1, -1, -1, meta, allocator);
+
+    BlockCacheKey key = new BlockCacheKey("testEvictionCount", 0);
+    ByteBuffer actualBuffer = ByteBuffer.allocate(length);
+    ByteBuffer block1Buffer = ByteBuffer.allocate(length);
+    ByteBuffer block2Buffer = ByteBuffer.allocate(length);
+    blockWithNextBlockMetadata.serialize(block1Buffer, true);
+    blockWithoutNextBlockMetadata.serialize(block2Buffer, true);
+
+    // Add blockWithNextBlockMetadata, expect blockWithNextBlockMetadata back.
+    CacheTestUtils.getBlockAndAssertEquals(cache, key, blockWithNextBlockMetadata, actualBuffer,
+      block1Buffer);
+
+    waitUntilFlushedToBucket(cache, key);
+
+    assertEquals(0, cache.getStats().getEvictionCount());
+
+    // evict call should return 1, but then eviction count be 0
+    assertEquals(1, cache.evictBlocksByHfileName("testEvictionCount"));
+    assertEquals(0, cache.getStats().getEvictionCount());
+
+    // add back
+    CacheTestUtils.getBlockAndAssertEquals(cache, key, blockWithNextBlockMetadata, actualBuffer,
+      block1Buffer);
+    waitUntilFlushedToBucket(cache, key);
+
+    // should not increment
+    assertTrue(cache.evictBlock(key));
+    assertEquals(0, cache.getStats().getEvictionCount());
+
+    // add back
+    CacheTestUtils.getBlockAndAssertEquals(cache, key, blockWithNextBlockMetadata, actualBuffer,
+      block1Buffer);
+    waitUntilFlushedToBucket(cache, key);
+
+    // should finally increment eviction count
+    cache.freeSpace("testing");
+    assertEquals(1, cache.getStats().getEvictionCount());
+  }
+
   @Test
   public void testCacheBlockNextBlockMetadataMissing() throws Exception {
     int size = 100;

hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCacheRefCnt.java

@@ -557,10 +557,10 @@ public class TestBucketCacheRefCnt {
     }
 
     @Override
-    void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry,
+    void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean decrementBlockNumber,
-      boolean decrementBlockNumber) {
+      boolean evictedByEvictionProcess) {
       blockEvictCounter.incrementAndGet();
-      super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber);
+      super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber, evictedByEvictionProcess);
     }
 
     /**
@@ -709,8 +709,8 @@ public class TestBucketCacheRefCnt {
     }
 
     @Override
-    void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry,
+    void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean decrementBlockNumber,
-      boolean decrementBlockNumber) {
+      boolean evictedByEvictionProcess) {
       /**
        * This is only invoked by {@link BucketCache.WriterThread}. {@link MyMyBucketCache2} create
        * only one {@link BucketCache.WriterThread}.
@@ -718,7 +718,7 @@ public class TestBucketCacheRefCnt {
       assertTrue(Thread.currentThread() == this.writerThreads[0]);
 
       blockEvictCounter.incrementAndGet();
-      super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber);
+      super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber, evictedByEvictionProcess);
     }
 
     /**