diff --git src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java index c7e83fe..6533c93 100644 --- src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java +++ src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java @@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.io.hfile; import java.io.IOException; import java.lang.ref.WeakReference; +import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; import java.util.EnumMap; @@ -267,19 +268,25 @@ public class LruBlockCache implements BlockCache, HeapSize { /** * Cache the block with the specified name and buffer. *
- * It is assumed this will NEVER be called on an already cached block. If - * that is done, an exception will be thrown. + * It is assumed this will NOT be called on an already cached block. In rare cases (HBASE-8547) + * this can happen, for which we compare the buffer contents. * @param cacheKey block's cache key * @param buf block buffer * @param inMemory if block is in-memory */ + @Override public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) { CachedBlock cb = map.get(cacheKey); if(cb != null) { + // compare the contents, if they are not equal, we are in big trouble + if (compare(buf, cb.getBuffer()) != 0) { + throw new RuntimeException("Cached block contents differ, which should not have happened." + + "cacheKey:" + cacheKey); + } String msg = "Cached an already cached block: " + cacheKey + " cb:" + cb.getCacheKey(); msg += ". This is harmless and can happen in rare cases (see HBASE-8547)"; LOG.warn(msg); - assert false : msg; + return; } cb = new CachedBlock(cacheKey, buf, count.incrementAndGet(), inMemory); long newSize = updateSizeMetrics(cb, false); @@ -290,6 +297,15 @@ public class LruBlockCache implements BlockCache, HeapSize { } } + private int compare(Cacheable left, Cacheable right) { + ByteBuffer l = ByteBuffer.allocate(left.getSerializedLength()); + left.serialize(l); + ByteBuffer r = ByteBuffer.allocate(right.getSerializedLength()); + right.serialize(r); + return Bytes.compareTo(l.array(), l.arrayOffset(), l.limit(), + r.array(), r.arrayOffset(), r.limit()); + } + /** * Cache the block with the specified name and buffer. *
diff --git src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java index 7d89531..65af0d7 100644 --- src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java +++ src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java @@ -143,17 +143,6 @@ public class TestLruBlockCache { assertEquals(buf.heapSize(), block.heapSize()); } - // Re-add same blocks and ensure nothing has changed - for (CachedItem block : blocks) { - try { - cache.cacheBlock(block.cacheKey, block); - assertTrue("Cache should not allow re-caching a block", false); - } catch(AssertionError re) { - // expected - assertTrue(re.getMessage().contains("Cached an already cached block")); - } - } - // Verify correctly calculated cache heap size assertEquals(expectedCacheSize, cache.heapSize());