fix(db): apply rocksdb table config #39
Open
bladehan1 wants to merge 1 commit into
Open
Conversation
…last setTableFormatConfig() materializes the native table factory at call time, so the BlockBasedTableConfig setters that followed it were silently dropped: block size, shared block cache, cache_index_and_filter_blocks, L0 pinning and the bloom filter all fell back to RocksDB defaults. Move the setTableFormatConfig() call after all tableCfg setters so they take effect. Verified on a fresh ROCKSDB private chain: OPTIONS now report block_size=16384, cache_index_and_filter_blocks=true, pin_l0=true and a BuiltinBloomFilter.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
What does this PR do?
Fixes a RocksDB option-ordering bug in
RocksDbSettings.getOptionsByDbName()so that theBlockBasedTableConfig(table-level) settings actually take effect.Options.setTableFormatConfig()materializes the native table factory at call time. It was being called before thetableCfg.setXxx()calls, so every table-level setting was silently dropped and RocksDB fell back to defaults:block_sizeblock_cachecache_index_and_filter_blockspin_l0_filter_and_index_blocks_in_cachefilter_policyThe fix moves
options.setTableFormatConfig(tableCfg)to be the last call, after alltableCfgsetters.Options-level settings (numLevels,maxOpenFiles, comparator, etc.) were unaffected and continue to work.Why are these changes required?
The intended table tuning never reached the native layer, so on real nodes RocksDB ran with near-worst-case defaults: no bloom filter (slow point lookups), 4 KB blocks (larger index), the shared 1 GB block cache was dead code (each DB used its own 8 MB default), and index/filter blocks were held by table readers instead of the cache. This degrades read performance, especially on large fullnode datasets.
This PR has been tested by:
ROCKSDB-engine private chain and confirmed via each DB'sOPTIONS-*file that the effective options now match the intended config:block_size=16384,cache_index_and_filter_blocks=true,pin_l0_filter_and_index_blocks_in_cache=true,filter_policy=rocksdb.BuiltinBloomFilter,no_block_cache=false. Node produced blocks normally.Follow up
estimate-table-readers-memdrops andblock-cache-usagerises after this fix; watch native RSS (≈ +650 MB as the shared cache fills).Extra details
Compatibility:
block_sizeand bloom filter are SST write-time properties stored per file, read back per each SST's own metadata, so existing data needs no migration — old and new SSTs coexist and the new format is adopted gradually via compaction.Summary by cubic
Fixes an option ordering bug in
RocksDbSettings.getOptionsByDbName()soBlockBasedTableConfigsettings are applied by callingoptions.setTableFormatConfig(tableCfg)last. This enables the intended block size, shared block cache, and bloom filter to take effect, improving read performance.Options.setTableFormatConfig()after alltableCfgsetters so table options are honored.OPTIONS-*:block_size=16384, shared cache used,cache_index_and_filter_blocks=true, L0 pinning and bloom filter enabled; no migration needed.Written for commit 1f16253. Summary will update on new commits.