Correct the comment of caching index & filter block with additional options to improve performance #2822
Conversation
…ptions to improve performance More internal details could refer to https://github.com/facebook/rocksdb/wiki/Block-Cache#caching-index-filter-and-compression-dictionary-blocks
| tableOptions.setCacheIndexAndFilterBlocks(true); | ||
| // Options below are tune to mitigate the performance regression when limiting the memory usage | ||
| tableOptions.setCacheIndexAndFilterBlocksWithHighPriority(true); |
There was a problem hiding this comment.
TBH I do not have enough information to make a call whether this is a good change or not:
- I don't have personal experience tuning this
- there is no link to a doc/KB article recommending these options to be used together
- no linked perf tests proving the change will improve something.
I can't make a call if the change that makes sense e.g. in case of SSD storage will work as well in case of HDD, and so on.
I think ideally we should move RocksDB options into config and provide reasonable defaults.
something like rocksdb.table.cacheIndexAndFilterBlocksWithHighPriority etc, to apply rocksdb.* settings without further code changes. there are also hardcoded configs in lines 134-139
There was a problem hiding this comment.
I think you could refer to rocksdb-wiki-cache-blocks to get some background information of setCacheIndexAndFilterBlocks. If this option is set as false, table reader would always keep the index & filter block in memory instead of limiting them in a LRU cache.
You could also refer to rocksdb-wiki-cache-blocks to see the suggestion to combine these options together.
hangc0276
requested review from
hangc0276,
eolivelli,
nicoloboschi,
hezhangjian and
zymap
|
fix old workflow,please see #3455 for detail |
Descriptions of the changes in this PR:
Motivation
The previous comment of
tableOptions.setCacheIndexAndFilterBlocks(true)isOptions best suited for HDDs. If the comment thought that we could cache index&filter blocks when using HDD for better performance, however, this is not correct as this option actually helps on limit the memory usage of index & filter blocks. More internal details could refer to https://github.com/facebook/rocksdb/wiki/Block-Cache#caching-index-filter-and-compression-dictionary-blocksApart from correcting the comment, this PR also add additional options to help improve performance.
Changes
Correct the comment of caching index & filter block with additional options to improve performance.