diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java index 5c550c0..c673460 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java @@ -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.ClassSize; 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 HeapSize { 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 HeapSize { @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 HeapSize { return hfileName; } - public DataBlockEncoding getDataBlockEncoding() { - return encoding; - } - public long getOffset() { return offset; } diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java index d63adbd..d43aed1 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java @@ -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; } diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java index b3d2c32..85f6da8 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java @@ -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 { diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java index 2f357f8..e8c17a1 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java @@ -218,8 +218,7 @@ public class HFileReaderV1 extends AbstractHFileReader { 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 AbstractHFileReader { 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 AbstractHFileReader { @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(); } diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java index 8c87836..0361864 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java @@ -221,23 +221,19 @@ public class HFileReaderV2 extends AbstractHFileReader { 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 AbstractHFileReader { } /** - * 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 AbstractHFileReader { * @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 AbstractHFileReader { + 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 AbstractHFileReader { 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 AbstractHFileReader { } 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 AbstractHFileReader { 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 AbstractHFileReader { "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 AbstractHFileReader { 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 AbstractHFileReader { LinkedBlockingQueue preloadAttempted; // This queue is to keep blocks that should be evicted LinkedBlockingQueue 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 AbstractHFileReader { } 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 AbstractHFileReader { 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 AbstractHFileReader { 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 AbstractHFileReader { // 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 AbstractHFileReader { LOG.info("Preloader metrics " + metrics); } } + + private DataBlockEncoding getEffectiveDataBlockEncoding() { + return hfileReaderV2.getEffectiveEncodingInCache(isCompaction); + } } /** @@ -854,7 +892,7 @@ public class HFileReaderV2 extends AbstractHFileReader { 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 AbstractHFileReader { 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 AbstractHFileReader { 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 AbstractHFileReader { 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 AbstractHFileReader { 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 AbstractHFileReader { 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 AbstractHFileReader { 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 AbstractHFileReader { } 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 AbstractHFileReader { } 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 AbstractHFileReader { } 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); } diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java index e9fb2f1..048f06c 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java @@ -140,8 +140,7 @@ public class HFileWriterV1 extends AbstractHFileWriter { 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++; diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java index 0b6b72a..4989b82 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java @@ -231,9 +231,8 @@ public class HFileWriterV2 extends AbstractHFileWriter { 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); } /** diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java index 9a24cbf..56edae6 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java @@ -292,8 +292,6 @@ 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. * @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 BlockCache, HeapSize { 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 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, 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 BlockCache, HeapSize { public Map getEncodingCountsForTest() { Map counts = new EnumMap(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); } diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java index 82d63df..6d5189f 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java @@ -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); } } diff --git a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilter.java b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilter.java index 047fcfc..dc5a168 100644 --- a/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilter.java +++ b/VENDOR.hbase/hbase-trunk/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilter.java @@ -97,7 +97,7 @@ public class CompoundBloomFilter extends CompoundBloomFilterBase // 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); diff --git a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/TestHeapSize.java b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/TestHeapSize.java index 1a78402..218aa51 100644 --- a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/TestHeapSize.java +++ b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/TestHeapSize.java @@ -294,10 +294,13 @@ public class TestHeapSize extends TestCase { } // 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 TestCase { 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); diff --git a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java index 78077cd..de4a0ee 100644 --- a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java +++ b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java @@ -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); + } } diff --git a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java index 251a086..1636f26 100644 --- a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java +++ b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java @@ -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) { diff --git a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java index 5941b36..b7914ce 100644 --- a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java +++ b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java @@ -47,6 +47,7 @@ import org.apache.hadoop.hbase.HBaseTestingUtility; 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; diff --git a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestL2BucketCache.java b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestL2BucketCache.java index 307cc37..d48d40c 100644 --- a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestL2BucketCache.java +++ b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestL2BucketCache.java @@ -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(), diff --git a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java index 659500c..bfe21dc 100644 --- a/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java +++ b/VENDOR.hbase/hbase-trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java @@ -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());