svn commit: r1484034 - in /hbase/branches/0.94/src: main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java

[en...@apache.org]

Author: enis
Date: Sat May 18 00:12:14 2013
New Revision: 1484034

URL: http://svn.apache.org/r1484034
Log:
HBASE-8547. Fix java.lang.RuntimeException: Cached an already cached block (Addendum2 and 3)

Modified:
    hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
    hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java?rev=1484034&r1=1484033&r2=1484034&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java Sat May 18 00:12:14 2013
@@ -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 Bl
   /**
    * Cache the block with the specified name and buffer.
    * <p>
-   * 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 Bl
     }
   }
 
+  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.
    * <p>

Modified: hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java?rev=1484034&r1=1484033&r2=1484034&view=diff
==============================================================================
--- hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java (original)
+++ hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java Sat May 18 00:12:14 2013
@@ -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());