diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java index 222e146..96395cd 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java @@ -77,6 +77,13 @@ public class HFileBlockIndex { public static final String MAX_CHUNK_SIZE_KEY = "hfile.index.block.max.size"; /** + * The maximum level of the multi-level block index + */ + public static final String MAX_INDEX_LEVEL_KEY = "hfile.index.block.max.level"; + + static final int DEFAULT_MAX_INDEX_LEVEL = 16; + + /** * The number of bytes stored in each "secondary index" entry in addition to * key bytes in the non-root index block format. The first long is the file * offset of the deeper-level block the entry points to, and the int that @@ -569,7 +576,7 @@ public class HFileBlockIndex { * Return the BlockWithScanInfo which contains the DataBlock with other scan * info such as nextIndexedKey. This function will only be called when the * HFile version is larger than 1. - * + * * @param key * the key we are looking for * @param currentBlock @@ -701,7 +708,7 @@ public class HFileBlockIndex { * Performs a binary search over a non-root level index block. Utilizes the * secondary index, which records the offsets of (offset, onDiskSize, * firstKey) tuples of all entries. - * + * * @param key * the key we are searching for offsets to individual entries in * the blockIndex buffer @@ -985,6 +992,9 @@ public class HFileBlockIndex { /** The maximum size guideline of all multi-level index blocks. */ private int maxChunkSize; + /** The maximum level of multi-level index blocks */ + private int maxIndexLevel; + /** Whether we require this block index to always be single-level. */ private boolean singleLevelOnly; @@ -1017,6 +1027,7 @@ public class HFileBlockIndex { this.cacheConf = cacheConf; this.nameForCaching = nameForCaching; this.maxChunkSize = HFileBlockIndex.DEFAULT_MAX_CHUNK_SIZE; + this.maxIndexLevel = HFileBlockIndex.DEFAULT_MAX_INDEX_LEVEL; } public void setMaxChunkSize(int maxChunkSize) { @@ -1026,6 +1037,13 @@ public class HFileBlockIndex { this.maxChunkSize = maxChunkSize; } + public void setMaxIndexLevel(int maxIndexLevel) { + if (maxIndexLevel <= 0) { + throw new IllegalArgumentException("Invald maximum index level"); + } + this.maxIndexLevel = maxIndexLevel; + } + /** * Writes the root level and intermediate levels of the block index into * the output stream, generating the tree from bottom up. Assumes that the @@ -1057,7 +1075,10 @@ public class HFileBlockIndex { : null; if (curInlineChunk != null) { - while (rootChunk.getRootSize() > maxChunkSize) { + while (rootChunk.getRootSize() > maxChunkSize + && rootChunk.getNumEntries() > 1 // HBASE-16288: if firstKey is larger than maxChunkSize + // we will loop indefinitely otherwise. + && numLevels < maxIndexLevel) { // Sanity check rootChunk = writeIntermediateLevel(out, rootChunk); numLevels += 1; } @@ -1153,8 +1174,11 @@ public class HFileBlockIndex { curChunk.add(currentLevel.getBlockKey(i), currentLevel.getBlockOffset(i), currentLevel.getOnDiskDataSize(i)); - if (curChunk.getRootSize() >= maxChunkSize) + // HBASE-16288: We have to have at least 2 items in the index. Otherwise, this means that + // the first key is larger than maxChunkSize, and will result in infinite recursion. + if (i > 1 && curChunk.getRootSize() >= maxChunkSize) { writeIntermediateBlock(out, parent, curChunk); + } } if (curChunk.getNumEntries() > 0) { @@ -1647,4 +1671,8 @@ public class HFileBlockIndex { public static int getMaxChunkSize(Configuration conf) { return conf.getInt(MAX_CHUNK_SIZE_KEY, DEFAULT_MAX_CHUNK_SIZE); } + + public static int getMaxIndexLevel(Configuration conf) { + return conf.getInt(MAX_INDEX_LEVEL_KEY, DEFAULT_MAX_INDEX_LEVEL); + } } diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java index 5769744..5cdd0f7 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java @@ -287,6 +287,8 @@ public class HFileWriterImpl implements HFile.Writer { cacheIndexesOnWrite ? name : null); dataBlockIndexWriter.setMaxChunkSize( HFileBlockIndex.getMaxChunkSize(conf)); + dataBlockIndexWriter.setMaxIndexLevel( + HFileBlockIndex.getMaxIndexLevel(conf)); inlineBlockWriters.add(dataBlockIndexWriter); // Meta data block index writer @@ -764,4 +766,4 @@ public class HFileWriterImpl implements HFile.Writer { outputStream = null; } } -} \ No newline at end of file +} diff --git hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java index 470d483..784b8be 100644 --- hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java +++ hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java @@ -671,6 +671,48 @@ public class TestHFileBlockIndex { valueRead); } + @Test(timeout=10000) + public void testIntermediateLevelIndicesWithLargeKeys() throws IOException { + testIntermediateLevelIndicesWithLargeKeys(16); + } + + @Test(timeout=10000) + public void testIntermediateLevelIndicesWithLargeKeysWithMaxLevel() throws IOException { + // because of the large rowKeys, we will end up with a 50-level block index + // we should still test the non-sanity code path + testIntermediateLevelIndicesWithLargeKeys(Integer.MAX_VALUE); + } + + public void testIntermediateLevelIndicesWithLargeKeys(int maxLevel) throws IOException { + Path hfPath = new Path(TEST_UTIL.getDataTestDir(), + "testIntermediateLevelIndicesWithLargeKeys.hfile"); + int maxChunkSize = 1024; + FileSystem fs = FileSystem.get(conf); + CacheConfig cacheConf = new CacheConfig(conf); + conf.setInt(HFileBlockIndex.MAX_CHUNK_SIZE_KEY, maxChunkSize); + HFileContext context = new HFileContextBuilder().withBlockSize(16).build(); + HFile.Writer hfw = new HFile.WriterFactory(conf, cacheConf) + .withFileContext(context) + .withPath(fs, hfPath).create(); + List keys = new ArrayList(); + + // This should result in leaf-level indices and a root level index + for (int i=0; i < 100; i++) { + byte[] rowkey = new byte[maxChunkSize + 1]; + byte[] b = Bytes.toBytes(i); + System.arraycopy(b, 0, rowkey, rowkey.length - b.length, b.length); + keys.add(rowkey); + hfw.append(CellUtil.createCell(rowkey)); + } + hfw.close(); + HFile.Reader reader = HFile.createReader(fs, hfPath, cacheConf, conf); + // Scanner doesn't do Cells yet. Fix. + HFileScanner scanner = reader.getScanner(true, true); + for (int i = 0; i < keys.size(); ++i) { + scanner.seekTo(CellUtil.createCell(keys.get(i))); + } + reader.close(); + } }