HBASE-4383 SlabCache reports negative heap sizes (Li Pi)

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1170930 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -262,6 +262,7 @@ Release 0.91.0 - Unreleased
    HBASE-4394  Add support for seeking hints to FilterList
    HBASE-4406  TestOpenRegionHandler failing after HBASE-4287 (todd)
    HBASE-4330  Fix races in slab cache (Li Pi & Todd)
+   HBASE-4383  SlabCache reports negative heap sizes (Li Pi)
 
   IMPROVEMENTS
    HBASE-3290  Max Compaction Size (Nicolas Spiegelberg via Stack)  

src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java

@@ -51,7 +51,7 @@ import com.google.common.collect.MapMaker;
  * ConcurrentLinkedHashMap.
  *
  **/
-public class SingleSizeCache implements BlockCache {
+public class SingleSizeCache implements BlockCache, HeapSize {
   private final Slab backingStore;
   private final ConcurrentMap<String, CacheablePair> backingMap;
   private final int numBlocks;
@@ -219,7 +219,7 @@ public class SingleSizeCache implements BlockCache {
         + StringUtils.humanReadableInt(this.heapSize()) + " bytes." + ", "
         + "churnTime=" + StringUtils.formatTime(milliseconds));
 
-    LOG.debug("Slab Stats: " + "accesses="
+    LOG.info("Slab Stats: " + "accesses="
         + stats.getRequestCount()
         + ", "
         + "hits="
@@ -248,19 +248,19 @@ public class SingleSizeCache implements BlockCache {
   }
 
   public long heapSize() {
-    return this.size() + backingStore.heapSize();
+    return this.size.get() + backingStore.heapSize();
   }
 
   public long size() {
-    return this.blockSize * this.numBlocks;
+    return (long) this.blockSize * (long) this.numBlocks;
   }
 
   public long getFreeSize() {
-    return backingStore.getBlocksRemaining() * blockSize;
+    return (long) backingStore.getBlocksRemaining() * (long) blockSize;
   }
 
   public long getOccupiedSize() {
-    return (numBlocks - backingStore.getBlocksRemaining()) * blockSize;
+    return (long) (numBlocks - backingStore.getBlocksRemaining()) * (long) blockSize;
   }
 
   public long getEvictedCount() {
@@ -328,7 +328,7 @@ public class SingleSizeCache implements BlockCache {
     @Override
     public long heapSize() {
       return ClassSize.align(ClassSize.OBJECT + ClassSize.REFERENCE * 3
-          + ClassSize.REENTRANT_LOCK);
+          + ClassSize.ATOMIC_LONG);
     }
   }
 }

src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java

@@ -325,10 +325,17 @@ public class SlabCache implements SlabItemEvictionWatcher, BlockCache, HeapSize
 
     @Override
     public void run() {
+      for (SingleSizeCache s : ourcache.sizer.values()) {
+        s.logStats();
+      }
+
+      SlabCache.LOG.info("Current heap size is: "
+          + StringUtils.humanReadableInt(ourcache.heapSize()));
+
       LOG.info("Request Stats");
-      ourcache.requestStats.logStats(ourcache);
+      ourcache.requestStats.logStats();
       LOG.info("Successfully Cached Stats");
-      ourcache.successfullyCachedStats.logStats(ourcache);
+      ourcache.successfullyCachedStats.logStats();
     }
 
   }
@@ -368,13 +375,8 @@ public class SlabCache implements SlabItemEvictionWatcher, BlockCache, HeapSize
       return Math.pow(Math.E, ((double) (index - 0.5) / (double) MULTIPLIER));
     }
 
-    public void logStats(SlabCache slabCache) {
-      for (SingleSizeCache s : slabCache.sizer.values()) {
-        s.logStats();
-      }
+    public void logStats() {
       AtomicLong[] fineGrainedStats = getUsage();
-      SlabCache.LOG.info("Current heap size is: "
-          + StringUtils.humanReadableInt(slabCache.heapSize()));
       for (int i = 0; i < fineGrainedStats.length; i++) {
 
         if (fineGrainedStats[i].get() > 0) {

src/test/java/org/apache/hadoop/hbase/io/hfile/CacheTestUtils.java

@@ -33,9 +33,25 @@ import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.MultithreadedTestUtil;
 import org.apache.hadoop.hbase.MultithreadedTestUtil.TestThread;
+import org.apache.hadoop.hbase.io.HeapSize;
 
 public class CacheTestUtils {
 
+  /*Just checks if heapsize grows when something is cached, and gets smaller when the same object is evicted*/
+
+  public static void testHeapSizeChanges(final BlockCache toBeTested, final int blockSize){
+    HFileBlockPair[] blocks = generateHFileBlocks(blockSize, 1);
+    long heapSize = ((HeapSize) toBeTested).heapSize();
+    toBeTested.cacheBlock(blocks[0].blockName, blocks[0].block);
+
+    /*When we cache something HeapSize should always increase */
+    assertTrue(heapSize < ((HeapSize) toBeTested).heapSize());
+
+    toBeTested.evictBlock(blocks[0].blockName);
+
+    /*Post eviction, heapsize should be the same */
+    assertEquals(heapSize, ((HeapSize) toBeTested).heapSize());
+  }
   public static void testCacheMultiThreaded(final BlockCache toBeTested,
       final int blockSize, final int numThreads, final int numQueries,
       final double passingScore) throws Exception {

src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSizeCache.java

@@ -63,10 +63,15 @@ public class TestSingleSizeCache {
   public void testCacheMultiThreadedSingleKey() throws Exception {
     CacheTestUtils.hammerSingleKey(cache, BLOCK_SIZE, NUM_THREADS, NUM_QUERIES);
   }
-  
+
   @Test
   public void testCacheMultiThreadedEviction() throws Exception {
     CacheTestUtils.hammerEviction(cache, BLOCK_SIZE, NUM_THREADS, NUM_QUERIES);
   }
 
+  @Test
+  public void testHeapSizeChanges(){
+    CacheTestUtils.testHeapSizeChanges(cache, BLOCK_SIZE);
+  }
+
 }

src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlabCache.java

@@ -97,4 +97,9 @@ public class TestSlabCache {
           test.getUpperBound(i) <= test.getLowerBound(i + 1));
     }
   }
+
+  @Test
+  public void testHeapSizeChanges(){
+    CacheTestUtils.testHeapSizeChanges(cache, BLOCK_SIZE);
+  }
 }