Merge pull request 'Don't fetch old values in cross-partition transactional inserts' (#877) from withings/garage:perf/kv/insert-no-return-cross-partition into main

Reviewed-on: Deuxfleurs/garage#877
This commit is contained in:
Alex 2024-09-14 15:57:27 +00:00
commit 6da1353541
4 changed files with 19 additions and 35 deletions

View file

@ -274,12 +274,12 @@ impl<'a> Transaction<'a> {
tree: &Tree, tree: &Tree,
key: T, key: T,
value: U, value: U,
) -> TxOpResult<Option<Value>> { ) -> TxOpResult<()> {
self.tx.insert(tree.1, key.as_ref(), value.as_ref()) self.tx.insert(tree.1, key.as_ref(), value.as_ref())
} }
/// Returns the old value if there was one /// Returns the old value if there was one
#[inline] #[inline]
pub fn remove<T: AsRef<[u8]>>(&mut self, tree: &Tree, key: T) -> TxOpResult<Option<Value>> { pub fn remove<T: AsRef<[u8]>>(&mut self, tree: &Tree, key: T) -> TxOpResult<()> {
self.tx.remove(tree.1, key.as_ref()) self.tx.remove(tree.1, key.as_ref())
} }
/// Clears all values in a tree /// Clears all values in a tree
@ -362,8 +362,8 @@ pub(crate) trait ITx {
fn get(&self, tree: usize, key: &[u8]) -> TxOpResult<Option<Value>>; fn get(&self, tree: usize, key: &[u8]) -> TxOpResult<Option<Value>>;
fn len(&self, tree: usize) -> TxOpResult<usize>; fn len(&self, tree: usize) -> TxOpResult<usize>;
fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult<Option<Value>>; fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult<()>;
fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult<Option<Value>>; fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult<()>;
fn clear(&mut self, tree: usize) -> TxOpResult<()>; fn clear(&mut self, tree: usize) -> TxOpResult<()>;
fn iter(&self, tree: usize) -> TxOpResult<TxValueIter<'_>>; fn iter(&self, tree: usize) -> TxOpResult<TxValueIter<'_>>;

View file

@ -252,17 +252,15 @@ impl<'a> ITx for LmdbTx<'a> {
Ok(tree.len(&self.tx)? as usize) Ok(tree.len(&self.tx)? as usize)
} }
fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult<Option<Value>> { fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult<()> {
let tree = *self.get_tree(tree)?; let tree = *self.get_tree(tree)?;
let old_val = tree.get(&self.tx, key)?.map(Vec::from);
tree.put(&mut self.tx, key, value)?; tree.put(&mut self.tx, key, value)?;
Ok(old_val) Ok(())
} }
fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult<Option<Value>> { fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult<()> {
let tree = *self.get_tree(tree)?; let tree = *self.get_tree(tree)?;
let old_val = tree.get(&self.tx, key)?.map(Vec::from);
tree.delete(&mut self.tx, key)?; tree.delete(&mut self.tx, key)?;
Ok(old_val) Ok(())
} }
fn clear(&mut self, tree: usize) -> TxOpResult<()> { fn clear(&mut self, tree: usize) -> TxOpResult<()> {
let tree = *self.get_tree(tree)?; let tree = *self.get_tree(tree)?;

View file

@ -192,7 +192,7 @@ impl IDb for SqliteDb {
let db = self.db.get()?; let db = self.db.get()?;
let lock = self.write_lock.lock(); 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); drop(lock);
Ok(()) Ok(())
@ -336,31 +336,17 @@ impl<'a> ITx for SqliteTx<'a> {
} }
} }
fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult<Option<Value>> { fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> TxOpResult<()> {
let tree = self.get_tree(tree)?; let tree = self.get_tree(tree)?;
let old_val = self.internal_get(tree, key)?; let sql = format!("INSERT OR REPLACE INTO {} (k, v) VALUES (?1, ?2)", tree);
self.tx.execute(&sql, params![key, value])?;
let sql = match &old_val { Ok(())
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)
} }
fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult<Option<Value>> { fn remove(&mut self, tree: usize, key: &[u8]) -> TxOpResult<()> {
let tree = self.get_tree(tree)?; let tree = self.get_tree(tree)?;
let old_val = self.internal_get(tree, key)?; self.tx
if old_val.is_some() {
let n = self
.tx
.execute(&format!("DELETE FROM {} WHERE k = ?1", tree), params![key])?; .execute(&format!("DELETE FROM {} WHERE k = ?1", tree), params![key])?;
assert_eq!(n, 1); Ok(())
}
Ok(old_val)
} }
fn clear(&mut self, tree: usize) -> TxOpResult<()> { fn clear(&mut self, tree: usize) -> TxOpResult<()> {
let tree = self.get_tree(tree)?; let tree = self.get_tree(tree)?;

View file

@ -21,7 +21,7 @@ fn test_suite(db: Db) {
let res = db.transaction::<_, (), _>(|tx| { let res = db.transaction::<_, (), _>(|tx| {
assert_eq!(tx.get(&tree, ka).unwrap().unwrap(), va); 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); assert_eq!(tx.get(&tree, ka).unwrap().unwrap(), vb);
@ -33,7 +33,7 @@ fn test_suite(db: Db) {
let res = db.transaction::<(), _, _>(|tx| { let res = db.transaction::<(), _, _>(|tx| {
assert_eq!(tx.get(&tree, ka).unwrap().unwrap(), vb); 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); assert_eq!(tx.get(&tree, ka).unwrap().unwrap(), vc);