From f6eafe1fffbe48401c04abb75ef1a9307234a7d0 Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Thu, 28 Jan 2021 13:46:16 -0800 Subject: [PATCH 01/30] Use RwLock --- src/column_family.rs | 1 + src/db.rs | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/column_family.rs b/src/column_family.rs index 2c1b50531..c581a3b18 100644 --- a/src/column_family.rs +++ b/src/column_family.rs @@ -43,6 +43,7 @@ impl ColumnFamilyDescriptor { /// An opaque type used to represent a column family. Returned from some functions, and used /// in others +#[derive(Clone)] pub struct ColumnFamily { pub(crate) inner: *mut ffi::rocksdb_column_family_handle_t, } diff --git a/src/db.rs b/src/db.rs index e49fe366e..01f0df115 100644 --- a/src/db.rs +++ b/src/db.rs @@ -32,6 +32,7 @@ use std::path::PathBuf; use std::ptr; use std::slice; use std::str; +use std::sync::RwLock; use std::time::Duration; /// A RocksDB database. @@ -39,7 +40,7 @@ use std::time::Duration; /// See crate level documentation for a simple usage example. pub struct DB { pub(crate) inner: *mut ffi::rocksdb_t, - cfs: BTreeMap, + cfs: RwLock>, path: PathBuf, } @@ -105,7 +106,7 @@ impl DB { Ok(DB { inner: db, - cfs: BTreeMap::new(), + cfs: RwLock::new(BTreeMap::new()), path: path.as_ref().to_path_buf(), }) } @@ -268,7 +269,7 @@ impl DB { Ok(DB { inner: db, - cfs: cf_map, + cfs: RwLock::new(cf_map), path: path.as_ref().to_path_buf(), }) } @@ -676,13 +677,15 @@ impl DB { )); self.cfs + .write() + .unwrap() .insert(name.as_ref().to_string(), ColumnFamily { inner }); }; Ok(()) } - pub fn drop_cf(&mut self, name: &str) -> Result<(), Error> { - if let Some(cf) = self.cfs.remove(name) { + pub fn drop_cf(&self, name: &str) -> Result<(), Error> { + if let Some(cf) = self.cfs.write().unwrap().remove(name) { unsafe { ffi_try!(ffi::rocksdb_drop_column_family(self.inner, cf.inner)); } @@ -693,8 +696,8 @@ impl DB { } /// Return the underlying column family handle. - pub fn cf_handle(&self, name: &str) -> Option<&ColumnFamily> { - self.cfs.get(name) + pub fn cf_handle(&self, name: &str) -> Option { + self.cfs.read().unwrap().get(name).cloned() } pub fn iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b> { @@ -1476,7 +1479,7 @@ impl DB { impl Drop for DB { fn drop(&mut self) { unsafe { - for cf in self.cfs.values() { + for cf in self.cfs.read().unwrap().values() { ffi::rocksdb_column_family_handle_destroy(cf.inner); } ffi::rocksdb_close(self.inner); From 123d613e8220562a7e5efd3b4215eb5e240be7ca Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Thu, 11 Feb 2021 16:59:32 -0700 Subject: [PATCH 02/30] Update create_cf --- src/db.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/db.rs b/src/db.rs index 01f0df115..b1a18f9c9 100644 --- a/src/db.rs +++ b/src/db.rs @@ -661,7 +661,7 @@ impl DB { Ok(convert_values(values, values_sizes)) } - pub fn create_cf>(&mut self, name: N, opts: &Options) -> Result<(), Error> { + pub fn create_cf>(&self, name: N, opts: &Options) -> Result<(), Error> { let cf_name = if let Ok(c) = CString::new(name.as_ref().as_bytes()) { c } else { @@ -1429,7 +1429,7 @@ impl DB { /// entirely in the range. /// /// Note: L0 files are left regardless of whether they're in the range. - /// + /// /// Snapshots before the delete might not see the data in the given range. pub fn delete_file_in_range>(&self, from: K, to: K) -> Result<(), Error> { let from = from.as_ref(); From 0ead89200022ce848be980df4158429841bfcb78 Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Thu, 11 Feb 2021 20:22:46 -0800 Subject: [PATCH 03/30] Test fixup --- tests/test_column_family.rs | 26 ++++++------- tests/test_db.rs | 74 ++++++++++++++++++------------------- tests/test_property.rs | 8 ++-- 3 files changed, 54 insertions(+), 54 deletions(-) diff --git a/tests/test_column_family.rs b/tests/test_column_family.rs index 40db91066..1ee182be4 100644 --- a/tests/test_column_family.rs +++ b/tests/test_column_family.rs @@ -28,7 +28,7 @@ fn test_column_family() { let mut opts = Options::default(); opts.create_if_missing(true); opts.set_merge_operator_associative("test operator", test_provided_merge); - let mut db = DB::open(&opts, &n).unwrap(); + let db = DB::open(&opts, &n).unwrap(); let opts = Options::default(); match db.create_cf("cf1", &opts) { Ok(()) => println!("cf1 created successfully"), @@ -78,7 +78,7 @@ fn test_column_family() { {} // should b able to drop a cf { - let mut db = DB::open_cf(&Options::default(), &n, &["cf1"]).unwrap(); + let db = DB::open_cf(&Options::default(), &n, &["cf1"]).unwrap(); match db.drop_cf("cf1") { Ok(_) => println!("cf1 successfully dropped."), Err(e) => panic!("failed to drop column family: {}", e), @@ -95,7 +95,7 @@ fn test_can_open_db_with_results_of_list_cf() { { let mut opts = Options::default(); opts.create_if_missing(true); - let mut db = DB::open(&opts, &n).unwrap(); + let db = DB::open(&opts, &n).unwrap(); let opts = Options::default(); assert!(db.create_cf("cf1", &opts).is_ok()); @@ -143,15 +143,15 @@ fn test_merge_operator() { Err(e) => panic!("failed to open db with column family: {}", e), }; let cf1 = db.cf_handle("cf1").unwrap(); - assert!(db.put_cf(cf1, b"k1", b"v1").is_ok()); - assert_eq!(db.get_cf(cf1, b"k1").unwrap().unwrap(), b"v1"); - let p = db.put_cf(cf1, b"k1", b"a"); + assert!(db.put_cf(&cf1, b"k1", b"v1").is_ok()); + assert_eq!(db.get_cf(&cf1, b"k1").unwrap().unwrap(), b"v1"); + let p = db.put_cf(&cf1, b"k1", b"a"); assert!(p.is_ok()); - db.merge_cf(cf1, b"k1", b"b").unwrap(); - db.merge_cf(cf1, b"k1", b"c").unwrap(); - db.merge_cf(cf1, b"k1", b"d").unwrap(); - db.merge_cf(cf1, b"k1", b"efg").unwrap(); - let m = db.merge_cf(cf1, b"k1", b"h"); + db.merge_cf(&cf1, b"k1", b"b").unwrap(); + db.merge_cf(&cf1, b"k1", b"c").unwrap(); + db.merge_cf(&cf1, b"k1", b"d").unwrap(); + db.merge_cf(&cf1, b"k1", b"efg").unwrap(); + let m = db.merge_cf(&cf1, b"k1", b"h"); println!("m is {:?}", m); // TODO assert!(m.is_ok()); match db.get(b"k1") { @@ -163,7 +163,7 @@ fn test_merge_operator() { _ => panic!("value not present!"), } - let _ = db.get_cf(cf1, b"k1"); + let _ = db.get_cf(&cf1, b"k1"); // TODO assert!(r.unwrap().as_ref() == b"abcdefgh"); assert!(db.delete(b"k1").is_ok()); assert!(db.get(b"k1").unwrap().is_none()); @@ -242,7 +242,7 @@ fn test_create_duplicate_column_family() { opts.create_if_missing(true); opts.create_missing_column_families(true); - let mut db = match DB::open_cf(&opts, &n, &["cf1"]) { + let db = match DB::open_cf(&opts, &n, &["cf1"]) { Ok(d) => d, Err(e) => panic!("failed to create new column family: {}", e), }; diff --git a/tests/test_db.rs b/tests/test_db.rs index d9ec85b24..f2f7f5750 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -347,27 +347,27 @@ fn set_option_cf_test() { let cf = db.cf_handle("cf1").unwrap(); // set an option to valid values assert!(db - .set_options_cf(cf, &[("disable_auto_compactions", "true")]) + .set_options_cf(&cf, &[("disable_auto_compactions", "true")]) .is_ok()); assert!(db - .set_options_cf(cf, &[("disable_auto_compactions", "false")]) + .set_options_cf(&cf, &[("disable_auto_compactions", "false")]) .is_ok()); // invalid names/values should result in an error assert!(db - .set_options_cf(cf, &[("disable_auto_compactions", "INVALID_VALUE")]) + .set_options_cf(&cf, &[("disable_auto_compactions", "INVALID_VALUE")]) .is_err()); assert!(db - .set_options_cf(cf, &[("INVALID_NAME", "INVALID_VALUE")]) + .set_options_cf(&cf, &[("INVALID_NAME", "INVALID_VALUE")]) .is_err()); // option names/values must not contain NULLs assert!(db - .set_options_cf(cf, &[("disable_auto_compactions", "true\0")]) + .set_options_cf(&cf, &[("disable_auto_compactions", "true\0")]) .is_err()); assert!(db - .set_options_cf(cf, &[("disable_auto_compactions\0", "true")]) + .set_options_cf(&cf, &[("disable_auto_compactions\0", "true")]) .is_err()); // empty options are not allowed - assert!(db.set_options_cf(cf, &[]).is_err()); + assert!(db.set_options_cf(&cf, &[]).is_err()); // multiple options can be set in a single API call let multiple_options = [ ("paranoid_file_checks", "true"), @@ -556,13 +556,13 @@ fn compact_range_test() { let cfs = vec!["cf1"]; let db = DB::open_cf(&opts, &path, cfs).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(cf1, b"k1", b"v1").unwrap(); - db.put_cf(cf1, b"k2", b"v2").unwrap(); - db.put_cf(cf1, b"k3", b"v3").unwrap(); - db.put_cf(cf1, b"k4", b"v4").unwrap(); - db.put_cf(cf1, b"k5", b"v5").unwrap(); - db.compact_range_cf(cf1, Some(b"k2"), Some(b"k4")); - db.compact_range_cf_opt(cf1, Some(b"k1"), None::<&str>, &compact_opts); + db.put_cf(&cf1, b"k1", b"v1").unwrap(); + db.put_cf(&cf1, b"k2", b"v2").unwrap(); + db.put_cf(&cf1, b"k3", b"v3").unwrap(); + db.put_cf(&cf1, b"k4", b"v4").unwrap(); + db.put_cf(&cf1, b"k5", b"v5").unwrap(); + db.compact_range_cf(&cf1, Some(b"k2"), Some(b"k4")); + db.compact_range_cf_opt(&cf1, Some(b"k1"), None::<&str>, &compact_opts); // put and compact default column family db.put(b"k1", b"v1").unwrap(); @@ -595,12 +595,12 @@ fn fifo_compaction_test() { let cfs = vec!["cf1"]; let db = DB::open_cf(&opts, &path, cfs).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(cf1, b"k1", b"v1").unwrap(); - db.put_cf(cf1, b"k2", b"v2").unwrap(); - db.put_cf(cf1, b"k3", b"v3").unwrap(); - db.put_cf(cf1, b"k4", b"v4").unwrap(); - db.put_cf(cf1, b"k5", b"v5").unwrap(); - db.compact_range_cf(cf1, Some(b"k2"), Some(b"k4")); + db.put_cf(&cf1, b"k1", b"v1").unwrap(); + db.put_cf(&cf1, b"k2", b"v2").unwrap(); + db.put_cf(&cf1, b"k3", b"v3").unwrap(); + db.put_cf(&cf1, b"k4", b"v4").unwrap(); + db.put_cf(&cf1, b"k5", b"v5").unwrap(); + db.compact_range_cf(&cf1, Some(b"k2"), Some(b"k4")); // check stats let ctx = PerfContext::default(); @@ -830,15 +830,15 @@ fn test_open_cf_for_read_only() { opts.create_missing_column_families(true); let db = DB::open_cf(&opts, &path, cfs.clone()).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(cf1, b"k1", b"v1").unwrap(); + db.put_cf(&cf1, b"k1", b"v1").unwrap(); } { let opts = Options::default(); let error_if_log_file_exist = false; let db = DB::open_cf_for_read_only(&opts, &path, cfs, error_if_log_file_exist).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - assert_eq!(db.get_cf(cf1, b"k1").unwrap().unwrap(), b"v1"); - assert!(db.put_cf(cf1, b"k2", b"v2").is_err()); + assert_eq!(db.get_cf(&cf1, b"k1").unwrap().unwrap(), b"v1"); + assert!(db.put_cf(&cf1, b"k2", b"v2").is_err()); } } @@ -854,18 +854,18 @@ fn delete_range_test() { let db = DB::open_cf(&opts, &path, cfs).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(cf1, b"k1", b"v1").unwrap(); - db.put_cf(cf1, b"k2", b"v2").unwrap(); - db.put_cf(cf1, b"k3", b"v3").unwrap(); - db.put_cf(cf1, b"k4", b"v4").unwrap(); - db.put_cf(cf1, b"k5", b"v5").unwrap(); + db.put_cf(&cf1, b"k1", b"v1").unwrap(); + db.put_cf(&cf1, b"k2", b"v2").unwrap(); + db.put_cf(&cf1, b"k3", b"v3").unwrap(); + db.put_cf(&cf1, b"k4", b"v4").unwrap(); + db.put_cf(&cf1, b"k5", b"v5").unwrap(); - db.delete_range_cf(cf1, b"k2", b"k4").unwrap(); - assert_eq!(db.get_cf(cf1, b"k1").unwrap().unwrap(), b"v1"); - assert_eq!(db.get_cf(cf1, b"k4").unwrap().unwrap(), b"v4"); - assert_eq!(db.get_cf(cf1, b"k5").unwrap().unwrap(), b"v5"); - assert!(db.get_cf(cf1, b"k2").unwrap().is_none()); - assert!(db.get_cf(cf1, b"k3").unwrap().is_none()); + db.delete_range_cf(&cf1, b"k2", b"k4").unwrap(); + assert_eq!(db.get_cf(&cf1, b"k1").unwrap().unwrap(), b"v1"); + assert_eq!(db.get_cf(&cf1, b"k4").unwrap().unwrap(), b"v4"); + assert_eq!(db.get_cf(&cf1, b"k5").unwrap().unwrap(), b"v5"); + assert!(db.get_cf(&cf1, b"k2").unwrap().is_none()); + assert!(db.get_cf(&cf1, b"k3").unwrap().is_none()); } } @@ -901,13 +901,13 @@ fn multi_get_cf() { let cf0 = db.cf_handle("cf0").unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(cf1, b"k1", b"v1").unwrap(); + db.put_cf(&cf1, b"k1", b"v1").unwrap(); let cf2 = db.cf_handle("cf2").unwrap(); - db.put_cf(cf2, b"k2", b"v2").unwrap(); + db.put_cf(&cf2, b"k2", b"v2").unwrap(); let values = db - .multi_get_cf(vec![(cf0, b"k0"), (cf1, b"k1"), (cf2, b"k2")]) + .multi_get_cf(vec![(&cf0, b"k0"), (&cf1, b"k1"), (&cf2, b"k2")]) .expect("multi_get failed"); assert_eq!(3, values.len()); assert!(values[0].is_empty()); diff --git a/tests/test_property.rs b/tests/test_property.rs index 2b9d39944..7b3aec811 100644 --- a/tests/test_property.rs +++ b/tests/test_property.rs @@ -35,10 +35,10 @@ fn property_cf_test() { let n = DBPath::new("_rust_rocksdb_property_cf_test"); { let opts = Options::default(); - let mut db = DB::open_default(&n).unwrap(); + let db = DB::open_default(&n).unwrap(); db.create_cf("cf1", &opts).unwrap(); let cf = db.cf_handle("cf1").unwrap(); - let value = db.property_value_cf(cf, "rocksdb.stats").unwrap().unwrap(); + let value = db.property_value_cf(&cf, "rocksdb.stats").unwrap().unwrap(); assert!(value.contains("Stats")); } @@ -62,11 +62,11 @@ fn property_int_cf_test() { let n = DBPath::new("_rust_rocksdb_property_int_cf_test"); { let opts = Options::default(); - let mut db = DB::open_default(&n).unwrap(); + let db = DB::open_default(&n).unwrap(); db.create_cf("cf1", &opts).unwrap(); let cf = db.cf_handle("cf1").unwrap(); let total_keys = db - .property_int_value_cf(cf, "rocksdb.estimate-num-keys") + .property_int_value_cf(&cf, "rocksdb.estimate-num-keys") .unwrap(); assert_eq!(total_keys, Some(0)); From c51484c6bbb7bf9472e9b2348ff1375de5fa0c85 Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Thu, 25 Feb 2021 20:04:39 -0800 Subject: [PATCH 04/30] Add multithread column family map option --- src/db.rs | 118 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 92 insertions(+), 26 deletions(-) diff --git a/src/db.rs b/src/db.rs index b1a18f9c9..6440a0f81 100644 --- a/src/db.rs +++ b/src/db.rs @@ -35,12 +35,42 @@ use std::str; use std::sync::RwLock; use std::time::Duration; +enum ColumnFamilyHandles { + MultiThread(RwLock>), + SingleThread(BTreeMap), +} + +impl ColumnFamilyHandles { + fn get(&self, f: impl Fn(&BTreeMap) -> T) -> T { + match self { + ColumnFamilyHandles::MultiThread(lock) => f(&lock.read().unwrap()), + ColumnFamilyHandles::SingleThread(map) => f(&map), + } + } + + fn get_mut(&mut self, mut f: impl FnMut(&mut BTreeMap) -> T) -> T { + match self { + ColumnFamilyHandles::MultiThread(lock) => f(&mut lock.write().unwrap()), + ColumnFamilyHandles::SingleThread(ref mut map) => f(map), + } + } + + fn get_mut_locked(&self, mut f: impl FnMut(&mut BTreeMap) -> T) -> T { + match self { + ColumnFamilyHandles::MultiThread(lock) => f(&mut lock.write().unwrap()), + ColumnFamilyHandles::SingleThread(_) => { + panic!("Not available on SingleThread implementation"); + } + } + } +} + /// A RocksDB database. /// /// See crate level documentation for a simple usage example. pub struct DB { pub(crate) inner: *mut ffi::rocksdb_t, - cfs: RwLock>, + cfs: ColumnFamilyHandles, path: PathBuf, } @@ -106,7 +136,7 @@ impl DB { Ok(DB { inner: db, - cfs: RwLock::new(BTreeMap::new()), + cfs: ColumnFamilyHandles::SingleThread(BTreeMap::new()), path: path.as_ref().to_path_buf(), }) } @@ -269,7 +299,7 @@ impl DB { Ok(DB { inner: db, - cfs: RwLock::new(cf_map), + cfs: ColumnFamilyHandles::SingleThread(cf_map), path: path.as_ref().to_path_buf(), }) } @@ -661,43 +691,77 @@ impl DB { Ok(convert_values(values, values_sizes)) } - pub fn create_cf>(&self, name: N, opts: &Options) -> Result<(), Error> { - let cf_name = if let Ok(c) = CString::new(name.as_ref().as_bytes()) { + fn create_inner_cf_handle( + &self, + name: &str, + opts: &Options, + ) -> Result<*mut ffi::rocksdb_column_family_handle_t, Error> { + let cf_name = if let Ok(c) = CString::new(name.as_bytes()) { c } else { return Err(Error::new( "Failed to convert path to CString when creating cf".to_owned(), )); }; - unsafe { - let inner = ffi_try!(ffi::rocksdb_create_column_family( + Ok(unsafe { + ffi_try!(ffi::rocksdb_create_column_family( self.inner, opts.inner, cf_name.as_ptr(), - )); + )) + }) + } - self.cfs - .write() - .unwrap() - .insert(name.as_ref().to_string(), ColumnFamily { inner }); - }; + pub fn create_cf_multithreaded>( + &self, + name: N, + opts: &Options, + ) -> Result<(), Error> { + let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; + self.cfs.get_mut_locked(|cf_map| { + cf_map.insert(name.as_ref().to_string(), ColumnFamily { inner }) + }); Ok(()) } - pub fn drop_cf(&self, name: &str) -> Result<(), Error> { - if let Some(cf) = self.cfs.write().unwrap().remove(name) { - unsafe { - ffi_try!(ffi::rocksdb_drop_column_family(self.inner, cf.inner)); + pub fn create_cf>(&mut self, name: N, opts: &Options) -> Result<(), Error> { + let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; + self.cfs + .get_mut(|cf_map| cf_map.insert(name.as_ref().to_string(), ColumnFamily { inner })); + Ok(()) + } + + pub fn drop_cf(&mut self, name: &str) -> Result<(), Error> { + let inner = self.inner; + self.cfs.get_mut(|cf_map| { + if let Some(cf) = cf_map.remove(name) { + unsafe { + ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); + } + Ok(()) + } else { + Err(Error::new(format!("Invalid column family: {}", name))) } - Ok(()) - } else { - Err(Error::new(format!("Invalid column family: {}", name))) - } + }) + } + + pub fn drop_cf_multi_threaded(&mut self, name: &str) -> Result<(), Error> { + let inner = self.inner; + self.cfs.get_mut_locked(|cf_map| { + if let Some(cf) = cf_map.remove(name) { + unsafe { + ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); + } + Ok(()) + } else { + Err(Error::new(format!("Invalid column family: {}", name))) + } + }) } /// Return the underlying column family handle. pub fn cf_handle(&self, name: &str) -> Option { - self.cfs.read().unwrap().get(name).cloned() + self.cfs.get(|cf_map| cf_map.get(name).cloned()) } pub fn iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b> { @@ -1479,10 +1543,12 @@ impl DB { impl Drop for DB { fn drop(&mut self) { unsafe { - for cf in self.cfs.read().unwrap().values() { - ffi::rocksdb_column_family_handle_destroy(cf.inner); - } - ffi::rocksdb_close(self.inner); + self.cfs.get(|cf_map| { + for cf in cf_map.values() { + ffi::rocksdb_column_family_handle_destroy(cf.inner); + ffi::rocksdb_close(self.inner); + } + }) } } } From 0f5a3cf1f2ebdb7527df8aab93eef86f865fd2bb Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Mon, 1 Mar 2021 20:41:56 -0800 Subject: [PATCH 05/30] Add comments/cleanup --- src/db.rs | 10 +++++++++- tests/test_property.rs | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/db.rs b/src/db.rs index 6440a0f81..23d8939b5 100644 --- a/src/db.rs +++ b/src/db.rs @@ -712,6 +712,9 @@ impl DB { }) } + /// Create column family with given name and options without grabbing the + /// inner column family lock. This is an optimization that is only safe to + /// use if the database was opened with multithreaded config pub fn create_cf_multithreaded>( &self, name: N, @@ -724,6 +727,7 @@ impl DB { Ok(()) } + /// Create column family with given name and options pub fn create_cf>(&mut self, name: N, opts: &Options) -> Result<(), Error> { let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; self.cfs @@ -731,6 +735,7 @@ impl DB { Ok(()) } + /// Drop the column family with the given name pub fn drop_cf(&mut self, name: &str) -> Result<(), Error> { let inner = self.inner; self.cfs.get_mut(|cf_map| { @@ -745,6 +750,9 @@ impl DB { }) } + /// Drop column family with given name without grabbing the inner column + /// family lock. This is an optimization that is only safe to use if the + /// database was opened with multithreaded config pub fn drop_cf_multi_threaded(&mut self, name: &str) -> Result<(), Error> { let inner = self.inner; self.cfs.get_mut_locked(|cf_map| { @@ -1546,8 +1554,8 @@ impl Drop for DB { self.cfs.get(|cf_map| { for cf in cf_map.values() { ffi::rocksdb_column_family_handle_destroy(cf.inner); - ffi::rocksdb_close(self.inner); } + ffi::rocksdb_close(self.inner); }) } } diff --git a/tests/test_property.rs b/tests/test_property.rs index 7b3aec811..d82fc6396 100644 --- a/tests/test_property.rs +++ b/tests/test_property.rs @@ -35,7 +35,7 @@ fn property_cf_test() { let n = DBPath::new("_rust_rocksdb_property_cf_test"); { let opts = Options::default(); - let db = DB::open_default(&n).unwrap(); + let mut db = DB::open_default(&n).unwrap(); db.create_cf("cf1", &opts).unwrap(); let cf = db.cf_handle("cf1").unwrap(); let value = db.property_value_cf(&cf, "rocksdb.stats").unwrap().unwrap(); From e88da394d7a8f61b1a70d46f50934aa9e28e83ac Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Mon, 1 Mar 2021 20:50:16 -0800 Subject: [PATCH 06/30] Plumb is_multithreaded through constructors --- src/db.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/src/db.rs b/src/db.rs index 23d8939b5..93ed7c67a 100644 --- a/src/db.rs +++ b/src/db.rs @@ -154,7 +154,24 @@ impl DB { .into_iter() .map(|name| ColumnFamilyDescriptor::new(name.as_ref(), Options::default())); - DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite) + DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, false) + } + + /// Opens a database with the given database options and column family names + /// with internal locking for column families. + /// + /// Column families opened using this function will be created with default `Options`. + pub fn open_cf_multithreaded(opts: &Options, path: P, cfs: I) -> Result + where + P: AsRef, + I: IntoIterator, + N: AsRef, + { + let cfs = cfs + .into_iter() + .map(|name| ColumnFamilyDescriptor::new(name.as_ref(), Options::default())); + + DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, true) } /// Opens a database for read only with the given database options and column family names. @@ -180,6 +197,7 @@ impl DB { &AccessType::ReadOnly { error_if_log_file_exist, }, + false, ) } @@ -206,6 +224,7 @@ impl DB { &AccessType::Secondary { secondary_path: secondary_path.as_ref(), }, + false, ) } @@ -215,7 +234,21 @@ impl DB { P: AsRef, I: IntoIterator, { - DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite) + DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, false) + } + + /// Opens a database with the given database options and column family descriptors, + /// with internal locking for column families + pub fn open_cf_descriptors_multithreaded( + opts: &Options, + path: P, + cfs: I, + ) -> Result + where + P: AsRef, + I: IntoIterator, + { + DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, true) } /// Internal implementation for opening RocksDB. @@ -224,6 +257,7 @@ impl DB { path: P, cfs: I, access_type: &AccessType, + is_multithreaded: bool, ) -> Result where P: AsRef, @@ -297,9 +331,15 @@ impl DB { return Err(Error::new("Could not initialize database.".to_owned())); } + let cfs = if is_multithreaded { + ColumnFamilyHandles::MultiThread(RwLock::new(cf_map)) + } else { + ColumnFamilyHandles::SingleThread(cf_map) + }; + Ok(DB { inner: db, - cfs: ColumnFamilyHandles::SingleThread(cf_map), + cfs, path: path.as_ref().to_path_buf(), }) } @@ -712,8 +752,8 @@ impl DB { }) } - /// Create column family with given name and options without grabbing the - /// inner column family lock. This is an optimization that is only safe to + /// Create column family with given name by internally locking the inner column + /// family map. This avoids needing `&mut self` reference, but is only safe to /// use if the database was opened with multithreaded config pub fn create_cf_multithreaded>( &self, @@ -750,10 +790,10 @@ impl DB { }) } - /// Drop column family with given name without grabbing the inner column - /// family lock. This is an optimization that is only safe to use if the - /// database was opened with multithreaded config - pub fn drop_cf_multi_threaded(&mut self, name: &str) -> Result<(), Error> { + /// Drop column family with given name by internally locking the inner column + /// family map. This avoids needing `&mut self` reference, but is only safe to + /// use if the database was opened with multithreaded config + pub fn drop_cf_multi_threaded(&self, name: &str) -> Result<(), Error> { let inner = self.inner; self.cfs.get_mut_locked(|cf_map| { if let Some(cf) = cf_map.remove(name) { From c1a203e9d4642380bf130e4d63e2a5e61bc355a4 Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Mon, 1 Mar 2021 20:54:11 -0800 Subject: [PATCH 07/30] Fix tests --- tests/test_column_family.rs | 8 ++++---- tests/test_property.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_column_family.rs b/tests/test_column_family.rs index 1ee182be4..4410e6c6f 100644 --- a/tests/test_column_family.rs +++ b/tests/test_column_family.rs @@ -28,7 +28,7 @@ fn test_column_family() { let mut opts = Options::default(); opts.create_if_missing(true); opts.set_merge_operator_associative("test operator", test_provided_merge); - let db = DB::open(&opts, &n).unwrap(); + let mut db = DB::open(&opts, &n).unwrap(); let opts = Options::default(); match db.create_cf("cf1", &opts) { Ok(()) => println!("cf1 created successfully"), @@ -78,7 +78,7 @@ fn test_column_family() { {} // should b able to drop a cf { - let db = DB::open_cf(&Options::default(), &n, &["cf1"]).unwrap(); + let mut db = DB::open_cf(&Options::default(), &n, &["cf1"]).unwrap(); match db.drop_cf("cf1") { Ok(_) => println!("cf1 successfully dropped."), Err(e) => panic!("failed to drop column family: {}", e), @@ -95,7 +95,7 @@ fn test_can_open_db_with_results_of_list_cf() { { let mut opts = Options::default(); opts.create_if_missing(true); - let db = DB::open(&opts, &n).unwrap(); + let mut db = DB::open(&opts, &n).unwrap(); let opts = Options::default(); assert!(db.create_cf("cf1", &opts).is_ok()); @@ -242,7 +242,7 @@ fn test_create_duplicate_column_family() { opts.create_if_missing(true); opts.create_missing_column_families(true); - let db = match DB::open_cf(&opts, &n, &["cf1"]) { + let mut db = match DB::open_cf(&opts, &n, &["cf1"]) { Ok(d) => d, Err(e) => panic!("failed to create new column family: {}", e), }; diff --git a/tests/test_property.rs b/tests/test_property.rs index d82fc6396..557144121 100644 --- a/tests/test_property.rs +++ b/tests/test_property.rs @@ -62,7 +62,7 @@ fn property_int_cf_test() { let n = DBPath::new("_rust_rocksdb_property_int_cf_test"); { let opts = Options::default(); - let db = DB::open_default(&n).unwrap(); + let mut db = DB::open_default(&n).unwrap(); db.create_cf("cf1", &opts).unwrap(); let cf = db.cf_handle("cf1").unwrap(); let total_keys = db From 1fc975f73453af006c621c14ed90c920ffaaa5f8 Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Mon, 1 Mar 2021 22:43:15 -0800 Subject: [PATCH 08/30] Add multithreaded test for column families --- src/db.rs | 14 +++++----- tests/test_column_family.rs | 56 +++++++++++++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/db.rs b/src/db.rs index 93ed7c67a..69a5f88a6 100644 --- a/src/db.rs +++ b/src/db.rs @@ -161,7 +161,7 @@ impl DB { /// with internal locking for column families. /// /// Column families opened using this function will be created with default `Options`. - pub fn open_cf_multithreaded(opts: &Options, path: P, cfs: I) -> Result + pub fn open_cf_multi_threaded(opts: &Options, path: P, cfs: I) -> Result where P: AsRef, I: IntoIterator, @@ -239,7 +239,7 @@ impl DB { /// Opens a database with the given database options and column family descriptors, /// with internal locking for column families - pub fn open_cf_descriptors_multithreaded( + pub fn open_cf_descriptors_multi_threaded( opts: &Options, path: P, cfs: I, @@ -257,7 +257,7 @@ impl DB { path: P, cfs: I, access_type: &AccessType, - is_multithreaded: bool, + is_multi_threaded: bool, ) -> Result where P: AsRef, @@ -331,7 +331,7 @@ impl DB { return Err(Error::new("Could not initialize database.".to_owned())); } - let cfs = if is_multithreaded { + let cfs = if is_multi_threaded { ColumnFamilyHandles::MultiThread(RwLock::new(cf_map)) } else { ColumnFamilyHandles::SingleThread(cf_map) @@ -754,8 +754,8 @@ impl DB { /// Create column family with given name by internally locking the inner column /// family map. This avoids needing `&mut self` reference, but is only safe to - /// use if the database was opened with multithreaded config - pub fn create_cf_multithreaded>( + /// use if the database was opened with multi-threaded config + pub fn create_cf_multi_threaded>( &self, name: N, opts: &Options, @@ -792,7 +792,7 @@ impl DB { /// Drop column family with given name by internally locking the inner column /// family map. This avoids needing `&mut self` reference, but is only safe to - /// use if the database was opened with multithreaded config + /// use if the database was opened with multi-threaded config pub fn drop_cf_multi_threaded(&self, name: &str) -> Result<(), Error> { let inner = self.inner; self.cfs.get_mut_locked(|cf_map| { diff --git a/tests/test_column_family.rs b/tests/test_column_family.rs index 4410e6c6f..d8a8c4f3d 100644 --- a/tests/test_column_family.rs +++ b/tests/test_column_family.rs @@ -19,8 +19,7 @@ use pretty_assertions::assert_eq; use rocksdb::{ColumnFamilyDescriptor, MergeOperands, Options, DB, DEFAULT_COLUMN_FAMILY_NAME}; use util::DBPath; -#[test] -fn test_column_family() { +fn run_test_column_family(is_multi_threaded: bool) { let n = DBPath::new("_rust_rocksdb_cftest"); // should be able to create column families @@ -28,9 +27,24 @@ fn test_column_family() { let mut opts = Options::default(); opts.create_if_missing(true); opts.set_merge_operator_associative("test operator", test_provided_merge); - let mut db = DB::open(&opts, &n).unwrap(); + let (multi_threaded_db, single_threaded_db) = if is_multi_threaded { + ( + Some(DB::open_cf_multi_threaded(&opts, &n, None::<&str>).unwrap()), + None, + ) + } else { + (None, Some(DB::open_cf(&opts, &n, None::<&str>).unwrap())) + }; let opts = Options::default(); - match db.create_cf("cf1", &opts) { + let create_cf_result = if is_multi_threaded { + // multithreaded db does not need mutable access to create a column family + let multi_threaded_db = multi_threaded_db.unwrap(); + multi_threaded_db.create_cf_multi_threaded("cf1", &opts) + } else { + let mut single_threaded_db = single_threaded_db.unwrap(); + single_threaded_db.create_cf("cf1", &opts) + }; + match create_cf_result { Ok(()) => println!("cf1 created successfully"), Err(e) => { panic!("could not create column family: {}", e); @@ -78,14 +92,44 @@ fn test_column_family() { {} // should b able to drop a cf { - let mut db = DB::open_cf(&Options::default(), &n, &["cf1"]).unwrap(); - match db.drop_cf("cf1") { + let (multi_threaded_db, single_threaded_db) = if is_multi_threaded { + ( + Some(DB::open_cf_multi_threaded(&Options::default(), &n, &["cf1"]).unwrap()), + None, + ) + } else { + ( + None, + Some(DB::open_cf(&Options::default(), &n, &["cf1"]).unwrap()), + ) + }; + + let drop_cf_result = if is_multi_threaded { + // multithreaded db does not need mutable access to drop a column family + let multi_threaded_db = multi_threaded_db.unwrap(); + multi_threaded_db.drop_cf_multi_threaded("cf1") + } else { + let mut single_threaded_db = single_threaded_db.unwrap(); + single_threaded_db.drop_cf("cf1") + }; + + match drop_cf_result { Ok(_) => println!("cf1 successfully dropped."), Err(e) => panic!("failed to drop column family: {}", e), } } } +#[test] +fn test_column_family() { + run_test_column_family(false); +} + +#[test] +fn test_column_family_multithreaded() { + run_test_column_family(true); +} + #[test] fn test_can_open_db_with_results_of_list_cf() { // Test scenario derived from GitHub issue #175 and 177 From 728b3490841fe386bcee1b2cd3f09f7f8ba06e81 Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Wed, 3 Mar 2021 23:05:59 -0800 Subject: [PATCH 09/30] Remove clone of handle --- src/column_family.rs | 1 - src/db.rs | 157 +++++++++++++++++++++++++------------------ tests/test_db.rs | 2 +- 3 files changed, 93 insertions(+), 67 deletions(-) diff --git a/src/column_family.rs b/src/column_family.rs index c581a3b18..2c1b50531 100644 --- a/src/column_family.rs +++ b/src/column_family.rs @@ -43,7 +43,6 @@ impl ColumnFamilyDescriptor { /// An opaque type used to represent a column family. Returned from some functions, and used /// in others -#[derive(Clone)] pub struct ColumnFamily { pub(crate) inner: *mut ffi::rocksdb_column_family_handle_t, } diff --git a/src/db.rs b/src/db.rs index 69a5f88a6..85f8446c5 100644 --- a/src/db.rs +++ b/src/db.rs @@ -32,39 +32,14 @@ use std::path::PathBuf; use std::ptr; use std::slice; use std::str; -use std::sync::RwLock; +use std::sync::{Arc, RwLock}; use std::time::Duration; enum ColumnFamilyHandles { - MultiThread(RwLock>), + MultiThread(RwLock>>), SingleThread(BTreeMap), } -impl ColumnFamilyHandles { - fn get(&self, f: impl Fn(&BTreeMap) -> T) -> T { - match self { - ColumnFamilyHandles::MultiThread(lock) => f(&lock.read().unwrap()), - ColumnFamilyHandles::SingleThread(map) => f(&map), - } - } - - fn get_mut(&mut self, mut f: impl FnMut(&mut BTreeMap) -> T) -> T { - match self { - ColumnFamilyHandles::MultiThread(lock) => f(&mut lock.write().unwrap()), - ColumnFamilyHandles::SingleThread(ref mut map) => f(map), - } - } - - fn get_mut_locked(&self, mut f: impl FnMut(&mut BTreeMap) -> T) -> T { - match self { - ColumnFamilyHandles::MultiThread(lock) => f(&mut lock.write().unwrap()), - ColumnFamilyHandles::SingleThread(_) => { - panic!("Not available on SingleThread implementation"); - } - } - } -} - /// A RocksDB database. /// /// See crate level documentation for a simple usage example. @@ -332,7 +307,12 @@ impl DB { } let cfs = if is_multi_threaded { - ColumnFamilyHandles::MultiThread(RwLock::new(cf_map)) + ColumnFamilyHandles::MultiThread(RwLock::new( + cf_map + .into_iter() + .map(|(name, cf)| (name, Arc::new(cf))) + .collect(), + )) } else { ColumnFamilyHandles::SingleThread(cf_map) }; @@ -752,7 +732,7 @@ impl DB { }) } - /// Create column family with given name by internally locking the inner column + /// Creates column family with given name by internally locking the inner column /// family map. This avoids needing `&mut self` reference, but is only safe to /// use if the database was opened with multi-threaded config pub fn create_cf_multi_threaded>( @@ -761,55 +741,94 @@ impl DB { opts: &Options, ) -> Result<(), Error> { let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; - self.cfs.get_mut_locked(|cf_map| { - cf_map.insert(name.as_ref().to_string(), ColumnFamily { inner }) - }); + match &self.cfs { + ColumnFamilyHandles::MultiThread(cf_map) => cf_map + .write() + .unwrap() + .insert(name.as_ref().to_string(), Arc::new(ColumnFamily { inner })), + ColumnFamilyHandles::SingleThread(_) => { + panic!("Not available on SingleThread implementation"); + } + }; Ok(()) } - /// Create column family with given name and options + /// Creates column family with given name and options pub fn create_cf>(&mut self, name: N, opts: &Options) -> Result<(), Error> { let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; - self.cfs - .get_mut(|cf_map| cf_map.insert(name.as_ref().to_string(), ColumnFamily { inner })); + match &mut self.cfs { + ColumnFamilyHandles::MultiThread(_) => { + panic!("Not available on MultiThread implementation") + } + ColumnFamilyHandles::SingleThread(cf_map) => { + cf_map.insert(name.as_ref().to_string(), ColumnFamily { inner }); + } + } Ok(()) } - /// Drop the column family with the given name - pub fn drop_cf(&mut self, name: &str) -> Result<(), Error> { + /// Drops the column family with the given name by internally locking the inner column + /// family map. This avoids needing `&mut self` reference, but is only safe to use if + /// the database was opened with multi-threaded config + pub fn drop_cf_multi_threaded(&self, name: &str) -> Result<(), Error> { let inner = self.inner; - self.cfs.get_mut(|cf_map| { - if let Some(cf) = cf_map.remove(name) { - unsafe { - ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); + match &self.cfs { + ColumnFamilyHandles::MultiThread(cf_map) => { + if let Some(cf) = cf_map.write().unwrap().remove(name) { + unsafe { + ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); + } + Ok(()) + } else { + Err(Error::new(format!("Invalid column family: {}", name))) } - Ok(()) - } else { - Err(Error::new(format!("Invalid column family: {}", name))) } - }) + ColumnFamilyHandles::SingleThread(_) => { + panic!("Not available on SingleThread implementation") + } + } } - /// Drop column family with given name by internally locking the inner column - /// family map. This avoids needing `&mut self` reference, but is only safe to - /// use if the database was opened with multi-threaded config - pub fn drop_cf_multi_threaded(&self, name: &str) -> Result<(), Error> { + /// Drops the column family with the given name + pub fn drop_cf(&mut self, name: &str) -> Result<(), Error> { let inner = self.inner; - self.cfs.get_mut_locked(|cf_map| { - if let Some(cf) = cf_map.remove(name) { - unsafe { - ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); + match &mut self.cfs { + ColumnFamilyHandles::MultiThread(_) => { + panic!("Not available on MultiThread implementation") + } + ColumnFamilyHandles::SingleThread(cf_map) => { + if let Some(cf) = cf_map.remove(name) { + unsafe { + ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); + } + Ok(()) + } else { + Err(Error::new(format!("Invalid column family: {}", name))) } - Ok(()) - } else { - Err(Error::new(format!("Invalid column family: {}", name))) } - }) + } } - /// Return the underlying column family handle. - pub fn cf_handle(&self, name: &str) -> Option { - self.cfs.get(|cf_map| cf_map.get(name).cloned()) + /// Return the underlying column family handle, only safe to use if the database + /// was configured to be multithreaded + pub fn cf_handle_multi_threaded(&self, name: &str) -> Option> { + match &self.cfs { + ColumnFamilyHandles::MultiThread(cf_map) => cf_map.write().unwrap().get(name).cloned(), + ColumnFamilyHandles::SingleThread(_) => { + panic!("Not available on SingleThread implementation") + } + } + } + + /// Return the underlying column family handle, only safe to use if the database + /// was not configured to be multithreaded + pub fn cf_handle(&self, name: &str) -> Option<&ColumnFamily> { + match &self.cfs { + ColumnFamilyHandles::MultiThread(_) => { + panic!("Not available on MultiThread implementation") + } + ColumnFamilyHandles::SingleThread(cf_map) => cf_map.get(name), + } } pub fn iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b> { @@ -1591,12 +1610,20 @@ impl DB { impl Drop for DB { fn drop(&mut self) { unsafe { - self.cfs.get(|cf_map| { - for cf in cf_map.values() { - ffi::rocksdb_column_family_handle_destroy(cf.inner); + match &self.cfs { + ColumnFamilyHandles::MultiThread(cf_map) => { + for cf in cf_map.read().unwrap().values() { + ffi::rocksdb_column_family_handle_destroy(cf.inner); + } + } + ColumnFamilyHandles::SingleThread(cf_map) => { + for cf in cf_map.values() { + ffi::rocksdb_column_family_handle_destroy(cf.inner); + } } - ffi::rocksdb_close(self.inner); - }) + } + + ffi::rocksdb_close(self.inner); } } } diff --git a/tests/test_db.rs b/tests/test_db.rs index f2f7f5750..1631bcdd0 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -907,7 +907,7 @@ fn multi_get_cf() { db.put_cf(&cf2, b"k2", b"v2").unwrap(); let values = db - .multi_get_cf(vec![(&cf0, b"k0"), (&cf1, b"k1"), (&cf2, b"k2")]) + .multi_get_cf(vec![(cf0, b"k0"), (cf1, b"k1"), (cf2, b"k2")]) .expect("multi_get failed"); assert_eq!(3, values.len()); assert!(values[0].is_empty()); From ac09e7515a8efdf4371ce16b7e5aa80c91a3d999 Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Wed, 10 Mar 2021 01:06:08 -0800 Subject: [PATCH 10/30] Rework test --- tests/test_column_family.rs | 111 +++++++++++++++++------------------- 1 file changed, 52 insertions(+), 59 deletions(-) diff --git a/tests/test_column_family.rs b/tests/test_column_family.rs index d8a8c4f3d..8b94812d4 100644 --- a/tests/test_column_family.rs +++ b/tests/test_column_family.rs @@ -19,39 +19,7 @@ use pretty_assertions::assert_eq; use rocksdb::{ColumnFamilyDescriptor, MergeOperands, Options, DB, DEFAULT_COLUMN_FAMILY_NAME}; use util::DBPath; -fn run_test_column_family(is_multi_threaded: bool) { - let n = DBPath::new("_rust_rocksdb_cftest"); - - // should be able to create column families - { - let mut opts = Options::default(); - opts.create_if_missing(true); - opts.set_merge_operator_associative("test operator", test_provided_merge); - let (multi_threaded_db, single_threaded_db) = if is_multi_threaded { - ( - Some(DB::open_cf_multi_threaded(&opts, &n, None::<&str>).unwrap()), - None, - ) - } else { - (None, Some(DB::open_cf(&opts, &n, None::<&str>).unwrap())) - }; - let opts = Options::default(); - let create_cf_result = if is_multi_threaded { - // multithreaded db does not need mutable access to create a column family - let multi_threaded_db = multi_threaded_db.unwrap(); - multi_threaded_db.create_cf_multi_threaded("cf1", &opts) - } else { - let mut single_threaded_db = single_threaded_db.unwrap(); - single_threaded_db.create_cf("cf1", &opts) - }; - match create_cf_result { - Ok(()) => println!("cf1 created successfully"), - Err(e) => { - panic!("could not create column family: {}", e); - } - } - } - +fn test_cf_common(n: &DBPath) { // should fail to open db without specifying same column families { let mut opts = Options::default(); @@ -91,29 +59,32 @@ fn run_test_column_family(is_multi_threaded: bool) { // TODO should be able to iterate over a cf {} // should b able to drop a cf +} + +#[test] +fn test_column_family() { + let n = DBPath::new("_rust_rocksdb_cftest"); + + // should be able to create column families { - let (multi_threaded_db, single_threaded_db) = if is_multi_threaded { - ( - Some(DB::open_cf_multi_threaded(&Options::default(), &n, &["cf1"]).unwrap()), - None, - ) - } else { - ( - None, - Some(DB::open_cf(&Options::default(), &n, &["cf1"]).unwrap()), - ) - }; + let mut opts = Options::default(); + opts.create_if_missing(true); + opts.set_merge_operator_associative("test operator", test_provided_merge); + let mut db = DB::open(&opts, &n).unwrap(); + let opts = Options::default(); + match db.create_cf("cf1", &opts) { + Ok(()) => println!("cf1 created successfully"), + Err(e) => { + panic!("could not create column family: {}", e); + } + } + } - let drop_cf_result = if is_multi_threaded { - // multithreaded db does not need mutable access to drop a column family - let multi_threaded_db = multi_threaded_db.unwrap(); - multi_threaded_db.drop_cf_multi_threaded("cf1") - } else { - let mut single_threaded_db = single_threaded_db.unwrap(); - single_threaded_db.drop_cf("cf1") - }; + test_cf_common(&n); - match drop_cf_result { + { + let mut db = DB::open_cf(&Options::default(), &n, &["cf1"]).unwrap(); + match db.drop_cf("cf1") { Ok(_) => println!("cf1 successfully dropped."), Err(e) => panic!("failed to drop column family: {}", e), } @@ -121,13 +92,35 @@ fn run_test_column_family(is_multi_threaded: bool) { } #[test] -fn test_column_family() { - run_test_column_family(false); -} +fn test_multi_threaded_column_family() { + let n = DBPath::new("_rust_rocksdb_cftest"); -#[test] -fn test_column_family_multithreaded() { - run_test_column_family(true); + // should be able to create column families + { + let mut opts = Options::default(); + opts.create_if_missing(true); + opts.set_merge_operator_associative("test operator", test_provided_merge); + let db = DB::open_cf_multi_threaded(&opts, &n, None::<&str>).unwrap(); + let opts = Options::default(); + + match db.create_cf_multi_threaded("cf1", &opts) { + Ok(()) => println!("cf1 created successfully"), + Err(e) => { + panic!("could not create column family: {}", e); + } + } + } + + test_cf_common(&n); + + // should be able to drop a cf + { + let db = DB::open_cf_multi_threaded(&Options::default(), &n, &["cf1"]).unwrap(); + match db.drop_cf_multi_threaded("cf1") { + Ok(_) => println!("cf1 successfully dropped."), + Err(e) => panic!("failed to drop column family: {}", e), + } + } } #[test] From 441834e4c1a4841c1858a16248b5ac368220fc7e Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Wed, 10 Mar 2021 01:34:30 -0800 Subject: [PATCH 11/30] Revert references --- src/db.rs | 4 +-- tests/test_column_family.rs | 18 +++++----- tests/test_db.rs | 72 ++++++++++++++++++------------------- tests/test_property.rs | 4 +-- 4 files changed, 49 insertions(+), 49 deletions(-) diff --git a/src/db.rs b/src/db.rs index 85f8446c5..9eaaa0c48 100644 --- a/src/db.rs +++ b/src/db.rs @@ -809,7 +809,7 @@ impl DB { } } - /// Return the underlying column family handle, only safe to use if the database + /// Returns the underlying column family handle, only safe to use if the database /// was configured to be multithreaded pub fn cf_handle_multi_threaded(&self, name: &str) -> Option> { match &self.cfs { @@ -820,7 +820,7 @@ impl DB { } } - /// Return the underlying column family handle, only safe to use if the database + /// Returns the underlying column family handle, only safe to use if the database /// was not configured to be multithreaded pub fn cf_handle(&self, name: &str) -> Option<&ColumnFamily> { match &self.cfs { diff --git a/tests/test_column_family.rs b/tests/test_column_family.rs index 8b94812d4..3b0404bf1 100644 --- a/tests/test_column_family.rs +++ b/tests/test_column_family.rs @@ -180,15 +180,15 @@ fn test_merge_operator() { Err(e) => panic!("failed to open db with column family: {}", e), }; let cf1 = db.cf_handle("cf1").unwrap(); - assert!(db.put_cf(&cf1, b"k1", b"v1").is_ok()); - assert_eq!(db.get_cf(&cf1, b"k1").unwrap().unwrap(), b"v1"); - let p = db.put_cf(&cf1, b"k1", b"a"); + assert!(db.put_cf(cf1, b"k1", b"v1").is_ok()); + assert_eq!(db.get_cf(cf1, b"k1").unwrap().unwrap(), b"v1"); + let p = db.put_cf(cf1, b"k1", b"a"); assert!(p.is_ok()); - db.merge_cf(&cf1, b"k1", b"b").unwrap(); - db.merge_cf(&cf1, b"k1", b"c").unwrap(); - db.merge_cf(&cf1, b"k1", b"d").unwrap(); - db.merge_cf(&cf1, b"k1", b"efg").unwrap(); - let m = db.merge_cf(&cf1, b"k1", b"h"); + db.merge_cf(cf1, b"k1", b"b").unwrap(); + db.merge_cf(cf1, b"k1", b"c").unwrap(); + db.merge_cf(cf1, b"k1", b"d").unwrap(); + db.merge_cf(cf1, b"k1", b"efg").unwrap(); + let m = db.merge_cf(cf1, b"k1", b"h"); println!("m is {:?}", m); // TODO assert!(m.is_ok()); match db.get(b"k1") { @@ -200,7 +200,7 @@ fn test_merge_operator() { _ => panic!("value not present!"), } - let _ = db.get_cf(&cf1, b"k1"); + let _ = db.get_cf(cf1, b"k1"); // TODO assert!(r.unwrap().as_ref() == b"abcdefgh"); assert!(db.delete(b"k1").is_ok()); assert!(db.get(b"k1").unwrap().is_none()); diff --git a/tests/test_db.rs b/tests/test_db.rs index 1631bcdd0..d9ec85b24 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -347,27 +347,27 @@ fn set_option_cf_test() { let cf = db.cf_handle("cf1").unwrap(); // set an option to valid values assert!(db - .set_options_cf(&cf, &[("disable_auto_compactions", "true")]) + .set_options_cf(cf, &[("disable_auto_compactions", "true")]) .is_ok()); assert!(db - .set_options_cf(&cf, &[("disable_auto_compactions", "false")]) + .set_options_cf(cf, &[("disable_auto_compactions", "false")]) .is_ok()); // invalid names/values should result in an error assert!(db - .set_options_cf(&cf, &[("disable_auto_compactions", "INVALID_VALUE")]) + .set_options_cf(cf, &[("disable_auto_compactions", "INVALID_VALUE")]) .is_err()); assert!(db - .set_options_cf(&cf, &[("INVALID_NAME", "INVALID_VALUE")]) + .set_options_cf(cf, &[("INVALID_NAME", "INVALID_VALUE")]) .is_err()); // option names/values must not contain NULLs assert!(db - .set_options_cf(&cf, &[("disable_auto_compactions", "true\0")]) + .set_options_cf(cf, &[("disable_auto_compactions", "true\0")]) .is_err()); assert!(db - .set_options_cf(&cf, &[("disable_auto_compactions\0", "true")]) + .set_options_cf(cf, &[("disable_auto_compactions\0", "true")]) .is_err()); // empty options are not allowed - assert!(db.set_options_cf(&cf, &[]).is_err()); + assert!(db.set_options_cf(cf, &[]).is_err()); // multiple options can be set in a single API call let multiple_options = [ ("paranoid_file_checks", "true"), @@ -556,13 +556,13 @@ fn compact_range_test() { let cfs = vec!["cf1"]; let db = DB::open_cf(&opts, &path, cfs).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(&cf1, b"k1", b"v1").unwrap(); - db.put_cf(&cf1, b"k2", b"v2").unwrap(); - db.put_cf(&cf1, b"k3", b"v3").unwrap(); - db.put_cf(&cf1, b"k4", b"v4").unwrap(); - db.put_cf(&cf1, b"k5", b"v5").unwrap(); - db.compact_range_cf(&cf1, Some(b"k2"), Some(b"k4")); - db.compact_range_cf_opt(&cf1, Some(b"k1"), None::<&str>, &compact_opts); + db.put_cf(cf1, b"k1", b"v1").unwrap(); + db.put_cf(cf1, b"k2", b"v2").unwrap(); + db.put_cf(cf1, b"k3", b"v3").unwrap(); + db.put_cf(cf1, b"k4", b"v4").unwrap(); + db.put_cf(cf1, b"k5", b"v5").unwrap(); + db.compact_range_cf(cf1, Some(b"k2"), Some(b"k4")); + db.compact_range_cf_opt(cf1, Some(b"k1"), None::<&str>, &compact_opts); // put and compact default column family db.put(b"k1", b"v1").unwrap(); @@ -595,12 +595,12 @@ fn fifo_compaction_test() { let cfs = vec!["cf1"]; let db = DB::open_cf(&opts, &path, cfs).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(&cf1, b"k1", b"v1").unwrap(); - db.put_cf(&cf1, b"k2", b"v2").unwrap(); - db.put_cf(&cf1, b"k3", b"v3").unwrap(); - db.put_cf(&cf1, b"k4", b"v4").unwrap(); - db.put_cf(&cf1, b"k5", b"v5").unwrap(); - db.compact_range_cf(&cf1, Some(b"k2"), Some(b"k4")); + db.put_cf(cf1, b"k1", b"v1").unwrap(); + db.put_cf(cf1, b"k2", b"v2").unwrap(); + db.put_cf(cf1, b"k3", b"v3").unwrap(); + db.put_cf(cf1, b"k4", b"v4").unwrap(); + db.put_cf(cf1, b"k5", b"v5").unwrap(); + db.compact_range_cf(cf1, Some(b"k2"), Some(b"k4")); // check stats let ctx = PerfContext::default(); @@ -830,15 +830,15 @@ fn test_open_cf_for_read_only() { opts.create_missing_column_families(true); let db = DB::open_cf(&opts, &path, cfs.clone()).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(&cf1, b"k1", b"v1").unwrap(); + db.put_cf(cf1, b"k1", b"v1").unwrap(); } { let opts = Options::default(); let error_if_log_file_exist = false; let db = DB::open_cf_for_read_only(&opts, &path, cfs, error_if_log_file_exist).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - assert_eq!(db.get_cf(&cf1, b"k1").unwrap().unwrap(), b"v1"); - assert!(db.put_cf(&cf1, b"k2", b"v2").is_err()); + assert_eq!(db.get_cf(cf1, b"k1").unwrap().unwrap(), b"v1"); + assert!(db.put_cf(cf1, b"k2", b"v2").is_err()); } } @@ -854,18 +854,18 @@ fn delete_range_test() { let db = DB::open_cf(&opts, &path, cfs).unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(&cf1, b"k1", b"v1").unwrap(); - db.put_cf(&cf1, b"k2", b"v2").unwrap(); - db.put_cf(&cf1, b"k3", b"v3").unwrap(); - db.put_cf(&cf1, b"k4", b"v4").unwrap(); - db.put_cf(&cf1, b"k5", b"v5").unwrap(); + db.put_cf(cf1, b"k1", b"v1").unwrap(); + db.put_cf(cf1, b"k2", b"v2").unwrap(); + db.put_cf(cf1, b"k3", b"v3").unwrap(); + db.put_cf(cf1, b"k4", b"v4").unwrap(); + db.put_cf(cf1, b"k5", b"v5").unwrap(); - db.delete_range_cf(&cf1, b"k2", b"k4").unwrap(); - assert_eq!(db.get_cf(&cf1, b"k1").unwrap().unwrap(), b"v1"); - assert_eq!(db.get_cf(&cf1, b"k4").unwrap().unwrap(), b"v4"); - assert_eq!(db.get_cf(&cf1, b"k5").unwrap().unwrap(), b"v5"); - assert!(db.get_cf(&cf1, b"k2").unwrap().is_none()); - assert!(db.get_cf(&cf1, b"k3").unwrap().is_none()); + db.delete_range_cf(cf1, b"k2", b"k4").unwrap(); + assert_eq!(db.get_cf(cf1, b"k1").unwrap().unwrap(), b"v1"); + assert_eq!(db.get_cf(cf1, b"k4").unwrap().unwrap(), b"v4"); + assert_eq!(db.get_cf(cf1, b"k5").unwrap().unwrap(), b"v5"); + assert!(db.get_cf(cf1, b"k2").unwrap().is_none()); + assert!(db.get_cf(cf1, b"k3").unwrap().is_none()); } } @@ -901,10 +901,10 @@ fn multi_get_cf() { let cf0 = db.cf_handle("cf0").unwrap(); let cf1 = db.cf_handle("cf1").unwrap(); - db.put_cf(&cf1, b"k1", b"v1").unwrap(); + db.put_cf(cf1, b"k1", b"v1").unwrap(); let cf2 = db.cf_handle("cf2").unwrap(); - db.put_cf(&cf2, b"k2", b"v2").unwrap(); + db.put_cf(cf2, b"k2", b"v2").unwrap(); let values = db .multi_get_cf(vec![(cf0, b"k0"), (cf1, b"k1"), (cf2, b"k2")]) diff --git a/tests/test_property.rs b/tests/test_property.rs index 557144121..2b9d39944 100644 --- a/tests/test_property.rs +++ b/tests/test_property.rs @@ -38,7 +38,7 @@ fn property_cf_test() { let mut db = DB::open_default(&n).unwrap(); db.create_cf("cf1", &opts).unwrap(); let cf = db.cf_handle("cf1").unwrap(); - let value = db.property_value_cf(&cf, "rocksdb.stats").unwrap().unwrap(); + let value = db.property_value_cf(cf, "rocksdb.stats").unwrap().unwrap(); assert!(value.contains("Stats")); } @@ -66,7 +66,7 @@ fn property_int_cf_test() { db.create_cf("cf1", &opts).unwrap(); let cf = db.cf_handle("cf1").unwrap(); let total_keys = db - .property_int_value_cf(&cf, "rocksdb.estimate-num-keys") + .property_int_value_cf(cf, "rocksdb.estimate-num-keys") .unwrap(); assert_eq!(total_keys, Some(0)); From a6b53661ca1500163e4e929303a6ebd808d7c139 Mon Sep 17 00:00:00 2001 From: Carl Lin Date: Wed, 10 Mar 2021 01:38:22 -0800 Subject: [PATCH 12/30] Add comment --- tests/test_column_family.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_column_family.rs b/tests/test_column_family.rs index 3b0404bf1..bda52f32b 100644 --- a/tests/test_column_family.rs +++ b/tests/test_column_family.rs @@ -95,14 +95,14 @@ fn test_column_family() { fn test_multi_threaded_column_family() { let n = DBPath::new("_rust_rocksdb_cftest"); - // should be able to create column families + // Should be able to create column families without a mutable reference + // to the db { let mut opts = Options::default(); opts.create_if_missing(true); opts.set_merge_operator_associative("test operator", test_provided_merge); let db = DB::open_cf_multi_threaded(&opts, &n, None::<&str>).unwrap(); let opts = Options::default(); - match db.create_cf_multi_threaded("cf1", &opts) { Ok(()) => println!("cf1 created successfully"), Err(e) => { @@ -113,7 +113,8 @@ fn test_multi_threaded_column_family() { test_cf_common(&n); - // should be able to drop a cf + // Should be able to drop column families without a mutable reference + // to the db { let db = DB::open_cf_multi_threaded(&Options::default(), &n, &["cf1"]).unwrap(); match db.drop_cf_multi_threaded("cf1") { From 1be386d3d388901398b29e67ecc5f56d6f59f9cf Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Wed, 24 Mar 2021 23:22:55 +0900 Subject: [PATCH 13/30] Encode single/multi threading into types --- src/db.rs | 341 +++++++++++++++++++++++------------- src/db_iterator.rs | 55 +++--- src/lib.rs | 4 +- src/snapshot.rs | 60 ------- tests/test_column_family.rs | 6 - 5 files changed, 255 insertions(+), 211 deletions(-) diff --git a/src/db.rs b/src/db.rs index 9eaaa0c48..3309cd53e 100644 --- a/src/db.rs +++ b/src/db.rs @@ -40,23 +40,114 @@ enum ColumnFamilyHandles { SingleThread(BTreeMap), } +pub trait ThreadMode {} + +#[derive(Default)] +pub struct SingleThread { + cfs: BTreeMap, +} + +#[derive(Default)] +pub struct MultiThread { + cfs: RwLock>, +} + +impl ThreadMode for SingleThread {} + +impl SingleThread { + fn cf_handle<'a>(&'a self, name: &str) -> Option<&'a ColumnFamily> { + self.cfs.get(name) + } + + fn create<'a>( + &'a mut self, + name: String, + opts: &Options, + inner: *mut ffi::rocksdb_column_family_handle_t, + ) -> Result<(), Error> { + self.cfs.insert(name, ColumnFamily { inner }); + Ok(()) + } +} + +pub struct BoundColumnFamily<'a> { + pub(crate) inner: *mut ffi::rocksdb_column_family_handle_t, + cfs2: std::marker::PhantomData<&'a ()>, +} + +impl ThreadMode for MultiThread {} + +impl MultiThread { + fn cf_handle<'a>(&'a self, name: &str) -> Option> { + self.cfs + .read() + .unwrap() + .get(name) + .map(|cf| BoundColumnFamily { + inner: cf.inner, + cfs2: std::marker::PhantomData::default(), + }) + } + + fn create<'a>( + &'a self, + name: String, + opts: &Options, + inner: *mut ffi::rocksdb_column_family_handle_t, + ) -> Result<(), Error> { + self.cfs + .write() + .unwrap() + .insert(name, ColumnFamily { inner }); + Ok(()) + } +} /// A RocksDB database. /// /// See crate level documentation for a simple usage example. -pub struct DB { +pub struct DbWithThreadMode { pub(crate) inner: *mut ffi::rocksdb_t, cfs: ColumnFamilyHandles, + cfs2: T, // will replace with cfs path: PathBuf, } +pub trait WithDbInner { + fn inner(&self) -> *mut ffi::rocksdb_t; +} + +pub trait WithColumnFamilyInner { + fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t; +} + +impl<'a> WithColumnFamilyInner for &'a ColumnFamily { + fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t { + self.inner + } +} + +impl<'a> WithColumnFamilyInner for BoundColumnFamily<'a> { + fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t { + self.inner + } +} + +impl WithDbInner for DbWithThreadMode { + fn inner(&self) -> *mut ffi::rocksdb_t { + self.inner + } +} + +pub type DB = DbWithThreadMode; + // Safety note: auto-implementing Send on most db-related types is prevented by the inner FFI // pointer. In most cases, however, this pointer is Send-safe because it is never aliased and // rocksdb internally does not rely on thread-local information for its user-exposed types. -unsafe impl Send for DB {} +unsafe impl Send for DbWithThreadMode {} // Sync is similarly safe for many types because they do not expose interior mutability, and their // use within the rocksdb library is generally behind a const reference -unsafe impl Sync for DB {} +unsafe impl Sync for DbWithThreadMode {} // Specifies whether open DB for read only. enum AccessType<'a> { @@ -66,17 +157,17 @@ enum AccessType<'a> { WithTTL { ttl: Duration }, } -impl DB { +impl DbWithThreadMode { /// Opens a database with default options. - pub fn open_default>(path: P) -> Result { + pub fn open_default>(path: P) -> Result { let mut opts = Options::default(); opts.create_if_missing(true); - DB::open(&opts, path) + Self::open(&opts, path) } /// Opens the database with the specified options. - pub fn open>(opts: &Options, path: P) -> Result { - DB::open_cf(opts, path, None::<&str>) + pub fn open>(opts: &Options, path: P) -> Result { + Self::open_cf(opts, path, None::<&str>) } /// Opens the database for read only with the specified options. @@ -84,8 +175,8 @@ impl DB { opts: &Options, path: P, error_if_log_file_exist: bool, - ) -> Result { - DB::open_cf_for_read_only(opts, path, None::<&str>, error_if_log_file_exist) + ) -> Result { + Self::open_cf_for_read_only(opts, path, None::<&str>, error_if_log_file_exist) } /// Opens the database as a secondary. @@ -93,8 +184,8 @@ impl DB { opts: &Options, primary_path: P, secondary_path: P, - ) -> Result { - DB::open_cf_as_secondary(opts, primary_path, secondary_path, None::<&str>) + ) -> Result { + Self::open_cf_as_secondary(opts, primary_path, secondary_path, None::<&str>) } /// Opens the database with a Time to Live compaction filter. @@ -102,24 +193,25 @@ impl DB { opts: &Options, path: P, ttl: Duration, - ) -> Result { + ) -> Result { let c_path = to_cpath(&path)?; - let db = DB::open_raw(opts, &c_path, &AccessType::WithTTL { ttl })?; + let db = Self::open_raw(opts, &c_path, &AccessType::WithTTL { ttl })?; if db.is_null() { return Err(Error::new("Could not initialize database.".to_owned())); } - Ok(DB { + Ok(Self { inner: db, cfs: ColumnFamilyHandles::SingleThread(BTreeMap::new()), path: path.as_ref().to_path_buf(), + cfs2: T::default(), }) } /// Opens a database with the given database options and column family names. /// /// Column families opened using this function will be created with default `Options`. - pub fn open_cf(opts: &Options, path: P, cfs: I) -> Result + pub fn open_cf(opts: &Options, path: P, cfs: I) -> Result where P: AsRef, I: IntoIterator, @@ -129,14 +221,14 @@ impl DB { .into_iter() .map(|name| ColumnFamilyDescriptor::new(name.as_ref(), Options::default())); - DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, false) + Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, false) } /// Opens a database with the given database options and column family names /// with internal locking for column families. /// /// Column families opened using this function will be created with default `Options`. - pub fn open_cf_multi_threaded(opts: &Options, path: P, cfs: I) -> Result + pub fn open_cf_multi_threaded(opts: &Options, path: P, cfs: I) -> Result where P: AsRef, I: IntoIterator, @@ -146,7 +238,7 @@ impl DB { .into_iter() .map(|name| ColumnFamilyDescriptor::new(name.as_ref(), Options::default())); - DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, true) + Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, true) } /// Opens a database for read only with the given database options and column family names. @@ -155,7 +247,7 @@ impl DB { path: P, cfs: I, error_if_log_file_exist: bool, - ) -> Result + ) -> Result where P: AsRef, I: IntoIterator, @@ -165,7 +257,7 @@ impl DB { .into_iter() .map(|name| ColumnFamilyDescriptor::new(name.as_ref(), Options::default())); - DB::open_cf_descriptors_internal( + Self::open_cf_descriptors_internal( opts, path, cfs, @@ -182,7 +274,7 @@ impl DB { primary_path: P, secondary_path: P, cfs: I, - ) -> Result + ) -> Result where P: AsRef, I: IntoIterator, @@ -192,7 +284,7 @@ impl DB { .into_iter() .map(|name| ColumnFamilyDescriptor::new(name.as_ref(), Options::default())); - DB::open_cf_descriptors_internal( + Self::open_cf_descriptors_internal( opts, primary_path, cfs, @@ -204,12 +296,12 @@ impl DB { } /// Opens a database with the given database options and column family descriptors. - pub fn open_cf_descriptors(opts: &Options, path: P, cfs: I) -> Result + pub fn open_cf_descriptors(opts: &Options, path: P, cfs: I) -> Result where P: AsRef, I: IntoIterator, { - DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, false) + Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, false) } /// Opens a database with the given database options and column family descriptors, @@ -218,12 +310,12 @@ impl DB { opts: &Options, path: P, cfs: I, - ) -> Result + ) -> Result where P: AsRef, I: IntoIterator, { - DB::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, true) + Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, true) } /// Internal implementation for opening RocksDB. @@ -233,7 +325,7 @@ impl DB { cfs: I, access_type: &AccessType, is_multi_threaded: bool, - ) -> Result + ) -> Result where P: AsRef, I: IntoIterator, @@ -253,7 +345,7 @@ impl DB { let mut cf_map = BTreeMap::new(); if cfs.is_empty() { - db = DB::open_raw(opts, &cpath, access_type)?; + db = Self::open_raw(opts, &cpath, access_type)?; } else { let mut cfs_v = cfs; // Always open the default column family. @@ -280,7 +372,7 @@ impl DB { .map(|cf| cf.options.inner as *const _) .collect(); - db = DB::open_cf_raw( + db = Self::open_cf_raw( opts, &cpath, &cfs_v, @@ -317,10 +409,11 @@ impl DB { ColumnFamilyHandles::SingleThread(cf_map) }; - Ok(DB { + Ok(Self { inner: db, cfs, path: path.as_ref().to_path_buf(), + cfs2: T::default(), }) } @@ -732,41 +825,6 @@ impl DB { }) } - /// Creates column family with given name by internally locking the inner column - /// family map. This avoids needing `&mut self` reference, but is only safe to - /// use if the database was opened with multi-threaded config - pub fn create_cf_multi_threaded>( - &self, - name: N, - opts: &Options, - ) -> Result<(), Error> { - let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; - match &self.cfs { - ColumnFamilyHandles::MultiThread(cf_map) => cf_map - .write() - .unwrap() - .insert(name.as_ref().to_string(), Arc::new(ColumnFamily { inner })), - ColumnFamilyHandles::SingleThread(_) => { - panic!("Not available on SingleThread implementation"); - } - }; - Ok(()) - } - - /// Creates column family with given name and options - pub fn create_cf>(&mut self, name: N, opts: &Options) -> Result<(), Error> { - let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; - match &mut self.cfs { - ColumnFamilyHandles::MultiThread(_) => { - panic!("Not available on MultiThread implementation") - } - ColumnFamilyHandles::SingleThread(cf_map) => { - cf_map.insert(name.as_ref().to_string(), ColumnFamily { inner }); - } - } - Ok(()) - } - /// Drops the column family with the given name by internally locking the inner column /// family map. This avoids needing `&mut self` reference, but is only safe to use if /// the database was opened with multi-threaded config @@ -820,18 +878,7 @@ impl DB { } } - /// Returns the underlying column family handle, only safe to use if the database - /// was not configured to be multithreaded - pub fn cf_handle(&self, name: &str) -> Option<&ColumnFamily> { - match &self.cfs { - ColumnFamilyHandles::MultiThread(_) => { - panic!("Not available on MultiThread implementation") - } - ColumnFamilyHandles::SingleThread(cf_map) => cf_map.get(name), - } - } - - pub fn iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b> { + pub fn iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b, Self> { let readopts = ReadOptions::default(); self.iterator_opt(mode, readopts) } @@ -840,67 +887,64 @@ impl DB { &'a self, mode: IteratorMode, readopts: ReadOptions, - ) -> DBIterator<'b> { - DBIterator::new(self, readopts, mode) - } - - /// Opens an iterator using the provided ReadOptions. - /// This is used when you want to iterate over a specific ColumnFamily with a modified ReadOptions - pub fn iterator_cf_opt<'a: 'b, 'b>( - &'a self, - cf_handle: &ColumnFamily, - readopts: ReadOptions, - mode: IteratorMode, - ) -> DBIterator<'b> { - DBIterator::new_cf(self, cf_handle, readopts, mode) + ) -> DBIterator<'b, Self> { + //DBIterator::new(self, readopts, mode) + panic!() } /// Opens an iterator with `set_total_order_seek` enabled. /// This must be used to iterate across prefixes when `set_memtable_factory` has been called /// with a Hash-based implementation. - pub fn full_iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b> { - let mut opts = ReadOptions::default(); - opts.set_total_order_seek(true); - DBIterator::new(self, opts, mode) + pub fn full_iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b, Self> { + //let mut opts = ReadOptions::default(); + //opts.set_total_order_seek(true); + //DBIterator::new(self, opts, mode) + panic!() } - pub fn prefix_iterator<'a: 'b, 'b, P: AsRef<[u8]>>(&'a self, prefix: P) -> DBIterator<'b> { + pub fn prefix_iterator<'a: 'b, 'b, P: AsRef<[u8]>>( + &'a self, + prefix: P, + ) -> DBIterator<'b, Self> { let mut opts = ReadOptions::default(); opts.set_prefix_same_as_start(true); - DBIterator::new( - self, - opts, - IteratorMode::From(prefix.as_ref(), Direction::Forward), - ) + //DBIterator::new( + // self, + // opts, + // IteratorMode::From(prefix.as_ref(), Direction::Forward), + //) + panic!() } pub fn iterator_cf<'a: 'b, 'b>( &'a self, cf_handle: &ColumnFamily, mode: IteratorMode, - ) -> DBIterator<'b> { + ) -> DBIterator<'b, Self> { let opts = ReadOptions::default(); - DBIterator::new_cf(self, cf_handle, opts, mode) + //DBIterator::new_cf(self, cf_handle, opts, mode) + panic!() } pub fn full_iterator_cf<'a: 'b, 'b>( &'a self, cf_handle: &ColumnFamily, mode: IteratorMode, - ) -> DBIterator<'b> { + ) -> DBIterator<'b, Self> { let mut opts = ReadOptions::default(); opts.set_total_order_seek(true); - DBIterator::new_cf(self, cf_handle, opts, mode) + //DBIterator::new_cf(self, cf_handle, opts, mode) + panic!() } - pub fn prefix_iterator_cf<'a: 'b, 'b, P: AsRef<[u8]>>( + pub fn prefix_iterator_cf<'a, P: AsRef<[u8]>>( &'a self, cf_handle: &ColumnFamily, prefix: P, - ) -> DBIterator<'b> { + ) -> DBIterator<'a, Self> { let mut opts = ReadOptions::default(); opts.set_prefix_same_as_start(true); - DBIterator::new_cf( + DBIterator::<'a, Self>::new_cf( self, cf_handle, opts, @@ -909,20 +953,29 @@ impl DB { } /// Opens a raw iterator over the database, using the default read options - pub fn raw_iterator<'a: 'b, 'b>(&'a self) -> DBRawIterator<'b> { + pub fn raw_iterator<'a: 'b, 'b>(&'a self) -> DBRawIterator<'b, Self> { let opts = ReadOptions::default(); - DBRawIterator::new(self, opts) + //DBRawIterator::new(self, opts) + panic!() } /// Opens a raw iterator over the given column family, using the default read options - pub fn raw_iterator_cf<'a: 'b, 'b>(&'a self, cf_handle: &ColumnFamily) -> DBRawIterator<'b> { + pub fn raw_iterator_cf<'a: 'b, 'b>( + &'a self, + cf_handle: &ColumnFamily, + ) -> DBRawIterator<'b, Self> { let opts = ReadOptions::default(); - DBRawIterator::new_cf(self, cf_handle, opts) + //DBRawIterator::new_cf(self, cf_handle, opts) + panic!() } /// Opens a raw iterator over the database, using the given read options - pub fn raw_iterator_opt<'a: 'b, 'b>(&'a self, readopts: ReadOptions) -> DBRawIterator<'b> { - DBRawIterator::new(self, readopts) + pub fn raw_iterator_opt<'a: 'b, 'b>( + &'a self, + readopts: ReadOptions, + ) -> DBRawIterator<'b, Self> { + //DBRawIterator::new(self, readopts) + panic!() } /// Opens a raw iterator over the given column family, using the given read options @@ -930,12 +983,14 @@ impl DB { &'a self, cf_handle: &ColumnFamily, readopts: ReadOptions, - ) -> DBRawIterator<'b> { - DBRawIterator::new_cf(self, cf_handle, readopts) + ) -> DBRawIterator<'b, Self> { + //DBRawIterator::new_cf(self, cf_handle, readopts) + panic!() } pub fn snapshot(&self) -> Snapshot { - Snapshot::new(self) + //Snapshot::new(self) + panic!() } pub fn put_opt(&self, key: K, value: V, writeopts: &WriteOptions) -> Result<(), Error> @@ -1607,7 +1662,55 @@ impl DB { } } -impl Drop for DB { +impl DbWithThreadMode { + /// Creates column family with given name and options + pub fn create_cf>(&mut self, name: N, opts: &Options) -> Result<(), Error> { + let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; + self.cfs2.create(name.as_ref().to_string(), opts, inner) + } + /// Returns the underlying column family handle, only safe to use if the database + /// was not configured to be multithreaded + pub fn cf_handle<'a>(&'a self, name: &str) -> Option<&'a ColumnFamily> { + self.cfs2.cf_handle(name) + } + + /// Opens an iterator using the provided ReadOptions. + /// This is used when you want to iterate over a specific ColumnFamily with a modified ReadOptions + pub fn iterator_cf_opt<'a: 'b, 'b>( + &'a self, + cf_handle: &'a ColumnFamily, + readopts: ReadOptions, + mode: IteratorMode, + ) -> DBIterator<'b, Self> { + DBIterator::new_cf(self, cf_handle, readopts, mode) + } +} + +impl DbWithThreadMode { + /// Creates column family with given name and options + pub fn create_cf>(&self, name: N, opts: &Options) -> Result<(), Error> { + let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; + self.cfs2.create(name.as_ref().to_string(), opts, inner) + } + /// Returns the underlying column family handle, only safe to use if the database + /// was not configured to be multithreaded + pub fn cf_handle<'a>(&'a self, name: &str) -> Option> { + self.cfs2.cf_handle(name) + } + + /// Opens an iterator using the provided ReadOptions. + /// This is used when you want to iterate over a specific ColumnFamily with a modified ReadOptions + pub fn iterator_cf_opt<'a: 'b, 'b>( + &'a self, + cf_handle: BoundColumnFamily<'a>, + readopts: ReadOptions, + mode: IteratorMode, + ) -> DBIterator<'b, Self> { + DBIterator::new_cf(self, cf_handle, readopts, mode) + } +} + +impl Drop for DbWithThreadMode { fn drop(&mut self) { unsafe { match &self.cfs { @@ -1628,7 +1731,7 @@ impl Drop for DB { } } -impl fmt::Debug for DB { +impl fmt::Debug for DbWithThreadMode { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "RocksDB {{ path: {:?} }}", self.path()) } diff --git a/src/db_iterator.rs b/src/db_iterator.rs index e1098857b..0ec207451 100644 --- a/src/db_iterator.rs +++ b/src/db_iterator.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::db::WithDbInner; use crate::{ffi, ColumnFamily, Error, ReadOptions, WriteBatch, DB}; use libc::{c_char, c_uchar, size_t}; use std::marker::PhantomData; @@ -65,7 +66,7 @@ use std::slice; /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` -pub struct DBRawIterator<'a> { +pub struct DBRawIterator<'a, T: WithDbInner> { inner: *mut ffi::rocksdb_iterator_t, /// When iterate_upper_bound is set, the inner C iterator keeps a pointer to the upper bound @@ -73,28 +74,34 @@ pub struct DBRawIterator<'a> { /// iterator is being used. _readopts: ReadOptions, - db: PhantomData<&'a DB>, + db: PhantomData<&'a T>, } -impl<'a> DBRawIterator<'a> { - pub(crate) fn new(db: &DB, readopts: ReadOptions) -> DBRawIterator<'a> { +impl<'a, T: WithDbInner> DBRawIterator<'a, T> { + pub(crate) fn new(db: &T, readopts: ReadOptions) -> DBRawIterator<'a, T> { + panic!(); + /* unsafe { DBRawIterator { inner: ffi::rocksdb_create_iterator(db.inner, readopts.inner), _readopts: readopts, db: PhantomData, } - } + }*/ } - pub(crate) fn new_cf( - db: &DB, - cf_handle: &ColumnFamily, + pub(crate) fn new_cf( + db: &'a T, + cf_handle: U, readopts: ReadOptions, - ) -> DBRawIterator<'a> { + ) -> DBRawIterator<'a, T> { unsafe { DBRawIterator { - inner: ffi::rocksdb_create_iterator_cf(db.inner, readopts.inner, cf_handle.inner), + inner: ffi::rocksdb_create_iterator_cf( + WithDbInner::inner(db), + readopts.inner, + cf_handle.inner(), + ), _readopts: readopts, db: PhantomData, } @@ -323,7 +330,7 @@ impl<'a> DBRawIterator<'a> { } } -impl<'a> Drop for DBRawIterator<'a> { +impl<'a, T: WithDbInner> Drop for DBRawIterator<'a, T> { fn drop(&mut self) { unsafe { ffi::rocksdb_iter_destroy(self.inner); @@ -331,8 +338,8 @@ impl<'a> Drop for DBRawIterator<'a> { } } -unsafe impl<'a> Send for DBRawIterator<'a> {} -unsafe impl<'a> Sync for DBRawIterator<'a> {} +unsafe impl<'a, T: WithDbInner> Send for DBRawIterator<'a, T> {} +unsafe impl<'a, T: WithDbInner> Sync for DBRawIterator<'a, T> {} /// An iterator over a database or column family, with specifiable /// ranges and direction. @@ -365,8 +372,8 @@ unsafe impl<'a> Sync for DBRawIterator<'a> {} /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` -pub struct DBIterator<'a> { - raw: DBRawIterator<'a>, +pub struct DBIterator<'a, T: WithDbInner> { + raw: DBRawIterator<'a, T>, direction: Direction, just_seeked: bool, } @@ -384,8 +391,8 @@ pub enum IteratorMode<'a> { From(&'a [u8], Direction), } -impl<'a> DBIterator<'a> { - pub(crate) fn new(db: &DB, readopts: ReadOptions, mode: IteratorMode) -> DBIterator<'a> { +impl<'a, T: WithDbInner> DBIterator<'a, T> { + pub(crate) fn new(db: &T, readopts: ReadOptions, mode: IteratorMode) -> Self { let mut rv = DBIterator { raw: DBRawIterator::new(db, readopts), direction: Direction::Forward, // blown away by set_mode() @@ -395,12 +402,12 @@ impl<'a> DBIterator<'a> { rv } - pub(crate) fn new_cf( - db: &DB, - cf_handle: &ColumnFamily, + pub(crate) fn new_cf( + db: &'a T, + cf_handle: U, readopts: ReadOptions, mode: IteratorMode, - ) -> DBIterator<'a> { + ) -> Self { let mut rv = DBIterator { raw: DBRawIterator::new_cf(db, cf_handle, readopts), direction: Direction::Forward, // blown away by set_mode() @@ -444,7 +451,7 @@ impl<'a> DBIterator<'a> { } } -impl<'a> Iterator for DBIterator<'a> { +impl<'a, T: WithDbInner> Iterator for DBIterator<'a, T> { type Item = KVBytes; fn next(&mut self) -> Option { @@ -475,8 +482,8 @@ impl<'a> Iterator for DBIterator<'a> { } } -impl<'a> Into> for DBIterator<'a> { - fn into(self) -> DBRawIterator<'a> { +impl<'a, T: WithDbInner> Into> for DBIterator<'a, T> { + fn into(self) -> DBRawIterator<'a, T> { self.raw } } diff --git a/src/lib.rs b/src/lib.rs index 74a902d6f..b84977e3d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -177,8 +177,8 @@ mod test { } is_send::(); - is_send::>(); - is_send::>(); + is_send::>(); + is_send::>(); is_send::(); is_send::(); is_send::(); diff --git a/src/snapshot.rs b/src/snapshot.rs index 41b2482b7..0f8e1bc5c 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -45,67 +45,7 @@ impl<'a> Snapshot<'a> { } } - /// Creates an iterator over the data in this snapshot, using the default read options. - pub fn iterator(&self, mode: IteratorMode) -> DBIterator<'a> { - let readopts = ReadOptions::default(); - self.iterator_opt(mode, readopts) - } - - /// Creates an iterator over the data in this snapshot under the given column family, using - /// the default read options. - pub fn iterator_cf(&self, cf_handle: &ColumnFamily, mode: IteratorMode) -> DBIterator { - let readopts = ReadOptions::default(); - self.iterator_cf_opt(cf_handle, readopts, mode) - } - - /// Creates an iterator over the data in this snapshot, using the given read options. - pub fn iterator_opt(&self, mode: IteratorMode, mut readopts: ReadOptions) -> DBIterator<'a> { - readopts.set_snapshot(self); - DBIterator::new(self.db, readopts, mode) - } - - /// Creates an iterator over the data in this snapshot under the given column family, using - /// the given read options. - pub fn iterator_cf_opt( - &self, - cf_handle: &ColumnFamily, - mut readopts: ReadOptions, - mode: IteratorMode, - ) -> DBIterator { - readopts.set_snapshot(self); - DBIterator::new_cf(self.db, cf_handle, readopts, mode) - } - /// Creates a raw iterator over the data in this snapshot, using the default read options. - pub fn raw_iterator(&self) -> DBRawIterator { - let readopts = ReadOptions::default(); - self.raw_iterator_opt(readopts) - } - - /// Creates a raw iterator over the data in this snapshot under the given column family, using - /// the default read options. - pub fn raw_iterator_cf(&self, cf_handle: &ColumnFamily) -> DBRawIterator { - let readopts = ReadOptions::default(); - self.raw_iterator_cf_opt(cf_handle, readopts) - } - - /// Creates a raw iterator over the data in this snapshot, using the given read options. - pub fn raw_iterator_opt(&self, mut readopts: ReadOptions) -> DBRawIterator { - readopts.set_snapshot(self); - DBRawIterator::new(self.db, readopts) - } - - /// Creates a raw iterator over the data in this snapshot under the given column family, using - /// the given read options. - pub fn raw_iterator_cf_opt( - &self, - cf_handle: &ColumnFamily, - mut readopts: ReadOptions, - ) -> DBRawIterator { - readopts.set_snapshot(self); - DBRawIterator::new_cf(self.db, cf_handle, readopts) - } - /// Returns the bytes associated with a key value with default read options. pub fn get>(&self, key: K) -> Result>, Error> { let readopts = ReadOptions::default(); diff --git a/tests/test_column_family.rs b/tests/test_column_family.rs index bda52f32b..4fcce8488 100644 --- a/tests/test_column_family.rs +++ b/tests/test_column_family.rs @@ -103,12 +103,6 @@ fn test_multi_threaded_column_family() { opts.set_merge_operator_associative("test operator", test_provided_merge); let db = DB::open_cf_multi_threaded(&opts, &n, None::<&str>).unwrap(); let opts = Options::default(); - match db.create_cf_multi_threaded("cf1", &opts) { - Ok(()) => println!("cf1 created successfully"), - Err(e) => { - panic!("could not create column family: {}", e); - } - } } test_cf_common(&n); From 78aeab5138b600722961a5b6252b9b0de6d461d5 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Thu, 25 Mar 2021 23:56:39 +0900 Subject: [PATCH 14/30] Finish off implementing and cleaning --- .github/workflows/rust.yml | 7 +- Cargo.toml | 1 + src/column_family.rs | 29 +- src/db.rs | 508 +++++++++++++++--------------------- src/db_iterator.rs | 58 ++-- src/db_options.rs | 3 +- src/lib.rs | 10 +- src/snapshot.rs | 85 +++++- tests/test_column_family.rs | 70 ++--- tests/test_db.rs | 31 ++- 10 files changed, 402 insertions(+), 400 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 3c75a87f5..81b4ebb8f 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -88,7 +88,12 @@ jobs: with: command: test args: --manifest-path=librocksdb-sys/Cargo.toml - - name: Run rocksdb tests + - name: Run rocksdb tests (single threaded cf alterations) uses: actions-rs/cargo@v1 with: command: test + - name: Run rocksdb tests (multi threaded cf alterations) + uses: actions-rs/cargo@v1 + with: + command: test + args: --features multi-threaded-as-default diff --git a/Cargo.toml b/Cargo.toml index edb395bdc..361e90776 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ lz4 = ["librocksdb-sys/lz4"] zstd = ["librocksdb-sys/zstd"] zlib = ["librocksdb-sys/zlib"] bzip2 = ["librocksdb-sys/bzip2"] +multi-threaded-as-default = [] [dependencies] libc = "0.2" diff --git a/src/column_family.rs b/src/column_family.rs index 2c1b50531..3f15241c0 100644 --- a/src/column_family.rs +++ b/src/column_family.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{ffi, Options}; +use crate::{db::MultiThreaded, ffi, Options}; /// The name of the default column family. /// @@ -48,3 +48,30 @@ pub struct ColumnFamily { } unsafe impl Send for ColumnFamily {} + +/// A specialized opaque type used to represent a column family. Used for multi-threaded +/// mode. Clone (and Copy) is derived to behave like &ColumnFamily (used for +/// single-threaded). Clone/Copy is safe because this lifetime is bound to DB like +/// iterators/snapshots. On top of it, this is cheap and small as &ColumnFamily because +/// this only has a single pointer-wide field. +#[derive(Clone, Copy)] +pub struct BoundColumnFamily<'a> { + pub(crate) inner: *mut ffi::rocksdb_column_family_handle_t, + pub(crate) multi_threaded_cfs: std::marker::PhantomData<&'a MultiThreaded>, +} + +pub trait ColumnFamilyRef { + fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t; +} + +impl<'a> ColumnFamilyRef for &'a ColumnFamily { + fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t { + self.inner + } +} + +impl<'a> ColumnFamilyRef for BoundColumnFamily<'a> { + fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t { + self.inner + } +} diff --git a/src/db.rs b/src/db.rs index 3309cd53e..eaa2268b0 100644 --- a/src/db.rs +++ b/src/db.rs @@ -14,6 +14,8 @@ // use crate::{ + column_family::BoundColumnFamily, + column_family::ColumnFamilyRef, ffi, ffi_util::{from_cstr, opt_bytes_to_ptr, raw_data, to_cpath}, ColumnFamily, ColumnFamilyDescriptor, CompactOptions, DBIterator, DBPinnableSlice, @@ -27,118 +29,114 @@ use std::collections::BTreeMap; use std::ffi::{CStr, CString}; use std::fmt; use std::fs; +use std::marker::PhantomData; use std::path::Path; use std::path::PathBuf; use std::ptr; use std::slice; use std::str; -use std::sync::{Arc, RwLock}; +use std::sync::RwLock; use std::time::Duration; -enum ColumnFamilyHandles { - MultiThread(RwLock>>), - SingleThread(BTreeMap), +// Marker trait to specify single or multi threaded column family alterations for DB +// Also, this is minimum common API sharable between SingleThreaded and +// MultiThreaded. Others differ in self mutability and return type. +pub trait ThreadMode { + fn new(cf_map: BTreeMap) -> Self; + unsafe fn cf_drop_all(&mut self); } -pub trait ThreadMode {} - -#[derive(Default)] -pub struct SingleThread { - cfs: BTreeMap, +pub struct SingleThreaded { + map: BTreeMap, } -#[derive(Default)] -pub struct MultiThread { - cfs: RwLock>, +pub struct MultiThreaded { + map: RwLock>, } -impl ThreadMode for SingleThread {} - -impl SingleThread { - fn cf_handle<'a>(&'a self, name: &str) -> Option<&'a ColumnFamily> { - self.cfs.get(name) +impl ThreadMode for SingleThreaded { + fn new(cf_map: BTreeMap) -> Self { + Self { map: cf_map } } - fn create<'a>( - &'a mut self, - name: String, - opts: &Options, - inner: *mut ffi::rocksdb_column_family_handle_t, - ) -> Result<(), Error> { - self.cfs.insert(name, ColumnFamily { inner }); - Ok(()) + unsafe fn cf_drop_all(&mut self) { + for cf in self.map.values() { + ffi::rocksdb_column_family_handle_destroy(cf.inner); + } } } -pub struct BoundColumnFamily<'a> { - pub(crate) inner: *mut ffi::rocksdb_column_family_handle_t, - cfs2: std::marker::PhantomData<&'a ()>, -} - -impl ThreadMode for MultiThread {} - -impl MultiThread { - fn cf_handle<'a>(&'a self, name: &str) -> Option> { - self.cfs - .read() - .unwrap() - .get(name) - .map(|cf| BoundColumnFamily { - inner: cf.inner, - cfs2: std::marker::PhantomData::default(), - }) +impl ThreadMode for MultiThreaded { + fn new(cf_map: BTreeMap) -> Self { + Self { + map: RwLock::new(cf_map), + } } - fn create<'a>( - &'a self, - name: String, - opts: &Options, - inner: *mut ffi::rocksdb_column_family_handle_t, - ) -> Result<(), Error> { - self.cfs - .write() - .unwrap() - .insert(name, ColumnFamily { inner }); - Ok(()) + unsafe fn cf_drop_all(&mut self) { + for cf in self.map.read().unwrap().values() { + ffi::rocksdb_column_family_handle_destroy(cf.inner); + } } } + /// A RocksDB database. /// /// See crate level documentation for a simple usage example. -pub struct DbWithThreadMode { +pub struct DbWithThreadMode { pub(crate) inner: *mut ffi::rocksdb_t, - cfs: ColumnFamilyHandles, - cfs2: T, // will replace with cfs + cfs: T, // Column families are held differently depending on thread mode path: PathBuf, } -pub trait WithDbInner { +pub trait InternalDbAdapter { fn inner(&self) -> *mut ffi::rocksdb_t; -} -pub trait WithColumnFamilyInner { - fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t; + fn get_opt>( + &self, + key: K, + readopts: &ReadOptions, + ) -> Result>, Error>; + + fn get_cf_opt>( + &self, + cf: impl ColumnFamilyRef, + key: K, + readopts: &ReadOptions, + ) -> Result>, Error>; } -impl<'a> WithColumnFamilyInner for &'a ColumnFamily { - fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t { +impl InternalDbAdapter for DbWithThreadMode { + fn inner(&self) -> *mut ffi::rocksdb_t { self.inner } -} -impl<'a> WithColumnFamilyInner for BoundColumnFamily<'a> { - fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t { - self.inner + fn get_opt>( + &self, + key: K, + readopts: &ReadOptions, + ) -> Result>, Error> { + self.get_opt(key, readopts) } -} -impl WithDbInner for DbWithThreadMode { - fn inner(&self) -> *mut ffi::rocksdb_t { - self.inner + fn get_cf_opt>( + &self, + cf: impl ColumnFamilyRef, + key: K, + readopts: &ReadOptions, + ) -> Result>, Error> { + self.get_cf_opt(cf, key, readopts) } } -pub type DB = DbWithThreadMode; +/// DB struct for single-threaded column family creations/deletions +/// Note: Previously this was direct struct; type-aliased for compatibility. Directly +/// use DbWithThreadMode for multi-threaded column family alternations. +#[cfg(not(feature = "multi-threaded-as-default"))] +pub type DB = DbWithThreadMode; + +#[cfg(feature = "multi-threaded-as-default")] +pub type DB = DbWithThreadMode; // Safety note: auto-implementing Send on most db-related types is prevented by the inner FFI // pointer. In most cases, however, this pointer is Send-safe because it is never aliased and @@ -157,7 +155,7 @@ enum AccessType<'a> { WithTTL { ttl: Duration }, } -impl DbWithThreadMode { +impl DbWithThreadMode { /// Opens a database with default options. pub fn open_default>(path: P) -> Result { let mut opts = Options::default(); @@ -202,9 +200,8 @@ impl DbWithThreadMode { Ok(Self { inner: db, - cfs: ColumnFamilyHandles::SingleThread(BTreeMap::new()), + cfs: T::new(BTreeMap::new()), path: path.as_ref().to_path_buf(), - cfs2: T::default(), }) } @@ -221,24 +218,7 @@ impl DbWithThreadMode { .into_iter() .map(|name| ColumnFamilyDescriptor::new(name.as_ref(), Options::default())); - Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, false) - } - - /// Opens a database with the given database options and column family names - /// with internal locking for column families. - /// - /// Column families opened using this function will be created with default `Options`. - pub fn open_cf_multi_threaded(opts: &Options, path: P, cfs: I) -> Result - where - P: AsRef, - I: IntoIterator, - N: AsRef, - { - let cfs = cfs - .into_iter() - .map(|name| ColumnFamilyDescriptor::new(name.as_ref(), Options::default())); - - Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, true) + Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite) } /// Opens a database for read only with the given database options and column family names. @@ -264,7 +244,6 @@ impl DbWithThreadMode { &AccessType::ReadOnly { error_if_log_file_exist, }, - false, ) } @@ -291,7 +270,6 @@ impl DbWithThreadMode { &AccessType::Secondary { secondary_path: secondary_path.as_ref(), }, - false, ) } @@ -301,21 +279,7 @@ impl DbWithThreadMode { P: AsRef, I: IntoIterator, { - Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, false) - } - - /// Opens a database with the given database options and column family descriptors, - /// with internal locking for column families - pub fn open_cf_descriptors_multi_threaded( - opts: &Options, - path: P, - cfs: I, - ) -> Result - where - P: AsRef, - I: IntoIterator, - { - Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite, true) + Self::open_cf_descriptors_internal(opts, path, cfs, &AccessType::ReadWrite) } /// Internal implementation for opening RocksDB. @@ -324,7 +288,6 @@ impl DbWithThreadMode { path: P, cfs: I, access_type: &AccessType, - is_multi_threaded: bool, ) -> Result where P: AsRef, @@ -398,22 +361,10 @@ impl DbWithThreadMode { return Err(Error::new("Could not initialize database.".to_owned())); } - let cfs = if is_multi_threaded { - ColumnFamilyHandles::MultiThread(RwLock::new( - cf_map - .into_iter() - .map(|(name, cf)| (name, Arc::new(cf))) - .collect(), - )) - } else { - ColumnFamilyHandles::SingleThread(cf_map) - }; - Ok(Self { inner: db, - cfs, path: path.as_ref().to_path_buf(), - cfs2: T::default(), + cfs: T::new(cf_map), }) } @@ -552,16 +503,24 @@ impl DbWithThreadMode { } /// Flushes database memtables to SST files on the disk for a given column family. - pub fn flush_cf_opt(&self, cf: &ColumnFamily, flushopts: &FlushOptions) -> Result<(), Error> { + pub fn flush_cf_opt( + &self, + cf: impl ColumnFamilyRef, + flushopts: &FlushOptions, + ) -> Result<(), Error> { unsafe { - ffi_try!(ffi::rocksdb_flush_cf(self.inner, flushopts.inner, cf.inner)); + ffi_try!(ffi::rocksdb_flush_cf( + self.inner, + flushopts.inner, + cf.inner() + )); } Ok(()) } /// Flushes database memtables to SST files on the disk for a given column family using default /// options. - pub fn flush_cf(&self, cf: &ColumnFamily) -> Result<(), Error> { + pub fn flush_cf(&self, cf: impl ColumnFamilyRef) -> Result<(), Error> { self.flush_cf_opt(cf, &FlushOptions::default()) } @@ -606,7 +565,7 @@ impl DbWithThreadMode { /// [`get_pinned_cf_opt`](#method.get_pinned_cf_opt) to avoid unnecessary memory. pub fn get_cf_opt>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, key: K, readopts: &ReadOptions, ) -> Result>, Error> { @@ -619,7 +578,7 @@ impl DbWithThreadMode { /// [`get_pinned_cf`](#method.get_pinned_cf) to avoid unnecessary memory. pub fn get_cf>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, key: K, ) -> Result>, Error> { self.get_cf_opt(cf, key.as_ref(), &ReadOptions::default()) @@ -668,7 +627,7 @@ impl DbWithThreadMode { /// allows specifying ColumnFamily pub fn get_pinned_cf_opt>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, key: K, readopts: &ReadOptions, ) -> Result, Error> { @@ -685,7 +644,7 @@ impl DbWithThreadMode { let val = ffi_try!(ffi::rocksdb_get_pinned_cf( self.inner, readopts.inner, - cf.inner, + cf.inner(), key.as_ptr() as *const c_char, key.len() as size_t, )); @@ -702,7 +661,7 @@ impl DbWithThreadMode { /// leverages default options. pub fn get_pinned_cf>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, key: K, ) -> Result, Error> { self.get_pinned_cf_opt(cf, key, &ReadOptions::default()) @@ -751,23 +710,25 @@ impl DbWithThreadMode { } /// Return the values associated with the given keys and column families. - pub fn multi_get_cf<'c, K, I>(&self, keys: I) -> Result>, Error> + pub fn multi_get_cf(&self, keys: I) -> Result>, Error> where K: AsRef<[u8]>, - I: IntoIterator, + I: IntoIterator, + W: ColumnFamilyRef, { self.multi_get_cf_opt(keys, &ReadOptions::default()) } /// Return the values associated with the given keys and column families using read options. - pub fn multi_get_cf_opt<'c, K, I>( + pub fn multi_get_cf_opt( &self, keys: I, readopts: &ReadOptions, ) -> Result>, Error> where K: AsRef<[u8]>, - I: IntoIterator, + I: IntoIterator, + W: ColumnFamilyRef, { let mut boxed_keys: Vec> = Vec::new(); let mut keys_sizes = Vec::new(); @@ -783,7 +744,7 @@ impl DbWithThreadMode { .collect(); let ptr_cfs: Vec<_> = column_families .iter() - .map(|c| c.inner as *const _) + .map(|c| c.inner() as *const _) .collect(); let mut values = vec![ptr::null_mut(); boxed_keys.len()]; @@ -825,59 +786,6 @@ impl DbWithThreadMode { }) } - /// Drops the column family with the given name by internally locking the inner column - /// family map. This avoids needing `&mut self` reference, but is only safe to use if - /// the database was opened with multi-threaded config - pub fn drop_cf_multi_threaded(&self, name: &str) -> Result<(), Error> { - let inner = self.inner; - match &self.cfs { - ColumnFamilyHandles::MultiThread(cf_map) => { - if let Some(cf) = cf_map.write().unwrap().remove(name) { - unsafe { - ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); - } - Ok(()) - } else { - Err(Error::new(format!("Invalid column family: {}", name))) - } - } - ColumnFamilyHandles::SingleThread(_) => { - panic!("Not available on SingleThread implementation") - } - } - } - - /// Drops the column family with the given name - pub fn drop_cf(&mut self, name: &str) -> Result<(), Error> { - let inner = self.inner; - match &mut self.cfs { - ColumnFamilyHandles::MultiThread(_) => { - panic!("Not available on MultiThread implementation") - } - ColumnFamilyHandles::SingleThread(cf_map) => { - if let Some(cf) = cf_map.remove(name) { - unsafe { - ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); - } - Ok(()) - } else { - Err(Error::new(format!("Invalid column family: {}", name))) - } - } - } - } - - /// Returns the underlying column family handle, only safe to use if the database - /// was configured to be multithreaded - pub fn cf_handle_multi_threaded(&self, name: &str) -> Option> { - match &self.cfs { - ColumnFamilyHandles::MultiThread(cf_map) => cf_map.write().unwrap().get(name).cloned(), - ColumnFamilyHandles::SingleThread(_) => { - panic!("Not available on SingleThread implementation") - } - } - } - pub fn iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b, Self> { let readopts = ReadOptions::default(); self.iterator_opt(mode, readopts) @@ -888,18 +796,27 @@ impl DbWithThreadMode { mode: IteratorMode, readopts: ReadOptions, ) -> DBIterator<'b, Self> { - //DBIterator::new(self, readopts, mode) - panic!() + DBIterator::new(self, readopts, mode) + } + + /// Opens an iterator using the provided ReadOptions. + /// This is used when you want to iterate over a specific ColumnFamily with a modified ReadOptions + pub fn iterator_cf_opt<'a: 'b, 'b>( + &'a self, + cf_handle: impl ColumnFamilyRef, + readopts: ReadOptions, + mode: IteratorMode, + ) -> DBIterator<'b, Self> { + DBIterator::new_cf(self, cf_handle.inner(), readopts, mode) } /// Opens an iterator with `set_total_order_seek` enabled. /// This must be used to iterate across prefixes when `set_memtable_factory` has been called /// with a Hash-based implementation. pub fn full_iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b, Self> { - //let mut opts = ReadOptions::default(); - //opts.set_total_order_seek(true); - //DBIterator::new(self, opts, mode) - panic!() + let mut opts = ReadOptions::default(); + opts.set_total_order_seek(true); + DBIterator::new(self, opts, mode) } pub fn prefix_iterator<'a: 'b, 'b, P: AsRef<[u8]>>( @@ -908,45 +825,42 @@ impl DbWithThreadMode { ) -> DBIterator<'b, Self> { let mut opts = ReadOptions::default(); opts.set_prefix_same_as_start(true); - //DBIterator::new( - // self, - // opts, - // IteratorMode::From(prefix.as_ref(), Direction::Forward), - //) - panic!() + DBIterator::new( + self, + opts, + IteratorMode::From(prefix.as_ref(), Direction::Forward), + ) } pub fn iterator_cf<'a: 'b, 'b>( &'a self, - cf_handle: &ColumnFamily, + cf_handle: impl ColumnFamilyRef, mode: IteratorMode, ) -> DBIterator<'b, Self> { let opts = ReadOptions::default(); - //DBIterator::new_cf(self, cf_handle, opts, mode) - panic!() + DBIterator::new_cf(self, cf_handle.inner(), opts, mode) } pub fn full_iterator_cf<'a: 'b, 'b>( &'a self, - cf_handle: &ColumnFamily, + cf_handle: impl ColumnFamilyRef, mode: IteratorMode, ) -> DBIterator<'b, Self> { let mut opts = ReadOptions::default(); opts.set_total_order_seek(true); - //DBIterator::new_cf(self, cf_handle, opts, mode) - panic!() + DBIterator::new_cf(self, cf_handle.inner(), opts, mode) } pub fn prefix_iterator_cf<'a, P: AsRef<[u8]>>( &'a self, - cf_handle: &ColumnFamily, + cf_handle: impl ColumnFamilyRef, prefix: P, ) -> DBIterator<'a, Self> { let mut opts = ReadOptions::default(); opts.set_prefix_same_as_start(true); DBIterator::<'a, Self>::new_cf( self, - cf_handle, + cf_handle.inner(), opts, IteratorMode::From(prefix.as_ref(), Direction::Forward), ) @@ -955,18 +869,16 @@ impl DbWithThreadMode { /// Opens a raw iterator over the database, using the default read options pub fn raw_iterator<'a: 'b, 'b>(&'a self) -> DBRawIterator<'b, Self> { let opts = ReadOptions::default(); - //DBRawIterator::new(self, opts) - panic!() + DBRawIterator::new(self, opts) } /// Opens a raw iterator over the given column family, using the default read options pub fn raw_iterator_cf<'a: 'b, 'b>( &'a self, - cf_handle: &ColumnFamily, + cf_handle: impl ColumnFamilyRef, ) -> DBRawIterator<'b, Self> { let opts = ReadOptions::default(); - //DBRawIterator::new_cf(self, cf_handle, opts) - panic!() + DBRawIterator::new_cf(self, cf_handle.inner(), opts) } /// Opens a raw iterator over the database, using the given read options @@ -974,23 +886,20 @@ impl DbWithThreadMode { &'a self, readopts: ReadOptions, ) -> DBRawIterator<'b, Self> { - //DBRawIterator::new(self, readopts) - panic!() + DBRawIterator::new(self, readopts) } /// Opens a raw iterator over the given column family, using the given read options pub fn raw_iterator_cf_opt<'a: 'b, 'b>( &'a self, - cf_handle: &ColumnFamily, + cf_handle: impl ColumnFamilyRef, readopts: ReadOptions, ) -> DBRawIterator<'b, Self> { - //DBRawIterator::new_cf(self, cf_handle, readopts) - panic!() + DBRawIterator::new_cf(self, cf_handle.inner(), readopts) } - pub fn snapshot(&self) -> Snapshot { - //Snapshot::new(self) - panic!() + pub fn snapshot(&self) -> Snapshot { + Snapshot::::new(self) } pub fn put_opt(&self, key: K, value: V, writeopts: &WriteOptions) -> Result<(), Error> @@ -1016,7 +925,7 @@ impl DbWithThreadMode { pub fn put_cf_opt( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, key: K, value: V, writeopts: &WriteOptions, @@ -1032,7 +941,7 @@ impl DbWithThreadMode { ffi_try!(ffi::rocksdb_put_cf( self.inner, writeopts.inner, - cf.inner, + cf.inner(), key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, @@ -1065,7 +974,7 @@ impl DbWithThreadMode { pub fn merge_cf_opt( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, key: K, value: V, writeopts: &WriteOptions, @@ -1081,7 +990,7 @@ impl DbWithThreadMode { ffi_try!(ffi::rocksdb_merge_cf( self.inner, writeopts.inner, - cf.inner, + cf.inner(), key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, @@ -1111,7 +1020,7 @@ impl DbWithThreadMode { pub fn delete_cf_opt>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, key: K, writeopts: &WriteOptions, ) -> Result<(), Error> { @@ -1121,7 +1030,7 @@ impl DbWithThreadMode { ffi_try!(ffi::rocksdb_delete_cf( self.inner, writeopts.inner, - cf.inner, + cf.inner(), key.as_ptr() as *const c_char, key.len() as size_t, )); @@ -1132,7 +1041,7 @@ impl DbWithThreadMode { /// Removes the database entries in the range `["from", "to")` using given write options. pub fn delete_range_cf_opt>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, from: K, to: K, writeopts: &WriteOptions, @@ -1144,7 +1053,7 @@ impl DbWithThreadMode { ffi_try!(ffi::rocksdb_delete_range_cf( self.inner, writeopts.inner, - cf.inner, + cf.inner(), from.as_ptr() as *const c_char, from.len() as size_t, to.as_ptr() as *const c_char, @@ -1162,7 +1071,7 @@ impl DbWithThreadMode { self.put_opt(key.as_ref(), value.as_ref(), &WriteOptions::default()) } - pub fn put_cf(&self, cf: &ColumnFamily, key: K, value: V) -> Result<(), Error> + pub fn put_cf(&self, cf: impl ColumnFamilyRef, key: K, value: V) -> Result<(), Error> where K: AsRef<[u8]>, V: AsRef<[u8]>, @@ -1178,7 +1087,7 @@ impl DbWithThreadMode { self.merge_opt(key.as_ref(), value.as_ref(), &WriteOptions::default()) } - pub fn merge_cf(&self, cf: &ColumnFamily, key: K, value: V) -> Result<(), Error> + pub fn merge_cf(&self, cf: impl ColumnFamilyRef, key: K, value: V) -> Result<(), Error> where K: AsRef<[u8]>, V: AsRef<[u8]>, @@ -1190,14 +1099,14 @@ impl DbWithThreadMode { self.delete_opt(key.as_ref(), &WriteOptions::default()) } - pub fn delete_cf>(&self, cf: &ColumnFamily, key: K) -> Result<(), Error> { + pub fn delete_cf>(&self, cf: impl ColumnFamilyRef, key: K) -> Result<(), Error> { self.delete_cf_opt(cf, key.as_ref(), &WriteOptions::default()) } /// Removes the database entries in the range `["from", "to")` using default write options. pub fn delete_range_cf>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, from: K, to: K, ) -> Result<(), Error> { @@ -1246,7 +1155,7 @@ impl DbWithThreadMode { /// given column family. This is not likely to be needed for typical usage. pub fn compact_range_cf, E: AsRef<[u8]>>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, start: Option, end: Option, ) { @@ -1256,7 +1165,7 @@ impl DbWithThreadMode { ffi::rocksdb_compact_range_cf( self.inner, - cf.inner, + cf.inner(), opt_bytes_to_ptr(start), start.map_or(0, |s| s.len()) as size_t, opt_bytes_to_ptr(end), @@ -1268,7 +1177,7 @@ impl DbWithThreadMode { /// Same as `compact_range_cf` but with custom options. pub fn compact_range_cf_opt, E: AsRef<[u8]>>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, start: Option, end: Option, opts: &CompactOptions, @@ -1279,7 +1188,7 @@ impl DbWithThreadMode { ffi::rocksdb_compact_range_cf_opt( self.inner, - cf.inner, + cf.inner(), opts.inner, opt_bytes_to_ptr(start), start.map_or(0, |s| s.len()) as size_t, @@ -1307,7 +1216,7 @@ impl DbWithThreadMode { pub fn set_options_cf( &self, - cf_handle: &ColumnFamily, + cf: impl ColumnFamilyRef, opts: &[(&str, &str)], ) -> Result<(), Error> { let copts = convert_options(opts)?; @@ -1317,7 +1226,7 @@ impl DbWithThreadMode { unsafe { ffi_try!(ffi::rocksdb_set_options_cf( self.inner, - cf_handle.inner, + cf.inner(), count, cnames.as_ptr(), cvalues.as_ptr(), @@ -1368,7 +1277,7 @@ impl DbWithThreadMode { /// [here](https://github.com/facebook/rocksdb/blob/08809f5e6cd9cc4bc3958dd4d59457ae78c76660/include/rocksdb/db.h#L428-L634). pub fn property_value_cf( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, name: &str, ) -> Result, Error> { let prop_name = match CString::new(name) { @@ -1382,7 +1291,7 @@ impl DbWithThreadMode { }; unsafe { - let value = ffi::rocksdb_property_value_cf(self.inner, cf.inner, prop_name.as_ptr()); + let value = ffi::rocksdb_property_value_cf(self.inner, cf.inner(), prop_name.as_ptr()); if value.is_null() { return Ok(None); } @@ -1426,7 +1335,7 @@ impl DbWithThreadMode { /// [here](https://github.com/facebook/rocksdb/blob/08809f5e6cd9cc4bc3958dd4d59457ae78c76660/include/rocksdb/db.h#L654-L689). pub fn property_int_value_cf( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, name: &str, ) -> Result, Error> { match self.property_value_cf(cf, name) { @@ -1503,17 +1412,17 @@ impl DbWithThreadMode { /// with default opts pub fn ingest_external_file_cf>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, paths: Vec

, ) -> Result<(), Error> { let opts = IngestExternalFileOptions::default(); - self.ingest_external_file_cf_opts(&cf, &opts, paths) + self.ingest_external_file_cf_opts(cf, &opts, paths) } /// Loads a list of external SST files created with SstFileWriter into the DB for given Column Family pub fn ingest_external_file_cf_opts>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, opts: &IngestExternalFileOptions, paths: Vec

, ) -> Result<(), Error> { @@ -1524,7 +1433,7 @@ impl DbWithThreadMode { let cpaths: Vec<_> = paths_v.iter().map(|path| path.as_ptr()).collect(); - self.ingest_external_file_raw_cf(&cf, &opts, &paths_v, &cpaths) + self.ingest_external_file_raw_cf(cf, &opts, &paths_v, &cpaths) } fn ingest_external_file_raw( @@ -1546,7 +1455,7 @@ impl DbWithThreadMode { fn ingest_external_file_raw_cf( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, opts: &IngestExternalFileOptions, paths_v: &[CString], cpaths: &[*const c_char], @@ -1554,7 +1463,7 @@ impl DbWithThreadMode { unsafe { ffi_try!(ffi::rocksdb_ingest_external_file_cf( self.inner, - cf.inner, + cf.inner(), cpaths.as_ptr(), paths_v.len(), opts.inner as *const _ @@ -1635,7 +1544,7 @@ impl DbWithThreadMode { /// Same as `delete_file_in_range` but only for specific column family pub fn delete_file_in_range_cf>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, from: K, to: K, ) -> Result<(), Error> { @@ -1644,7 +1553,7 @@ impl DbWithThreadMode { unsafe { ffi_try!(ffi::rocksdb_delete_file_in_range_cf( self.inner, - cf.inner, + cf.inner(), from.as_ptr() as *const c_char, from.len() as size_t, to.as_ptr() as *const c_char, @@ -1662,76 +1571,85 @@ impl DbWithThreadMode { } } -impl DbWithThreadMode { +impl DbWithThreadMode { /// Creates column family with given name and options pub fn create_cf>(&mut self, name: N, opts: &Options) -> Result<(), Error> { let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; - self.cfs2.create(name.as_ref().to_string(), opts, inner) + self.cfs + .map + .insert(name.as_ref().to_string(), ColumnFamily { inner }); + Ok(()) } - /// Returns the underlying column family handle, only safe to use if the database - /// was not configured to be multithreaded - pub fn cf_handle<'a>(&'a self, name: &str) -> Option<&'a ColumnFamily> { - self.cfs2.cf_handle(name) + + /// Drops the column family with the given name + pub fn drop_cf(&mut self, name: &str) -> Result<(), Error> { + let inner = self.inner; + if let Some(cf) = self.cfs.map.remove(name) { + unsafe { + ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); + } + Ok(()) + } else { + Err(Error::new(format!("Invalid column family: {}", name))) + } } - /// Opens an iterator using the provided ReadOptions. - /// This is used when you want to iterate over a specific ColumnFamily with a modified ReadOptions - pub fn iterator_cf_opt<'a: 'b, 'b>( - &'a self, - cf_handle: &'a ColumnFamily, - readopts: ReadOptions, - mode: IteratorMode, - ) -> DBIterator<'b, Self> { - DBIterator::new_cf(self, cf_handle, readopts, mode) + /// Returns the underlying column family handle + pub fn cf_handle<'a>(&'a self, name: &str) -> Option<&'a ColumnFamily> { + self.cfs.map.get(name) } } -impl DbWithThreadMode { +impl DbWithThreadMode { /// Creates column family with given name and options pub fn create_cf>(&self, name: N, opts: &Options) -> Result<(), Error> { let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; - self.cfs2.create(name.as_ref().to_string(), opts, inner) + self.cfs + .map + .write() + .unwrap() + .insert(name.as_ref().to_string(), ColumnFamily { inner }); + Ok(()) } - /// Returns the underlying column family handle, only safe to use if the database - /// was not configured to be multithreaded - pub fn cf_handle<'a>(&'a self, name: &str) -> Option> { - self.cfs2.cf_handle(name) + + /// Drops the column family with the given name by internally locking the inner column + /// family map. This avoids needing `&mut self` reference + pub fn drop_cf(&self, name: &str) -> Result<(), Error> { + let inner = self.inner; + if let Some(cf) = self.cfs.map.write().unwrap().remove(name) { + unsafe { + ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); + } + Ok(()) + } else { + Err(Error::new(format!("Invalid column family: {}", name))) + } } - /// Opens an iterator using the provided ReadOptions. - /// This is used when you want to iterate over a specific ColumnFamily with a modified ReadOptions - pub fn iterator_cf_opt<'a: 'b, 'b>( - &'a self, - cf_handle: BoundColumnFamily<'a>, - readopts: ReadOptions, - mode: IteratorMode, - ) -> DBIterator<'b, Self> { - DBIterator::new_cf(self, cf_handle, readopts, mode) + /// Returns the underlying column family handle + pub fn cf_handle(&self, name: &str) -> Option { + self.cfs + .map + .read() + .unwrap() + .get(name) + .map(|cf| BoundColumnFamily { + inner: cf.inner, + multi_threaded_cfs: PhantomData, + }) } } impl Drop for DbWithThreadMode { fn drop(&mut self) { unsafe { - match &self.cfs { - ColumnFamilyHandles::MultiThread(cf_map) => { - for cf in cf_map.read().unwrap().values() { - ffi::rocksdb_column_family_handle_destroy(cf.inner); - } - } - ColumnFamilyHandles::SingleThread(cf_map) => { - for cf in cf_map.values() { - ffi::rocksdb_column_family_handle_destroy(cf.inner); - } - } - } - + self.cfs.cf_drop_all(); ffi::rocksdb_close(self.inner); } } } -impl fmt::Debug for DbWithThreadMode { +impl fmt::Debug for DbWithThreadMode { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "RocksDB {{ path: {:?} }}", self.path()) } diff --git a/src/db_iterator.rs b/src/db_iterator.rs index 0ec207451..901f3ae94 100644 --- a/src/db_iterator.rs +++ b/src/db_iterator.rs @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::db::WithDbInner; -use crate::{ffi, ColumnFamily, Error, ReadOptions, WriteBatch, DB}; +use crate::db::InternalDbAdapter; +use crate::{ffi, Error, ReadOptions, WriteBatch}; use libc::{c_char, c_uchar, size_t}; use std::marker::PhantomData; use std::slice; @@ -66,7 +66,7 @@ use std::slice; /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` -pub struct DBRawIterator<'a, T: WithDbInner> { +pub struct DBRawIterator<'a, D: InternalDbAdapter> { inner: *mut ffi::rocksdb_iterator_t, /// When iterate_upper_bound is set, the inner C iterator keeps a pointer to the upper bound @@ -74,34 +74,28 @@ pub struct DBRawIterator<'a, T: WithDbInner> { /// iterator is being used. _readopts: ReadOptions, - db: PhantomData<&'a T>, + db: PhantomData<&'a D>, } -impl<'a, T: WithDbInner> DBRawIterator<'a, T> { - pub(crate) fn new(db: &T, readopts: ReadOptions) -> DBRawIterator<'a, T> { - panic!(); - /* +impl<'a, D: InternalDbAdapter> DBRawIterator<'a, D> { + pub(crate) fn new(db: &D, readopts: ReadOptions) -> DBRawIterator<'a, D> { unsafe { DBRawIterator { - inner: ffi::rocksdb_create_iterator(db.inner, readopts.inner), + inner: ffi::rocksdb_create_iterator(db.inner(), readopts.inner), _readopts: readopts, db: PhantomData, } - }*/ + } } - pub(crate) fn new_cf( - db: &'a T, - cf_handle: U, + pub(crate) fn new_cf( + db: &'a D, + cf_handle: *mut ffi::rocksdb_column_family_handle_t, readopts: ReadOptions, - ) -> DBRawIterator<'a, T> { + ) -> DBRawIterator<'a, D> { unsafe { DBRawIterator { - inner: ffi::rocksdb_create_iterator_cf( - WithDbInner::inner(db), - readopts.inner, - cf_handle.inner(), - ), + inner: ffi::rocksdb_create_iterator_cf(db.inner(), readopts.inner, cf_handle), _readopts: readopts, db: PhantomData, } @@ -330,7 +324,7 @@ impl<'a, T: WithDbInner> DBRawIterator<'a, T> { } } -impl<'a, T: WithDbInner> Drop for DBRawIterator<'a, T> { +impl<'a, D: InternalDbAdapter> Drop for DBRawIterator<'a, D> { fn drop(&mut self) { unsafe { ffi::rocksdb_iter_destroy(self.inner); @@ -338,8 +332,8 @@ impl<'a, T: WithDbInner> Drop for DBRawIterator<'a, T> { } } -unsafe impl<'a, T: WithDbInner> Send for DBRawIterator<'a, T> {} -unsafe impl<'a, T: WithDbInner> Sync for DBRawIterator<'a, T> {} +unsafe impl<'a, D: InternalDbAdapter> Send for DBRawIterator<'a, D> {} +unsafe impl<'a, D: InternalDbAdapter> Sync for DBRawIterator<'a, D> {} /// An iterator over a database or column family, with specifiable /// ranges and direction. @@ -372,8 +366,8 @@ unsafe impl<'a, T: WithDbInner> Sync for DBRawIterator<'a, T> {} /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` -pub struct DBIterator<'a, T: WithDbInner> { - raw: DBRawIterator<'a, T>, +pub struct DBIterator<'a, D: InternalDbAdapter> { + raw: DBRawIterator<'a, D>, direction: Direction, just_seeked: bool, } @@ -391,8 +385,8 @@ pub enum IteratorMode<'a> { From(&'a [u8], Direction), } -impl<'a, T: WithDbInner> DBIterator<'a, T> { - pub(crate) fn new(db: &T, readopts: ReadOptions, mode: IteratorMode) -> Self { +impl<'a, D: InternalDbAdapter> DBIterator<'a, D> { + pub(crate) fn new(db: &D, readopts: ReadOptions, mode: IteratorMode) -> Self { let mut rv = DBIterator { raw: DBRawIterator::new(db, readopts), direction: Direction::Forward, // blown away by set_mode() @@ -402,9 +396,9 @@ impl<'a, T: WithDbInner> DBIterator<'a, T> { rv } - pub(crate) fn new_cf( - db: &'a T, - cf_handle: U, + pub(crate) fn new_cf( + db: &'a D, + cf_handle: *mut ffi::rocksdb_column_family_handle_t, readopts: ReadOptions, mode: IteratorMode, ) -> Self { @@ -451,7 +445,7 @@ impl<'a, T: WithDbInner> DBIterator<'a, T> { } } -impl<'a, T: WithDbInner> Iterator for DBIterator<'a, T> { +impl<'a, D: InternalDbAdapter> Iterator for DBIterator<'a, D> { type Item = KVBytes; fn next(&mut self) -> Option { @@ -482,8 +476,8 @@ impl<'a, T: WithDbInner> Iterator for DBIterator<'a, T> { } } -impl<'a, T: WithDbInner> Into> for DBIterator<'a, T> { - fn into(self) -> DBRawIterator<'a, T> { +impl<'a, D: InternalDbAdapter> Into> for DBIterator<'a, D> { + fn into(self) -> DBRawIterator<'a, D> { self.raw } } diff --git a/src/db_options.rs b/src/db_options.rs index 40d177158..d0dba7f35 100644 --- a/src/db_options.rs +++ b/src/db_options.rs @@ -22,6 +22,7 @@ use crate::{ compaction_filter::{self, CompactionFilterCallback, CompactionFilterFn}, compaction_filter_factory::{self, CompactionFilterFactory}, comparator::{self, ComparatorCallback, CompareFn}, + db::InternalDbAdapter, ffi, merge_operator::{ self, full_merge_callback, partial_merge_callback, MergeFn, MergeOperatorCallback, @@ -2849,7 +2850,7 @@ impl ReadOptions { /// Sets the snapshot which should be used for the read. /// The snapshot must belong to the DB that is being read and must /// not have been released. - pub(crate) fn set_snapshot(&mut self, snapshot: &Snapshot) { + pub(crate) fn set_snapshot(&mut self, snapshot: &Snapshot) { unsafe { ffi::rocksdb_readoptions_set_snapshot(self.inner, snapshot.inner); } diff --git a/src/lib.rs b/src/lib.rs index b84977e3d..b38686d8e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -93,9 +93,11 @@ mod sst_file_writer; mod write_batch; pub use crate::{ - column_family::{ColumnFamily, ColumnFamilyDescriptor, DEFAULT_COLUMN_FAMILY_NAME}, + column_family::{ + BoundColumnFamily, ColumnFamily, ColumnFamilyDescriptor, DEFAULT_COLUMN_FAMILY_NAME, + }, compaction_filter::Decision as CompactionDecision, - db::{LiveFile, DB}, + db::{DbWithThreadMode, LiveFile, MultiThreaded, SingleThreaded, DB}, db_iterator::{DBIterator, DBRawIterator, DBWALIterator, Direction, IteratorMode}, db_options::{ BlockBasedIndexType, BlockBasedOptions, BottommostLevelCompaction, Cache, CompactOptions, @@ -179,7 +181,7 @@ mod test { is_send::(); is_send::>(); is_send::>(); - is_send::(); + is_send::>(); is_send::(); is_send::(); is_send::(); @@ -201,7 +203,7 @@ mod test { } is_sync::(); - is_sync::(); + is_sync::>(); is_sync::(); is_sync::(); is_sync::(); diff --git a/src/snapshot.rs b/src/snapshot.rs index 0f8e1bc5c..147fd9d0f 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -12,7 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{ffi, ColumnFamily, DBIterator, DBRawIterator, Error, IteratorMode, ReadOptions, DB}; +use crate::{ + column_family::ColumnFamilyRef, db::InternalDbAdapter, ffi, ColumnFamily, DBIterator, + DBRawIterator, Error, IteratorMode, ReadOptions, +}; /// A consistent view of the database at the point of creation. /// @@ -30,22 +33,82 @@ use crate::{ffi, ColumnFamily, DBIterator, DBRawIterator, Error, IteratorMode, R /// let _ = DB::destroy(&Options::default(), path); /// ``` /// -pub struct Snapshot<'a> { - db: &'a DB, +pub struct Snapshot<'a, D: InternalDbAdapter> { + db: &'a D, pub(crate) inner: *const ffi::rocksdb_snapshot_t, } -impl<'a> Snapshot<'a> { +impl<'a, D: InternalDbAdapter> Snapshot<'a, D> { /// Creates a new `Snapshot` of the database `db`. - pub fn new(db: &DB) -> Snapshot { - let snapshot = unsafe { ffi::rocksdb_create_snapshot(db.inner) }; - Snapshot { + pub fn new(db: &'a D) -> Self { + let snapshot = unsafe { ffi::rocksdb_create_snapshot(db.inner()) }; + Self { db, inner: snapshot, } } + /// Creates an iterator over the data in this snapshot, using the default read options. + pub fn iterator(&self, mode: IteratorMode) -> DBIterator<'a, D> { + let readopts = ReadOptions::default(); + self.iterator_opt(mode, readopts) + } + + /// Creates an iterator over the data in this snapshot under the given column family, using + /// the default read options. + pub fn iterator_cf(&self, cf_handle: &ColumnFamily, mode: IteratorMode) -> DBIterator { + let readopts = ReadOptions::default(); + self.iterator_cf_opt(cf_handle, readopts, mode) + } + + /// Creates an iterator over the data in this snapshot, using the given read options. + pub fn iterator_opt(&self, mode: IteratorMode, mut readopts: ReadOptions) -> DBIterator<'a, D> { + readopts.set_snapshot(self); + DBIterator::::new(self.db, readopts, mode) + } + + /// Creates an iterator over the data in this snapshot under the given column family, using + /// the given read options. + pub fn iterator_cf_opt( + &self, + cf_handle: impl ColumnFamilyRef, + mut readopts: ReadOptions, + mode: IteratorMode, + ) -> DBIterator { + readopts.set_snapshot(self); + DBIterator::new_cf(self.db, cf_handle.inner(), readopts, mode) + } + /// Creates a raw iterator over the data in this snapshot, using the default read options. + pub fn raw_iterator(&self) -> DBRawIterator { + let readopts = ReadOptions::default(); + self.raw_iterator_opt(readopts) + } + + /// Creates a raw iterator over the data in this snapshot under the given column family, using + /// the default read options. + pub fn raw_iterator_cf(&self, cf_handle: &ColumnFamily) -> DBRawIterator { + let readopts = ReadOptions::default(); + self.raw_iterator_cf_opt(cf_handle, readopts) + } + + /// Creates a raw iterator over the data in this snapshot, using the given read options. + pub fn raw_iterator_opt(&self, mut readopts: ReadOptions) -> DBRawIterator { + readopts.set_snapshot(self); + DBRawIterator::new(self.db, readopts) + } + + /// Creates a raw iterator over the data in this snapshot under the given column family, using + /// the given read options. + pub fn raw_iterator_cf_opt( + &self, + cf_handle: &ColumnFamily, + mut readopts: ReadOptions, + ) -> DBRawIterator { + readopts.set_snapshot(self); + DBRawIterator::new_cf(self.db, cf_handle.inner(), readopts) + } + /// Returns the bytes associated with a key value with default read options. pub fn get>(&self, key: K) -> Result>, Error> { let readopts = ReadOptions::default(); @@ -85,15 +148,15 @@ impl<'a> Snapshot<'a> { } } -impl<'a> Drop for Snapshot<'a> { +impl<'a, D: InternalDbAdapter> Drop for Snapshot<'a, D> { fn drop(&mut self) { unsafe { - ffi::rocksdb_release_snapshot(self.db.inner, self.inner); + ffi::rocksdb_release_snapshot(self.db.inner(), self.inner); } } } /// `Send` and `Sync` implementations for `Snapshot` are safe, because `Snapshot` is /// immutable and can be safely shared between threads. -unsafe impl<'a> Send for Snapshot<'a> {} -unsafe impl<'a> Sync for Snapshot<'a> {} +unsafe impl<'a, D: InternalDbAdapter> Send for Snapshot<'a, D> {} +unsafe impl<'a, D: InternalDbAdapter> Sync for Snapshot<'a, D> {} diff --git a/tests/test_column_family.rs b/tests/test_column_family.rs index 4fcce8488..40db91066 100644 --- a/tests/test_column_family.rs +++ b/tests/test_column_family.rs @@ -19,7 +19,25 @@ use pretty_assertions::assert_eq; use rocksdb::{ColumnFamilyDescriptor, MergeOperands, Options, DB, DEFAULT_COLUMN_FAMILY_NAME}; use util::DBPath; -fn test_cf_common(n: &DBPath) { +#[test] +fn test_column_family() { + let n = DBPath::new("_rust_rocksdb_cftest"); + + // should be able to create column families + { + let mut opts = Options::default(); + opts.create_if_missing(true); + opts.set_merge_operator_associative("test operator", test_provided_merge); + let mut db = DB::open(&opts, &n).unwrap(); + let opts = Options::default(); + match db.create_cf("cf1", &opts) { + Ok(()) => println!("cf1 created successfully"), + Err(e) => { + panic!("could not create column family: {}", e); + } + } + } + // should fail to open db without specifying same column families { let mut opts = Options::default(); @@ -59,29 +77,6 @@ fn test_cf_common(n: &DBPath) { // TODO should be able to iterate over a cf {} // should b able to drop a cf -} - -#[test] -fn test_column_family() { - let n = DBPath::new("_rust_rocksdb_cftest"); - - // should be able to create column families - { - let mut opts = Options::default(); - opts.create_if_missing(true); - opts.set_merge_operator_associative("test operator", test_provided_merge); - let mut db = DB::open(&opts, &n).unwrap(); - let opts = Options::default(); - match db.create_cf("cf1", &opts) { - Ok(()) => println!("cf1 created successfully"), - Err(e) => { - panic!("could not create column family: {}", e); - } - } - } - - test_cf_common(&n); - { let mut db = DB::open_cf(&Options::default(), &n, &["cf1"]).unwrap(); match db.drop_cf("cf1") { @@ -91,33 +86,6 @@ fn test_column_family() { } } -#[test] -fn test_multi_threaded_column_family() { - let n = DBPath::new("_rust_rocksdb_cftest"); - - // Should be able to create column families without a mutable reference - // to the db - { - let mut opts = Options::default(); - opts.create_if_missing(true); - opts.set_merge_operator_associative("test operator", test_provided_merge); - let db = DB::open_cf_multi_threaded(&opts, &n, None::<&str>).unwrap(); - let opts = Options::default(); - } - - test_cf_common(&n); - - // Should be able to drop column families without a mutable reference - // to the db - { - let db = DB::open_cf_multi_threaded(&Options::default(), &n, &["cf1"]).unwrap(); - match db.drop_cf_multi_threaded("cf1") { - Ok(_) => println!("cf1 successfully dropped."), - Err(e) => panic!("failed to drop column family: {}", e), - } - } -} - #[test] fn test_can_open_db_with_results_of_list_cf() { // Test scenario derived from GitHub issue #175 and 177 diff --git a/tests/test_db.rs b/tests/test_db.rs index d9ec85b24..1991171f1 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -20,9 +20,10 @@ use pretty_assertions::assert_eq; use rocksdb::{ perf::get_memory_usage_stats, BlockBasedOptions, BottommostLevelCompaction, Cache, - CompactOptions, DBCompactionStyle, Env, Error, FifoCompactOptions, IteratorMode, Options, - PerfContext, PerfMetric, ReadOptions, SliceTransform, Snapshot, UniversalCompactOptions, - UniversalCompactionStopStyle, WriteBatch, DB, + CompactOptions, DBCompactionStyle, DbWithThreadMode, Env, Error, FifoCompactOptions, + IteratorMode, MultiThreaded, Options, PerfContext, PerfMetric, ReadOptions, SingleThreaded, + SliceTransform, Snapshot, UniversalCompactOptions, UniversalCompactionStopStyle, WriteBatch, + DB, }; use util::DBPath; @@ -264,7 +265,7 @@ fn snapshot_test() { #[derive(Clone)] struct SnapshotWrapper { - snapshot: Arc>, + snapshot: Arc>, } impl SnapshotWrapper { @@ -528,6 +529,28 @@ fn test_open_with_ttl() { assert!(db.get(b"key1").unwrap().is_none()); } +#[test] +fn test_open_as_single_threaded() { + let primary_path = DBPath::new("_rust_rocksdb_test_open_as_single_threaded"); + + let mut db = DbWithThreadMode::::open_default(&primary_path).unwrap(); + let db_ref1 = &mut db; + let opts = Options::default(); + db_ref1.create_cf("cf1", &opts).unwrap(); +} + +#[test] +fn test_open_as_multi_threaded() { + let primary_path = DBPath::new("_rust_rocksdb_test_open_as_multi_threaded"); + + let db = DbWithThreadMode::::open_default(&primary_path).unwrap(); + let db_ref1 = &db; + let db_ref2 = &db; + let opts = Options::default(); + db_ref1.create_cf("cf1", &opts).unwrap(); + db_ref2.create_cf("cf2", &opts).unwrap(); +} + #[test] fn compact_range_test() { let path = DBPath::new("_rust_rocksdb_compact_range_test"); From 6c9fce886cbc6aa409de43e8eeb02aad0f2a40d0 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 27 Mar 2021 16:55:27 +0900 Subject: [PATCH 15/30] Fix ci --- src/backup.rs | 4 ++-- src/lib.rs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backup.rs b/src/backup.rs index f84e0e931..f0790426e 100644 --- a/src/backup.rs +++ b/src/backup.rs @@ -235,7 +235,7 @@ impl Default for BackupEngineOptions { unsafe { let opts = ffi::rocksdb_options_create(); if opts.is_null() { - panic!("Could not create RocksDB backup options".to_owned()); + panic!("Could not create RocksDB backup options"); } BackupEngineOptions { inner: opts } } @@ -247,7 +247,7 @@ impl Default for RestoreOptions { unsafe { let opts = ffi::rocksdb_restore_options_create(); if opts.is_null() { - panic!("Could not create RocksDB restore options".to_owned()); + panic!("Could not create RocksDB restore options"); } RestoreOptions { inner: opts } } diff --git a/src/lib.rs b/src/lib.rs index b38686d8e..a15e32bc7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -70,6 +70,7 @@ clippy::missing_safety_doc, clippy::needless_pass_by_value, clippy::option_if_let_else, + clippy::upper_case_acronyms, )] #[macro_use] From 2ce61eceb083da94918b303945ffc94e65a6e035 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 27 Mar 2021 17:12:30 +0900 Subject: [PATCH 16/30] Further fix ci --- src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index a15e32bc7..905ffc8e5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -70,6 +70,9 @@ clippy::missing_safety_doc, clippy::needless_pass_by_value, clippy::option_if_let_else, + clippy::ptr_as_ptr, + clippy::missing_panics_doc, + clippy::from_over_into, clippy::upper_case_acronyms, )] From 99993603b4e0891b1d1c3900ddc59660e1ea9521 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 27 Mar 2021 18:10:51 +0900 Subject: [PATCH 17/30] Further tweak ci actions --- .github/workflows/rust.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 81b4ebb8f..836847955 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -88,12 +88,14 @@ jobs: with: command: test args: --manifest-path=librocksdb-sys/Cargo.toml - - name: Run rocksdb tests (single threaded cf alterations) + - name: Run rocksdb tests (single-threaded cf alterations) uses: actions-rs/cargo@v1 with: command: test - - name: Run rocksdb tests (multi threaded cf alterations) + - name: Run rocksdb tests (multi-threaded cf alterations) uses: actions-rs/cargo@v1 + env: + RUSTFLAGS=-Awarnings # Suppress "variable does not need to be mutable" warnings with: command: test args: --features multi-threaded-as-default From 82e57d182437e7338b4673557fc9a2874528907b Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 27 Mar 2021 18:11:14 +0900 Subject: [PATCH 18/30] Fix typo --- src/column_family.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/column_family.rs b/src/column_family.rs index 3f15241c0..0d1cd35d5 100644 --- a/src/column_family.rs +++ b/src/column_family.rs @@ -52,7 +52,7 @@ unsafe impl Send for ColumnFamily {} /// A specialized opaque type used to represent a column family. Used for multi-threaded /// mode. Clone (and Copy) is derived to behave like &ColumnFamily (used for /// single-threaded). Clone/Copy is safe because this lifetime is bound to DB like -/// iterators/snapshots. On top of it, this is cheap and small as &ColumnFamily because +/// iterators/snapshots. On top of it, this is as cheap and small as &ColumnFamily because /// this only has a single pointer-wide field. #[derive(Clone, Copy)] pub struct BoundColumnFamily<'a> { From 42bdf42ee6ea124fd119e76030155a423aac6b20 Mon Sep 17 00:00:00 2001 From: Oleksandr Anyshchenko Date: Mon, 29 Mar 2021 11:19:59 +0300 Subject: [PATCH 19/30] Update rust.yml --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 836847955..dbd6ab17e 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -95,7 +95,7 @@ jobs: - name: Run rocksdb tests (multi-threaded cf alterations) uses: actions-rs/cargo@v1 env: - RUSTFLAGS=-Awarnings # Suppress "variable does not need to be mutable" warnings + RUSTFLAGS: -Awarnings # Suppress "variable does not need to be mutable" warnings with: command: test args: --features multi-threaded-as-default From 67a180ac33520dba9cde43266010531d77da8e58 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 30 Mar 2021 19:30:27 +0900 Subject: [PATCH 20/30] Impl Send for BoundCF & add type alias for compat --- src/column_family.rs | 5 ++-- src/db.rs | 66 ++++++++++++++++++++++++-------------------- src/db_iterator.rs | 65 ++++++++++++++++++++++++++----------------- src/db_options.rs | 9 ++++-- src/lib.rs | 22 +++++++++------ src/snapshot.rs | 56 +++++++++++++++++++++++-------------- tests/test_db.rs | 6 ++-- 7 files changed, 136 insertions(+), 93 deletions(-) diff --git a/src/column_family.rs b/src/column_family.rs index 0d1cd35d5..0a7a58b8b 100644 --- a/src/column_family.rs +++ b/src/column_family.rs @@ -47,8 +47,6 @@ pub struct ColumnFamily { pub(crate) inner: *mut ffi::rocksdb_column_family_handle_t, } -unsafe impl Send for ColumnFamily {} - /// A specialized opaque type used to represent a column family. Used for multi-threaded /// mode. Clone (and Copy) is derived to behave like &ColumnFamily (used for /// single-threaded). Clone/Copy is safe because this lifetime is bound to DB like @@ -75,3 +73,6 @@ impl<'a> ColumnFamilyRef for BoundColumnFamily<'a> { self.inner } } + +unsafe impl Send for ColumnFamily {} +unsafe impl<'a> Send for BoundColumnFamily<'a> {} diff --git a/src/db.rs b/src/db.rs index eaa2268b0..0457f8bab 100644 --- a/src/db.rs +++ b/src/db.rs @@ -18,10 +18,10 @@ use crate::{ column_family::ColumnFamilyRef, ffi, ffi_util::{from_cstr, opt_bytes_to_ptr, raw_data, to_cpath}, - ColumnFamily, ColumnFamilyDescriptor, CompactOptions, DBIterator, DBPinnableSlice, - DBRawIterator, DBWALIterator, Direction, Error, FlushOptions, IngestExternalFileOptions, - IteratorMode, Options, ReadOptions, Snapshot, WriteBatch, WriteOptions, - DEFAULT_COLUMN_FAMILY_NAME, + ColumnFamily, ColumnFamilyDescriptor, CompactOptions, DBIteratorWithThreadMode, + DBPinnableSlice, DBRawIteratorWithThreadMode, DBWALIterator, Direction, Error, FlushOptions, + IngestExternalFileOptions, IteratorMode, Options, ReadOptions, SnapshotWithThreadMode, + WriteBatch, WriteOptions, DEFAULT_COLUMN_FAMILY_NAME, }; use libc::{self, c_char, c_int, c_uchar, c_void, size_t}; @@ -786,7 +786,10 @@ impl DbWithThreadMode { }) } - pub fn iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b, Self> { + pub fn iterator<'a: 'b, 'b>( + &'a self, + mode: IteratorMode, + ) -> DBIteratorWithThreadMode<'b, Self> { let readopts = ReadOptions::default(); self.iterator_opt(mode, readopts) } @@ -795,8 +798,8 @@ impl DbWithThreadMode { &'a self, mode: IteratorMode, readopts: ReadOptions, - ) -> DBIterator<'b, Self> { - DBIterator::new(self, readopts, mode) + ) -> DBIteratorWithThreadMode<'b, Self> { + DBIteratorWithThreadMode::new(self, readopts, mode) } /// Opens an iterator using the provided ReadOptions. @@ -806,26 +809,29 @@ impl DbWithThreadMode { cf_handle: impl ColumnFamilyRef, readopts: ReadOptions, mode: IteratorMode, - ) -> DBIterator<'b, Self> { - DBIterator::new_cf(self, cf_handle.inner(), readopts, mode) + ) -> DBIteratorWithThreadMode<'b, Self> { + DBIteratorWithThreadMode::new_cf(self, cf_handle.inner(), readopts, mode) } /// Opens an iterator with `set_total_order_seek` enabled. /// This must be used to iterate across prefixes when `set_memtable_factory` has been called /// with a Hash-based implementation. - pub fn full_iterator<'a: 'b, 'b>(&'a self, mode: IteratorMode) -> DBIterator<'b, Self> { + pub fn full_iterator<'a: 'b, 'b>( + &'a self, + mode: IteratorMode, + ) -> DBIteratorWithThreadMode<'b, Self> { let mut opts = ReadOptions::default(); opts.set_total_order_seek(true); - DBIterator::new(self, opts, mode) + DBIteratorWithThreadMode::new(self, opts, mode) } pub fn prefix_iterator<'a: 'b, 'b, P: AsRef<[u8]>>( &'a self, prefix: P, - ) -> DBIterator<'b, Self> { + ) -> DBIteratorWithThreadMode<'b, Self> { let mut opts = ReadOptions::default(); opts.set_prefix_same_as_start(true); - DBIterator::new( + DBIteratorWithThreadMode::new( self, opts, IteratorMode::From(prefix.as_ref(), Direction::Forward), @@ -836,29 +842,29 @@ impl DbWithThreadMode { &'a self, cf_handle: impl ColumnFamilyRef, mode: IteratorMode, - ) -> DBIterator<'b, Self> { + ) -> DBIteratorWithThreadMode<'b, Self> { let opts = ReadOptions::default(); - DBIterator::new_cf(self, cf_handle.inner(), opts, mode) + DBIteratorWithThreadMode::new_cf(self, cf_handle.inner(), opts, mode) } pub fn full_iterator_cf<'a: 'b, 'b>( &'a self, cf_handle: impl ColumnFamilyRef, mode: IteratorMode, - ) -> DBIterator<'b, Self> { + ) -> DBIteratorWithThreadMode<'b, Self> { let mut opts = ReadOptions::default(); opts.set_total_order_seek(true); - DBIterator::new_cf(self, cf_handle.inner(), opts, mode) + DBIteratorWithThreadMode::new_cf(self, cf_handle.inner(), opts, mode) } pub fn prefix_iterator_cf<'a, P: AsRef<[u8]>>( &'a self, cf_handle: impl ColumnFamilyRef, prefix: P, - ) -> DBIterator<'a, Self> { + ) -> DBIteratorWithThreadMode<'a, Self> { let mut opts = ReadOptions::default(); opts.set_prefix_same_as_start(true); - DBIterator::<'a, Self>::new_cf( + DBIteratorWithThreadMode::<'a, Self>::new_cf( self, cf_handle.inner(), opts, @@ -867,26 +873,26 @@ impl DbWithThreadMode { } /// Opens a raw iterator over the database, using the default read options - pub fn raw_iterator<'a: 'b, 'b>(&'a self) -> DBRawIterator<'b, Self> { + pub fn raw_iterator<'a: 'b, 'b>(&'a self) -> DBRawIteratorWithThreadMode<'b, Self> { let opts = ReadOptions::default(); - DBRawIterator::new(self, opts) + DBRawIteratorWithThreadMode::new(self, opts) } /// Opens a raw iterator over the given column family, using the default read options pub fn raw_iterator_cf<'a: 'b, 'b>( &'a self, cf_handle: impl ColumnFamilyRef, - ) -> DBRawIterator<'b, Self> { + ) -> DBRawIteratorWithThreadMode<'b, Self> { let opts = ReadOptions::default(); - DBRawIterator::new_cf(self, cf_handle.inner(), opts) + DBRawIteratorWithThreadMode::new_cf(self, cf_handle.inner(), opts) } /// Opens a raw iterator over the database, using the given read options pub fn raw_iterator_opt<'a: 'b, 'b>( &'a self, readopts: ReadOptions, - ) -> DBRawIterator<'b, Self> { - DBRawIterator::new(self, readopts) + ) -> DBRawIteratorWithThreadMode<'b, Self> { + DBRawIteratorWithThreadMode::new(self, readopts) } /// Opens a raw iterator over the given column family, using the given read options @@ -894,12 +900,12 @@ impl DbWithThreadMode { &'a self, cf_handle: impl ColumnFamilyRef, readopts: ReadOptions, - ) -> DBRawIterator<'b, Self> { - DBRawIterator::new_cf(self, cf_handle.inner(), readopts) + ) -> DBRawIteratorWithThreadMode<'b, Self> { + DBRawIteratorWithThreadMode::new_cf(self, cf_handle.inner(), readopts) } - pub fn snapshot(&self) -> Snapshot { - Snapshot::::new(self) + pub fn snapshot(&self) -> SnapshotWithThreadMode { + SnapshotWithThreadMode::::new(self) } pub fn put_opt(&self, key: K, value: V, writeopts: &WriteOptions) -> Result<(), Error> @@ -1525,7 +1531,7 @@ impl DbWithThreadMode { /// /// Note: L0 files are left regardless of whether they're in the range. /// - /// Snapshots before the delete might not see the data in the given range. + /// SnapshotWithThreadModes before the delete might not see the data in the given range. pub fn delete_file_in_range>(&self, from: K, to: K) -> Result<(), Error> { let from = from.as_ref(); let to = to.as_ref(); diff --git a/src/db_iterator.rs b/src/db_iterator.rs index 901f3ae94..72194e935 100644 --- a/src/db_iterator.rs +++ b/src/db_iterator.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::db::InternalDbAdapter; +use crate::db::{InternalDbAdapter, DB}; use crate::{ffi, Error, ReadOptions, WriteBatch}; use libc::{c_char, c_uchar, size_t}; use std::marker::PhantomData; @@ -21,7 +21,7 @@ use std::slice; /// An iterator over a database or column family, with specifiable /// ranges and direction. /// -/// This iterator is different to the standard ``DBIterator`` as it aims Into +/// This iterator is different to the standard ``DBIteratorWithThreadMode`` as it aims Into /// replicate the underlying iterator API within RocksDB itself. This should /// give access to more performance and flexibility but departs from the /// widely recognised Rust idioms. @@ -66,7 +66,14 @@ use std::slice; /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` -pub struct DBRawIterator<'a, D: InternalDbAdapter> { + +#[cfg(not(feature = "multi-threaded-as-default"))] +pub type DBRawIterator<'a> = DBRawIteratorWithThreadMode<'a, DB>; + +#[cfg(feature = "multi-threaded-as-default")] +pub type DBRawIterator<'a> = DBRawIteratorWithThreadMode<'a, DB>; + +pub struct DBRawIteratorWithThreadMode<'a, D: InternalDbAdapter> { inner: *mut ffi::rocksdb_iterator_t, /// When iterate_upper_bound is set, the inner C iterator keeps a pointer to the upper bound @@ -77,10 +84,10 @@ pub struct DBRawIterator<'a, D: InternalDbAdapter> { db: PhantomData<&'a D>, } -impl<'a, D: InternalDbAdapter> DBRawIterator<'a, D> { - pub(crate) fn new(db: &D, readopts: ReadOptions) -> DBRawIterator<'a, D> { +impl<'a, D: InternalDbAdapter> DBRawIteratorWithThreadMode<'a, D> { + pub(crate) fn new(db: &D, readopts: ReadOptions) -> DBRawIteratorWithThreadMode<'a, D> { unsafe { - DBRawIterator { + DBRawIteratorWithThreadMode { inner: ffi::rocksdb_create_iterator(db.inner(), readopts.inner), _readopts: readopts, db: PhantomData, @@ -92,9 +99,9 @@ impl<'a, D: InternalDbAdapter> DBRawIterator<'a, D> { db: &'a D, cf_handle: *mut ffi::rocksdb_column_family_handle_t, readopts: ReadOptions, - ) -> DBRawIterator<'a, D> { + ) -> DBRawIteratorWithThreadMode<'a, D> { unsafe { - DBRawIterator { + DBRawIteratorWithThreadMode { inner: ffi::rocksdb_create_iterator_cf(db.inner(), readopts.inner, cf_handle), _readopts: readopts, db: PhantomData, @@ -106,7 +113,7 @@ impl<'a, D: InternalDbAdapter> DBRawIterator<'a, D> { /// it reaches the end of its defined range, or when it encounters an error. /// /// To check whether the iterator encountered an error after `valid` has - /// returned `false`, use the [`status`](DBRawIterator::status) method. `status` will never + /// returned `false`, use the [`status`](DBRawIteratorWithThreadMode::status) method. `status` will never /// return an error when `valid` is `true`. pub fn valid(&self) -> bool { unsafe { ffi::rocksdb_iter_valid(self.inner) != 0 } @@ -114,7 +121,7 @@ impl<'a, D: InternalDbAdapter> DBRawIterator<'a, D> { /// Returns an error `Result` if the iterator has encountered an error /// during operation. When an error is encountered, the iterator is - /// invalidated and [`valid`](DBRawIterator::valid) will return `false` when called. + /// invalidated and [`valid`](DBRawIteratorWithThreadMode::valid) will return `false` when called. /// /// Performing a seek will discard the current status. pub fn status(&self) -> Result<(), Error> { @@ -324,7 +331,7 @@ impl<'a, D: InternalDbAdapter> DBRawIterator<'a, D> { } } -impl<'a, D: InternalDbAdapter> Drop for DBRawIterator<'a, D> { +impl<'a, D: InternalDbAdapter> Drop for DBRawIteratorWithThreadMode<'a, D> { fn drop(&mut self) { unsafe { ffi::rocksdb_iter_destroy(self.inner); @@ -332,8 +339,8 @@ impl<'a, D: InternalDbAdapter> Drop for DBRawIterator<'a, D> { } } -unsafe impl<'a, D: InternalDbAdapter> Send for DBRawIterator<'a, D> {} -unsafe impl<'a, D: InternalDbAdapter> Sync for DBRawIterator<'a, D> {} +unsafe impl<'a, D: InternalDbAdapter> Send for DBRawIteratorWithThreadMode<'a, D> {} +unsafe impl<'a, D: InternalDbAdapter> Sync for DBRawIteratorWithThreadMode<'a, D> {} /// An iterator over a database or column family, with specifiable /// ranges and direction. @@ -366,8 +373,14 @@ unsafe impl<'a, D: InternalDbAdapter> Sync for DBRawIterator<'a, D> {} /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` -pub struct DBIterator<'a, D: InternalDbAdapter> { - raw: DBRawIterator<'a, D>, +#[cfg(not(feature = "multi-threaded-as-default"))] +pub type DBIterator<'a> = DBIteratorWithThreadMode<'a, DB>; + +#[cfg(feature = "multi-threaded-as-default")] +pub type DBIterator<'a> = DBIteratorWithThreadMode<'a, DB>; + +pub struct DBIteratorWithThreadMode<'a, D: InternalDbAdapter> { + raw: DBRawIteratorWithThreadMode<'a, D>, direction: Direction, just_seeked: bool, } @@ -385,10 +398,10 @@ pub enum IteratorMode<'a> { From(&'a [u8], Direction), } -impl<'a, D: InternalDbAdapter> DBIterator<'a, D> { +impl<'a, D: InternalDbAdapter> DBIteratorWithThreadMode<'a, D> { pub(crate) fn new(db: &D, readopts: ReadOptions, mode: IteratorMode) -> Self { - let mut rv = DBIterator { - raw: DBRawIterator::new(db, readopts), + let mut rv = DBIteratorWithThreadMode { + raw: DBRawIteratorWithThreadMode::new(db, readopts), direction: Direction::Forward, // blown away by set_mode() just_seeked: false, }; @@ -402,8 +415,8 @@ impl<'a, D: InternalDbAdapter> DBIterator<'a, D> { readopts: ReadOptions, mode: IteratorMode, ) -> Self { - let mut rv = DBIterator { - raw: DBRawIterator::new_cf(db, cf_handle, readopts), + let mut rv = DBIteratorWithThreadMode { + raw: DBRawIteratorWithThreadMode::new_cf(db, cf_handle, readopts), direction: Direction::Forward, // blown away by set_mode() just_seeked: false, }; @@ -434,18 +447,18 @@ impl<'a, D: InternalDbAdapter> DBIterator<'a, D> { self.just_seeked = true; } - /// See [`valid`](DBRawIterator::valid) + /// See [`valid`](DBRawIteratorWithThreadMode::valid) pub fn valid(&self) -> bool { self.raw.valid() } - /// See [`status`](DBRawIterator::status) + /// See [`status`](DBRawIteratorWithThreadMode::status) pub fn status(&self) -> Result<(), Error> { self.raw.status() } } -impl<'a, D: InternalDbAdapter> Iterator for DBIterator<'a, D> { +impl<'a, D: InternalDbAdapter> Iterator for DBIteratorWithThreadMode<'a, D> { type Item = KVBytes; fn next(&mut self) -> Option { @@ -476,8 +489,10 @@ impl<'a, D: InternalDbAdapter> Iterator for DBIterator<'a, D> { } } -impl<'a, D: InternalDbAdapter> Into> for DBIterator<'a, D> { - fn into(self) -> DBRawIterator<'a, D> { +impl<'a, D: InternalDbAdapter> Into> + for DBIteratorWithThreadMode<'a, D> +{ + fn into(self) -> DBRawIteratorWithThreadMode<'a, D> { self.raw } } diff --git a/src/db_options.rs b/src/db_options.rs index d0dba7f35..9da33b4ed 100644 --- a/src/db_options.rs +++ b/src/db_options.rs @@ -28,7 +28,7 @@ use crate::{ self, full_merge_callback, partial_merge_callback, MergeFn, MergeOperatorCallback, }, slice_transform::SliceTransform, - Error, Snapshot, + Error, SnapshotWithThreadMode, }; fn new_cache(capacity: size_t) -> *mut ffi::rocksdb_cache_t { @@ -2022,7 +2022,7 @@ impl Options { /// to not being able to determine whether there were any write conflicts. /// /// When using a TransactionDB: - /// If Transaction::SetSnapshot is used, TransactionDB will read either + /// If Transaction::SetSnapshotWithThreadMode is used, TransactionDB will read either /// in-memory write buffers or SST files to do write-conflict checking. /// Increasing this value can reduce the number of reads to SST files /// done for conflict detection. @@ -2850,7 +2850,10 @@ impl ReadOptions { /// Sets the snapshot which should be used for the read. /// The snapshot must belong to the DB that is being read and must /// not have been released. - pub(crate) fn set_snapshot(&mut self, snapshot: &Snapshot) { + pub(crate) fn set_snapshot( + &mut self, + snapshot: &SnapshotWithThreadMode, + ) { unsafe { ffi::rocksdb_readoptions_set_snapshot(self.inner, snapshot.inner); } diff --git a/src/lib.rs b/src/lib.rs index 905ffc8e5..6b5d6a80f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -102,7 +102,10 @@ pub use crate::{ }, compaction_filter::Decision as CompactionDecision, db::{DbWithThreadMode, LiveFile, MultiThreaded, SingleThreaded, DB}, - db_iterator::{DBIterator, DBRawIterator, DBWALIterator, Direction, IteratorMode}, + db_iterator::{ + DBIterator, DBIteratorWithThreadMode, DBRawIterator, DBRawIteratorWithThreadMode, + DBWALIterator, Direction, IteratorMode, + }, db_options::{ BlockBasedIndexType, BlockBasedOptions, BottommostLevelCompaction, Cache, CompactOptions, DBCompactionStyle, DBCompressionType, DBPath, DBRecoveryMode, DataBlockIndexType, Env, @@ -114,7 +117,7 @@ pub use crate::{ merge_operator::MergeOperands, perf::{PerfContext, PerfMetric, PerfStatsLevel}, slice_transform::SliceTransform, - snapshot::Snapshot, + snapshot::{Snapshot, SnapshotWithThreadMode}, sst_file_writer::SstFileWriter, write_batch::{WriteBatch, WriteBatchIterator}, }; @@ -168,9 +171,9 @@ impl fmt::Display for Error { #[cfg(test)] mod test { use super::{ - BlockBasedOptions, ColumnFamily, ColumnFamilyDescriptor, DBIterator, DBRawIterator, - IngestExternalFileOptions, Options, PlainTableFactoryOptions, ReadOptions, Snapshot, - SstFileWriter, WriteBatch, WriteOptions, DB, + BlockBasedOptions, BoundColumnFamily, ColumnFamily, ColumnFamilyDescriptor, DBIterator, + DBRawIterator, IngestExternalFileOptions, Options, PlainTableFactoryOptions, ReadOptions, + Snapshot, SstFileWriter, WriteBatch, WriteOptions, DB, }; #[test] @@ -183,9 +186,9 @@ mod test { } is_send::(); - is_send::>(); - is_send::>(); - is_send::>(); + is_send::>(); + is_send::>(); + is_send::(); is_send::(); is_send::(); is_send::(); @@ -194,6 +197,7 @@ mod test { is_send::(); is_send::(); is_send::(); + is_send::>(); is_send::(); is_send::(); } @@ -207,7 +211,7 @@ mod test { } is_sync::(); - is_sync::>(); + is_sync::(); is_sync::(); is_sync::(); is_sync::(); diff --git a/src/snapshot.rs b/src/snapshot.rs index 147fd9d0f..a291d3667 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -13,8 +13,8 @@ // limitations under the License. use crate::{ - column_family::ColumnFamilyRef, db::InternalDbAdapter, ffi, ColumnFamily, DBIterator, - DBRawIterator, Error, IteratorMode, ReadOptions, + column_family::ColumnFamilyRef, db::InternalDbAdapter, ffi, ColumnFamily, + DBIteratorWithThreadMode, DBRawIteratorWithThreadMode, Error, IteratorMode, ReadOptions, DB, }; /// A consistent view of the database at the point of creation. @@ -33,13 +33,19 @@ use crate::{ /// let _ = DB::destroy(&Options::default(), path); /// ``` /// -pub struct Snapshot<'a, D: InternalDbAdapter> { +#[cfg(not(feature = "multi-threaded-as-default"))] +pub type Snapshot<'a> = SnapshotWithThreadMode<'a, DB>; + +#[cfg(feature = "multi-threaded-as-default")] +pub type Snapshot<'a> = SnapshotWithThreadMode<'a, DB>; + +pub struct SnapshotWithThreadMode<'a, D: InternalDbAdapter> { db: &'a D, pub(crate) inner: *const ffi::rocksdb_snapshot_t, } -impl<'a, D: InternalDbAdapter> Snapshot<'a, D> { - /// Creates a new `Snapshot` of the database `db`. +impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { + /// Creates a new `SnapshotWithThreadMode` of the database `db`. pub fn new(db: &'a D) -> Self { let snapshot = unsafe { ffi::rocksdb_create_snapshot(db.inner()) }; Self { @@ -49,22 +55,30 @@ impl<'a, D: InternalDbAdapter> Snapshot<'a, D> { } /// Creates an iterator over the data in this snapshot, using the default read options. - pub fn iterator(&self, mode: IteratorMode) -> DBIterator<'a, D> { + pub fn iterator(&self, mode: IteratorMode) -> DBIteratorWithThreadMode<'a, D> { let readopts = ReadOptions::default(); self.iterator_opt(mode, readopts) } /// Creates an iterator over the data in this snapshot under the given column family, using /// the default read options. - pub fn iterator_cf(&self, cf_handle: &ColumnFamily, mode: IteratorMode) -> DBIterator { + pub fn iterator_cf( + &self, + cf_handle: &ColumnFamily, + mode: IteratorMode, + ) -> DBIteratorWithThreadMode { let readopts = ReadOptions::default(); self.iterator_cf_opt(cf_handle, readopts, mode) } /// Creates an iterator over the data in this snapshot, using the given read options. - pub fn iterator_opt(&self, mode: IteratorMode, mut readopts: ReadOptions) -> DBIterator<'a, D> { + pub fn iterator_opt( + &self, + mode: IteratorMode, + mut readopts: ReadOptions, + ) -> DBIteratorWithThreadMode<'a, D> { readopts.set_snapshot(self); - DBIterator::::new(self.db, readopts, mode) + DBIteratorWithThreadMode::::new(self.db, readopts, mode) } /// Creates an iterator over the data in this snapshot under the given column family, using @@ -74,28 +88,28 @@ impl<'a, D: InternalDbAdapter> Snapshot<'a, D> { cf_handle: impl ColumnFamilyRef, mut readopts: ReadOptions, mode: IteratorMode, - ) -> DBIterator { + ) -> DBIteratorWithThreadMode { readopts.set_snapshot(self); - DBIterator::new_cf(self.db, cf_handle.inner(), readopts, mode) + DBIteratorWithThreadMode::new_cf(self.db, cf_handle.inner(), readopts, mode) } /// Creates a raw iterator over the data in this snapshot, using the default read options. - pub fn raw_iterator(&self) -> DBRawIterator { + pub fn raw_iterator(&self) -> DBRawIteratorWithThreadMode { let readopts = ReadOptions::default(); self.raw_iterator_opt(readopts) } /// Creates a raw iterator over the data in this snapshot under the given column family, using /// the default read options. - pub fn raw_iterator_cf(&self, cf_handle: &ColumnFamily) -> DBRawIterator { + pub fn raw_iterator_cf(&self, cf_handle: &ColumnFamily) -> DBRawIteratorWithThreadMode { let readopts = ReadOptions::default(); self.raw_iterator_cf_opt(cf_handle, readopts) } /// Creates a raw iterator over the data in this snapshot, using the given read options. - pub fn raw_iterator_opt(&self, mut readopts: ReadOptions) -> DBRawIterator { + pub fn raw_iterator_opt(&self, mut readopts: ReadOptions) -> DBRawIteratorWithThreadMode { readopts.set_snapshot(self); - DBRawIterator::new(self.db, readopts) + DBRawIteratorWithThreadMode::new(self.db, readopts) } /// Creates a raw iterator over the data in this snapshot under the given column family, using @@ -104,9 +118,9 @@ impl<'a, D: InternalDbAdapter> Snapshot<'a, D> { &self, cf_handle: &ColumnFamily, mut readopts: ReadOptions, - ) -> DBRawIterator { + ) -> DBRawIteratorWithThreadMode { readopts.set_snapshot(self); - DBRawIterator::new_cf(self.db, cf_handle.inner(), readopts) + DBRawIteratorWithThreadMode::new_cf(self.db, cf_handle.inner(), readopts) } /// Returns the bytes associated with a key value with default read options. @@ -148,7 +162,7 @@ impl<'a, D: InternalDbAdapter> Snapshot<'a, D> { } } -impl<'a, D: InternalDbAdapter> Drop for Snapshot<'a, D> { +impl<'a, D: InternalDbAdapter> Drop for SnapshotWithThreadMode<'a, D> { fn drop(&mut self) { unsafe { ffi::rocksdb_release_snapshot(self.db.inner(), self.inner); @@ -156,7 +170,7 @@ impl<'a, D: InternalDbAdapter> Drop for Snapshot<'a, D> { } } -/// `Send` and `Sync` implementations for `Snapshot` are safe, because `Snapshot` is +/// `Send` and `Sync` implementations for `SnapshotWithThreadMode` are safe, because `SnapshotWithThreadMode` is /// immutable and can be safely shared between threads. -unsafe impl<'a, D: InternalDbAdapter> Send for Snapshot<'a, D> {} -unsafe impl<'a, D: InternalDbAdapter> Sync for Snapshot<'a, D> {} +unsafe impl<'a, D: InternalDbAdapter> Send for SnapshotWithThreadMode<'a, D> {} +unsafe impl<'a, D: InternalDbAdapter> Sync for SnapshotWithThreadMode<'a, D> {} diff --git a/tests/test_db.rs b/tests/test_db.rs index 1991171f1..30d4b521d 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -22,8 +22,8 @@ use rocksdb::{ perf::get_memory_usage_stats, BlockBasedOptions, BottommostLevelCompaction, Cache, CompactOptions, DBCompactionStyle, DbWithThreadMode, Env, Error, FifoCompactOptions, IteratorMode, MultiThreaded, Options, PerfContext, PerfMetric, ReadOptions, SingleThreaded, - SliceTransform, Snapshot, UniversalCompactOptions, UniversalCompactionStopStyle, WriteBatch, - DB, + SliceTransform, SnapshotWithThreadMode, UniversalCompactOptions, UniversalCompactionStopStyle, + WriteBatch, DB, }; use util::DBPath; @@ -265,7 +265,7 @@ fn snapshot_test() { #[derive(Clone)] struct SnapshotWrapper { - snapshot: Arc>, + snapshot: Arc>, } impl SnapshotWrapper { From c722e164c42cdedf61ea0a75f8bdbb20fdf5abe6 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 30 Mar 2021 20:26:42 +0900 Subject: [PATCH 21/30] Replace all remaining &CF with impl CFRef --- src/lib.rs | 3 ++- src/snapshot.rs | 17 ++++++++++------- src/write_batch.rs | 18 +++++++++--------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 6b5d6a80f..31a01e678 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -98,7 +98,8 @@ mod write_batch; pub use crate::{ column_family::{ - BoundColumnFamily, ColumnFamily, ColumnFamilyDescriptor, DEFAULT_COLUMN_FAMILY_NAME, + BoundColumnFamily, ColumnFamily, ColumnFamilyDescriptor, ColumnFamilyRef, + DEFAULT_COLUMN_FAMILY_NAME, }, compaction_filter::Decision as CompactionDecision, db::{DbWithThreadMode, LiveFile, MultiThreaded, SingleThreaded, DB}, diff --git a/src/snapshot.rs b/src/snapshot.rs index a291d3667..458410e8f 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -13,8 +13,8 @@ // limitations under the License. use crate::{ - column_family::ColumnFamilyRef, db::InternalDbAdapter, ffi, ColumnFamily, - DBIteratorWithThreadMode, DBRawIteratorWithThreadMode, Error, IteratorMode, ReadOptions, DB, + db::InternalDbAdapter, ffi, ColumnFamilyRef, DBIteratorWithThreadMode, + DBRawIteratorWithThreadMode, Error, IteratorMode, ReadOptions, DB, }; /// A consistent view of the database at the point of creation. @@ -64,7 +64,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// the default read options. pub fn iterator_cf( &self, - cf_handle: &ColumnFamily, + cf_handle: impl ColumnFamilyRef, mode: IteratorMode, ) -> DBIteratorWithThreadMode { let readopts = ReadOptions::default(); @@ -101,7 +101,10 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// Creates a raw iterator over the data in this snapshot under the given column family, using /// the default read options. - pub fn raw_iterator_cf(&self, cf_handle: &ColumnFamily) -> DBRawIteratorWithThreadMode { + pub fn raw_iterator_cf( + &self, + cf_handle: impl ColumnFamilyRef, + ) -> DBRawIteratorWithThreadMode { let readopts = ReadOptions::default(); self.raw_iterator_cf_opt(cf_handle, readopts) } @@ -116,7 +119,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// the given read options. pub fn raw_iterator_cf_opt( &self, - cf_handle: &ColumnFamily, + cf_handle: impl ColumnFamilyRef, mut readopts: ReadOptions, ) -> DBRawIteratorWithThreadMode { readopts.set_snapshot(self); @@ -133,7 +136,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// options. pub fn get_cf>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, key: K, ) -> Result>, Error> { let readopts = ReadOptions::default(); @@ -153,7 +156,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// Returns the bytes associated with a key value, given column family and read options. pub fn get_cf_opt>( &self, - cf: &ColumnFamily, + cf: impl ColumnFamilyRef, key: K, mut readopts: ReadOptions, ) -> Result>, Error> { diff --git a/src/write_batch.rs b/src/write_batch.rs index a2566bd36..d4dd7d14d 100644 --- a/src/write_batch.rs +++ b/src/write_batch.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{ffi, ColumnFamily}; +use crate::{ffi, ColumnFamilyRef}; use libc::{c_char, c_void, size_t}; use std::slice; @@ -134,7 +134,7 @@ impl WriteBatch { } } - pub fn put_cf(&mut self, cf: &ColumnFamily, key: K, value: V) + pub fn put_cf(&mut self, cf: impl ColumnFamilyRef, key: K, value: V) where K: AsRef<[u8]>, V: AsRef<[u8]>, @@ -145,7 +145,7 @@ impl WriteBatch { unsafe { ffi::rocksdb_writebatch_put_cf( self.inner, - cf.inner, + cf.inner(), key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, @@ -173,7 +173,7 @@ impl WriteBatch { } } - pub fn merge_cf(&mut self, cf: &ColumnFamily, key: K, value: V) + pub fn merge_cf(&mut self, cf: impl ColumnFamilyRef, key: K, value: V) where K: AsRef<[u8]>, V: AsRef<[u8]>, @@ -184,7 +184,7 @@ impl WriteBatch { unsafe { ffi::rocksdb_writebatch_merge_cf( self.inner, - cf.inner, + cf.inner(), key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, @@ -206,13 +206,13 @@ impl WriteBatch { } } - pub fn delete_cf>(&mut self, cf: &ColumnFamily, key: K) { + pub fn delete_cf>(&mut self, cf: impl ColumnFamilyRef, key: K) { let key = key.as_ref(); unsafe { ffi::rocksdb_writebatch_delete_cf( self.inner, - cf.inner, + cf.inner(), key.as_ptr() as *const c_char, key.len() as size_t, ); @@ -243,13 +243,13 @@ impl WriteBatch { /// Removes the database entries in the range ["begin_key", "end_key"), i.e., /// including "begin_key" and excluding "end_key". It is not an error if no /// keys exist in the range ["begin_key", "end_key"). - pub fn delete_range_cf>(&mut self, cf: &ColumnFamily, from: K, to: K) { + pub fn delete_range_cf>(&mut self, cf: impl ColumnFamilyRef, from: K, to: K) { let (start_key, end_key) = (from.as_ref(), to.as_ref()); unsafe { ffi::rocksdb_writebatch_delete_range_cf( self.inner, - cf.inner, + cf.inner(), start_key.as_ptr() as *const c_char, start_key.len() as size_t, end_key.as_ptr() as *const c_char, From 4016943551e22aeb7fd1f948ff02d7a8ed1e030c Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 30 Mar 2021 22:58:35 +0900 Subject: [PATCH 22/30] Expose compile-time handy ColumnFamilyRef alias --- src/column_family.rs | 12 ++++++-- src/db.rs | 72 +++++++++++++++++++++++--------------------- src/lib.rs | 4 +-- src/snapshot.rs | 14 ++++----- src/write_batch.rs | 10 +++--- 5 files changed, 61 insertions(+), 51 deletions(-) diff --git a/src/column_family.rs b/src/column_family.rs index 0a7a58b8b..60148691e 100644 --- a/src/column_family.rs +++ b/src/column_family.rs @@ -58,17 +58,23 @@ pub struct BoundColumnFamily<'a> { pub(crate) multi_threaded_cfs: std::marker::PhantomData<&'a MultiThreaded>, } -pub trait ColumnFamilyRef { +#[cfg(not(feature = "multi-threaded-as-default"))] +pub type ColumnFamilyRef<'a> = &'a ColumnFamily; + +#[cfg(feature = "multi-threaded-as-default")] +pub type ColumnFamilyRef<'a> = BoundColumnFamily<'a>; + +pub trait AsColumnFamilyRef { fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t; } -impl<'a> ColumnFamilyRef for &'a ColumnFamily { +impl<'a> AsColumnFamilyRef for &'a ColumnFamily { fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t { self.inner } } -impl<'a> ColumnFamilyRef for BoundColumnFamily<'a> { +impl<'a> AsColumnFamilyRef for BoundColumnFamily<'a> { fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t { self.inner } diff --git a/src/db.rs b/src/db.rs index 0457f8bab..77f9ba110 100644 --- a/src/db.rs +++ b/src/db.rs @@ -14,8 +14,8 @@ // use crate::{ + column_family::AsColumnFamilyRef, column_family::BoundColumnFamily, - column_family::ColumnFamilyRef, ffi, ffi_util::{from_cstr, opt_bytes_to_ptr, raw_data, to_cpath}, ColumnFamily, ColumnFamilyDescriptor, CompactOptions, DBIteratorWithThreadMode, @@ -100,7 +100,7 @@ pub trait InternalDbAdapter { fn get_cf_opt>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, readopts: &ReadOptions, ) -> Result>, Error>; @@ -121,7 +121,7 @@ impl InternalDbAdapter for DbWithThreadMode { fn get_cf_opt>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, readopts: &ReadOptions, ) -> Result>, Error> { @@ -505,7 +505,7 @@ impl DbWithThreadMode { /// Flushes database memtables to SST files on the disk for a given column family. pub fn flush_cf_opt( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, flushopts: &FlushOptions, ) -> Result<(), Error> { unsafe { @@ -520,7 +520,7 @@ impl DbWithThreadMode { /// Flushes database memtables to SST files on the disk for a given column family using default /// options. - pub fn flush_cf(&self, cf: impl ColumnFamilyRef) -> Result<(), Error> { + pub fn flush_cf(&self, cf: impl AsColumnFamilyRef) -> Result<(), Error> { self.flush_cf_opt(cf, &FlushOptions::default()) } @@ -565,7 +565,7 @@ impl DbWithThreadMode { /// [`get_pinned_cf_opt`](#method.get_pinned_cf_opt) to avoid unnecessary memory. pub fn get_cf_opt>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, readopts: &ReadOptions, ) -> Result>, Error> { @@ -578,7 +578,7 @@ impl DbWithThreadMode { /// [`get_pinned_cf`](#method.get_pinned_cf) to avoid unnecessary memory. pub fn get_cf>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, ) -> Result>, Error> { self.get_cf_opt(cf, key.as_ref(), &ReadOptions::default()) @@ -627,7 +627,7 @@ impl DbWithThreadMode { /// allows specifying ColumnFamily pub fn get_pinned_cf_opt>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, readopts: &ReadOptions, ) -> Result, Error> { @@ -661,7 +661,7 @@ impl DbWithThreadMode { /// leverages default options. pub fn get_pinned_cf>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, ) -> Result, Error> { self.get_pinned_cf_opt(cf, key, &ReadOptions::default()) @@ -714,7 +714,7 @@ impl DbWithThreadMode { where K: AsRef<[u8]>, I: IntoIterator, - W: ColumnFamilyRef, + W: AsColumnFamilyRef, { self.multi_get_cf_opt(keys, &ReadOptions::default()) } @@ -728,7 +728,7 @@ impl DbWithThreadMode { where K: AsRef<[u8]>, I: IntoIterator, - W: ColumnFamilyRef, + W: AsColumnFamilyRef, { let mut boxed_keys: Vec> = Vec::new(); let mut keys_sizes = Vec::new(); @@ -806,7 +806,7 @@ impl DbWithThreadMode { /// This is used when you want to iterate over a specific ColumnFamily with a modified ReadOptions pub fn iterator_cf_opt<'a: 'b, 'b>( &'a self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, readopts: ReadOptions, mode: IteratorMode, ) -> DBIteratorWithThreadMode<'b, Self> { @@ -840,7 +840,7 @@ impl DbWithThreadMode { pub fn iterator_cf<'a: 'b, 'b>( &'a self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, mode: IteratorMode, ) -> DBIteratorWithThreadMode<'b, Self> { let opts = ReadOptions::default(); @@ -849,7 +849,7 @@ impl DbWithThreadMode { pub fn full_iterator_cf<'a: 'b, 'b>( &'a self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, mode: IteratorMode, ) -> DBIteratorWithThreadMode<'b, Self> { let mut opts = ReadOptions::default(); @@ -859,7 +859,7 @@ impl DbWithThreadMode { pub fn prefix_iterator_cf<'a, P: AsRef<[u8]>>( &'a self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, prefix: P, ) -> DBIteratorWithThreadMode<'a, Self> { let mut opts = ReadOptions::default(); @@ -881,7 +881,7 @@ impl DbWithThreadMode { /// Opens a raw iterator over the given column family, using the default read options pub fn raw_iterator_cf<'a: 'b, 'b>( &'a self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, ) -> DBRawIteratorWithThreadMode<'b, Self> { let opts = ReadOptions::default(); DBRawIteratorWithThreadMode::new_cf(self, cf_handle.inner(), opts) @@ -898,7 +898,7 @@ impl DbWithThreadMode { /// Opens a raw iterator over the given column family, using the given read options pub fn raw_iterator_cf_opt<'a: 'b, 'b>( &'a self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, readopts: ReadOptions, ) -> DBRawIteratorWithThreadMode<'b, Self> { DBRawIteratorWithThreadMode::new_cf(self, cf_handle.inner(), readopts) @@ -931,7 +931,7 @@ impl DbWithThreadMode { pub fn put_cf_opt( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, value: V, writeopts: &WriteOptions, @@ -980,7 +980,7 @@ impl DbWithThreadMode { pub fn merge_cf_opt( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, value: V, writeopts: &WriteOptions, @@ -1026,7 +1026,7 @@ impl DbWithThreadMode { pub fn delete_cf_opt>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, writeopts: &WriteOptions, ) -> Result<(), Error> { @@ -1047,7 +1047,7 @@ impl DbWithThreadMode { /// Removes the database entries in the range `["from", "to")` using given write options. pub fn delete_range_cf_opt>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, from: K, to: K, writeopts: &WriteOptions, @@ -1077,7 +1077,7 @@ impl DbWithThreadMode { self.put_opt(key.as_ref(), value.as_ref(), &WriteOptions::default()) } - pub fn put_cf(&self, cf: impl ColumnFamilyRef, key: K, value: V) -> Result<(), Error> + pub fn put_cf(&self, cf: impl AsColumnFamilyRef, key: K, value: V) -> Result<(), Error> where K: AsRef<[u8]>, V: AsRef<[u8]>, @@ -1093,7 +1093,7 @@ impl DbWithThreadMode { self.merge_opt(key.as_ref(), value.as_ref(), &WriteOptions::default()) } - pub fn merge_cf(&self, cf: impl ColumnFamilyRef, key: K, value: V) -> Result<(), Error> + pub fn merge_cf(&self, cf: impl AsColumnFamilyRef, key: K, value: V) -> Result<(), Error> where K: AsRef<[u8]>, V: AsRef<[u8]>, @@ -1105,14 +1105,18 @@ impl DbWithThreadMode { self.delete_opt(key.as_ref(), &WriteOptions::default()) } - pub fn delete_cf>(&self, cf: impl ColumnFamilyRef, key: K) -> Result<(), Error> { + pub fn delete_cf>( + &self, + cf: impl AsColumnFamilyRef, + key: K, + ) -> Result<(), Error> { self.delete_cf_opt(cf, key.as_ref(), &WriteOptions::default()) } /// Removes the database entries in the range `["from", "to")` using default write options. pub fn delete_range_cf>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, from: K, to: K, ) -> Result<(), Error> { @@ -1161,7 +1165,7 @@ impl DbWithThreadMode { /// given column family. This is not likely to be needed for typical usage. pub fn compact_range_cf, E: AsRef<[u8]>>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, start: Option, end: Option, ) { @@ -1183,7 +1187,7 @@ impl DbWithThreadMode { /// Same as `compact_range_cf` but with custom options. pub fn compact_range_cf_opt, E: AsRef<[u8]>>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, start: Option, end: Option, opts: &CompactOptions, @@ -1222,7 +1226,7 @@ impl DbWithThreadMode { pub fn set_options_cf( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, opts: &[(&str, &str)], ) -> Result<(), Error> { let copts = convert_options(opts)?; @@ -1283,7 +1287,7 @@ impl DbWithThreadMode { /// [here](https://github.com/facebook/rocksdb/blob/08809f5e6cd9cc4bc3958dd4d59457ae78c76660/include/rocksdb/db.h#L428-L634). pub fn property_value_cf( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, name: &str, ) -> Result, Error> { let prop_name = match CString::new(name) { @@ -1341,7 +1345,7 @@ impl DbWithThreadMode { /// [here](https://github.com/facebook/rocksdb/blob/08809f5e6cd9cc4bc3958dd4d59457ae78c76660/include/rocksdb/db.h#L654-L689). pub fn property_int_value_cf( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, name: &str, ) -> Result, Error> { match self.property_value_cf(cf, name) { @@ -1418,7 +1422,7 @@ impl DbWithThreadMode { /// with default opts pub fn ingest_external_file_cf>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, paths: Vec

, ) -> Result<(), Error> { let opts = IngestExternalFileOptions::default(); @@ -1428,7 +1432,7 @@ impl DbWithThreadMode { /// Loads a list of external SST files created with SstFileWriter into the DB for given Column Family pub fn ingest_external_file_cf_opts>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, opts: &IngestExternalFileOptions, paths: Vec

, ) -> Result<(), Error> { @@ -1461,7 +1465,7 @@ impl DbWithThreadMode { fn ingest_external_file_raw_cf( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, opts: &IngestExternalFileOptions, paths_v: &[CString], cpaths: &[*const c_char], @@ -1550,7 +1554,7 @@ impl DbWithThreadMode { /// Same as `delete_file_in_range` but only for specific column family pub fn delete_file_in_range_cf>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, from: K, to: K, ) -> Result<(), Error> { diff --git a/src/lib.rs b/src/lib.rs index 31a01e678..42a61c490 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -98,8 +98,8 @@ mod write_batch; pub use crate::{ column_family::{ - BoundColumnFamily, ColumnFamily, ColumnFamilyDescriptor, ColumnFamilyRef, - DEFAULT_COLUMN_FAMILY_NAME, + AsColumnFamilyRef, BoundColumnFamily, ColumnFamily, ColumnFamilyDescriptor, + ColumnFamilyRef, DEFAULT_COLUMN_FAMILY_NAME, }, compaction_filter::Decision as CompactionDecision, db::{DbWithThreadMode, LiveFile, MultiThreaded, SingleThreaded, DB}, diff --git a/src/snapshot.rs b/src/snapshot.rs index 458410e8f..d3da190c1 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -13,7 +13,7 @@ // limitations under the License. use crate::{ - db::InternalDbAdapter, ffi, ColumnFamilyRef, DBIteratorWithThreadMode, + db::InternalDbAdapter, ffi, AsColumnFamilyRef, DBIteratorWithThreadMode, DBRawIteratorWithThreadMode, Error, IteratorMode, ReadOptions, DB, }; @@ -64,7 +64,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// the default read options. pub fn iterator_cf( &self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, mode: IteratorMode, ) -> DBIteratorWithThreadMode { let readopts = ReadOptions::default(); @@ -85,7 +85,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// the given read options. pub fn iterator_cf_opt( &self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, mut readopts: ReadOptions, mode: IteratorMode, ) -> DBIteratorWithThreadMode { @@ -103,7 +103,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// the default read options. pub fn raw_iterator_cf( &self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, ) -> DBRawIteratorWithThreadMode { let readopts = ReadOptions::default(); self.raw_iterator_cf_opt(cf_handle, readopts) @@ -119,7 +119,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// the given read options. pub fn raw_iterator_cf_opt( &self, - cf_handle: impl ColumnFamilyRef, + cf_handle: impl AsColumnFamilyRef, mut readopts: ReadOptions, ) -> DBRawIteratorWithThreadMode { readopts.set_snapshot(self); @@ -136,7 +136,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// options. pub fn get_cf>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, ) -> Result>, Error> { let readopts = ReadOptions::default(); @@ -156,7 +156,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { /// Returns the bytes associated with a key value, given column family and read options. pub fn get_cf_opt>( &self, - cf: impl ColumnFamilyRef, + cf: impl AsColumnFamilyRef, key: K, mut readopts: ReadOptions, ) -> Result>, Error> { diff --git a/src/write_batch.rs b/src/write_batch.rs index d4dd7d14d..00707f931 100644 --- a/src/write_batch.rs +++ b/src/write_batch.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{ffi, ColumnFamilyRef}; +use crate::{ffi, AsColumnFamilyRef}; use libc::{c_char, c_void, size_t}; use std::slice; @@ -134,7 +134,7 @@ impl WriteBatch { } } - pub fn put_cf(&mut self, cf: impl ColumnFamilyRef, key: K, value: V) + pub fn put_cf(&mut self, cf: impl AsColumnFamilyRef, key: K, value: V) where K: AsRef<[u8]>, V: AsRef<[u8]>, @@ -173,7 +173,7 @@ impl WriteBatch { } } - pub fn merge_cf(&mut self, cf: impl ColumnFamilyRef, key: K, value: V) + pub fn merge_cf(&mut self, cf: impl AsColumnFamilyRef, key: K, value: V) where K: AsRef<[u8]>, V: AsRef<[u8]>, @@ -206,7 +206,7 @@ impl WriteBatch { } } - pub fn delete_cf>(&mut self, cf: impl ColumnFamilyRef, key: K) { + pub fn delete_cf>(&mut self, cf: impl AsColumnFamilyRef, key: K) { let key = key.as_ref(); unsafe { @@ -243,7 +243,7 @@ impl WriteBatch { /// Removes the database entries in the range ["begin_key", "end_key"), i.e., /// including "begin_key" and excluding "end_key". It is not an error if no /// keys exist in the range ["begin_key", "end_key"). - pub fn delete_range_cf>(&mut self, cf: impl ColumnFamilyRef, from: K, to: K) { + pub fn delete_range_cf>(&mut self, cf: impl AsColumnFamilyRef, from: K, to: K) { let (start_key, end_key) = (from.as_ref(), to.as_ref()); unsafe { From d687029273f9e51ec5ca907d5e4bac36e1000d25 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 30 Mar 2021 23:07:42 +0900 Subject: [PATCH 23/30] Tweaks --- src/db_options.rs | 2 +- tests/test_db.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/db_options.rs b/src/db_options.rs index 9da33b4ed..a5a209286 100644 --- a/src/db_options.rs +++ b/src/db_options.rs @@ -2022,7 +2022,7 @@ impl Options { /// to not being able to determine whether there were any write conflicts. /// /// When using a TransactionDB: - /// If Transaction::SetSnapshotWithThreadMode is used, TransactionDB will read either + /// If Transaction::SetSnapshot is used, TransactionDB will read either /// in-memory write buffers or SST files to do write-conflict checking. /// Increasing this value can reduce the number of reads to SST files /// done for conflict detection. diff --git a/tests/test_db.rs b/tests/test_db.rs index 30d4b521d..543a4ac36 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -22,8 +22,8 @@ use rocksdb::{ perf::get_memory_usage_stats, BlockBasedOptions, BottommostLevelCompaction, Cache, CompactOptions, DBCompactionStyle, DbWithThreadMode, Env, Error, FifoCompactOptions, IteratorMode, MultiThreaded, Options, PerfContext, PerfMetric, ReadOptions, SingleThreaded, - SliceTransform, SnapshotWithThreadMode, UniversalCompactOptions, UniversalCompactionStopStyle, - WriteBatch, DB, + SliceTransform, Snapshot, UniversalCompactOptions, UniversalCompactionStopStyle, WriteBatch, + DB, }; use util::DBPath; @@ -265,7 +265,7 @@ fn snapshot_test() { #[derive(Clone)] struct SnapshotWrapper { - snapshot: Arc>, + snapshot: Arc>, } impl SnapshotWrapper { From 61f323bedab7c12a1db2dcfad943761d1eb4fc64 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 30 Mar 2021 23:28:17 +0900 Subject: [PATCH 24/30] Remove unneeded cfgs... --- src/db_iterator.rs | 9 --------- src/snapshot.rs | 4 ---- 2 files changed, 13 deletions(-) diff --git a/src/db_iterator.rs b/src/db_iterator.rs index 72194e935..68aad868e 100644 --- a/src/db_iterator.rs +++ b/src/db_iterator.rs @@ -66,11 +66,6 @@ use std::slice; /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` - -#[cfg(not(feature = "multi-threaded-as-default"))] -pub type DBRawIterator<'a> = DBRawIteratorWithThreadMode<'a, DB>; - -#[cfg(feature = "multi-threaded-as-default")] pub type DBRawIterator<'a> = DBRawIteratorWithThreadMode<'a, DB>; pub struct DBRawIteratorWithThreadMode<'a, D: InternalDbAdapter> { @@ -373,10 +368,6 @@ unsafe impl<'a, D: InternalDbAdapter> Sync for DBRawIteratorWithThreadMode<'a, D /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` -#[cfg(not(feature = "multi-threaded-as-default"))] -pub type DBIterator<'a> = DBIteratorWithThreadMode<'a, DB>; - -#[cfg(feature = "multi-threaded-as-default")] pub type DBIterator<'a> = DBIteratorWithThreadMode<'a, DB>; pub struct DBIteratorWithThreadMode<'a, D: InternalDbAdapter> { diff --git a/src/snapshot.rs b/src/snapshot.rs index d3da190c1..67ecb78c4 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -33,10 +33,6 @@ use crate::{ /// let _ = DB::destroy(&Options::default(), path); /// ``` /// -#[cfg(not(feature = "multi-threaded-as-default"))] -pub type Snapshot<'a> = SnapshotWithThreadMode<'a, DB>; - -#[cfg(feature = "multi-threaded-as-default")] pub type Snapshot<'a> = SnapshotWithThreadMode<'a, DB>; pub struct SnapshotWithThreadMode<'a, D: InternalDbAdapter> { From 7a16581438ec7864651915cc9a78411df3cf15a6 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 6 Apr 2021 14:35:02 +0900 Subject: [PATCH 25/30] Rename InternalDbAdapter => DbAccess --- src/db.rs | 4 ++-- src/db_iterator.rs | 22 ++++++++++------------ src/db_options.rs | 7 ++----- src/snapshot.rs | 14 +++++++------- 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/db.rs b/src/db.rs index 77f9ba110..5c489c602 100644 --- a/src/db.rs +++ b/src/db.rs @@ -89,7 +89,7 @@ pub struct DbWithThreadMode { path: PathBuf, } -pub trait InternalDbAdapter { +pub trait DbAccess { fn inner(&self) -> *mut ffi::rocksdb_t; fn get_opt>( @@ -106,7 +106,7 @@ pub trait InternalDbAdapter { ) -> Result>, Error>; } -impl InternalDbAdapter for DbWithThreadMode { +impl DbAccess for DbWithThreadMode { fn inner(&self) -> *mut ffi::rocksdb_t { self.inner } diff --git a/src/db_iterator.rs b/src/db_iterator.rs index 68aad868e..cbce2e853 100644 --- a/src/db_iterator.rs +++ b/src/db_iterator.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::db::{InternalDbAdapter, DB}; +use crate::db::{DbAccess, DB}; use crate::{ffi, Error, ReadOptions, WriteBatch}; use libc::{c_char, c_uchar, size_t}; use std::marker::PhantomData; @@ -68,7 +68,7 @@ use std::slice; /// ``` pub type DBRawIterator<'a> = DBRawIteratorWithThreadMode<'a, DB>; -pub struct DBRawIteratorWithThreadMode<'a, D: InternalDbAdapter> { +pub struct DBRawIteratorWithThreadMode<'a, D: DbAccess> { inner: *mut ffi::rocksdb_iterator_t, /// When iterate_upper_bound is set, the inner C iterator keeps a pointer to the upper bound @@ -79,7 +79,7 @@ pub struct DBRawIteratorWithThreadMode<'a, D: InternalDbAdapter> { db: PhantomData<&'a D>, } -impl<'a, D: InternalDbAdapter> DBRawIteratorWithThreadMode<'a, D> { +impl<'a, D: DbAccess> DBRawIteratorWithThreadMode<'a, D> { pub(crate) fn new(db: &D, readopts: ReadOptions) -> DBRawIteratorWithThreadMode<'a, D> { unsafe { DBRawIteratorWithThreadMode { @@ -326,7 +326,7 @@ impl<'a, D: InternalDbAdapter> DBRawIteratorWithThreadMode<'a, D> { } } -impl<'a, D: InternalDbAdapter> Drop for DBRawIteratorWithThreadMode<'a, D> { +impl<'a, D: DbAccess> Drop for DBRawIteratorWithThreadMode<'a, D> { fn drop(&mut self) { unsafe { ffi::rocksdb_iter_destroy(self.inner); @@ -334,8 +334,8 @@ impl<'a, D: InternalDbAdapter> Drop for DBRawIteratorWithThreadMode<'a, D> { } } -unsafe impl<'a, D: InternalDbAdapter> Send for DBRawIteratorWithThreadMode<'a, D> {} -unsafe impl<'a, D: InternalDbAdapter> Sync for DBRawIteratorWithThreadMode<'a, D> {} +unsafe impl<'a, D: DbAccess> Send for DBRawIteratorWithThreadMode<'a, D> {} +unsafe impl<'a, D: DbAccess> Sync for DBRawIteratorWithThreadMode<'a, D> {} /// An iterator over a database or column family, with specifiable /// ranges and direction. @@ -370,7 +370,7 @@ unsafe impl<'a, D: InternalDbAdapter> Sync for DBRawIteratorWithThreadMode<'a, D /// ``` pub type DBIterator<'a> = DBIteratorWithThreadMode<'a, DB>; -pub struct DBIteratorWithThreadMode<'a, D: InternalDbAdapter> { +pub struct DBIteratorWithThreadMode<'a, D: DbAccess> { raw: DBRawIteratorWithThreadMode<'a, D>, direction: Direction, just_seeked: bool, @@ -389,7 +389,7 @@ pub enum IteratorMode<'a> { From(&'a [u8], Direction), } -impl<'a, D: InternalDbAdapter> DBIteratorWithThreadMode<'a, D> { +impl<'a, D: DbAccess> DBIteratorWithThreadMode<'a, D> { pub(crate) fn new(db: &D, readopts: ReadOptions, mode: IteratorMode) -> Self { let mut rv = DBIteratorWithThreadMode { raw: DBRawIteratorWithThreadMode::new(db, readopts), @@ -449,7 +449,7 @@ impl<'a, D: InternalDbAdapter> DBIteratorWithThreadMode<'a, D> { } } -impl<'a, D: InternalDbAdapter> Iterator for DBIteratorWithThreadMode<'a, D> { +impl<'a, D: DbAccess> Iterator for DBIteratorWithThreadMode<'a, D> { type Item = KVBytes; fn next(&mut self) -> Option { @@ -480,9 +480,7 @@ impl<'a, D: InternalDbAdapter> Iterator for DBIteratorWithThreadMode<'a, D> { } } -impl<'a, D: InternalDbAdapter> Into> - for DBIteratorWithThreadMode<'a, D> -{ +impl<'a, D: DbAccess> Into> for DBIteratorWithThreadMode<'a, D> { fn into(self) -> DBRawIteratorWithThreadMode<'a, D> { self.raw } diff --git a/src/db_options.rs b/src/db_options.rs index a5a209286..17c0bcade 100644 --- a/src/db_options.rs +++ b/src/db_options.rs @@ -22,7 +22,7 @@ use crate::{ compaction_filter::{self, CompactionFilterCallback, CompactionFilterFn}, compaction_filter_factory::{self, CompactionFilterFactory}, comparator::{self, ComparatorCallback, CompareFn}, - db::InternalDbAdapter, + db::DbAccess, ffi, merge_operator::{ self, full_merge_callback, partial_merge_callback, MergeFn, MergeOperatorCallback, @@ -2850,10 +2850,7 @@ impl ReadOptions { /// Sets the snapshot which should be used for the read. /// The snapshot must belong to the DB that is being read and must /// not have been released. - pub(crate) fn set_snapshot( - &mut self, - snapshot: &SnapshotWithThreadMode, - ) { + pub(crate) fn set_snapshot(&mut self, snapshot: &SnapshotWithThreadMode) { unsafe { ffi::rocksdb_readoptions_set_snapshot(self.inner, snapshot.inner); } diff --git a/src/snapshot.rs b/src/snapshot.rs index 67ecb78c4..c02224522 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -13,8 +13,8 @@ // limitations under the License. use crate::{ - db::InternalDbAdapter, ffi, AsColumnFamilyRef, DBIteratorWithThreadMode, - DBRawIteratorWithThreadMode, Error, IteratorMode, ReadOptions, DB, + db::DbAccess, ffi, AsColumnFamilyRef, DBIteratorWithThreadMode, DBRawIteratorWithThreadMode, + Error, IteratorMode, ReadOptions, DB, }; /// A consistent view of the database at the point of creation. @@ -35,12 +35,12 @@ use crate::{ /// pub type Snapshot<'a> = SnapshotWithThreadMode<'a, DB>; -pub struct SnapshotWithThreadMode<'a, D: InternalDbAdapter> { +pub struct SnapshotWithThreadMode<'a, D: DbAccess> { db: &'a D, pub(crate) inner: *const ffi::rocksdb_snapshot_t, } -impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { +impl<'a, D: DbAccess> SnapshotWithThreadMode<'a, D> { /// Creates a new `SnapshotWithThreadMode` of the database `db`. pub fn new(db: &'a D) -> Self { let snapshot = unsafe { ffi::rocksdb_create_snapshot(db.inner()) }; @@ -161,7 +161,7 @@ impl<'a, D: InternalDbAdapter> SnapshotWithThreadMode<'a, D> { } } -impl<'a, D: InternalDbAdapter> Drop for SnapshotWithThreadMode<'a, D> { +impl<'a, D: DbAccess> Drop for SnapshotWithThreadMode<'a, D> { fn drop(&mut self) { unsafe { ffi::rocksdb_release_snapshot(self.db.inner(), self.inner); @@ -171,5 +171,5 @@ impl<'a, D: InternalDbAdapter> Drop for SnapshotWithThreadMode<'a, D> { /// `Send` and `Sync` implementations for `SnapshotWithThreadMode` are safe, because `SnapshotWithThreadMode` is /// immutable and can be safely shared between threads. -unsafe impl<'a, D: InternalDbAdapter> Send for SnapshotWithThreadMode<'a, D> {} -unsafe impl<'a, D: InternalDbAdapter> Sync for SnapshotWithThreadMode<'a, D> {} +unsafe impl<'a, D: DbAccess> Send for SnapshotWithThreadMode<'a, D> {} +unsafe impl<'a, D: DbAccess> Sync for SnapshotWithThreadMode<'a, D> {} From f8c7fd4cb54f24f34e14155f7e0269fc277e6fdb Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 6 Apr 2021 14:36:49 +0900 Subject: [PATCH 26/30] Rename: map => cfs --- src/db.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/db.rs b/src/db.rs index 5c489c602..972958675 100644 --- a/src/db.rs +++ b/src/db.rs @@ -47,34 +47,34 @@ pub trait ThreadMode { } pub struct SingleThreaded { - map: BTreeMap, + cfs: BTreeMap, } pub struct MultiThreaded { - map: RwLock>, + cfs: RwLock>, } impl ThreadMode for SingleThreaded { - fn new(cf_map: BTreeMap) -> Self { - Self { map: cf_map } + fn new(cfs: BTreeMap) -> Self { + Self { cfs } } unsafe fn cf_drop_all(&mut self) { - for cf in self.map.values() { + for cf in self.cfs.values() { ffi::rocksdb_column_family_handle_destroy(cf.inner); } } } impl ThreadMode for MultiThreaded { - fn new(cf_map: BTreeMap) -> Self { + fn new(cfs: BTreeMap) -> Self { Self { - map: RwLock::new(cf_map), + cfs: RwLock::new(cfs), } } unsafe fn cf_drop_all(&mut self) { - for cf in self.map.read().unwrap().values() { + for cf in self.cfs.read().unwrap().values() { ffi::rocksdb_column_family_handle_destroy(cf.inner); } } @@ -1586,7 +1586,7 @@ impl DbWithThreadMode { pub fn create_cf>(&mut self, name: N, opts: &Options) -> Result<(), Error> { let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; self.cfs - .map + .cfs .insert(name.as_ref().to_string(), ColumnFamily { inner }); Ok(()) } @@ -1594,7 +1594,7 @@ impl DbWithThreadMode { /// Drops the column family with the given name pub fn drop_cf(&mut self, name: &str) -> Result<(), Error> { let inner = self.inner; - if let Some(cf) = self.cfs.map.remove(name) { + if let Some(cf) = self.cfs.cfs.remove(name) { unsafe { ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); } @@ -1606,7 +1606,7 @@ impl DbWithThreadMode { /// Returns the underlying column family handle pub fn cf_handle<'a>(&'a self, name: &str) -> Option<&'a ColumnFamily> { - self.cfs.map.get(name) + self.cfs.cfs.get(name) } } @@ -1615,7 +1615,7 @@ impl DbWithThreadMode { pub fn create_cf>(&self, name: N, opts: &Options) -> Result<(), Error> { let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; self.cfs - .map + .cfs .write() .unwrap() .insert(name.as_ref().to_string(), ColumnFamily { inner }); @@ -1626,7 +1626,7 @@ impl DbWithThreadMode { /// family map. This avoids needing `&mut self` reference pub fn drop_cf(&self, name: &str) -> Result<(), Error> { let inner = self.inner; - if let Some(cf) = self.cfs.map.write().unwrap().remove(name) { + if let Some(cf) = self.cfs.cfs.write().unwrap().remove(name) { unsafe { ffi_try!(ffi::rocksdb_drop_column_family(inner, cf.inner)); } @@ -1639,7 +1639,7 @@ impl DbWithThreadMode { /// Returns the underlying column family handle pub fn cf_handle(&self, name: &str) -> Option { self.cfs - .map + .cfs .read() .unwrap() .get(name) From a59059750bab108f48df040613f6b6be1d56476a Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 6 Apr 2021 14:52:39 +0900 Subject: [PATCH 27/30] Rename: multi-threaded-as-default => multi-threaded-cf-alteration --- .github/workflows/rust.yml | 2 +- Cargo.toml | 2 +- src/column_family.rs | 4 ++-- src/db.rs | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index dbd6ab17e..0c7eb59b4 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -98,4 +98,4 @@ jobs: RUSTFLAGS: -Awarnings # Suppress "variable does not need to be mutable" warnings with: command: test - args: --features multi-threaded-as-default + args: --features multi-threaded-cf-alteration diff --git a/Cargo.toml b/Cargo.toml index 361e90776..29d36a761 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ lz4 = ["librocksdb-sys/lz4"] zstd = ["librocksdb-sys/zstd"] zlib = ["librocksdb-sys/zlib"] bzip2 = ["librocksdb-sys/bzip2"] -multi-threaded-as-default = [] +multi-threaded-cf-alteration = [] [dependencies] libc = "0.2" diff --git a/src/column_family.rs b/src/column_family.rs index 60148691e..e0f1d57ab 100644 --- a/src/column_family.rs +++ b/src/column_family.rs @@ -58,10 +58,10 @@ pub struct BoundColumnFamily<'a> { pub(crate) multi_threaded_cfs: std::marker::PhantomData<&'a MultiThreaded>, } -#[cfg(not(feature = "multi-threaded-as-default"))] +#[cfg(not(feature = "multi-threaded-cf-alteration"))] pub type ColumnFamilyRef<'a> = &'a ColumnFamily; -#[cfg(feature = "multi-threaded-as-default")] +#[cfg(feature = "multi-threaded-cf-alteration")] pub type ColumnFamilyRef<'a> = BoundColumnFamily<'a>; pub trait AsColumnFamilyRef { diff --git a/src/db.rs b/src/db.rs index 972958675..f198a7c6d 100644 --- a/src/db.rs +++ b/src/db.rs @@ -132,10 +132,10 @@ impl DbAccess for DbWithThreadMode { /// DB struct for single-threaded column family creations/deletions /// Note: Previously this was direct struct; type-aliased for compatibility. Directly /// use DbWithThreadMode for multi-threaded column family alternations. -#[cfg(not(feature = "multi-threaded-as-default"))] +#[cfg(not(feature = "multi-threaded-cf-alteration"))] pub type DB = DbWithThreadMode; -#[cfg(feature = "multi-threaded-as-default")] +#[cfg(feature = "multi-threaded-cf-alteration")] pub type DB = DbWithThreadMode; // Safety note: auto-implementing Send on most db-related types is prevented by the inner FFI From 1a11b5f5b68a0a6cf0456d20d004954b2b483666 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 6 Apr 2021 17:17:33 +0900 Subject: [PATCH 28/30] Remove unsafe from trait definition --- src/db.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/db.rs b/src/db.rs index f198a7c6d..7c4097c0d 100644 --- a/src/db.rs +++ b/src/db.rs @@ -43,7 +43,7 @@ use std::time::Duration; // MultiThreaded. Others differ in self mutability and return type. pub trait ThreadMode { fn new(cf_map: BTreeMap) -> Self; - unsafe fn cf_drop_all(&mut self); + fn cf_drop_all(&mut self); } pub struct SingleThreaded { @@ -59,9 +59,11 @@ impl ThreadMode for SingleThreaded { Self { cfs } } - unsafe fn cf_drop_all(&mut self) { + fn cf_drop_all(&mut self) { for cf in self.cfs.values() { - ffi::rocksdb_column_family_handle_destroy(cf.inner); + unsafe { + ffi::rocksdb_column_family_handle_destroy(cf.inner); + } } } } @@ -73,9 +75,11 @@ impl ThreadMode for MultiThreaded { } } - unsafe fn cf_drop_all(&mut self) { + fn cf_drop_all(&mut self) { for cf in self.cfs.read().unwrap().values() { - ffi::rocksdb_column_family_handle_destroy(cf.inner); + unsafe { + ffi::rocksdb_column_family_handle_destroy(cf.inner); + } } } } From 2cfe746ae0dbe5a40c92c0e61f925bddd1f59714 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 6 Apr 2021 15:05:09 +0900 Subject: [PATCH 29/30] Improve docs --- .github/workflows/rust.yml | 6 ++-- Cargo.toml | 2 +- README.md | 9 +++++ src/column_family.rs | 16 +++++---- src/db.rs | 68 ++++++++++++++++++++++++++++---------- src/db_iterator.rs | 30 +++++++++-------- src/db_options.rs | 4 +-- src/lib.rs | 2 +- src/snapshot.rs | 17 +++++----- tests/test_db.rs | 6 ++-- 10 files changed, 104 insertions(+), 56 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 0c7eb59b4..a21299f3e 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -88,14 +88,14 @@ jobs: with: command: test args: --manifest-path=librocksdb-sys/Cargo.toml - - name: Run rocksdb tests (single-threaded cf alterations) + - name: Run rocksdb tests (single-threaded cf) uses: actions-rs/cargo@v1 with: command: test - - name: Run rocksdb tests (multi-threaded cf alterations) + - name: Run rocksdb tests (multi-threaded cf) uses: actions-rs/cargo@v1 env: RUSTFLAGS: -Awarnings # Suppress "variable does not need to be mutable" warnings with: command: test - args: --features multi-threaded-cf-alteration + args: --features multi-threaded-cf diff --git a/Cargo.toml b/Cargo.toml index 29d36a761..b333b97d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ lz4 = ["librocksdb-sys/lz4"] zstd = ["librocksdb-sys/zstd"] zlib = ["librocksdb-sys/zlib"] bzip2 = ["librocksdb-sys/bzip2"] -multi-threaded-cf-alteration = [] +multi-threaded-cf = [] [dependencies] libc = "0.2" diff --git a/README.md b/README.md index 62102f380..d0541a4ad 100644 --- a/README.md +++ b/README.md @@ -42,3 +42,12 @@ compression support, make these changes to your Cargo.toml: default-features = false features = ["lz4"] ``` + +## Multi-threaded ColumnFamily alternation + +The underlying RocksDB does allow column families to be created and dropped +from multiple threads concurrently. But this crate doesn't allow it by default +for compatibility. If you need to modify column families concurrently, enable +crate feature called `multi-threaded-cf`, which makes this binding's +data structures to use RwLock by default. Alternatively, you can directly create +`DBWithThreadMode` without enabling the crate feature. diff --git a/src/column_family.rs b/src/column_family.rs index e0f1d57ab..1967f7d81 100644 --- a/src/column_family.rs +++ b/src/column_family.rs @@ -47,10 +47,10 @@ pub struct ColumnFamily { pub(crate) inner: *mut ffi::rocksdb_column_family_handle_t, } -/// A specialized opaque type used to represent a column family. Used for multi-threaded -/// mode. Clone (and Copy) is derived to behave like &ColumnFamily (used for -/// single-threaded). Clone/Copy is safe because this lifetime is bound to DB like -/// iterators/snapshots. On top of it, this is as cheap and small as &ColumnFamily because +/// A specialized opaque type used to represent a column family by the [`MultiThreaded`] +/// mode. Clone (and Copy) is derived to behave like `&ColumnFamily` (this is used for +/// single-threaded mode). `Clone`/`Copy` is safe because this lifetime is bound to DB like +/// iterators/snapshots. On top of it, this is as cheap and small as `&ColumnFamily` because /// this only has a single pointer-wide field. #[derive(Clone, Copy)] pub struct BoundColumnFamily<'a> { @@ -58,12 +58,16 @@ pub struct BoundColumnFamily<'a> { pub(crate) multi_threaded_cfs: std::marker::PhantomData<&'a MultiThreaded>, } -#[cfg(not(feature = "multi-threaded-cf-alteration"))] +/// Handy type alias to hide actual type difference to reference [`ColumnFamily`] +/// depending on the `multi-threaded-cf` crate feature. +#[cfg(not(feature = "multi-threaded-cf"))] pub type ColumnFamilyRef<'a> = &'a ColumnFamily; -#[cfg(feature = "multi-threaded-cf-alteration")] +#[cfg(feature = "multi-threaded-cf")] pub type ColumnFamilyRef<'a> = BoundColumnFamily<'a>; +/// Utility trait to accept both supported references to `ColumnFamily` +/// (`&ColumnFamily` and `BoundColumnFamily`) pub trait AsColumnFamilyRef { fn inner(&self) -> *mut ffi::rocksdb_column_family_handle_t; } diff --git a/src/db.rs b/src/db.rs index 7c4097c0d..684d6b172 100644 --- a/src/db.rs +++ b/src/db.rs @@ -38,7 +38,7 @@ use std::str; use std::sync::RwLock; use std::time::Duration; -// Marker trait to specify single or multi threaded column family alterations for DB +// Marker trait to specify single or multi threaded column family alternations for DB // Also, this is minimum common API sharable between SingleThreaded and // MultiThreaded. Others differ in self mutability and return type. pub trait ThreadMode { @@ -46,10 +46,21 @@ pub trait ThreadMode { fn cf_drop_all(&mut self); } +/// Actual marker type for the internal marker trait `ThreadMode`, which holds +/// a collection of column families without synchronization primitive, providing +/// no overhead for the single-threaded column family alternations. The other +/// mode is [`MultiThreaded`]. +/// +/// See [`DB`] for more details, including performance implications for each mode pub struct SingleThreaded { cfs: BTreeMap, } +/// Actual marker type for the internal marker trait `ThreadMode`, which holds +/// a collection of column families wrapped in a RwLock to be mutated +/// concurrently. The other mode is [`SingleThreaded`]. +/// +/// See [`DB`] for more details, including performance implications for each mode pub struct MultiThreaded { cfs: RwLock>, } @@ -87,13 +98,15 @@ impl ThreadMode for MultiThreaded { /// A RocksDB database. /// /// See crate level documentation for a simple usage example. -pub struct DbWithThreadMode { +pub struct DBWithThreadMode { pub(crate) inner: *mut ffi::rocksdb_t, cfs: T, // Column families are held differently depending on thread mode path: PathBuf, } -pub trait DbAccess { +/// Minimal set of DB-related methods, intended to be generic over +/// `DBWithThreadMode`. Mainly used internally +pub trait DBAccess { fn inner(&self) -> *mut ffi::rocksdb_t; fn get_opt>( @@ -110,7 +123,7 @@ pub trait DbAccess { ) -> Result>, Error>; } -impl DbAccess for DbWithThreadMode { +impl DBAccess for DBWithThreadMode { fn inner(&self) -> *mut ffi::rocksdb_t { self.inner } @@ -133,23 +146,42 @@ impl DbAccess for DbWithThreadMode { } } -/// DB struct for single-threaded column family creations/deletions -/// Note: Previously this was direct struct; type-aliased for compatibility. Directly -/// use DbWithThreadMode for multi-threaded column family alternations. -#[cfg(not(feature = "multi-threaded-cf-alteration"))] -pub type DB = DbWithThreadMode; +/// A type alias to DB instance type with the single-threaded column family +/// creations/deletions +/// +/// # Compatibility and multi-threaded mode +/// +/// Previously, `DB` was defined as a direct struct. Now, it's type-aliased for +/// compatibility. Use `DBWithThreadMode` for multi-threaded +/// column family alternations. +/// +/// # Limited performance implication for single-threaded mode +/// +/// Even with [`SingleThreaded`], almost all of RocksDB operations is +/// multi-threaded unless the underlying RocksDB instance is +/// specifically configured otherwise. `SingleThreaded` only forces +/// serialization of column family alternations by requring `&mut self` of DB +/// instance due to its wrapper implementation details. +/// +/// # Multi-threaded mode +/// +/// [`MultiThreaded`] can be appropriate for the situation of multi-threaded +/// workload including multi-threaded column family alternations, costing the +/// RwLock overhead inside `DB`. +#[cfg(not(feature = "multi-threaded-cf"))] +pub type DB = DBWithThreadMode; -#[cfg(feature = "multi-threaded-cf-alteration")] -pub type DB = DbWithThreadMode; +#[cfg(feature = "multi-threaded-cf")] +pub type DB = DBWithThreadMode; // Safety note: auto-implementing Send on most db-related types is prevented by the inner FFI // pointer. In most cases, however, this pointer is Send-safe because it is never aliased and // rocksdb internally does not rely on thread-local information for its user-exposed types. -unsafe impl Send for DbWithThreadMode {} +unsafe impl Send for DBWithThreadMode {} // Sync is similarly safe for many types because they do not expose interior mutability, and their // use within the rocksdb library is generally behind a const reference -unsafe impl Sync for DbWithThreadMode {} +unsafe impl Sync for DBWithThreadMode {} // Specifies whether open DB for read only. enum AccessType<'a> { @@ -159,7 +191,7 @@ enum AccessType<'a> { WithTTL { ttl: Duration }, } -impl DbWithThreadMode { +impl DBWithThreadMode { /// Opens a database with default options. pub fn open_default>(path: P) -> Result { let mut opts = Options::default(); @@ -1585,7 +1617,7 @@ impl DbWithThreadMode { } } -impl DbWithThreadMode { +impl DBWithThreadMode { /// Creates column family with given name and options pub fn create_cf>(&mut self, name: N, opts: &Options) -> Result<(), Error> { let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; @@ -1614,7 +1646,7 @@ impl DbWithThreadMode { } } -impl DbWithThreadMode { +impl DBWithThreadMode { /// Creates column family with given name and options pub fn create_cf>(&self, name: N, opts: &Options) -> Result<(), Error> { let inner = self.create_inner_cf_handle(name.as_ref(), opts)?; @@ -1654,7 +1686,7 @@ impl DbWithThreadMode { } } -impl Drop for DbWithThreadMode { +impl Drop for DBWithThreadMode { fn drop(&mut self) { unsafe { self.cfs.cf_drop_all(); @@ -1663,7 +1695,7 @@ impl Drop for DbWithThreadMode { } } -impl fmt::Debug for DbWithThreadMode { +impl fmt::Debug for DBWithThreadMode { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "RocksDB {{ path: {:?} }}", self.path()) } diff --git a/src/db_iterator.rs b/src/db_iterator.rs index cbce2e853..2a958c87e 100644 --- a/src/db_iterator.rs +++ b/src/db_iterator.rs @@ -12,12 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::db::{DbAccess, DB}; +use crate::db::{DBAccess, DB}; use crate::{ffi, Error, ReadOptions, WriteBatch}; use libc::{c_char, c_uchar, size_t}; use std::marker::PhantomData; use std::slice; +/// A type alias to keep compatibility. See [`DBRawIteratorWithThreadMode`] for details +pub type DBRawIterator<'a> = DBRawIteratorWithThreadMode<'a, DB>; + /// An iterator over a database or column family, with specifiable /// ranges and direction. /// @@ -66,9 +69,7 @@ use std::slice; /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` -pub type DBRawIterator<'a> = DBRawIteratorWithThreadMode<'a, DB>; - -pub struct DBRawIteratorWithThreadMode<'a, D: DbAccess> { +pub struct DBRawIteratorWithThreadMode<'a, D: DBAccess> { inner: *mut ffi::rocksdb_iterator_t, /// When iterate_upper_bound is set, the inner C iterator keeps a pointer to the upper bound @@ -79,7 +80,7 @@ pub struct DBRawIteratorWithThreadMode<'a, D: DbAccess> { db: PhantomData<&'a D>, } -impl<'a, D: DbAccess> DBRawIteratorWithThreadMode<'a, D> { +impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> { pub(crate) fn new(db: &D, readopts: ReadOptions) -> DBRawIteratorWithThreadMode<'a, D> { unsafe { DBRawIteratorWithThreadMode { @@ -326,7 +327,7 @@ impl<'a, D: DbAccess> DBRawIteratorWithThreadMode<'a, D> { } } -impl<'a, D: DbAccess> Drop for DBRawIteratorWithThreadMode<'a, D> { +impl<'a, D: DBAccess> Drop for DBRawIteratorWithThreadMode<'a, D> { fn drop(&mut self) { unsafe { ffi::rocksdb_iter_destroy(self.inner); @@ -334,8 +335,11 @@ impl<'a, D: DbAccess> Drop for DBRawIteratorWithThreadMode<'a, D> { } } -unsafe impl<'a, D: DbAccess> Send for DBRawIteratorWithThreadMode<'a, D> {} -unsafe impl<'a, D: DbAccess> Sync for DBRawIteratorWithThreadMode<'a, D> {} +unsafe impl<'a, D: DBAccess> Send for DBRawIteratorWithThreadMode<'a, D> {} +unsafe impl<'a, D: DBAccess> Sync for DBRawIteratorWithThreadMode<'a, D> {} + +/// A type alias to keep compatibility. See [`DBIteratorWithThreadMode`] for details +pub type DBIterator<'a> = DBIteratorWithThreadMode<'a, DB>; /// An iterator over a database or column family, with specifiable /// ranges and direction. @@ -368,9 +372,7 @@ unsafe impl<'a, D: DbAccess> Sync for DBRawIteratorWithThreadMode<'a, D> {} /// } /// let _ = DB::destroy(&Options::default(), path); /// ``` -pub type DBIterator<'a> = DBIteratorWithThreadMode<'a, DB>; - -pub struct DBIteratorWithThreadMode<'a, D: DbAccess> { +pub struct DBIteratorWithThreadMode<'a, D: DBAccess> { raw: DBRawIteratorWithThreadMode<'a, D>, direction: Direction, just_seeked: bool, @@ -389,7 +391,7 @@ pub enum IteratorMode<'a> { From(&'a [u8], Direction), } -impl<'a, D: DbAccess> DBIteratorWithThreadMode<'a, D> { +impl<'a, D: DBAccess> DBIteratorWithThreadMode<'a, D> { pub(crate) fn new(db: &D, readopts: ReadOptions, mode: IteratorMode) -> Self { let mut rv = DBIteratorWithThreadMode { raw: DBRawIteratorWithThreadMode::new(db, readopts), @@ -449,7 +451,7 @@ impl<'a, D: DbAccess> DBIteratorWithThreadMode<'a, D> { } } -impl<'a, D: DbAccess> Iterator for DBIteratorWithThreadMode<'a, D> { +impl<'a, D: DBAccess> Iterator for DBIteratorWithThreadMode<'a, D> { type Item = KVBytes; fn next(&mut self) -> Option { @@ -480,7 +482,7 @@ impl<'a, D: DbAccess> Iterator for DBIteratorWithThreadMode<'a, D> { } } -impl<'a, D: DbAccess> Into> for DBIteratorWithThreadMode<'a, D> { +impl<'a, D: DBAccess> Into> for DBIteratorWithThreadMode<'a, D> { fn into(self) -> DBRawIteratorWithThreadMode<'a, D> { self.raw } diff --git a/src/db_options.rs b/src/db_options.rs index 17c0bcade..c99207e38 100644 --- a/src/db_options.rs +++ b/src/db_options.rs @@ -22,7 +22,7 @@ use crate::{ compaction_filter::{self, CompactionFilterCallback, CompactionFilterFn}, compaction_filter_factory::{self, CompactionFilterFactory}, comparator::{self, ComparatorCallback, CompareFn}, - db::DbAccess, + db::DBAccess, ffi, merge_operator::{ self, full_merge_callback, partial_merge_callback, MergeFn, MergeOperatorCallback, @@ -2850,7 +2850,7 @@ impl ReadOptions { /// Sets the snapshot which should be used for the read. /// The snapshot must belong to the DB that is being read and must /// not have been released. - pub(crate) fn set_snapshot(&mut self, snapshot: &SnapshotWithThreadMode) { + pub(crate) fn set_snapshot(&mut self, snapshot: &SnapshotWithThreadMode) { unsafe { ffi::rocksdb_readoptions_set_snapshot(self.inner, snapshot.inner); } diff --git a/src/lib.rs b/src/lib.rs index 42a61c490..2858c4af1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -102,7 +102,7 @@ pub use crate::{ ColumnFamilyRef, DEFAULT_COLUMN_FAMILY_NAME, }, compaction_filter::Decision as CompactionDecision, - db::{DbWithThreadMode, LiveFile, MultiThreaded, SingleThreaded, DB}, + db::{DBWithThreadMode, LiveFile, MultiThreaded, SingleThreaded, DB}, db_iterator::{ DBIterator, DBIteratorWithThreadMode, DBRawIterator, DBRawIteratorWithThreadMode, DBWALIterator, Direction, IteratorMode, diff --git a/src/snapshot.rs b/src/snapshot.rs index c02224522..0f77b03cf 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -13,10 +13,13 @@ // limitations under the License. use crate::{ - db::DbAccess, ffi, AsColumnFamilyRef, DBIteratorWithThreadMode, DBRawIteratorWithThreadMode, + db::DBAccess, ffi, AsColumnFamilyRef, DBIteratorWithThreadMode, DBRawIteratorWithThreadMode, Error, IteratorMode, ReadOptions, DB, }; +/// A type alias to keep compatibility. See [`SnapshotWithThreadMode`] for details +pub type Snapshot<'a> = SnapshotWithThreadMode<'a, DB>; + /// A consistent view of the database at the point of creation. /// /// # Examples @@ -33,14 +36,12 @@ use crate::{ /// let _ = DB::destroy(&Options::default(), path); /// ``` /// -pub type Snapshot<'a> = SnapshotWithThreadMode<'a, DB>; - -pub struct SnapshotWithThreadMode<'a, D: DbAccess> { +pub struct SnapshotWithThreadMode<'a, D: DBAccess> { db: &'a D, pub(crate) inner: *const ffi::rocksdb_snapshot_t, } -impl<'a, D: DbAccess> SnapshotWithThreadMode<'a, D> { +impl<'a, D: DBAccess> SnapshotWithThreadMode<'a, D> { /// Creates a new `SnapshotWithThreadMode` of the database `db`. pub fn new(db: &'a D) -> Self { let snapshot = unsafe { ffi::rocksdb_create_snapshot(db.inner()) }; @@ -161,7 +162,7 @@ impl<'a, D: DbAccess> SnapshotWithThreadMode<'a, D> { } } -impl<'a, D: DbAccess> Drop for SnapshotWithThreadMode<'a, D> { +impl<'a, D: DBAccess> Drop for SnapshotWithThreadMode<'a, D> { fn drop(&mut self) { unsafe { ffi::rocksdb_release_snapshot(self.db.inner(), self.inner); @@ -171,5 +172,5 @@ impl<'a, D: DbAccess> Drop for SnapshotWithThreadMode<'a, D> { /// `Send` and `Sync` implementations for `SnapshotWithThreadMode` are safe, because `SnapshotWithThreadMode` is /// immutable and can be safely shared between threads. -unsafe impl<'a, D: DbAccess> Send for SnapshotWithThreadMode<'a, D> {} -unsafe impl<'a, D: DbAccess> Sync for SnapshotWithThreadMode<'a, D> {} +unsafe impl<'a, D: DBAccess> Send for SnapshotWithThreadMode<'a, D> {} +unsafe impl<'a, D: DBAccess> Sync for SnapshotWithThreadMode<'a, D> {} diff --git a/tests/test_db.rs b/tests/test_db.rs index 543a4ac36..92d4b8297 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -20,7 +20,7 @@ use pretty_assertions::assert_eq; use rocksdb::{ perf::get_memory_usage_stats, BlockBasedOptions, BottommostLevelCompaction, Cache, - CompactOptions, DBCompactionStyle, DbWithThreadMode, Env, Error, FifoCompactOptions, + CompactOptions, DBCompactionStyle, DBWithThreadMode, Env, Error, FifoCompactOptions, IteratorMode, MultiThreaded, Options, PerfContext, PerfMetric, ReadOptions, SingleThreaded, SliceTransform, Snapshot, UniversalCompactOptions, UniversalCompactionStopStyle, WriteBatch, DB, @@ -533,7 +533,7 @@ fn test_open_with_ttl() { fn test_open_as_single_threaded() { let primary_path = DBPath::new("_rust_rocksdb_test_open_as_single_threaded"); - let mut db = DbWithThreadMode::::open_default(&primary_path).unwrap(); + let mut db = DBWithThreadMode::::open_default(&primary_path).unwrap(); let db_ref1 = &mut db; let opts = Options::default(); db_ref1.create_cf("cf1", &opts).unwrap(); @@ -543,7 +543,7 @@ fn test_open_as_single_threaded() { fn test_open_as_multi_threaded() { let primary_path = DBPath::new("_rust_rocksdb_test_open_as_multi_threaded"); - let db = DbWithThreadMode::::open_default(&primary_path).unwrap(); + let db = DBWithThreadMode::::open_default(&primary_path).unwrap(); let db_ref1 = &db; let db_ref2 = &db; let opts = Options::default(); From 9fd26a3666322b7cdd9363e0d8d8e467999c5329 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 6 Apr 2021 19:45:54 +0900 Subject: [PATCH 30/30] Test error by creating CFs via SingleThreaded &ref --- ...pen_with_multiple_refs_as_single_threaded.rs | 10 ++++++++++ ...with_multiple_refs_as_single_threaded.stderr | 17 +++++++++++++++++ tests/test_db.rs | 10 +++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 tests/fail/open_with_multiple_refs_as_single_threaded.rs create mode 100644 tests/fail/open_with_multiple_refs_as_single_threaded.stderr diff --git a/tests/fail/open_with_multiple_refs_as_single_threaded.rs b/tests/fail/open_with_multiple_refs_as_single_threaded.rs new file mode 100644 index 000000000..8bf3524c0 --- /dev/null +++ b/tests/fail/open_with_multiple_refs_as_single_threaded.rs @@ -0,0 +1,10 @@ +use rocksdb::{SingleThreaded, DBWithThreadMode, Options}; + +fn main() { + let db = DBWithThreadMode::::open_default("/path/to/dummy").unwrap(); + let db_ref1 = &db; + let db_ref2 = &db; + let opts = Options::default(); + db_ref1.create_cf("cf1", &opts).unwrap(); + db_ref2.create_cf("cf2", &opts).unwrap(); +} diff --git a/tests/fail/open_with_multiple_refs_as_single_threaded.stderr b/tests/fail/open_with_multiple_refs_as_single_threaded.stderr new file mode 100644 index 000000000..61879c0cd --- /dev/null +++ b/tests/fail/open_with_multiple_refs_as_single_threaded.stderr @@ -0,0 +1,17 @@ +error[E0596]: cannot borrow `*db_ref1` as mutable, as it is behind a `&` reference + --> $DIR/open_with_multiple_refs_as_single_threaded.rs:8:5 + | +5 | let db_ref1 = &db; + | --- help: consider changing this to be a mutable reference: `&mut db` +... +8 | db_ref1.create_cf("cf1", &opts).unwrap(); + | ^^^^^^^ `db_ref1` is a `&` reference, so the data it refers to cannot be borrowed as mutable + +error[E0596]: cannot borrow `*db_ref2` as mutable, as it is behind a `&` reference + --> $DIR/open_with_multiple_refs_as_single_threaded.rs:9:5 + | +6 | let db_ref2 = &db; + | --- help: consider changing this to be a mutable reference: `&mut db` +... +9 | db_ref2.create_cf("cf2", &opts).unwrap(); + | ^^^^^^^ `db_ref2` is a `&` reference, so the data it refers to cannot be borrowed as mutable diff --git a/tests/test_db.rs b/tests/test_db.rs index 92d4b8297..9c82c5e00 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -540,7 +540,8 @@ fn test_open_as_single_threaded() { } #[test] -fn test_open_as_multi_threaded() { +fn test_open_with_multiple_refs_as_multi_threaded() { + // This tests multiple references can be allowed while creating column families let primary_path = DBPath::new("_rust_rocksdb_test_open_as_multi_threaded"); let db = DBWithThreadMode::::open_default(&primary_path).unwrap(); @@ -551,6 +552,13 @@ fn test_open_as_multi_threaded() { db_ref2.create_cf("cf2", &opts).unwrap(); } +#[test] +fn test_open_with_multiple_refs_as_single_threaded() { + // This tests multiple references CANNOT be allowed while creating column families + let t = trybuild::TestCases::new(); + t.compile_fail("tests/fail/open_with_multiple_refs_as_single_threaded.rs"); +} + #[test] fn compact_range_test() { let path = DBPath::new("_rust_rocksdb_compact_range_test");