dlg99 commented on a change in pull request #2822:
URL: https://github.com/apache/bookkeeper/pull/2822#discussion_r728345728
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java
##########
@@ -149,8 +149,12 @@ public KeyValueStorageRocksDB(String basePath, String
subPath, DbConfigType dbCo
tableOptions.setFilterPolicy(new
BloomFilter(bloomFilterBitsPerKey, false));
}
- // Options best suited for HDDs
+ // Cache index & filter in block cache to limit the memory
usage
tableOptions.setCacheIndexAndFilterBlocks(true);
+ // Options below are tune to mitigate the performance
regression when limiting the memory usage
+
tableOptions.setCacheIndexAndFilterBlocksWithHighPriority(true);
Review 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
