From 51ced6036602826ff653c6fc283cbf11fc7b4865 Mon Sep 17 00:00:00 2001 From: Julien Kritter Date: Thu, 12 Sep 2024 10:26:28 +0200 Subject: [PATCH 1/2] Don't fetch old values in cross-partition transactional inserts --- src/db/lib.rs | 8 ++++---- src/db/lmdb_adapter.rs | 10 ++++------ src/db/sqlite_adapter.rs | 32 +++++++++----------------------- 3 files changed, 17 insertions(+), 33 deletions(-) diff --git a/src/db/lib.rs b/src/db/lib.rs index d6057505..3485745a 100644 --- a/src/db/lib.rs +++ b/src/db/lib.rs @@ -274,12 +274,12 @@ impl<'a> Transaction<'a> { tree: &Tree, key: T, value: U, - ) -> TxOpResult> { + ) -> TxOpResult<()> { self.tx.insert(tree.1, key.as_ref(), value.as_ref()) } /// Returns the old value if there was one #[inline] - pub fn remove>(&mut self, tree: &Tree, key: T) -> TxOpResult> { + pub fn remove>(&mut self, tree: &Tree, key: T) -> TxOpResult<()> { self.tx.remove(tree.1, key.as_ref()) } /// Clears all values in a tree @@ -362,8 +362,8 @@ pub(crate) trait ITx { fn get(&self, tree: usize, key: &[u8]) -> TxOpResult>; fn len(&self, tree: usize) -> TxOpResult; - fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult>; - fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult>; + fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult<()>; + fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult<()>; fn clear(&mut self, tree: usize) -> TxOpResult<()>; fn iter(&self, tree: usize) -> TxOpResult>; diff --git a/src/db/lmdb_adapter.rs b/src/db/lmdb_adapter.rs index 436a67fa..de4c3910 100644 --- a/src/db/lmdb_adapter.rs +++ b/src/db/lmdb_adapter.rs @@ -252,17 +252,15 @@ impl<'a> ITx for LmdbTx<'a> { Ok(tree.len(&self.tx)? as usize) } - fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult> { + fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult<()> { let tree = *self.get_tree(tree)?; - let old_val = tree.get(&self.tx, key)?.map(Vec::from); tree.put(&mut self.tx, key, value)?; - Ok(old_val) + Ok(()) } - fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult> { + fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult<()> { let tree = *self.get_tree(tree)?; - let old_val = tree.get(&self.tx, key)?.map(Vec::from); tree.delete(&mut self.tx, key)?; - Ok(old_val) + Ok(()) } fn clear(&mut self, tree: usize) -> TxOpResult<()> { let tree = *self.get_tree(tree)?; diff --git a/src/db/sqlite_adapter.rs b/src/db/sqlite_adapter.rs index 5a142117..9c9a668d 100644 --- a/src/db/sqlite_adapter.rs +++ b/src/db/sqlite_adapter.rs @@ -192,7 +192,7 @@ impl IDb for SqliteDb { let db = self.db.get()?; let lock = self.write_lock.lock(); - let n = db.execute(&format!("DELETE FROM {} WHERE k = ?1", tree), params![key])?; + db.execute(&format!("DELETE FROM {} WHERE k = ?1", tree), params![key])?; drop(lock); Ok(()) @@ -336,31 +336,17 @@ impl<'a> ITx for SqliteTx<'a> { } } - fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult> { + fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult<()> { let tree = self.get_tree(tree)?; - let old_val = self.internal_get(tree, key)?; - - let sql = match &old_val { - Some(_) => format!("UPDATE {} SET v = ?2 WHERE k = ?1", tree), - None => format!("INSERT INTO {} (k, v) VALUES (?1, ?2)", tree), - }; - let n = self.tx.execute(&sql, params![key, value])?; - assert_eq!(n, 1); - - Ok(old_val) + let sql = format!("INSERT OR REPLACE INTO {} (k, v) VALUES (?1, ?2)", tree); + self.tx.execute(&sql, params![key, value])?; + Ok(()) } - fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult> { + fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult<()> { let tree = self.get_tree(tree)?; - let old_val = self.internal_get(tree, key)?; - - if old_val.is_some() { - let n = self - .tx - .execute(&format!("DELETE FROM {} WHERE k = ?1", tree), params![key])?; - assert_eq!(n, 1); - } - - Ok(old_val) + self.tx + .execute(&format!("DELETE FROM {} WHERE k = ?1", tree), params![key])?; + Ok(()) } fn clear(&mut self, tree: usize) -> TxOpResult<()> { let tree = self.get_tree(tree)?; From bd71728874e7f03c547246cb0076804f62db102f Mon Sep 17 00:00:00 2001 From: Julien Kritter Date: Thu, 12 Sep 2024 10:50:53 +0200 Subject: [PATCH 2/2] Tests: don't expect old value after transactional insert --- 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 e3b7badf..26b816b8 100644 --- a/src/db/test.rs +++ b/src/db/test.rs @@ -21,7 +21,7 @@ fn test_suite(db: Db) { let res = db.transaction::<_, (), _>(|tx| { assert_eq!(tx.get(&tree, ka).unwrap().unwrap(), va); - assert_eq!(tx.insert(&tree, ka, vb).unwrap().unwrap(), va); + assert_eq!(tx.insert(&tree, ka, vb).unwrap(), ()); assert_eq!(tx.get(&tree, ka).unwrap().unwrap(), vb); @@ -33,7 +33,7 @@ fn test_suite(db: Db) { let res = db.transaction::<(), _, _>(|tx| { assert_eq!(tx.get(&tree, ka).unwrap().unwrap(), vb); - assert_eq!(tx.insert(&tree, ka, vc).unwrap().unwrap(), vb); + assert_eq!(tx.insert(&tree, ka, vc).unwrap(), ()); assert_eq!(tx.get(&tree, ka).unwrap().unwrap(), vc);