-
Notifications
You must be signed in to change notification settings - Fork 704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose LRU and blob cache options #837
base: master
Are you sure you want to change the base?
Conversation
d0c3172
to
3a87972
Compare
Added one more commit to also expose the advanced LRU cache constructor with custom options. |
@athre0z overall looks good, but could you add some tests, please? |
This also LGTM, adding tests would be great |
I can write some tests, but it might take a while until I get to it depending on how deep these tests should go: I'm unfortunately rather short on time right now. Simply adding tests that invoke the new methods to make sure it links & doesn't segfault is quickly done, but I might have to read up a bit deeper if you'd like tests that actually make sure that RocksDB is caching things as expected etc (like some other test cases seem to do). What's the expectation here? |
a simple test case where you set the new options being exposed here, and put some kvs without segfaulting is good enough |
This PR exposes the following two DB options related to BlobDB caching:
https://github.com/facebook/rocksdb/blob/f6fd4b9dbd15dba36f7e5ad23de407b5c26b1460/include/rocksdb/advanced_options.h#L1129-L1148