From 74363c906065a559beef4c3c93e8f73a7ecff437 Mon Sep 17 00:00:00 2001 From: marvin-j97 Date: Wed, 4 Sep 2024 18:45:17 +0200 Subject: [PATCH 1/3] perf(kv): dont retrieve values for write ops see https://git.deuxfleurs.fr/Deuxfleurs/garage/issues/851 --- src/db/lib.rs | 12 ++++-------- src/db/lmdb_adapter.rs | 10 ++++------ src/db/sqlite_adapter.rs | 16 ++++++---------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/db/lib.rs b/src/db/lib.rs index c8f9e13f..d6057505 100644 --- a/src/db/lib.rs +++ b/src/db/lib.rs @@ -211,16 +211,12 @@ impl Tree { /// Returns the old value if there was one #[inline] - pub fn insert, U: AsRef<[u8]>>( - &self, - key: T, - value: U, - ) -> Result> { + pub fn insert, U: AsRef<[u8]>>(&self, key: T, value: U) -> Result<()> { self.0.insert(self.1, key.as_ref(), value.as_ref()) } /// Returns the old value if there was one #[inline] - pub fn remove>(&self, key: T) -> Result> { + pub fn remove>(&self, key: T) -> Result<()> { self.0.remove(self.1, key.as_ref()) } /// Clears all values from the tree @@ -339,8 +335,8 @@ pub(crate) trait IDb: Send + Sync { fn get(&self, tree: usize, key: &[u8]) -> Result>; fn len(&self, tree: usize) -> Result; - fn insert(&self, tree: usize, key: &[u8], value: &[u8]) -> Result>; - fn remove(&self, tree: usize, key: &[u8]) -> Result>; + fn insert(&self, tree: usize, key: &[u8], value: &[u8]) -> Result<()>; + fn remove(&self, tree: usize, key: &[u8]) -> Result<()>; fn clear(&self, tree: usize) -> Result<()>; fn iter(&self, tree: usize) -> Result>; diff --git a/src/db/lmdb_adapter.rs b/src/db/lmdb_adapter.rs index d5066664..436a67fa 100644 --- a/src/db/lmdb_adapter.rs +++ b/src/db/lmdb_adapter.rs @@ -132,22 +132,20 @@ impl IDb for LmdbDb { Ok(tree.len(&tx)?.try_into().unwrap()) } - fn insert(&self, tree: usize, key: &[u8], value: &[u8]) -> Result> { + fn insert(&self, tree: usize, key: &[u8], value: &[u8]) -> Result<()> { let tree = self.get_tree(tree)?; let mut tx = self.db.write_txn()?; - let old_val = tree.get(&tx, key)?.map(Vec::from); tree.put(&mut tx, key, value)?; tx.commit()?; - Ok(old_val) + Ok(()) } - fn remove(&self, tree: usize, key: &[u8]) -> Result> { + fn remove(&self, tree: usize, key: &[u8]) -> Result<()> { let tree = self.get_tree(tree)?; let mut tx = self.db.write_txn()?; - let old_val = tree.get(&tx, key)?.map(Vec::from); tree.delete(&mut tx, key)?; tx.commit()?; - Ok(old_val) + Ok(()) } fn clear(&self, tree: usize) -> Result<()> { diff --git a/src/db/sqlite_adapter.rs b/src/db/sqlite_adapter.rs index a91b9011..eb106a31 100644 --- a/src/db/sqlite_adapter.rs +++ b/src/db/sqlite_adapter.rs @@ -169,7 +169,7 @@ impl IDb for SqliteDb { } } - fn insert(&self, tree: usize, key: &[u8], value: &[u8]) -> Result> { + fn insert(&self, tree: usize, key: &[u8], value: &[u8]) -> Result<()> { let tree = self.get_tree(tree)?; let db = self.db.get()?; let lock = self.write_lock.lock(); @@ -184,23 +184,19 @@ impl IDb for SqliteDb { assert_eq!(n, 1); drop(lock); - Ok(old_val) + Ok(()) } - fn remove(&self, tree: usize, key: &[u8]) -> Result> { + fn remove(&self, tree: usize, key: &[u8]) -> Result<()> { let tree = self.get_tree(tree)?; let db = self.db.get()?; let lock = self.write_lock.lock(); - let old_val = self.internal_get(&db, &tree, key)?; - - if old_val.is_some() { - let n = db.execute(&format!("DELETE FROM {} WHERE k = ?1", tree), params![key])?; - assert_eq!(n, 1); - } + let n = db.execute(&format!("DELETE FROM {} WHERE k = ?1", tree), params![key])?; + assert_eq!(n, 1); drop(lock); - Ok(old_val) + Ok(()) } fn clear(&self, tree: usize) -> Result<()> { From eb416a02fb9f6d7fef23d52d87624e2c23ec0336 Mon Sep 17 00:00:00 2001 From: marvin-j97 Date: Wed, 4 Sep 2024 18:51:51 +0200 Subject: [PATCH 2/3] dont assert deletion count in sqlite KV adapter --- src/db/sqlite_adapter.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/db/sqlite_adapter.rs b/src/db/sqlite_adapter.rs index eb106a31..5a142117 100644 --- a/src/db/sqlite_adapter.rs +++ b/src/db/sqlite_adapter.rs @@ -193,7 +193,6 @@ impl IDb for SqliteDb { let lock = self.write_lock.lock(); let n = db.execute(&format!("DELETE FROM {} WHERE k = ?1", tree), params![key])?; - assert_eq!(n, 1); drop(lock); Ok(()) From 8062ec7b4b6115e6158ec1b5523b4bd8dc0185be Mon Sep 17 00:00:00 2001 From: marvin-j97 Date: Wed, 4 Sep 2024 19:24:36 +0200 Subject: [PATCH 3/3] test: fix db tests --- src/db/test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/db/test.rs b/src/db/test.rs index adb429e7..e3b7badf 100644 --- a/src/db/test.rs +++ b/src/db/test.rs @@ -12,7 +12,7 @@ fn test_suite(db: Db) { // ---- test simple insert/delete ---- - assert!(tree.insert(ka, va).unwrap().is_none()); + assert!(tree.insert(ka, va).is_ok()); assert_eq!(tree.get(ka).unwrap().unwrap(), va); assert_eq!(tree.len().unwrap(), 1); @@ -50,7 +50,7 @@ fn test_suite(db: Db) { assert!(iter.next().is_none()); drop(iter); - assert!(tree.insert(kb, vc).unwrap().is_none()); + assert!(tree.insert(kb, vc).is_ok()); assert_eq!(tree.get(kb).unwrap().unwrap(), vc); let mut iter = tree.iter().unwrap();