From 4e3fbbce6ff692fa56358e94e3368cd234f4065b Mon Sep 17 00:00:00 2001 From: Christopher Berner Date: Thu, 21 Dec 2023 09:16:50 -0800 Subject: [PATCH] Remove lifetime from Btree --- src/db.rs | 22 ++++++++++++++++++---- src/multimap_table.rs | 24 +++++++++++++++++++----- src/table.rs | 13 ++++++++++--- src/transactions.rs | 34 ++++++++++++++++++++++++++-------- src/tree_store/btree.rs | 27 ++++++++++++++++++--------- src/tree_store/table_tree.rs | 4 +++- 6 files changed, 94 insertions(+), 30 deletions(-) diff --git a/src/db.rs b/src/db.rs index d090823e..d6394116 100644 --- a/src/db.rs +++ b/src/db.rs @@ -337,12 +337,21 @@ impl Database { pub(crate) fn verify_primary_checksums(mem: Arc) -> Result { let fake_freed_pages = Arc::new(Mutex::new(vec![])); - let table_tree = TableTree::new(mem.get_data_root(), mem.clone(), fake_freed_pages.clone()); + let table_tree = TableTree::new( + mem.get_data_root(), + Arc::new(TransactionGuard::fake()), + mem.clone(), + fake_freed_pages.clone(), + ); if !table_tree.verify_checksums()? { return Ok(false); } - let system_table_tree = - TableTree::new(mem.get_system_root(), mem.clone(), fake_freed_pages.clone()); + let system_table_tree = TableTree::new( + mem.get_system_root(), + Arc::new(TransactionGuard::fake()), + mem.clone(), + fake_freed_pages.clone(), + ); if !system_table_tree.verify_checksums()? { return Ok(false); } @@ -444,7 +453,12 @@ impl Database { oldest_unprocessed_free_transaction: TransactionId, ) -> Result { let freed_list = Arc::new(Mutex::new(vec![])); - let table_tree = TableTree::new(system_root, mem.clone(), freed_list); + let table_tree = TableTree::new( + system_root, + Arc::new(TransactionGuard::fake()), + mem.clone(), + freed_list, + ); let fake_transaction_tracker = Arc::new(TransactionTracker::new(TransactionId::new(0))); if let Some(savepoint_table_def) = table_tree .get_table::( diff --git a/src/multimap_table.rs b/src/multimap_table.rs index 71112d01..5eb6cc5c 100644 --- a/src/multimap_table.rs +++ b/src/multimap_table.rs @@ -769,7 +769,12 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> MultimapTable<'db, ' name: name.to_string(), transaction, freed_pages: freed_pages.clone(), - tree: BtreeMut::new(table_root, mem.clone(), freed_pages), + tree: BtreeMut::new( + table_root, + transaction.transaction_guard(), + mem.clone(), + freed_pages, + ), mem, _value_type: Default::default(), } @@ -866,6 +871,7 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> MultimapTable<'db, ' // Don't bother computing the checksum, since we're about to modify the tree let mut subtree: BtreeMut<'_, V, ()> = BtreeMut::new( Some((page_number, 0)), + self.transaction.transaction_guard(), self.mem.clone(), self.freed_pages.clone(), ); @@ -883,6 +889,7 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> MultimapTable<'db, ' Subtree => { let mut subtree: BtreeMut<'_, V, ()> = BtreeMut::new( Some(guard.value().as_subtree()), + self.transaction.transaction_guard(), self.mem.clone(), self.freed_pages.clone(), ); @@ -920,8 +927,12 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> MultimapTable<'db, ' self.tree .insert(key.borrow(), &DynamicCollection::new(&inline_data))?; } else { - let mut subtree: BtreeMut<'_, V, ()> = - BtreeMut::new(None, self.mem.clone(), self.freed_pages.clone()); + let mut subtree: BtreeMut<'_, V, ()> = BtreeMut::new( + None, + self.transaction.transaction_guard(), + self.mem.clone(), + self.freed_pages.clone(), + ); subtree.insert(value.borrow(), &())?; let (new_root, new_checksum) = subtree.get_root().unwrap(); let subtree_data = @@ -1008,6 +1019,7 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> MultimapTable<'db, ' Subtree => { let mut subtree: BtreeMut = BtreeMut::new( Some(v.as_subtree()), + self.transaction.transaction_guard(), self.mem.clone(), self.freed_pages.clone(), ); @@ -1262,10 +1274,11 @@ impl<'txn> ReadOnlyUntypedMultimapTable<'txn> { /// A read-only multimap table pub struct ReadOnlyMultimapTable<'txn, K: RedbKey + 'static, V: RedbKey + 'static> { - tree: Btree<'txn, K, &'static DynamicCollection>, + tree: Btree>, mem: Arc, transaction_guard: Arc, _value_type: PhantomData, + _lifetime: PhantomData<&'txn ()>, } impl<'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadOnlyMultimapTable<'txn, K, V> { @@ -1276,10 +1289,11 @@ impl<'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadOnlyMultimapTable<'tx mem: Arc, ) -> Result> { Ok(ReadOnlyMultimapTable { - tree: Btree::new(root_page, hint, mem.clone())?, + tree: Btree::new(root_page, hint, guard.clone(), mem.clone())?, mem, transaction_guard: guard, _value_type: Default::default(), + _lifetime: Default::default(), }) } } diff --git a/src/table.rs b/src/table.rs index 959d0614..2541df77 100644 --- a/src/table.rs +++ b/src/table.rs @@ -81,7 +81,12 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbValue + 'static> Table<'db, 'txn, K Table { name: name.to_string(), transaction, - tree: BtreeMut::new(table_root, mem, freed_pages), + tree: BtreeMut::new( + table_root, + transaction.transaction_guard(), + mem, + freed_pages, + ), } } @@ -414,8 +419,9 @@ impl<'txn> ReadOnlyUntypedTable<'txn> { /// A read-only table pub struct ReadOnlyTable<'txn, K: RedbKey + 'static, V: RedbValue + 'static> { name: String, - tree: Btree<'txn, K, V>, + tree: Btree, transaction_guard: Arc, + _lifetime: PhantomData<&'txn ()>, } impl<'txn, K: RedbKey + 'static, V: RedbValue + 'static> ReadOnlyTable<'txn, K, V> { @@ -428,8 +434,9 @@ impl<'txn, K: RedbKey + 'static, V: RedbValue + 'static> ReadOnlyTable<'txn, K, ) -> Result> { Ok(ReadOnlyTable { name, - tree: Btree::new(root_page, hint, mem)?, + tree: Btree::new(root_page, hint, guard.clone(), mem)?, transaction_guard: guard, + _lifetime: Default::default(), }) } } diff --git a/src/transactions.rs b/src/transactions.rs index 79151bd2..2782eee2 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -209,7 +209,7 @@ impl<'db, 's, K: RedbKey + 'static, V: RedbValue + 'static> SystemTable<'db, 's, SystemTable { name: name.to_string(), namespace, - tree: BtreeMut::new(table_root, mem, freed_pages), + tree: BtreeMut::new(table_root, guard.clone(), mem, freed_pages), transaction_guard: guard, } } @@ -457,10 +457,20 @@ impl<'db> WriteTransaction<'db> { let tables = TableNamespace { open_tables: Default::default(), - table_tree: TableTree::new(root_page, db.get_memory(), freed_pages.clone()), + table_tree: TableTree::new( + root_page, + guard.clone(), + db.get_memory(), + freed_pages.clone(), + ), }; let system_tables = SystemNamespace { - table_tree: TableTree::new(system_page, db.get_memory(), freed_pages.clone()), + table_tree: TableTree::new( + system_page, + guard.clone(), + db.get_memory(), + freed_pages.clone(), + ), transaction_guard: guard.clone(), }; @@ -468,12 +478,13 @@ impl<'db> WriteTransaction<'db> { db, transaction_tracker, mem: db.get_memory(), - transaction_guard: guard, + transaction_guard: guard.clone(), transaction_id, tables: Mutex::new(tables), system_tables: Mutex::new(system_tables), freed_tree: Mutex::new(BtreeMut::new( freed_root, + guard, db.get_memory(), post_commit_frees.clone(), )), @@ -682,6 +693,7 @@ impl<'db> WriteTransaction<'db> { *self.freed_pages.lock().unwrap() = freed_pages; self.tables.lock().unwrap().table_tree = TableTree::new( savepoint.get_user_root(), + self.transaction_guard.clone(), self.mem.clone(), self.freed_pages.clone(), ); @@ -702,6 +714,7 @@ impl<'db> WriteTransaction<'db> { let mut freed_tree = BtreeMut::new( savepoint.get_freed_root(), + self.transaction_guard.clone(), self.mem.clone(), self.post_commit_frees.clone(), ); @@ -1122,8 +1135,12 @@ impl<'db> WriteTransaction<'db> { .unwrap() { eprintln!("Master tree:"); - let master_tree: Btree<&str, InternalTableDefinition> = - Btree::new(Some(page), PageHint::None, self.mem.clone())?; + let master_tree: Btree<&str, InternalTableDefinition> = Btree::new( + Some(page), + PageHint::None, + self.transaction_guard.clone(), + self.mem.clone(), + )?; master_tree.print_debug(true)?; } @@ -1155,10 +1172,11 @@ pub struct ReadTransaction<'a> { impl<'db> ReadTransaction<'db> { pub(crate) fn new(mem: Arc, guard: TransactionGuard) -> Self { let root_page = mem.get_data_root(); + let guard = Arc::new(guard); Self { mem: mem.clone(), - tree: TableTree::new(root_page, mem, Default::default()), - transaction_guard: Arc::new(guard), + tree: TableTree::new(root_page, guard.clone(), mem, Default::default()), + transaction_guard: guard, } } diff --git a/src/tree_store/btree.rs b/src/tree_store/btree.rs index a2cfe8b7..7495c85b 100644 --- a/src/tree_store/btree.rs +++ b/src/tree_store/btree.rs @@ -1,3 +1,4 @@ +use crate::db::TransactionGuard; use crate::tree_store::btree_base::{ branch_checksum, leaf_checksum, BranchAccessor, BranchMutator, Checksum, LeafAccessor, BRANCH, DEFERRED, LEAF, @@ -218,6 +219,7 @@ impl UntypedBtreeMut { pub(crate) struct BtreeMut<'a, K: RedbKey, V: RedbValue> { mem: Arc, + transaction_guard: Arc, root: Arc>>, freed_pages: Arc>>, _key_type: PhantomData, @@ -228,11 +230,13 @@ pub(crate) struct BtreeMut<'a, K: RedbKey, V: RedbValue> { impl<'a, K: RedbKey + 'a, V: RedbValue + 'a> BtreeMut<'a, K, V> { pub(crate) fn new( root: Option<(PageNumber, Checksum)>, + guard: Arc, mem: Arc, freed_pages: Arc>>, ) -> Self { Self { mem, + transaction_guard: guard, root: Arc::new(Mutex::new(root)), freed_pages, _key_type: Default::default(), @@ -342,8 +346,13 @@ impl<'a, K: RedbKey + 'a, V: RedbValue + 'a> BtreeMut<'a, K, V> { ) } - fn read_tree(&self) -> Result> { - Btree::new(self.get_root(), PageHint::None, self.mem.clone()) + fn read_tree(&self) -> Result> { + Btree::new( + self.get_root(), + PageHint::None, + self.transaction_guard.clone(), + self.mem.clone(), + ) } pub(crate) fn get(&self, key: &K::SelfType<'_>) -> Result>> { @@ -543,22 +552,22 @@ impl RawBtree { } } -pub(crate) struct Btree<'a, K: RedbKey, V: RedbValue> { +pub(crate) struct Btree { mem: Arc, + _transaction_guard: Arc, // Cache of the root page to avoid repeated lookups cached_root: Option, root: Option<(PageNumber, Checksum)>, hint: PageHint, _key_type: PhantomData, _value_type: PhantomData, - // TODO: can we remove this and replace it with a TransactionGuard? - _lifetime: PhantomData<&'a ()>, } -impl<'a, K: RedbKey, V: RedbValue> Btree<'a, K, V> { +impl Btree { pub(crate) fn new( root: Option<(PageNumber, Checksum)>, hint: PageHint, + guard: Arc, mem: Arc, ) -> Result { let cached_root = if let Some((r, _)) = root { @@ -568,12 +577,12 @@ impl<'a, K: RedbKey, V: RedbValue> Btree<'a, K, V> { }; Ok(Self { mem, + _transaction_guard: guard, cached_root, root, hint, _key_type: Default::default(), _value_type: Default::default(), - _lifetime: Default::default(), }) } @@ -581,7 +590,7 @@ impl<'a, K: RedbKey, V: RedbValue> Btree<'a, K, V> { self.root } - pub(crate) fn get(&self, key: &K::SelfType<'_>) -> Result>> { + pub(crate) fn get(&self, key: &K::SelfType<'_>) -> Result>> { if let Some(ref root_page) = self.cached_root { self.get_helper(root_page.clone(), K::as_bytes(key).as_ref()) } else { @@ -590,7 +599,7 @@ impl<'a, K: RedbKey, V: RedbValue> Btree<'a, K, V> { } // Returns the value for the queried key, if present - fn get_helper(&self, page: PageImpl, query: &[u8]) -> Result>> { + fn get_helper(&self, page: PageImpl, query: &[u8]) -> Result>> { let node_mem = page.memory(); match node_mem[0] { LEAF => { diff --git a/src/tree_store/table_tree.rs b/src/tree_store/table_tree.rs index a09a4419..2fd55fac 100644 --- a/src/tree_store/table_tree.rs +++ b/src/tree_store/table_tree.rs @@ -1,3 +1,4 @@ +use crate::db::TransactionGuard; use crate::error::TableError; use crate::multimap_table::{ finalize_tree_and_subtree_checksums, multimap_btree_stats, verify_tree_and_subtree_checksums, @@ -417,11 +418,12 @@ pub(crate) struct TableTree<'txn> { impl<'txn> TableTree<'txn> { pub(crate) fn new( master_root: Option<(PageNumber, Checksum)>, + guard: Arc, mem: Arc, freed_pages: Arc>>, ) -> Self { Self { - tree: BtreeMut::new(master_root, mem.clone(), freed_pages.clone()), + tree: BtreeMut::new(master_root, guard, mem.clone(), freed_pages.clone()), mem, pending_table_updates: Default::default(), freed_pages,