diff --git a/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java b/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java index 3798a06..55375e7 100644 --- a/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java +++ b/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java @@ -58,8 +58,8 @@ public class SingleSizeCache implements BlockCache, HeapSize { private final int blockSize; private final CacheStats stats; private final SlabItemEvictionWatcher evictionWatcher; - private AtomicLong size; - private AtomicLong timeSinceLastAccess; + private final AtomicLong size; + private final AtomicLong timeSinceLastAccess; public final static long CACHE_FIXED_OVERHEAD = ClassSize .align((2 * Bytes.SIZEOF_INT) + (5 * ClassSize.REFERENCE) + +ClassSize.OBJECT); @@ -87,14 +87,15 @@ public class SingleSizeCache implements BlockCache, HeapSize { this.size = new AtomicLong(CACHE_FIXED_OVERHEAD + backingStore.heapSize()); this.timeSinceLastAccess = new AtomicLong(); - // This evictionListener is called whenever the cache automatically evicts + // This evictionListener is called whenever the cache automatically + // evicts // something. MapEvictionListener listener = new MapEvictionListener() { @Override public void onEviction(String key, CacheablePair value) { timeSinceLastAccess.set(System.nanoTime() - value.recentlyAccessed.get()); - doEviction(key, value); + doEviction(key, value, true); } }; @@ -171,14 +172,19 @@ public class SingleSizeCache implements BlockCache, HeapSize { public boolean evictBlock(String key) { stats.evict(); CacheablePair evictedBlock = backingMap.remove(key); + + // If we are evicting a block from here, either the call came from the + // master, or we are using this cache as a standalone cache. Either way, + // we do not call our listener during oneviction. if (evictedBlock != null) { - doEviction(key, evictedBlock); + doEviction(key, evictedBlock, false); } return evictedBlock != null; } - private void doEviction(String key, CacheablePair evictedBlock) { + private void doEviction(String key, CacheablePair evictedBlock, + boolean callListener) { long evictedHeap = 0; synchronized (evictedBlock) { if (evictedBlock.serializedData == null) { @@ -200,8 +206,9 @@ public class SingleSizeCache implements BlockCache, HeapSize { // Thread A sees the null serializedData, and returns null // Thread A calls cacheBlock on the same block, and gets // "already cached" since the block is still in backingStore - if (evictionWatcher != null) { - evictionWatcher.onEviction(key, false); + + if (evictionWatcher != null && callListener) { + evictionWatcher.onEviction(key); } } stats.evicted(); diff --git a/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java b/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java index fe8b95a..f4ac12d 100644 --- a/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java +++ b/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java @@ -21,8 +21,8 @@ package org.apache.hadoop.hbase.io.hfile.slab; import java.math.BigDecimal; -import java.util.Map.Entry; import java.util.List; +import java.util.Map.Entry; import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; @@ -122,7 +122,9 @@ public class SlabCache implements SlabItemEvictionWatcher, BlockCache, HeapSize + sizes.length + " slabs " + "offheapslabporportions and offheapslabsizes"); } - /* We use BigDecimals instead of floats because float rounding is annoying */ + /* + * We use BigDecimals instead of floats because float rounding is annoying + */ BigDecimal[] parsedProportions = stringArrayToBigDecimalArray(porportions); BigDecimal[] parsedSizes = stringArrayToBigDecimalArray(sizes); @@ -205,10 +207,16 @@ public class SlabCache implements SlabItemEvictionWatcher, BlockCache, HeapSize this.successfullyCachedStats.addin(cachedItem.getSerializedLength()); SingleSizeCache scache = scacheEntry.getValue(); - /*This will throw a runtime exception if we try to cache the same value twice*/ + /* + * This will throw a runtime exception if we try to cache the same value + * twice + */ scache.cacheBlock(blockName, cachedItem); - /*Spinlock, if we're spinlocking, that means an eviction hasn't taken place yet*/ + /* + * Spinlock, if we're spinlocking, that means an eviction hasn't taken place + * yet + */ while (backingStore.putIfAbsent(blockName, scache) != null) { Thread.yield(); } @@ -255,24 +263,51 @@ public class SlabCache implements SlabItemEvictionWatcher, BlockCache, HeapSize */ public boolean evictBlock(String key) { stats.evict(); - return onEviction(key, true); - } - - @Override - public boolean onEviction(String key, boolean callAssignedCache) { SingleSizeCache cacheEntry = backingStore.remove(key); if (cacheEntry == null) { return false; - } - /* we need to bump up stats.evict, as this call came from the assignedCache. */ - if (callAssignedCache == false) { - stats.evict(); - } - stats.evicted(); - if (callAssignedCache) { + } else { cacheEntry.evictBlock(key); + stats.evicted(); + return true; + } + } + + @Override + public void onEviction(String key) { + /* + * Without the while loop below, the following can occur: + * + * Invariant: Anything in SingleSizeCache will have a representation in + * SlabCache, and vice-versa. + * + * Start: Key A is in both SingleSizeCache and SlabCache. Invariant is + * satisfied + * + * Thread A: Caches something, starting eviction of Key A in SingleSizeCache + * + * Thread B: Checks for Key A -> Returns Gets Null, as eviction has begun + * + * Thread B: Recaches Key A, gets to SingleSizeCache, does not get the + * PutIfAbsentLoop yet... + * + * Thread C: Caches another key, starting the second eviction of Key A. + * + * Thread A: does its onEviction, removing the entry of Key A from + * SlabCache. + * + * Thread C: does its onEviction, removing the (blank) entry of Key A from + * SlabCache: + * + * Thread B: goes to putifabsent, and puts its entry into SlabCache. + * + * Result: SlabCache has an entry for A, while SingleSizeCache has no + * entries for A. Invariant is violated. + */ + + while ((backingStore.remove(key)) == null) { + Thread.yield(); } - return true; } /** @@ -346,7 +381,8 @@ public class SlabCache implements SlabItemEvictionWatcher, BlockCache, HeapSize * */ static class SlabStats { - // the maximum size somebody will ever try to cache, then we multiply by 10 + // the maximum size somebody will ever try to cache, then we multiply by + // 10 // so we have finer grained stats. final int MULTIPLIER = 10; final int NUMDIVISIONS = (int) (Math.log(Integer.MAX_VALUE) * MULTIPLIER); diff --git a/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabItemEvictionWatcher.java b/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabItemEvictionWatcher.java index 91b1603..e917b6e 100644 --- a/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabItemEvictionWatcher.java +++ b/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabItemEvictionWatcher.java @@ -33,6 +33,6 @@ interface SlabItemEvictionWatcher { * @param boolean callAssignedCache whether we should call the cache which the * key was originally assigned to. */ - boolean onEviction(String key, boolean callAssignedCache); + void onEviction(String key); } diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index 0c06f4f..d4603a5 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -1269,6 +1269,7 @@ public class HRegionServer implements HRegionInterface, HBaseRPCErrorHandler, (int) (totalStaticIndexSize / 1024)); this.metrics.totalStaticBloomSizeKB.set( (int) (totalStaticBloomSize / 1024)); + LOG.info("readRequestsCount: " + readRequestsCount); this.metrics.readRequestsCount.set(readRequestsCount); this.metrics.writeRequestsCount.set(writeRequestsCount); diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java index fd9e7ef..1e38f22 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java @@ -378,18 +378,15 @@ public class StoreFile { MemoryUsage mu = ManagementFactory.getMemoryMXBean().getHeapMemoryUsage(); long cacheSize = (long)(mu.getMax() * cachePercentage); int blockSize = conf.getInt("hbase.offheapcache.minblocksize", HFile.DEFAULT_BLOCKSIZE); + long offHeapCacheSize = (long) (conf.getFloat("hbase.offheapcache.percentage", (float) 0.95) * DirectMemoryUtils.getDirectMemorySize()); boolean enableOffHeapCache = conf.getBoolean("hbase.offheapcache.enable", false); - long offHeapCacheSize = enableOffHeapCache ? - (long) (conf.getFloat("hbase.offheapcache.percentage", - (float) 0.95) * DirectMemoryUtils.getDirectMemorySize()) : - 0; LOG.info("Allocating LruBlockCache with maximum size " + StringUtils.humanReadableInt(cacheSize)); - if(offHeapCacheSize <= 0) { + if(offHeapCacheSize <= 0 && enableOffHeapCache) { hfileBlockCache = new LruBlockCache(cacheSize, DEFAULT_BLOCKSIZE_SMALL); } else { - LOG.info("Allocating OffHeapCache with maximum size " + - StringUtils.humanReadableInt(offHeapCacheSize)); + LOG.info("Allocating OffHeapCache with maximum size " + + StringUtils.humanReadableInt(offHeapCacheSize)); hfileBlockCache = new DoubleBlockCache(cacheSize, offHeapCacheSize, DEFAULT_BLOCKSIZE_SMALL, blockSize, conf); } return hfileBlockCache; diff --git a/src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSizeCache.java b/src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSizeCache.java index e021780..ef2d147 100644 --- a/src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSizeCache.java +++ b/src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSizeCache.java @@ -20,8 +20,9 @@ package org.apache.hadoop.hbase.io.hfile.slab; import org.apache.hadoop.hbase.io.hfile.CacheTestUtils; -import org.apache.hadoop.hbase.io.hfile.slab.SingleSizeCache; -import org.junit.*; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; /** * Tests SingleSlabCache. @@ -48,28 +49,28 @@ public class TestSingleSizeCache { cache.shutdown(); } - @Ignore @Test + @Test public void testCacheSimple() throws Exception { CacheTestUtils.testCacheSimple(cache, BLOCK_SIZE, NUM_QUERIES); } - @Ignore @Test + @Test public void testCacheMultiThreaded() throws Exception { CacheTestUtils.testCacheMultiThreaded(cache, BLOCK_SIZE, NUM_THREADS, NUM_QUERIES, 0.80); } - @Ignore @Test + @Test public void testCacheMultiThreadedSingleKey() throws Exception { CacheTestUtils.hammerSingleKey(cache, BLOCK_SIZE, NUM_THREADS, NUM_QUERIES); } - @Ignore @Test + @Test public void testCacheMultiThreadedEviction() throws Exception { CacheTestUtils.hammerEviction(cache, BLOCK_SIZE, NUM_THREADS, NUM_QUERIES); } - @Ignore @Test + @Test public void testHeapSizeChanges(){ CacheTestUtils.testHeapSizeChanges(cache, BLOCK_SIZE); } diff --git a/src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlabCache.java b/src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlabCache.java index 8dd5159..9891349 100644 --- a/src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlabCache.java +++ b/src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlabCache.java @@ -19,16 +19,16 @@ */ package org.apache.hadoop.hbase.io.hfile.slab; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.io.hfile.CacheTestUtils; -import org.apache.hadoop.hbase.io.hfile.slab.SlabCache; import org.apache.hadoop.hbase.io.hfile.slab.SlabCache.SlabStats; import org.junit.After; import org.junit.Before; -import org.junit.Test; import org.junit.Ignore; - -import static org.junit.Assert.*; +import org.junit.Test; /** * Basic test of SlabCache. Puts and gets. @@ -61,34 +61,34 @@ public class TestSlabCache { @Ignore @Test public void testElementPlacement() { - assertEquals(cache.getHigherBlock((int) BLOCK_SIZE).getKey().intValue(), - (int) (BLOCK_SIZE * 11 / 10)); - assertEquals(cache.getHigherBlock((int) (BLOCK_SIZE * 2)).getKey() - .intValue(), (int) (BLOCK_SIZE * 21 / 10)); + assertEquals(cache.getHigherBlock(BLOCK_SIZE).getKey().intValue(), + (BLOCK_SIZE * 11 / 10)); + assertEquals(cache.getHigherBlock((BLOCK_SIZE * 2)).getKey() + .intValue(), (BLOCK_SIZE * 21 / 10)); } - @Ignore @Test + @Test public void testCacheSimple() throws Exception { CacheTestUtils.testCacheSimple(cache, BLOCK_SIZE, NUM_QUERIES); } - @Ignore @Test + @Test public void testCacheMultiThreaded() throws Exception { CacheTestUtils.testCacheMultiThreaded(cache, BLOCK_SIZE, NUM_THREADS, NUM_QUERIES, 0.80); } - @Ignore @Test + @Test public void testCacheMultiThreadedSingleKey() throws Exception { CacheTestUtils.hammerSingleKey(cache, BLOCK_SIZE, NUM_THREADS, NUM_QUERIES); } - @Ignore @Test + @Test public void testCacheMultiThreadedEviction() throws Exception { CacheTestUtils.hammerEviction(cache, BLOCK_SIZE, 10, NUM_QUERIES); } - @Ignore @Test + @Test /*Just checks if ranges overlap*/ public void testStatsArithmetic(){ SlabStats test = cache.requestStats; @@ -99,7 +99,7 @@ public class TestSlabCache { } } - @Ignore @Test + @Test public void testHeapSizeChanges(){ CacheTestUtils.testHeapSizeChanges(cache, BLOCK_SIZE); }