From 0fbe2eae2274cbaa12560f227eed0b2357859424 Mon Sep 17 00:00:00 2001 From: Niklas Fiekas Date: Wed, 5 Apr 2023 13:17:49 +0200 Subject: [PATCH] Block cache creation failure is not recoverable (#763) --- src/db_options.rs | 55 ++++++++++------------------------- src/perf.rs | 2 +- tests/test_db.rs | 4 +-- tests/test_rocksdb_options.rs | 2 +- 4 files changed, 20 insertions(+), 43 deletions(-) diff --git a/src/db_options.rs b/src/db_options.rs index fe426bc..1e8d4cc 100644 --- a/src/db_options.rs +++ b/src/db_options.rs @@ -14,7 +14,7 @@ use std::ffi::CStr; use std::path::Path; -use std::ptr::null_mut; +use std::ptr::{null_mut, NonNull}; use std::slice; use std::sync::Arc; @@ -35,18 +35,14 @@ use crate::{ ColumnFamilyDescriptor, Error, SnapshotWithThreadMode, }; -fn new_cache(capacity: size_t) -> *mut ffi::rocksdb_cache_t { - unsafe { ffi::rocksdb_cache_create_lru(capacity) } -} - pub(crate) struct CacheWrapper { - pub(crate) inner: *mut ffi::rocksdb_cache_t, + pub(crate) inner: NonNull, } impl Drop for CacheWrapper { fn drop(&mut self) { unsafe { - ffi::rocksdb_cache_destroy(self.inner); + ffi::rocksdb_cache_destroy(self.inner.as_ptr()); } } } @@ -55,30 +51,26 @@ impl Drop for CacheWrapper { pub struct Cache(pub(crate) Arc); impl Cache { - /// Create a lru cache with capacity - pub fn new_lru_cache(capacity: size_t) -> Result { - let cache = new_cache(capacity); - if cache.is_null() { - Err(Error::new("Could not create Cache".to_owned())) - } else { - Ok(Cache(Arc::new(CacheWrapper { inner: cache }))) - } + /// Creates an LRU cache with capacity in bytes. + pub fn new_lru_cache(capacity: size_t) -> Cache { + let inner = NonNull::new(unsafe { ffi::rocksdb_cache_create_lru(capacity) }).unwrap(); + Cache(Arc::new(CacheWrapper { inner })) } - /// Returns the Cache memory usage + /// Returns the cache memory usage in bytes. pub fn get_usage(&self) -> usize { - unsafe { ffi::rocksdb_cache_get_usage(self.0.inner) } + unsafe { ffi::rocksdb_cache_get_usage(self.0.inner.as_ptr()) } } - /// Returns pinned memory usage + /// Returns the pinned memory usage in bytes. pub fn get_pinned_usage(&self) -> usize { - unsafe { ffi::rocksdb_cache_get_pinned_usage(self.0.inner) } + unsafe { ffi::rocksdb_cache_get_pinned_usage(self.0.inner.as_ptr()) } } - /// Sets cache capacity + /// Sets cache capacity in bytes. pub fn set_capacity(&mut self, capacity: size_t) { unsafe { - ffi::rocksdb_cache_set_capacity(self.0.inner, capacity); + ffi::rocksdb_cache_set_capacity(self.0.inner.as_ptr(), capacity); } } } @@ -376,21 +368,6 @@ impl BlockBasedOptions { } } - /// When provided: use the specified cache for blocks. - /// Otherwise rocksdb will automatically create and use an 8MB internal cache. - #[deprecated( - since = "0.15.0", - note = "This function will be removed in next release. Use set_block_cache instead" - )] - pub fn set_lru_cache(&mut self, size: size_t) { - let cache = new_cache(size); - unsafe { - // Since cache is wrapped in shared_ptr, we don't need to - // call rocksdb_cache_destroy explicitly. - ffi::rocksdb_block_based_options_set_block_cache(self.inner, cache); - } - } - /// Sets global cache for blocks (user data is stored in a set of blocks, and /// a block is the unit of reading from disk). Cache must outlive DB instance which uses it. /// @@ -398,7 +375,7 @@ impl BlockBasedOptions { /// 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); + ffi::rocksdb_block_based_options_set_block_cache(self.inner, cache.0.inner.as_ptr()); } self.outlive.block_cache = Some(cache.clone()); } @@ -750,7 +727,7 @@ impl Options { path.as_ptr(), env.0.inner, ignore_unknown_options, - cache.0.inner, + cache.0.inner.as_ptr(), &mut db_options, &mut num_column_families, &mut column_family_names, @@ -2828,7 +2805,7 @@ impl Options { /// 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); + ffi::rocksdb_options_set_row_cache(self.inner, cache.0.inner.as_ptr()); } self.outlive.row_cache = Some(cache.clone()); } diff --git a/src/perf.rs b/src/perf.rs index 29eb68b..6fa7ac0 100644 --- a/src/perf.rs +++ b/src/perf.rs @@ -249,7 +249,7 @@ impl MemoryUsageBuilder { /// Add a cache to collect memory usage from it and add up in total stats fn add_cache(&mut self, cache: &Cache) { unsafe { - ffi::rocksdb_memory_consumers_add_cache(self.inner, cache.0.inner); + ffi::rocksdb_memory_consumers_add_cache(self.inner, cache.0.inner.as_ptr()); } } diff --git a/tests/test_db.rs b/tests/test_db.rs index 1ac85b8..1a91ebd 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -816,7 +816,7 @@ fn get_with_cache_and_bulkload_test() { { // set block based table and cache - let cache = Cache::new_lru_cache(512 << 10).unwrap(); + let cache = Cache::new_lru_cache(512 << 10); assert_eq!(cache.get_usage(), 0); let mut block_based_opts = BlockBasedOptions::default(); block_based_opts.set_block_cache(&cache); @@ -951,7 +951,7 @@ fn get_with_cache_and_bulkload_and_blobs_test() { { // set block based table and cache - let cache = Cache::new_lru_cache(512 << 10).unwrap(); + let cache = Cache::new_lru_cache(512 << 10); assert_eq!(cache.get_usage(), 0); let mut block_based_opts = BlockBasedOptions::default(); block_based_opts.set_block_cache(&cache); diff --git a/tests/test_rocksdb_options.rs b/tests/test_rocksdb_options.rs index e992d8b..7810c80 100644 --- a/tests/test_rocksdb_options.rs +++ b/tests/test_rocksdb_options.rs @@ -34,7 +34,7 @@ fn test_load_latest() { &n, Env::new().unwrap(), true, - Cache::new_lru_cache(1024 * 8).unwrap(), + Cache::new_lru_cache(1024 * 8), ) .unwrap(); assert!(cfs.iter().any(|cf| cf.name() == "default"));