[core] Improve local KV database lookup performance#8857
Conversation
| File sstFile = newSstFile(); | ||
| SortLookupStoreWriter writer = storeFactory.createWriter(sstFile, null); | ||
| SortLookupStoreWriter writer = | ||
| storeFactory.createWriter(sstFile, bloomFilterBuilderFactory.apply(data.size())); |
There was a problem hiding this comment.
Could we make the Bloom-filter hash consistent with the configured key comparator, or avoid enabling the Bloom filter when comparator equality is not guaranteed to imply byte equality?
LocalKvDb uses the configured comparator for MemTable/SST ordering and lookup, but the Bloom filter hashes the raw serialized key bytes. Thus two keys that compare equal but have different encodings can be found before flush and incorrectly rejected after flush.
This occurs with Paimon's own RowCompactedSerializer comparator: two FLOAT NaNs with different payload bits serialize to different bytes, while the slice comparator returns 0. I reproduced:
rawKeysEqual=false, comparator=0
beforeFlush=value
afterFlush=null
Disabling the Bloom filter restores the expected result. Please add a regression test and either provide a comparator-consistent key hasher/canonical encoding or conservatively disable this optimization where byte equality is not guaranteed.
There was a problem hiding this comment.
Thanks for pointing this out. This is a known limitation: the Bloom filter hashes the raw serialized key bytes, while a custom comparator can define broader equality, notably for different FLOAT/DOUBLE NaN payloads. We are leaving this limitation unchanged in this PR for now and will address comparator-consistent hashing or canonicalization separately.
| targetLevel); | ||
| } | ||
|
|
||
| checkArgument( |
There was a problem hiding this comment.
Could we clean up all SST files created by bulkLoad when validation or iteration fails?
This validation can run after one or more output files have already been closed. For example, bulk-loading three entries with numEntries=4 throws as expected, but leaves all three SST files in the directory while the database still reports zero level files:
sstFilesAfterFailure=3
dbLevelFilesAfterFailure=0
The same leak can occur when the iterator or an ordering check fails after earlier files have been finalized. Please delete currentSstFile and every file recorded in bulkLoadFiles before propagating the exception, and extend the failure tests to verify directory cleanup.
leaves12138
left a comment
There was a problem hiding this comment.
I found two issues that should be addressed before merging. The first can cause false-negative lookups after a MemTable is flushed; the second leaves orphan SST files after a failed bulk load. Please see the inline comments for reproductions and suggested fixes.
What changed
LocalKvDblocal-kv-db.block-sizeCacheManager-backed caches and remove the Guava cache implementationWhy
Random point lookups in a local KV database larger than its cache were dominated by block read amplification and cache lookup overhead. Missing-key lookups also visited SST files unnecessarily when no Bloom filter was available.
Impact
With 3 million records, 1 million operations, a 128 MB cache, 4 KB blocks, LZ4, and Bloom-filter FPP 0.1:
The mixed workload still shows periodic compaction latency, so its average throughput remains roughly level with RocksDB.
Validation
LocalKvDbTest: 51 tests passedCacheManagerTestandFileBasedBloomFilterTest: 2 tests passed