HBASE-22663 The HeapAllocationRatio in WebUI is not accurate because all of the heap allocation will happen in another separated allocator named HEAP (#365)

Author: openinx

hbase-common/src/main/java/org/apache/hadoop/hbase/io/ByteBuffAllocator.java

@@ -37,6 +37,7 @@
 import org.slf4j.LoggerFactory;
 
 import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
 
 /**
  * ByteBuffAllocator is used for allocating/freeing the ByteBuffers from/to NIO ByteBuffer pool, and
@@ -159,7 +160,10 @@ public interface Recycler {
    * reservoir is enabled and the reservoir has enough buffers, otherwise the allocator will just
    * allocate the insufficient buffers from on-heap to meet the requirement.
    * @param conf which get the arguments to initialize the allocator.
-   * @param reservoirEnabled indicate whether the reservoir is enabled or disabled.
+   * @param reservoirEnabled indicate whether the reservoir is enabled or disabled. NOTICE: if
+   *          reservoir is enabled, then we will use the pool allocator to allocate off-heap
+   *          ByteBuffers and use the HEAP allocator to allocate heap ByteBuffers. Otherwise if
+   *          reservoir is disabled then all allocations will happen in HEAP instance.
    * @return ByteBuffAllocator to manage the byte buffers.
    */
   public static ByteBuffAllocator create(Configuration conf, boolean reservoirEnabled) {
@@ -192,7 +196,7 @@ public static ByteBuffAllocator create(Configuration conf, boolean reservoirEnab
       int minSizeForReservoirUse = conf.getInt(MIN_ALLOCATE_SIZE_KEY, poolBufSize / 6);
       return new ByteBuffAllocator(true, maxBuffCount, poolBufSize, minSizeForReservoirUse);
     } else {
-      return new ByteBuffAllocator(false, 0, poolBufSize, Integer.MAX_VALUE);
+      return HEAP;
     }
   }
 
@@ -247,12 +251,22 @@ public int getTotalBufferCount() {
     return maxBufCount;
   }
 
-  public double getHeapAllocationRatio() {
-    long heapAllocBytes = heapAllocationBytes.sum(), poolAllocBytes = poolAllocationBytes.sum();
-    double heapDelta = heapAllocBytes - lastHeapAllocationBytes;
-    double poolDelta = poolAllocBytes - lastPoolAllocationBytes;
-    lastHeapAllocationBytes = heapAllocBytes;
-    lastPoolAllocationBytes = poolAllocBytes;
+  public static double getHeapAllocationRatio(ByteBuffAllocator... allocators) {
+    double heapDelta = 0.0, poolDelta = 0.0;
+    long heapAllocBytes, poolAllocBytes;
+    // If disabled the pool allocator, then we use the global HEAP allocator. otherwise we use
+    // the pool allocator to allocate offheap ByteBuffers and use the HEAP to allocate heap
+    // ByteBuffers. So here we use a HashSet to remove the duplicated allocator object in disable
+    // case.
+    for (ByteBuffAllocator alloc : Sets.newHashSet(allocators)) {
+      heapAllocBytes = alloc.heapAllocationBytes.sum();
+      poolAllocBytes = alloc.poolAllocationBytes.sum();
+      heapDelta += (heapAllocBytes - alloc.lastHeapAllocationBytes);
+      poolDelta += (poolAllocBytes - alloc.lastPoolAllocationBytes);
+      alloc.lastHeapAllocationBytes = heapAllocBytes;
+      alloc.lastPoolAllocationBytes = poolAllocBytes;
+    }
+    // Calculate the heap allocation ratio.
     if (Math.abs(heapDelta + poolDelta) < 1e-3) {
       return 0.0;
     }

hbase-common/src/test/java/org/apache/hadoop/hbase/io/TestByteBuffAllocator.java

@@ -18,6 +18,8 @@
 
 package org.apache.hadoop.hbase.io;
 
+import static org.apache.hadoop.hbase.io.ByteBuffAllocator.HEAP;
+import static org.apache.hadoop.hbase.io.ByteBuffAllocator.getHeapAllocationRatio;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -162,7 +164,7 @@ public void testNegativeAllocatedSize() {
   @Test
   public void testAllocateOneBuffer() {
     // Allocate from on-heap
-    ByteBuffAllocator allocator = ByteBuffAllocator.HEAP;
+    ByteBuffAllocator allocator = HEAP;
     ByteBuff buf = allocator.allocateOneBuffer();
     assertTrue(buf.hasArray());
     assertEquals(ByteBuffAllocator.DEFAULT_BUFFER_SIZE, buf.remaining());
@@ -367,4 +369,37 @@ public void testDeprecatedConfigs() {
     conf.setBoolean(ByteBuffAllocator.ALLOCATOR_POOL_ENABLED_KEY, false);
     Assert.assertFalse(conf.getBoolean(ByteBuffAllocator.ALLOCATOR_POOL_ENABLED_KEY, true));
   }
+
+  @Test
+  public void testHeapAllocationRatio() {
+    Configuration conf = new Configuration();
+    conf.setInt(ByteBuffAllocator.MAX_BUFFER_COUNT_KEY, 11);
+    conf.setInt(ByteBuffAllocator.BUFFER_SIZE_KEY, 2048);
+    ByteBuffAllocator alloc1 = ByteBuffAllocator.create(conf, true);
+    Assert.assertEquals(getHeapAllocationRatio(alloc1), 0.0f, 1e-6);
+    alloc1.allocate(1);
+    Assert.assertEquals(getHeapAllocationRatio(alloc1), 1.0f, 1e-6);
+
+    alloc1.allocate(2048 / 6 - 1);
+    Assert.assertEquals(getHeapAllocationRatio(alloc1), 1.0f, 1e-6);
+
+    alloc1.allocate(24);
+    alloc1.allocate(1024);
+    Assert.assertEquals(getHeapAllocationRatio(alloc1), 24 / (24f + 2048), 1e-6);
+    Assert.assertEquals(getHeapAllocationRatio(alloc1), 0.0f, 1e-6);
+
+    // Allocate something from HEAP
+    HEAP.allocate(1024);
+    alloc1.allocate(24);
+    alloc1.allocate(1024);
+    Assert.assertEquals(getHeapAllocationRatio(HEAP, alloc1), (1024f + 24) / (1024f + 24 + 2048),
+      1e-6);
+    Assert.assertEquals(getHeapAllocationRatio(HEAP, alloc1), 0.0f, 1e-6);
+
+    // Check duplicated heap allocator, say even if we passed (HEAP, HEAP, alloc1), it will only
+    // caculate the allocation from (HEAP, alloc1).
+    HEAP.allocate(1024);
+    alloc1.allocate(1024);
+    Assert.assertEquals(getHeapAllocationRatio(HEAP, HEAP, alloc1), 1024f / (1024f + 2048f), 1e-6);
+  }
 }

hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/ServerMetricsTmpl.jamon

@@ -248,7 +248,7 @@ ByteBuffAllocator bbAllocator;
 <tr>
     <td><% bbAllocator.getHeapAllocationBytes() %></td>
     <td><% bbAllocator.getPoolAllocationBytes() %></td>
-    <td><% String.format("%.3f", bbAllocator.getHeapAllocationRatio() * 100) %><% "%" %></td>
+    <td><% String.format("%.3f", ByteBuffAllocator.getHeapAllocationRatio(bbAllocator, ByteBuffAllocator.HEAP) * 100) %><% "%" %></td>
     <td><% bbAllocator.getTotalBufferCount() %></td>
     <td><% bbAllocator.getUsedBufferCount() %></td>
     <td><% bbAllocator.getBufferSize() %></td>

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java

@@ -1021,7 +1021,7 @@ public long getByteBuffAllocatorPoolAllocationBytes() {
 
   @Override
   public double getByteBuffAllocatorHeapAllocRatio() {
-    return this.allocator.getHeapAllocationRatio();
+    return ByteBuffAllocator.getHeapAllocationRatio(allocator, ByteBuffAllocator.HEAP);
   }
 
   @Override