Block cache creation failure is not recoverable (#763)

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<ffi::rocksdb_cache_t>,
 }
 
 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<CacheWrapper>);
 
 impl Cache {
-    /// Create a lru cache with capacity
+    /// Creates an LRU cache with capacity in bytes.
-    pub fn new_lru_cache(capacity: size_t) -> Result<Cache, Error> {
+    pub fn new_lru_cache(capacity: size_t) -> Cache {
-        let cache = new_cache(capacity);
+        let inner = NonNull::new(unsafe { ffi::rocksdb_cache_create_lru(capacity) }).unwrap();
-        if cache.is_null() {
+        Cache(Arc::new(CacheWrapper { inner }))
-            Err(Error::new("Could not create Cache".to_owned()))
-        } else {
-            Ok(Cache(Arc::new(CacheWrapper { inner: cache })))
-        }
     }
 
-    /// 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());
     }

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());
         }
     }
 

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);

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"));