Skip to content
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

Remove wrong outlive requirements for Cache in docs #812

Merged

Conversation

zheland
Copy link
Contributor

@zheland zheland commented Aug 11, 2023

Phrase Cache must outlive DB instance which uses it. was added to docs of BlockBasedOptions::set_block_cache and Options::set_row_cache in #446.

But this requirement for Cache methods argument should no longer be valid after #497 was merged because it:

Wrap Cache and Env in an Rc and keep a copy of that in the database to ensure that they are not dropped before the database is.

I suggest to remove these outdated requirements from docs.

/// a block is the unit of reading from disk). Cache must outlive DB instance which uses it.
///
/// If set, use the specified cache for blocks.
/// By default, rocksdb will automatically create and use an 8MB internal cache.
pub fn set_block_cache(&mut self, cache: &Cache) {
unsafe {
ffi::rocksdb_block_based_options_set_block_cache(self.inner, cache.0.inner.as_ptr());
}
self.outlive.block_cache = Some(cache.clone());
}

rust-rocksdb/src/db_options.rs

Lines 2823 to 2832 in b993275

/// Sets global cache for table-level rows. Cache must outlive DB instance which uses it.
///
/// Default: null (disabled)
/// Not supported in ROCKSDB_LITE mode!
pub fn set_row_cache(&mut self, cache: &Cache) {
unsafe {
ffi::rocksdb_options_set_row_cache(self.inner, cache.0.inner.as_ptr());
}
self.outlive.row_cache = Some(cache.clone());
}

#[derive(Clone)]
pub struct Cache(pub(crate) Arc<CacheWrapper>);

pub(crate) struct CacheWrapper {
pub(crate) inner: NonNull<ffi::rocksdb_cache_t>,
}
impl Drop for CacheWrapper {
fn drop(&mut self) {
unsafe {
ffi::rocksdb_cache_destroy(self.inner.as_ptr());
}
}
}

@aleksuss aleksuss merged commit 0690527 into rust-rocksdb:master Sep 19, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants