(hbase) branch branch-2.4 updated: HBASE-28211 BucketCache.blocksByHFile may leak on allocationFailure or if we reach io errors tolerated (#5530)

[wc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wchevreuil pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2.4 by this push:
     new 14f2734b636 HBASE-28211 BucketCache.blocksByHFile may leak on allocationFailure or if we reach io errors tolerated (#5530)
14f2734b636 is described below

commit 14f2734b636f129c677e605e4cb89f4a1b439112
Author: Wellington Ramos Chevreuil <wc...@apache.org>
AuthorDate: Wed Nov 29 10:40:33 2023 +0000

    HBASE-28211 BucketCache.blocksByHFile may leak on allocationFailure or if we reach io errors tolerated (#5530)
    
    Signed-off-by: Duo Zhang <zh...@apache.org>
---
 .../java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java    | 5 +++--
 .../apache/hadoop/hbase/io/hfile/bucket/TestBucketWriterThread.java  | 2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java
index 8ac6e6ee331..10469fb8a41 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java
@@ -191,7 +191,7 @@ public class BucketCache implements BlockCache, HeapSize {
    */
   transient final IdReadWriteLock<Long> offsetLock = new IdReadWriteLock<>(ReferenceType.SOFT);
 
-  private final NavigableSet<BlockCacheKey> blocksByHFile = new ConcurrentSkipListSet<>((a, b) -> {
+  final NavigableSet<BlockCacheKey> blocksByHFile = new ConcurrentSkipListSet<>((a, b) -> {
     int nameComparison = a.getHfileName().compareTo(b.getHfileName());
     if (nameComparison != 0) {
       return nameComparison;
@@ -496,7 +496,6 @@ public class BucketCache implements BlockCache, HeapSize {
     } else {
       this.blockNumber.increment();
       this.heapSize.add(cachedItem.heapSize());
-      blocksByHFile.add(cacheKey);
     }
   }
 
@@ -996,6 +995,7 @@ public class BucketCache implements BlockCache, HeapSize {
    */
   protected void putIntoBackingMap(BlockCacheKey key, BucketEntry bucketEntry) {
     BucketEntry previousEntry = backingMap.put(key, bucketEntry);
+    blocksByHFile.add(key);
     if (previousEntry != null && previousEntry != bucketEntry) {
       previousEntry.withWriteLock(offsetLock, () -> {
         blockEvicted(key, previousEntry, false);
@@ -1304,6 +1304,7 @@ public class BucketCache implements BlockCache, HeapSize {
     if (!ioEngine.isPersistent() || persistencePath == null) {
       // If persistent ioengine and a path, we will serialize out the backingMap.
       this.backingMap.clear();
+      this.blocksByHFile.clear();
     }
   }
 
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketWriterThread.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketWriterThread.java
index d033dabbcff..158ca501afd 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketWriterThread.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketWriterThread.java
@@ -120,6 +120,7 @@ public class TestBucketWriterThread {
     Mockito.when(tooBigCacheable.getSerializedLength()).thenReturn(Integer.MAX_VALUE);
     this.bc.cacheBlock(this.plainKey, tooBigCacheable);
     doDrainOfOneEntry(this.bc, this.wt, this.q);
+    assertTrue(bc.blocksByHFile.isEmpty());
   }
 
   /**
@@ -136,6 +137,7 @@ public class TestBucketWriterThread {
       Mockito.any(), Mockito.any(), Mockito.any());
     this.q.add(spiedRqe);
     doDrainOfOneEntry(bc, wt, q);
+    assertTrue(bc.blocksByHFile.isEmpty());
     // Cache disabled when ioes w/o ever healing.
     assertTrue(!bc.isCacheEnabled());
   }