hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From li...@apache.org
Subject svn commit: r1555210 - in /hbase/branches/0.89-fb/src: main/java/org/apache/hadoop/hbase/io/hfile/ main/java/org/apache/hadoop/hbase/regionserver/metrics/ main/java/org/apache/hadoop/hbase/util/ test/java/org/apache/hadoop/hbase/io/ test/java/org/apach...
Date Fri, 03 Jan 2014 19:18:29 GMT
Author: liyin
Date: Fri Jan  3 19:18:29 2014
New Revision: 1555210

URL: http://svn.apache.org/r1555210
Log:
[0.89-fb] [HBASE-10270] Remove DataBlockEncoding from BlockCacheKey

Author: arjen

Summary:
When a block is added to the block cache its DataBlockEncoding type is stored on the BlockCacheKey. This block encoding is used in the calculation of the hashCode and as such matters when cache lookups are done. The reason the encoding is part of the key is because ScannerV2 expects data blocks where the EncodedScannerV2 expects encoded data blocks. Adding the encoding to the key implicitly defined the block type and caused a cache miss in case there was a mismatch between the block in cache and the scanner type.

However, this has the potential of double caching the same block (with different encoding) and in the case of the scan preloader a read is done without knowing the block type or the expected encoding, causing the preloader to miss encoded blocks already in cache. Removing the encoding from the key allows for a CompoundScannerV2, which can read all data blocks potentially saving IOPS when compacting recent data. Finally, removing the encoding from the BlockCacheKey and making it part of the read call makes more sense, allows keys to be shared between the BlockCache and BucketCache, allowing them to be more uniform and easier to deal with.

Test Plan: Ran the unit tests under org.apache.hadoop.hbase.io.

Reviewers: liyintang, aaiyer, rshroff

Reviewed By: liyintang

CC: hbase-eng@

Differential Revision: https://phabricator.fb.com/D1112436

Task ID: 2567910

Modified:
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilter.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/TestHeapSize.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestL2BucketCache.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java Fri Jan  3 19:18:29 2014
@@ -18,7 +18,6 @@
 package org.apache.hadoop.hbase.io.hfile;
 
 import org.apache.hadoop.hbase.io.HeapSize;
-import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.ClassSize;
 
@@ -28,32 +27,20 @@ import org.apache.hadoop.hbase.util.Clas
 public class BlockCacheKey implements HeapSize {
   private final String hfileName;
   private final long offset;
-  private final DataBlockEncoding encoding;
-
-  public BlockCacheKey(String file, long offset, DataBlockEncoding encoding,
-      BlockType blockType) {
-    this.hfileName = file;
-    this.offset = offset;
-    // We add encoding to the cache key only for data blocks. If the block type
-    // is unknown (this should never be the case in production), we just use
-    // the provided encoding, because it might be a data block.
-    this.encoding = (blockType == null || blockType.isData()) ? encoding :
-        DataBlockEncoding.NONE;
-  }
 
   /**
    * Construct a new BlockCacheKey
-   * @param file The name of the HFile this block belongs to.
+   * @param hfileName The name of the HFile this block belongs to.
    * @param offset Offset of the block into the file
    */
-  public BlockCacheKey(String file, long offset) {
-    this(file, offset, DataBlockEncoding.NONE, null);
+  public BlockCacheKey(String hfileName, long offset) {
+    this.hfileName = hfileName;
+    this.offset = offset;
   }
 
   @Override
   public int hashCode() {
-    return hfileName.hashCode() * 127 + (int) (offset ^ (offset >>> 32)
-     + encoding.ordinal() * 17);
+    return hfileName.hashCode() * 127 + (int) (offset ^ (offset >>> 32));
   }
 
   @Override
@@ -62,9 +49,7 @@ public class BlockCacheKey implements He
       BlockCacheKey k = (BlockCacheKey) o;
       return offset == k.offset
           && (hfileName == null ? k.hfileName == null : hfileName
-              .equals(k.hfileName))
-          && (encoding == null ? k.encoding == null :
-              encoding.ordinal() == k.encoding.ordinal());
+              .equals(k.hfileName));
     } else {
       return false;
     }
@@ -72,18 +57,21 @@ public class BlockCacheKey implements He
 
   @Override
   public String toString() {
-    return hfileName + "_" + offset
-        + (encoding == DataBlockEncoding.NONE ? "" : "_" + encoding);
+    return String.format("%s_%d", hfileName, offset);
   }
 
+  public static final long FIXED_OVERHEAD = ClassSize.OBJECT +
+          ClassSize.REFERENCE + // this.hfileName
+          Bytes.SIZEOF_LONG;    // this.offset
+
   /**
    * Strings have two bytes per character due to default Java Unicode encoding
    * (hence length times 2).
    */
   @Override
   public long heapSize() {
-    return ClassSize.align(ClassSize.OBJECT + 2 * hfileName.length() +
-        Bytes.SIZEOF_LONG + 2 * ClassSize.REFERENCE);
+    return ClassSize.align(FIXED_OVERHEAD + ClassSize.STRING +
+            2 * hfileName.length());
   }
 
   // can't avoid this unfortunately
@@ -94,10 +82,6 @@ public class BlockCacheKey implements He
     return hfileName;
   }
 
-  public DataBlockEncoding getDataBlockEncoding() {
-    return encoding;
-  }
-
   public long getOffset() {
     return offset;
   }

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java Fri Jan  3 19:18:29 2014
@@ -435,7 +435,7 @@ public class HFile {
   public interface CachingBlockReader {
     /**
      * Read in a file block.
-     * @param dataBlockOffset offset to read.
+     * @param offset offset to read.
      * @param onDiskBlockSize size of the block
      * @param cacheBlock
      * @param isCompaction is this block being read as part of a compaction
@@ -443,13 +443,19 @@ public class HFile {
      * @param expectedBlockType the block type we are expecting to read with this read operation, or
      *          null to read whatever block type is available and avoid checking (that might reduce
      *          caching efficiency of encoded data blocks)
-     * @param obtainedFromCache
+     * @param expectedDataBlockEncoding the data block encoding the caller is
+     *          expecting data blocks to be in, or null to not perform this
+     *          check and return the block irrespective of the encoding. This
+     *          check only applies to data blocks and can be set to null when
+     *          the caller is expecting to read a non-data block and has set
+     *          {@param expectedBlockType} accordingly.
      * @return Block wrapped in a ByteBuffer.
      * @throws IOException
      */
     HFileBlock readBlock(long offset, long onDiskBlockSize,
         boolean cacheBlock, final boolean isCompaction, boolean cacheOnPreload,
-        BlockType expectedBlockType, KeyValueContext kvContext)
+        BlockType expectedBlockType,
+        DataBlockEncoding expectedDataBlockEncoding, KeyValueContext kvContext)
         throws IOException;
   }
 

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java Fri Jan  3 19:18:29 2014
@@ -177,15 +177,22 @@ public class HFileBlockIndex {
      * @param keyLength the length of the key
      * @param currentBlock the current block, to avoid re-reading the same
      *          block
+     * @param cacheBlocks
+     * @param isCompaction whether this seek is done on behalf of a compaction
+     * @param expectedDataBlockEncoding the data block encoding the caller is
+     *          expecting the data block to be in, or null to not perform this
+     *          check and return the block irrespective of the encoding.
      * @return reader a basic way to load blocks
      * @throws IOException
      */
     public HFileBlock seekToDataBlock(final byte[] key, int keyOffset,
         int keyLength, HFileBlock currentBlock, boolean cacheBlocks,
-        boolean isCompaction, KeyValueContext kvContext)
-        throws IOException {
-      BlockWithScanInfo blockWithScanInfo = loadDataBlockWithScanInfo(key, keyOffset, keyLength,
-          currentBlock, cacheBlocks, isCompaction, kvContext);
+        boolean isCompaction, DataBlockEncoding expectedDataBlockEncoding,
+        KeyValueContext kvContext) throws IOException {
+      BlockWithScanInfo blockWithScanInfo =
+              loadDataBlockWithScanInfo(key, keyOffset, keyLength, currentBlock,
+                      cacheBlocks, isCompaction, expectedDataBlockEncoding,
+                      kvContext);
       if (blockWithScanInfo == null) {
         return null;
       } else {
@@ -205,6 +212,9 @@ public class HFileBlockIndex {
      *          block
      * @param cacheBlocks
      * @param isCompaction
+     * @param expectedDataBlockEncoding the data block encoding the caller is
+     *          expecting the data block to be in, or null to not perform this
+     *          check and return the block irrespective of the encoding.
      * @param kvContext
      * @return the BlockWithScanInfo which contains the DataBlock with other scan info 
      *         such as nextIndexedKey.
@@ -212,8 +222,8 @@ public class HFileBlockIndex {
      */
     public BlockWithScanInfo loadDataBlockWithScanInfo(final byte[] key, int keyOffset,
         int keyLength, HFileBlock currentBlock, boolean cacheBlocks,
-        boolean isCompaction, KeyValueContext kvContext)
-        throws IOException {
+        boolean isCompaction, DataBlockEncoding expectedDataBlockEncoding,
+        KeyValueContext kvContext) throws IOException {
       int rootLevelIndex = rootBlockContainingKey(key, keyOffset, keyLength);
       if (rootLevelIndex < 0 || rootLevelIndex >= blockOffsets.length) {
         return null;
@@ -255,12 +265,11 @@ public class HFileBlockIndex {
           } else if (lookupLevel == searchTreeLevel - 1) {
             expectedBlockType = BlockType.LEAF_INDEX;
           } else {
-            // this also accounts for ENCODED_DATA
             expectedBlockType = BlockType.DATA;
           }
           block = cachingBlockReader.readBlock(currentOffset,
               currentOnDiskSize, shouldCache, isCompaction, false,
-              expectedBlockType, kvContext);
+              expectedBlockType, expectedDataBlockEncoding, kvContext);
         }
 
         if (block == null) {
@@ -335,8 +344,9 @@ public class HFileBlockIndex {
 
         // Caching, using pread, assuming this is not a compaction.
         HFileBlock midLeafBlock =
-            cachingBlockReader.readBlock(midLeafBlockOffset, midLeafBlockOnDiskSize, true, false,
-              false, BlockType.LEAF_INDEX, null);
+            cachingBlockReader.readBlock(midLeafBlockOffset,
+                    midLeafBlockOnDiskSize, true, false, false,
+                    BlockType.LEAF_INDEX, null, null);
 
         ByteBuffer b = midLeafBlock.getBufferWithoutHeader();
         int numDataBlocks = b.getInt();
@@ -938,9 +948,8 @@ public class HFileBlockIndex {
       if (blockCache != null) {
         HFileBlock blockForCaching = blockWriter.getBlockForCaching();
         passSchemaMetricsTo(blockForCaching);
-        blockCache.cacheBlock(new BlockCacheKey(nameForCaching,
-            beginOffset, DataBlockEncoding.NONE,
-            blockForCaching.getBlockType()), blockForCaching);
+        blockCache.cacheBlock(new BlockCacheKey(nameForCaching, beginOffset),
+                blockForCaching);
       }
 
       if (l2Cache != null) {
@@ -1030,8 +1039,6 @@ public class HFileBlockIndex {
      * blocks, so the non-root index format is used.
      *
      * @param out
-     * @param position The beginning offset of the inline block in the file not
-     *          include the header.
      */
     @Override
     public void writeInlineBlock(DataOutput out) throws IOException {

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java Fri Jan  3 19:18:29 2014
@@ -218,8 +218,7 @@ public class HFileReaderV1 extends Abstr
 
     long startTimeNs = System.nanoTime();
 
-    BlockCacheKey cacheKey = new BlockCacheKey(name, offset,
-        DataBlockEncoding.NONE, BlockType.META);
+    BlockCacheKey cacheKey = new BlockCacheKey(name, offset);
 
     BlockCategory effectiveCategory = BlockCategory.META;
     if (metaBlockName.equals(HFileWriterV1.BLOOM_FILTER_META_KEY) ||
@@ -393,8 +392,7 @@ public class HFileReaderV1 extends Abstr
       for (int i = 0; i < dataBlockIndexReader.getRootBlockCount(); i++) {
         if (cacheConf.getBlockCache().evictBlock(
             new BlockCacheKey(name,
-                dataBlockIndexReader.getRootBlockOffset(i),
-                DataBlockEncoding.NONE, BlockType.DATA))) {
+                dataBlockIndexReader.getRootBlockOffset(i)))) {
           numEvicted++;
         }
       }
@@ -715,7 +713,7 @@ public class HFileReaderV1 extends Abstr
   @Override
   public HFileBlock readBlock(long offset, long onDiskBlockSize, boolean cacheBlock,
       boolean isCompaction, boolean cacheOnPreload, BlockType expectedBlockType,
-      KeyValueContext kvContext) {
+      DataBlockEncoding expectedDataBlockEncoding, KeyValueContext kvContext) {
     throw new UnsupportedOperationException();
   }
 

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java Fri Jan  3 19:18:29 2014
@@ -221,23 +221,19 @@ public class HFileReaderV2 extends Abstr
     synchronized (metaBlockIndexReader.getRootBlockKey(block)) {
       // Check cache for block. If found return.
       long metaBlockOffset = metaBlockIndexReader.getRootBlockOffset(block);
-      BlockCacheKey cacheKey = new BlockCacheKey(name, metaBlockOffset,
-          DataBlockEncoding.NONE, BlockType.META);
+      BlockCacheKey cacheKey = new BlockCacheKey(name, metaBlockOffset);
 
       boolean cacheInL2 = cacheBlock && cacheConf.isL2CacheEnabled();
       cacheBlock &= cacheConf.shouldCacheDataOnRead();
-      if (cacheConf.isBlockCacheEnabled()) {
-        HFileBlock cachedBlock =
-          (HFileBlock) cacheConf.getBlockCache().getBlock(cacheKey, cacheBlock);
-        if (cachedBlock != null) {
-          // Return a distinct 'shallow copy' of the block,
-          // so pos does not get messed by the scanner
-          getSchemaMetrics().updateOnCacheHit(BlockCategory.META, false);
-          return cachedBlock.getBufferWithoutHeader();
-        }
-        // Cache Miss, please load.
+      HFileBlock cachedBlock = getCachedBlock(cacheKey, cacheBlock, false,
+              BlockType.META, null);
+      if (cachedBlock != null) {
+        // Return a distinct 'shallow copy' of the block,
+        // so pos does not get messed by the scanner
+        getSchemaMetrics().updateOnCacheHit(BlockCategory.META, false);
+        return cachedBlock.getBufferWithoutHeader();
       }
-
+      // Cache Miss, please load.
       HFileBlock metaBlock = fsBlockReader.readBlockData(metaBlockOffset,
           blockSize, -1, cacheInL2);
       passSchemaMetricsTo(metaBlock);
@@ -260,7 +256,8 @@ public class HFileReaderV2 extends Abstr
   }
 
   /**
-   * Read in a file block.
+   * Read in a file block of the given {@link BlockType} and
+   * {@link DataBlockEncoding}.
    * @param dataBlockOffset offset to read.
    * @param onDiskBlockSize size of the block
    * @param cacheBlock
@@ -268,16 +265,24 @@ public class HFileReaderV2 extends Abstr
    * @param cacheOnPreload should we cache this block because we are preloading
    * @param expectedBlockType the block type we are expecting to read with this
    *          read operation, or null to read whatever block type is available
-   *          and avoid checking (that might reduce caching efficiency of
-   *          encoded data blocks)
-   * @param obtainedFromCache
+   *          and avoid checking. See AbstractScannerV2.readNextDataBlock() as
+   *          an appropriate example of a read being performed without knowing
+   *          the block type in advance.
+   * @param expectedDataBlockEncoding the data block encoding the caller is
+   *          expecting data blocks to be in, or null to not perform this
+   *          check and return the block irrespective of the encoding. This
+   *          check only applies to data blocks and can be set to null when
+   *          the caller is expecting to read a non-data block and has set
+   *          {@param expectedBlockType} accordingly.
+   * @param kvContext
    * @return Block wrapped in a ByteBuffer.
    * @throws IOException
    */
   public HFileBlock readBlock(long dataBlockOffset, long onDiskBlockSize,
       final boolean cacheBlock, final boolean isCompaction,
       boolean cacheOnPreload, BlockType expectedBlockType,
-      KeyValueContext kvContext) throws IOException {
+      DataBlockEncoding expectedDataBlockEncoding, KeyValueContext kvContext)
+          throws IOException {
     /*
      * time at which we entered the function, for metrics we use readTime as the whole time spent in
      * this function not just the time spent reading disk, this is just to add extra cacheHits into
@@ -294,20 +299,18 @@ public class HFileReaderV2 extends Abstr
           + dataBlockOffset + ", lastDataBlockOffset: "
           + trailer.getLastDataBlockOffset());
     }
+
     // For any given block from any given file, synchronize reads for said
     // block.
     // Without a cache, this synchronizing is needless overhead, but really
     // the other choice is to duplicate work (which the cache would prevent you
     // from doing).
-
     BlockCacheKey cacheKey =
-        new BlockCacheKey(name, dataBlockOffset,
-            dataBlockEncoder.getEffectiveEncodingInCache(isCompaction),
-            expectedBlockType);
+        new BlockCacheKey(name, dataBlockOffset);
     // Checking the block cache.
     HFileBlock cachedBlock =
-        this.getCachedBlock(cacheKey, cacheBlock, isCompaction,
-          expectedBlockType);
+        getCachedBlock(cacheKey, cacheBlock, isCompaction, expectedBlockType,
+                expectedDataBlockEncoding);
     if (cachedBlock != null) {
       if (kvContext != null) {
         if (LOG.isTraceEnabled()) {
@@ -333,8 +336,8 @@ public class HFileReaderV2 extends Abstr
     try {
       // Double checking the block cache again within the IdLock
       cachedBlock =
-          this.getCachedBlock(cacheKey, cacheBlock, isCompaction,
-            expectedBlockType);
+          getCachedBlock(cacheKey, cacheBlock, isCompaction, expectedBlockType,
+                  expectedDataBlockEncoding);
       if (cachedBlock != null) {
         if (kvContext != null) {
           kvContext.setObtainedFromCache(true);
@@ -442,23 +445,52 @@ public class HFileReaderV2 extends Abstr
   }
 
   private HFileBlock getCachedBlock(BlockCacheKey cacheKey, boolean cacheBlock,
-      boolean isCompaction, BlockType expectedBlockType) throws IOException {
+      boolean isCompaction, BlockType expectedBlockType,
+      DataBlockEncoding expectedDataBlockEncoding) throws IOException {
     // Check cache for block. If found return.
     if (cacheConf.isBlockCacheEnabled()) {
-      HFileBlock cachedBlock = (HFileBlock)
-          cacheConf.getBlockCache().getBlock(cacheKey, cacheBlock);
+      BlockCache cache = cacheConf.getBlockCache();
+      HFileBlock cachedBlock =
+              (HFileBlock) cache.getBlock(cacheKey, cacheBlock);
       if (cachedBlock != null) {
-        // Validate the block type first
         validateBlockType(cachedBlock, expectedBlockType);
 
-        // Validate encoding type for encoded blocks. We include encoding
-        // type in the cache key, and we expect it to match on a cache hit.
-        if (cachedBlock.getBlockType() == BlockType.ENCODED_DATA &&
-            cachedBlock.getDataBlockEncoding() !=
-            dataBlockEncoder.getEncodingInCache()) {
-          throw new IOException("Cached block under key " + cacheKey + " " +
-              "has wrong encoding: " + cachedBlock.getDataBlockEncoding() +
-              " (expected: " + dataBlockEncoder.getEncodingInCache() + ")");
+        if (expectedDataBlockEncoding == null) {
+          return cachedBlock;
+        }
+        DataBlockEncoding actualDataBlockEncoding =
+                cachedBlock.getDataBlockEncoding();
+        // Block types other than data blocks always have
+        // DataBlockEncoding.NONE. To avoid false negative cache misses, only
+        // perform this check if cached block is a data block.
+        if (cachedBlock.getBlockType().isData() &&
+                !actualDataBlockEncoding.equals(expectedDataBlockEncoding)) {
+          // This mismatch may happen if a ScannerV2, which is used for say a
+          // compaction, tries to read an encoded block from the block cache.
+          // The reverse might happen when an EncodedScannerV2 tries to read
+          // un-encoded blocks which were cached earlier.
+          //
+          // Because returning a data block with an implicit BlockType mismatch
+          // will cause the requesting scanner to throw a disk read should be
+          // forced here. This will potentially cause a significant number of
+          // cache misses, so update so we should keep track of this as it might
+          // justify the work on a CompoundScannerV2.
+          getSchemaMetrics().updateOnDataBlockEncodingMismatch(isCompaction);
+          if (!expectedDataBlockEncoding.equals(DataBlockEncoding.NONE) &&
+                  !actualDataBlockEncoding.equals(DataBlockEncoding.NONE)) {
+            // If the block is encoded but the encoding does not match the
+            // expected encoding it is likely the encoding was changed but the
+            // block was not yet evicted. Evictions on file close happen async
+            // so blocks with the old encoding still linger in cache for some
+            // period of time. This event should be rare as it only happens on
+            // schema definition change.
+            LOG.info("Evicting cached block with key " + cacheKey +
+                    " because of a data block encoding mismatch" +
+                    "; expected: " + expectedDataBlockEncoding +
+                    ", actual: " + actualDataBlockEncoding);
+            cache.evictBlock(cacheKey);
+          }
+          return null;
         }
         return cachedBlock;
       }
@@ -517,8 +549,7 @@ public class HFileReaderV2 extends Abstr
       return;
     }
     BlockType actualBlockType = block.getBlockType();
-    if (actualBlockType == BlockType.ENCODED_DATA &&
-        expectedBlockType == BlockType.DATA) {
+    if (expectedBlockType.isData() && actualBlockType.isData()) {
       // We consider DATA to match ENCODED_DATA for the purpose of this
       // verification.
       return;
@@ -528,7 +559,7 @@ public class HFileReaderV2 extends Abstr
           "but got " + actualBlockType + ": " + block);
     }
   }
-  
+
   /**
    * @return Last key in the file. May be null if file has no entries. Note that
    *         this is not the last row key, but rather the byte form of the last
@@ -582,6 +613,10 @@ public class HFileReaderV2 extends Abstr
       istream = null;
     }
   }
+
+  public DataBlockEncoding getEffectiveEncodingInCache(boolean isCompaction) {
+    return dataBlockEncoder.getEffectiveEncodingInCache(isCompaction);
+  }
   
   protected abstract static class AbstractScannerV2
       extends AbstractHFileReader.Scanner {
@@ -653,9 +688,9 @@ public class HFileReaderV2 extends Abstr
       LinkedBlockingQueue<Long> preloadAttempted;
       // This queue is to keep blocks that should be evicted
       LinkedBlockingQueue<Long> evictQueue;
-      //Offset of the next block to preload
+      // Offset of the next block to preload
       long startOffset;
-      //size of the next block to preload
+      // Size of the next block to preload
       long startSize;
       BlockType expectedType;
       // Number of blocks left to preload
@@ -696,9 +731,9 @@ public class HFileReaderV2 extends Abstr
               }
               HFileBlock block = null;
               try {
-                block =
-                    reader.readBlock(startOffset, startSize, cacheBlocks,
-                      isCompaction, ON_PRELOAD, null, preloaderKvContext);
+                block = reader.readBlock(startOffset, startSize, cacheBlocks,
+                        isCompaction, ON_PRELOAD, null,
+                        getEffectiveDataBlockEncoding(), preloaderKvContext);
               } catch (Throwable e) {
                 // in case of ANY kind of error, we'll mark this block as attempted and let the IPC
                 // Caller handler catch this exception
@@ -760,9 +795,8 @@ public class HFileReaderV2 extends Abstr
             leftToPreload.set(scanPreloadBlocksCount - preloadAttempted.size());
             startNewPreloader();
           }
-          block =
-              reader.readBlock(offset, size, cacheBlocks, isCompaction, false,
-                blocktype, kvContext);
+          block = reader.readBlock(offset, size, cacheBlocks, isCompaction,
+                  false, blocktype, getEffectiveDataBlockEncoding(), kvContext);
           metrics.lastRequestOffset = offset;
           return block;
         } else {
@@ -780,9 +814,9 @@ public class HFileReaderV2 extends Abstr
           if (read != null && read.equals(offset)) {
             preloadAttempted.poll();
             // continue wherever you stopped you were in the right direction
-            block =
-                reader.readBlock(offset, size, cacheBlocks, isCompaction,
-                  false, blocktype, kvContext);
+            block = reader.readBlock(offset, size, cacheBlocks, isCompaction,
+                    false, blocktype, getEffectiveDataBlockEncoding(),
+                    kvContext);
             leftToPreload.set(scanPreloadBlocksCount - preloadAttempted.size());
             startNewPreloader();
           } else {
@@ -793,7 +827,7 @@ public class HFileReaderV2 extends Abstr
             // you're in the wrong place read the block, clear and reset offset
             block =
                 reader.readBlock(offset, size, cacheBlocks, isCompaction,
-                  false, blocktype, kvContext);
+                  false, blocktype, getEffectiveDataBlockEncoding(), kvContext);
             preloadAttempted.clear();
             if (block == null) {
               return null;
@@ -837,6 +871,10 @@ public class HFileReaderV2 extends Abstr
           LOG.info("Preloader metrics " + metrics);
         }
       }
+
+      private DataBlockEncoding getEffectiveDataBlockEncoding() {
+        return hfileReaderV2.getEffectiveEncodingInCache(isCompaction);
+      }
     }
 
     /**
@@ -854,7 +892,7 @@ public class HFileReaderV2 extends Abstr
         return blockManager.getPreloadBlock(offset, size, blocktype);
       } catch (InterruptedException e) {
         return reader.readBlock(offset, size, cacheBlocks, isCompaction, false,
-          blocktype, kvContext);
+          blocktype, getEffectiveDataBlockEncoding(), kvContext);
       }
     }
 
@@ -891,7 +929,8 @@ public class HFileReaderV2 extends Abstr
           reader.getDataBlockIndexReader();
       BlockWithScanInfo blockWithScanInfo =
           indexReader.loadDataBlockWithScanInfo(key, offset, length, block,
-            cacheBlocks, isCompaction, this.kvContext);
+            cacheBlocks, isCompaction, getEffectiveDataBlockEncoding(),
+                  kvContext);
       if (blockWithScanInfo == null
           || blockWithScanInfo.getHFileBlock() == null) {
         // This happens if the key e.g. falls before the beginning of the file.
@@ -950,7 +989,7 @@ public class HFileReaderV2 extends Abstr
       HFileBlock seekToBlock =
           reader.getDataBlockIndexReader().seekToDataBlock(key, offset, length,
             block, cacheBlocks || preloadBlocks, isCompaction,
-            this.kvContext);
+                  getEffectiveDataBlockEncoding(), this.kvContext);
       if (seekToBlock == null) {
         return false;
       }
@@ -972,12 +1011,13 @@ public class HFileReaderV2 extends Abstr
         if (preloadBlocks) {
           seekToBlock =
               readBlockUsingBlockingPreloadManager(previousBlockOffset,
-                seekToBlock.getOffset() - previousBlockOffset, BlockType.DATA);
+                seekToBlock.getOffset() - previousBlockOffset, null);
         } else {
           seekToBlock =
-              reader.readBlock(previousBlockOffset, seekToBlock.getOffset()
-                  - previousBlockOffset, cacheBlocks, isCompaction, false,
-                BlockType.DATA, this.kvContext);
+              reader.readBlock(previousBlockOffset,
+                      seekToBlock.getOffset() - previousBlockOffset,
+                      cacheBlocks, isCompaction, false, null,
+                      getEffectiveDataBlockEncoding(), this.kvContext);
           // TODO shortcut: seek forward in this block to the last key of the
           // block.
         }
@@ -1001,7 +1041,8 @@ public class HFileReaderV2 extends Abstr
         return null;
 
       HFileBlock curBlock = block;
-
+      // The next block might not be a data block, so keep reading until a block
+      // of the expected type is returned.
       do {
         if (curBlock.getOffset() >= lastDataBlockOffset)
           return null;
@@ -1010,8 +1051,6 @@ public class HFileReaderV2 extends Abstr
           throw new IOException("Invalid block file offset: " + block);
         }
 
-        // We are reading the next block without block type validation, because
-        // it might turn out to be a non-data block.
         if (preloadBlocks) {
           curBlock =
               readBlockUsingBlockingPreloadManager(curBlock.getOffset()
@@ -1022,12 +1061,16 @@ public class HFileReaderV2 extends Abstr
               reader.readBlock(
                 curBlock.getOffset() + curBlock.getOnDiskSizeWithHeader(),
                 curBlock.getNextBlockOnDiskSizeWithHeader(), cacheBlocks,
-                isCompaction, false, null, this.kvContext);
+                isCompaction, false, null, getEffectiveDataBlockEncoding(),
+                this.kvContext);
         }
-      } while (!(curBlock.getBlockType().equals(BlockType.DATA) ||
-          curBlock.getBlockType().equals(BlockType.ENCODED_DATA)));
+      } while (!curBlock.getBlockType().isData());
       return curBlock;
     }
+
+    public DataBlockEncoding getEffectiveDataBlockEncoding() {
+      return hfileReaderV2.getEffectiveEncodingInCache(isCompaction);
+    };
   }
 
   /**
@@ -1156,7 +1199,8 @@ public class HFileReaderV2 extends Abstr
       }
 
       block = reader.readBlock(firstDataBlockOffset, -1, cacheBlocks,
-          isCompaction, false, BlockType.DATA, this.kvContext);
+          isCompaction, false, BlockType.DATA, getEffectiveDataBlockEncoding(),
+              this.kvContext);
       if (block.getOffset() < 0) {
         throw new IOException("Invalid block offset: " + block.getOffset());
       }
@@ -1426,7 +1470,8 @@ public class HFileReaderV2 extends Abstr
       }
 
       block = reader.readBlock(firstDataBlockOffset, -1, cacheBlocks,
-          isCompaction, false, BlockType.DATA, this.kvContext);
+          isCompaction, false, BlockType.ENCODED_DATA,
+              getEffectiveDataBlockEncoding(), this.kvContext);
       if (block.getOffset() < 0) {
         throw new IOException("Invalid block offset: " + block.getOffset());
       }
@@ -1552,9 +1597,7 @@ public class HFileReaderV2 extends Abstr
   }
   
   public void evictBlock(long offset, boolean isCompaction) {
-    BlockCacheKey cacheKey =
-        new BlockCacheKey(name, offset,
-            dataBlockEncoder.getEffectiveEncodingInCache(isCompaction), null);
+    BlockCacheKey cacheKey = new BlockCacheKey(name, offset);
     if (cacheConf.isBlockCacheEnabled()) {
       cacheConf.getBlockCache().evictBlock(cacheKey);
     }

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java Fri Jan  3 19:18:29 2014
@@ -140,8 +140,7 @@ public class HFileWriterV1 extends Abstr
       block = blockEncoder.diskToCacheFormat(block, false);
       passSchemaMetricsTo(block);
       cacheConf.getBlockCache().cacheBlock(
-          new BlockCacheKey(name, blockBegin, DataBlockEncoding.NONE,
-              block.getBlockType()), block);
+          new BlockCacheKey(name, blockBegin), block);
       baosDos.close();
     }
     blockNumber++;

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java Fri Jan  3 19:18:29 2014
@@ -231,9 +231,8 @@ public class HFileWriterV2 extends Abstr
     HFileBlock cacheFormatBlock = blockEncoder.diskToCacheFormat(
         fsBlockWriter.getBlockForCaching(), isCompaction);
     passSchemaMetricsTo(cacheFormatBlock);
-    cacheConf.getBlockCache().cacheBlock(
-        new BlockCacheKey(name, offset, blockEncoder.getEncodingInCache(),
-            cacheFormatBlock.getBlockType()), cacheFormatBlock);
+    cacheConf.getBlockCache().cacheBlock(new BlockCacheKey(name, offset),
+            cacheFormatBlock);
   }
 
   /**

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java Fri Jan  3 19:18:29 2014
@@ -292,8 +292,6 @@ 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.
    * @param cacheKey block's cache key
    * @param buf block buffer
    * @param inMemory if block is in-memory
@@ -301,7 +299,7 @@ public class LruBlockCache implements Bl
   public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) {
     CachedBlock cb = map.get(cacheKey);
     if(cb != null) {
-      throw new RuntimeException("Cached an already cached block");
+      return;
     }
     cb = new CachedBlock(cacheKey, buf, count.incrementAndGet(), inMemory);
     long newSize = updateSizeMetrics(cb, false);
@@ -315,10 +313,6 @@ 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, it is assumed that you are reinserting the same exact
-   * block due to a race condition and will update the buffer but not modify
-   * the size of the cache.
    * @param cacheKey block's cache key
    * @param buf block buffer
    */
@@ -964,8 +958,9 @@ public class LruBlockCache implements Bl
   public Map<DataBlockEncoding, Integer> getEncodingCountsForTest() {
     Map<DataBlockEncoding, Integer> counts =
         new EnumMap<DataBlockEncoding, Integer>(DataBlockEncoding.class);
-    for (BlockCacheKey cacheKey : map.keySet()) {
-      DataBlockEncoding encoding = cacheKey.getDataBlockEncoding();
+    for (CachedBlock block : map.values()) {
+      DataBlockEncoding encoding =
+              ((HFileBlock) block.getBuffer()).getDataBlockEncoding();
       Integer count = counts.get(encoding);
       counts.put(encoding, (count == null ? 0 : count) + 1);
     }

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java Fri Jan  3 19:18:29 2014
@@ -117,10 +117,11 @@ public class SchemaMetrics {
     READ_COUNT("BlockReadCnt", COMPACTION_AWARE_METRIC_FLAG),
     CACHE_HIT("BlockReadCacheHitCnt", COMPACTION_AWARE_METRIC_FLAG),
     CACHE_MISS("BlockReadCacheMissCnt", COMPACTION_AWARE_METRIC_FLAG),
-  
+
     PRELOAD_CACHE_HIT("PreloadCacheHitCnt",COMPACTION_AWARE_METRIC_FLAG),
     PRELOAD_CACHE_MISS("PreloadCacheMissCnt",COMPACTION_AWARE_METRIC_FLAG),
-    PRELOAD_READ_TIME("PreloadReadTime",COMPACTION_AWARE_METRIC_FLAG | TIME_VARYING_METRIC_FLAG),
+    PRELOAD_READ_TIME("PreloadReadTime",
+            COMPACTION_AWARE_METRIC_FLAG | TIME_VARYING_METRIC_FLAG),
     
     CACHE_SIZE("blockCacheSize", PERSISTENT_METRIC_FLAG),
     UNENCODED_CACHE_SIZE("blockCacheUnencodedSize", PERSISTENT_METRIC_FLAG),
@@ -306,6 +307,7 @@ public class SchemaMetrics {
   private final String[] bloomMetricNames = new String[2];
   private final String[] storeMetricNames = new String[NUM_STORE_METRIC_TYPES];
   private final String[] storeMetricNamesMax = new String[NUM_STORE_METRIC_TYPES];
+  private final String[] dataBlockEncodingMismatchMetricNames = new String[2];
 
   private SchemaMetrics(final String tableName, final String cfName) {
     String metricPrefix =
@@ -350,6 +352,13 @@ public class SchemaMetrics {
           + (isInBloom ? "keyMaybeInBloomCnt" : "keyNotInBloomCnt");
     }
 
+    for (boolean isCompaction : BOOL_VALUES) {
+      dataBlockEncodingMismatchMetricNames[isCompaction ? 1 : 0] =
+              String.format("%s%sDataBlockEncodingMismatchCnt", metricPrefix,
+                      isCompaction ? COMPACTION_METRIC_PREFIX :
+                              NON_COMPACTION_METRIC_PREFIX);
+    }
+
     for (StoreMetricType storeMetric : StoreMetricType.values()) {
       String coreName = metricPrefix + storeMetric.toString();
       storeMetricNames[storeMetric.ordinal()] = coreName;
@@ -423,6 +432,10 @@ public class SchemaMetrics {
     return bloomMetricNames[isInBloom ? 1 : 0];
   }
 
+  public String getDataBlockEncodingMismatchMetricNames(boolean isCompaction) {
+    return dataBlockEncodingMismatchMetricNames[isCompaction ? 1 : 0];
+  }
+
   /**
    * Increments the given metric, both per-CF and aggregate, for both the given
    * category and all categories in aggregate (four counters total).
@@ -669,6 +682,19 @@ public class SchemaMetrics {
   }
 
   /**
+   * Updates the number of times a block was present in the block cache, but
+   * could not be used due to a type mismatch. The assumption is this should
+   * really only happen upon compactions of CFs which use block encoding.
+   */
+  public void updateOnDataBlockEncodingMismatch(boolean isCompaction) {
+    HRegion.incrNumericMetric(
+            getDataBlockEncodingMismatchMetricNames(isCompaction), 1);
+    if (this != ALL_SCHEMA_METRICS) {
+      ALL_SCHEMA_METRICS.updateOnDataBlockEncodingMismatch(isCompaction);
+    }
+  }
+
+  /**
    * Sets the flag whether to use table name in metric names according to the
    * given configuration. This must be called at least once before
    * instantiating HFile readers/writers.
@@ -1107,7 +1133,7 @@ public class SchemaMetrics {
       long metricValue = HRegion.getNumericPersistentMetric(metricName);
       long expectedValue = blockCategoryCounts[blockCategory.ordinal()];
       if (metricValue != expectedValue) {
-        throw new AssertionError("Expected " + expectedValue + " blocks of category " + 
+        throw new AssertionError("Expected " + expectedValue + " blocks of category " +
             blockCategory + " in cache, but found " + metricName + "=" + metricValue);
       }
     }

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilter.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilter.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilter.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilter.java Fri Jan  3 19:18:29 2014
@@ -97,7 +97,7 @@ public class CompoundBloomFilter extends
         // We cache the block and use a positional read.
         bloomBlock = reader.readBlock(index.getRootBlockOffset(block),
             index.getRootBlockDataSize(block), true, false, false,
-            BlockType.BLOOM_CHUNK, null);
+            BlockType.BLOOM_CHUNK, null, null);
       } catch (IOException ex) {
        LOG.error( "Failed to load Bloom block. Returning a potential false positive " +
          "for key " + Bytes.toStringBinary(key, keyOffset, keyLength),  ex);

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/TestHeapSize.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/TestHeapSize.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/TestHeapSize.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/TestHeapSize.java Fri Jan  3 19:18:29 2014
@@ -294,10 +294,13 @@ public class TestHeapSize extends TestCa
     }
 
     // SchemaConfigured
-    LOG.debug("Heap size for: " + SchemaConfigured.class.getName());
-    SchemaConfigured sc = new SchemaConfigured(null, "myTable", "myCF");
-    assertEquals(ClassSize.estimateBase(SchemaConfigured.class, true),
-        sc.heapSize());
+    cl = SchemaConfigured.class;
+    actual = new SchemaConfigured(null, "myTable", "myCF").heapSize();
+    expected = ClassSize.estimateBase(cl, false);
+    if (expected != actual) {
+      ClassSize.estimateBase(cl, true);
+      assertEquals(expected, actual);
+    }
 
     // Store Overhead
     cl = Store.class;
@@ -317,11 +320,10 @@ public class TestHeapSize extends TestCa
       assertEquals(expected, actual);
     }
 
-    // Block cache key overhead
+    // Block cache key overhead. Only tests fixed overhead as estimating heap
+    // size of strings is hard.
     cl = BlockCacheKey.class;
-    // Passing zero length file name, because estimateBase does not handle
-    // deep overhead.
-    actual = new BlockCacheKey("", 0).heapSize();
+    actual = BlockCacheKey.FIXED_OVERHEAD;
     expected  = ClassSize.estimateBase(cl, false);
     if (expected != actual) {
       ClassSize.estimateBase(cl, true);

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java Fri Jan  3 19:18:29 2014
@@ -16,29 +16,26 @@
  */
 package org.apache.hadoop.hbase.io.encoding;
 
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
-import java.util.Random;
+import java.util.*;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.HBaseTestingUtility;
-import org.apache.hadoop.hbase.HColumnDescriptor;
-import org.apache.hadoop.hbase.HConstants;
-import org.apache.hadoop.hbase.HTableDescriptor;
-import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.*;
 import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
 import org.apache.hadoop.hbase.client.HTable;
 import org.apache.hadoop.hbase.client.Put;
 import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.regionserver.HRegionServer;
+import org.apache.hadoop.hbase.regionserver.metrics.SchemaConfigured;
+import org.apache.hadoop.hbase.regionserver.metrics.SchemaMetrics;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.Threads;
 import org.junit.AfterClass;
@@ -129,7 +126,7 @@ public class TestChangingEncoding {
       Put put = new Put(getRowKey(batchId, i));
       for (int j = 0; j < NUM_COLS_PER_ROW; ++j) {
         put.add(CF_BYTES, getQualifier(j),
-            getValue(batchId, i, j));
+                getValue(batchId, i, j));
         table.put(put);
       }
     }
@@ -145,6 +142,7 @@ public class TestChangingEncoding {
       Result result = table.get(get);
       for (int j = 0; j < NUM_COLS_PER_ROW; ++j) {
         KeyValue kv = result.getColumnLatest(CF_BYTES, getQualifier(j));
+        assertNotNull(kv);
         assertEquals(Bytes.toStringBinary(getValue(batchId, i, j)),
             Bytes.toStringBinary(kv.getValue()));
       }
@@ -166,7 +164,7 @@ public class TestChangingEncoding {
   private void setEncodingConf(DataBlockEncoding encoding,
       boolean encodeOnDisk) throws IOException {
     LOG.debug("Setting CF encoding to " + encoding + " (ordinal="
-        + encoding.ordinal() + "), encodeOnDisk=" + encodeOnDisk);
+            + encoding.ordinal() + "), encodeOnDisk=" + encodeOnDisk);
     admin.disableTable(tableName);
     hcd.setDataBlockEncoding(encoding);
     hcd.setEncodeOnDisk(encodeOnDisk);
@@ -250,4 +248,37 @@ public class TestChangingEncoding {
     }
   }
 
+  private long getDataBlockEncodingMismatchCount(boolean isCompaction) {
+    SchemaMetrics metrics =
+            new SchemaConfigured(conf, tableName, CF).getSchemaMetrics();
+    return HRegion.getNumericMetric(
+            metrics.getDataBlockEncodingMismatchMetricNames(isCompaction));
+  }
+
+  @Test
+  public void testDataBlockEncodingMismatchMetrics() throws Exception {
+    prepareTest("DataEncodingMismatchMetrics");
+
+    long oldCountCompaction = getDataBlockEncodingMismatchCount(true);
+    long oldCountNonCompaction = getDataBlockEncodingMismatchCount(false);
+
+    setEncodingConf(DataBlockEncoding.FAST_DIFF, false);
+    // Read and write several batches of data to force flushes and encoded
+    // blocks to be read into cache.
+    for (int i = 0; i < 10; i++) {
+      writeSomeNewData();
+      verifyAllData();
+    }
+    // Force a major compaction which uses a un-encoded scanner and causes data
+    // block encoding mismatches in the cache.
+    compactAndWait();
+
+    long newCountCompaction = getDataBlockEncodingMismatchCount(true);
+    long newCountNonCompaction = getDataBlockEncodingMismatchCount(false);
+
+    assertTrue("Compaction scanners should not match encoded data blocks",
+            newCountCompaction - oldCountCompaction > 0);
+    assertEquals("Get scanners should match encoded data", 0,
+            newCountNonCompaction - oldCountNonCompaction);
+  }
 }

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java Fri Jan  3 19:18:29 2014
@@ -229,9 +229,8 @@ public class TestCacheOnWrite {
       // Flags: don't cache the block, use pread, this is not a compaction.
       // Also, pass null for expected block type to avoid checking it.
       HFileBlock block = reader.readBlock(offset, onDiskSize, false,
-          false, false, null, null);
-      BlockCacheKey blockCacheKey = new BlockCacheKey(reader.getName(),
-          offset, encodingInCache, block.getBlockType());
+          false, false, null, encodingInCache, null);
+      BlockCacheKey blockCacheKey = new BlockCacheKey(reader.getName(), offset);
       boolean isCached = blockCache.getBlock(blockCacheKey, true) != null;
       boolean shouldBeCached = cowType.shouldBeCached(block.getBlockType());
       if (shouldBeCached != isCached) {

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java Fri Jan  3 19:18:29 2014
@@ -47,6 +47,7 @@ import org.apache.hadoop.hbase.HBaseTest
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.KeyValue;
 import org.apache.hadoop.hbase.KeyValueContext;
+import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding;
 import org.apache.hadoop.hbase.io.hfile.HFileBlockIndex.BlockIndexChunk;
 import org.apache.hadoop.hbase.io.hfile.HFileBlockIndex.BlockIndexReader;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -142,7 +143,8 @@ public class TestHFileBlockIndex {
     @Override
     public HFileBlock readBlock(long offset, long onDiskSize,
         boolean cacheBlock, boolean isCompaction, boolean preloadBlocks,
-        BlockType expectedBlockType, KeyValueContext kvContext)
+        BlockType expectedBlockType,
+        DataBlockEncoding expectedDataBlockEncoding, KeyValueContext kvContext)
         throws IOException {
       if (offset == prevOffset && onDiskSize == prevOnDiskSize) {
         hitCount += 1;
@@ -185,7 +187,7 @@ public class TestHFileBlockIndex {
       assertTrue(key != null);
       assertTrue(indexReader != null);
       HFileBlock b = indexReader.seekToDataBlock(key, 0, key.length, null,
-          true, false, null);
+          true, false, null, null);
       if (Bytes.BYTES_RAWCOMPARATOR.compare(key, firstKeyInFile) < 0) {
         assertTrue(b == null);
         ++i;

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestL2BucketCache.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestL2BucketCache.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestL2BucketCache.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestL2BucketCache.java Fri Jan  3 19:18:29 2014
@@ -136,13 +136,13 @@ public class TestL2BucketCache {
       HFileBlock blockFromDisk;
       try {
         blockFromDisk =
-            reader.readBlock(offset, -1, false, false, false, null, null);
+            reader.readBlock(offset, -1, false, false, false, null,
+                    encodingInCache, null);
       } finally {
         mockedL2Cache.enableReads.set(true);
       }
       boolean isInL1Lcache = cacheConf.getBlockCache().getBlock(
-          new BlockCacheKey(reader.getName(), offset, encodingInCache,
-              blockFromDisk.getBlockType()), true) != null;
+          new BlockCacheKey(reader.getName(), offset), true) != null;
       if (isInL1Lcache) {
         cachedCount++;
         byte[] blockFromCacheRaw =
@@ -174,12 +174,13 @@ public class TestL2BucketCache {
     cacheConf.getBlockCache().clearCache();
     underlyingCache.clearCache();
     while (offset < reader.getTrailer().getLoadOnOpenDataOffset()) {
-      HFileBlock blockFromDisk = reader.readBlock(offset, -1, true, false, false, null, null);
+      HFileBlock blockFromDisk = reader.readBlock(offset, -1, true, false,
+              false, null, encodingInCache, null);
       assertNotNull(mockedL2Cache.getRawBlock(reader.getName(), offset));
       cacheConf.getBlockCache().evictBlock(new BlockCacheKey(reader.getName(),
-          offset, encodingInCache, blockFromDisk.getBlockType()));
-      HFileBlock blockFromL2Cache = reader.readBlock(offset, -1, true, false, false,
-          null, null);
+              offset));
+      HFileBlock blockFromL2Cache = reader.readBlock(offset, -1, true, false,
+              false, null, encodingInCache, null);
       assertEquals("Data in block from disk (" + blockFromDisk +
           ") should match data in block from cache (" + blockFromL2Cache +
           ").", blockFromL2Cache.getBufferWithHeader(),

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java?rev=1555210&r1=1555209&r2=1555210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java Fri Jan  3 19:18:29 2014
@@ -19,7 +19,6 @@
  */
 package org.apache.hadoop.hbase.io.hfile;
 
-import java.nio.ByteBuffer;
 import java.util.Collection;
 import java.util.Map;
 import java.util.Random;
@@ -130,14 +129,13 @@ public class TestLruBlockCache {
     }
 
     // Re-add same blocks and ensure nothing has changed
+    long expectedBlockCount = cache.getBlockCount();
     for (CachedItem block : blocks) {
-      try {
-        cache.cacheBlock(block.cacheKey, block);
-        assertTrue("Cache should not allow re-caching a block", false);
-      } catch(RuntimeException re) {
-        // expected
-      }
+      cache.cacheBlock(block.cacheKey, block);
     }
+    assertEquals(
+            "Cache should ignore cache requests for blocks already in cache",
+            expectedBlockCount, cache.getBlockCount());
 
     // Verify correctly calculated cache heap size
     assertEquals(expectedCacheSize, cache.heapSize());



Mime
View raw message