[common] Always accept the first entry in bitmap index block packing - #9614
Open
LuciferYang wants to merge 1 commit into
Open
[common] Always accept the first entry in bitmap index block packing#9614LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
BitmapFileIndexMetaV2's block packing rejected an entry whenever it pushed the block past index-block-size, including when the block was still empty. A single dictionary key larger than the block size (default 16kb, e.g. one long string value) therefore made the whole bitmap file index serialization fail with "index fail" instead of producing a usable index. Treat the limit as the packing target it is: an empty block always admits its first entry, so an oversized key gets its own block and serialization proceeds; subsequent entries spill to new blocks as before. Reading makes no assumption about block sizes.
Contributor
Author
|
The red I cannot re-run the job myself; a re-run of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
close #9613
BitmapFileIndexMetaV2.BitmapIndexBlock.tryAddrefused an entry whenever it pushed the block pastindex-block-size, without asking whether the block already held anything.serializeanswers a refusal by opening one new block and retrying once, and that block is empty too, so the same entry is refused again and the write ends atthrow new RuntimeException("index fail"). A single dictionary key larger than the limit therefore fails the whole data file, not just its index.The threshold is
index-block-sizeminus 16 bytes: a block starts atInteger.BYTESfor its entry count, and a STRING entry costs2 * Integer.BYTESplus theInteger.BYTES + sizeInBytesthatgetSerializeSizeMeasurereports. With the default 16kb that means one 16369-byte value, and a single row carrying it is enough. Fixed-width types cannot get close to the limit, so this is a variable-length column problem.An empty block now takes its first entry unconditionally, so an oversized key gets a block to itself and the entries after it spill over as before. That treats the limit as the packing target it is, which is also how the two sibling implementations read it:
BitmapGlobalIndexFormat.writeonly enforces its dictionary block size once the blockhasEntries(), andChunkedDictionary.appendpasses each chunk's first key through the chunk constructor rather thantryAdd.Nothing on the read side depends on a block fitting inside the limit. Block offsets accumulate from each block's real
serializedBytes, the entry count written ahead of a block bounds it, and the block keys stay sorted, sofindBlock's binary search still lands on the right block. Inputs that used to serialize produce identical bytes, since an entry an empty block rejected could not produce a file at all before.One thing I left alone: now that an empty block never refuses, the
index failthrow inserializeis unreachable. Removing it means restructuring that loop, which felt like more than this fix should carry, but I'm glad to fold it in if you would rather see it go.Tests
BitmapFileIndexTest.testV2EntryLargerThanBlockSizebuilds an index over a 20000-character value, a null and two short values at the default 16kb block size, then reads all four back through the reader. The two short keys share the first block, so the size check runs and admits the second one; the long key is rejected there and then accepted as the first entry of the next block. Both sides of the new condition run within one index.Against the unfixed writer the test errors with
RuntimeException: index failout ofBitmapFileIndex$Writer.serializedBytes.mvn -pl paimon-common -Dtest=BitmapFileIndexTest teston JDK 8: 7 tests, 0 failures.spotless:checkandcheckstyle:checkon paimon-common are clean.