Skip to content

[common] Always accept the first entry in bitmap index block packing - #9614

Open
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:fix/bitmap-index-oversized-entry
Open

[common] Always accept the first entry in bitmap index block packing#9614
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:fix/bitmap-index-oversized-entry

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

Purpose

close #9613

BitmapFileIndexMetaV2.BitmapIndexBlock.tryAdd refused an entry whenever it pushed the block past index-block-size, without asking whether the block already held anything. serialize answers 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 at throw 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-size minus 16 bytes: a block starts at Integer.BYTES for its entry count, and a STRING entry costs 2 * Integer.BYTES plus the Integer.BYTES + sizeInBytes that getSerializeSizeMeasure reports. 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.write only enforces its dictionary block size once the block hasEntries(), and ChunkedDictionary.append passes each chunk's first key through the chunk constructor rather than tryAdd.

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, so findBlock'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 fail throw in serialize is 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.testV2EntryLargerThanBlockSize builds 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 fail out of BitmapFileIndex$Writer.serializedBytes.

mvn -pl paimon-common -Dtest=BitmapFileIndexTest test on JDK 8: 7 tests, 0 failures. spotless:check and checkstyle:check on paimon-common are clean.

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.
@LuciferYang

Copy link
Copy Markdown
Contributor Author

The red build_test here is an unrelated flake: AppendTableSavepointTagFailoverITCase.testRegionFailoverPreservesSavepointTag aborts its second savepoint with "Not all required tasks are currently running" after the injected region failover. This PR touches only paimon-common, no Flink or checkpointing code, and the same test failed the same way on another unrelated PR in the same hour. Filed as #9653 with both runs.

I cannot re-run the job myself; a re-run of build_test should be green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Bitmap file index write fails with "index fail" when one value exceeds index-block-size

1 participant