From 916aff189b239a575a3c64ef2d280a40dea2f1b8 Mon Sep 17 00:00:00 2001 From: Christopher Berner Date: Thu, 21 Dec 2023 08:59:10 -0800 Subject: [PATCH] Pass TransactionGuard into Range --- src/db.rs | 25 +++++++++-- src/multimap_table.rs | 96 +++++++++++++++++++++++++++++-------------- src/table.rs | 16 ++++++-- src/transactions.rs | 25 ++++++++--- 4 files changed, 120 insertions(+), 42 deletions(-) diff --git a/src/db.rs b/src/db.rs index c5aa3df6..d090823e 100644 --- a/src/db.rs +++ b/src/db.rs @@ -215,7 +215,7 @@ impl<'a, K: RedbKey + 'static, V: RedbKey + 'static> Display for MultimapTableDe } pub(crate) struct TransactionGuard { - transaction_tracker: Arc, + transaction_tracker: Option>, transaction_id: Option, write_transaction: bool, } @@ -226,7 +226,7 @@ impl TransactionGuard { tracker: Arc, ) -> Self { Self { - transaction_tracker: tracker, + transaction_tracker: Some(tracker), transaction_id: Some(transaction_id), write_transaction: false, } @@ -237,12 +237,21 @@ impl TransactionGuard { tracker: Arc, ) -> Self { Self { - transaction_tracker: tracker, + transaction_tracker: Some(tracker), transaction_id: Some(transaction_id), write_transaction: true, } } + // TODO: remove this hack + pub(crate) fn fake() -> Self { + Self { + transaction_tracker: None, + transaction_id: None, + write_transaction: false, + } + } + pub(crate) fn id(&self) -> TransactionId { self.transaction_id.unwrap() } @@ -254,12 +263,19 @@ impl TransactionGuard { impl Drop for TransactionGuard { fn drop(&mut self) { + if self.transaction_tracker.is_none() { + return; + } if let Some(transaction_id) = self.transaction_id { if self.write_transaction { self.transaction_tracker + .as_ref() + .unwrap() .end_write_transaction(transaction_id); } else { self.transaction_tracker + .as_ref() + .unwrap() .deallocate_read_transaction(transaction_id); } } @@ -444,6 +460,7 @@ impl Database { "internal savepoint table".to_string(), savepoint_table_def.get_root(), PageHint::None, + Arc::new(TransactionGuard::fake()), mem.clone(), )?; for result in savepoint_table.range::(..)? { @@ -484,6 +501,7 @@ impl Database { "internal freed table".to_string(), freed_root, PageHint::None, + Arc::new(TransactionGuard::fake()), mem.clone(), )?; let lookup_key = FreedTableKey { @@ -618,6 +636,7 @@ impl Database { "internal freed table".to_string(), freed_root, PageHint::None, + Arc::new(TransactionGuard::fake()), mem.clone(), )?; // The persistent savepoints might hold references to older freed trees that are partially processed. diff --git a/src/multimap_table.rs b/src/multimap_table.rs index 0b1e5ef4..71112d01 100644 --- a/src/multimap_table.rs +++ b/src/multimap_table.rs @@ -1,3 +1,4 @@ +use crate::db::TransactionGuard; use crate::multimap_table::DynamicCollectionType::{Inline, Subtree}; use crate::sealed::Sealed; use crate::table::TableStats; @@ -477,6 +478,7 @@ impl DynamicCollection { impl DynamicCollection { fn iter<'a>( collection: AccessGuard<'a, &'static DynamicCollection>, + guard: Arc, mem: Arc, ) -> Result> { Ok(match collection.value().collection_type() { @@ -486,15 +488,14 @@ impl DynamicCollection { V::fixed_width(), <() as RedbValue>::fixed_width(), ); - MultimapValue::new_inline(leaf_iter) + MultimapValue::new_inline(leaf_iter, guard) } Subtree => { let root = collection.value().as_subtree().0; - MultimapValue::new_subtree(BtreeRangeIter::new::>( - &(..), - Some(root), - mem, - )?) + MultimapValue::new_subtree( + BtreeRangeIter::new::>(&(..), Some(root), mem)?, + guard, + ) } }) } @@ -503,6 +504,7 @@ impl DynamicCollection { collection: AccessGuard<'a, &'static DynamicCollection>, pages: Vec, freed_pages: Arc>>, + guard: Arc, mem: Arc, ) -> Result> { Ok(match collection.value().collection_type() { @@ -512,7 +514,7 @@ impl DynamicCollection { V::fixed_width(), <() as RedbValue>::fixed_width(), ); - MultimapValue::new_inline(leaf_iter) + MultimapValue::new_inline(leaf_iter, guard) } Subtree => { let root = collection.value().as_subtree().0; @@ -521,7 +523,7 @@ impl DynamicCollection { Some(root), mem.clone(), )?; - MultimapValue::new_subtree_free_on_drop(inner, freed_pages, pages, mem) + MultimapValue::new_subtree_free_on_drop(inner, freed_pages, pages, guard, mem) } }) } @@ -568,16 +570,18 @@ pub struct MultimapValue<'a, V: RedbKey + 'static> { inner: Option>, freed_pages: Option>>>, free_on_drop: Vec, + _transaction_guard: Arc, mem: Option>, _value_type: PhantomData, } impl<'a, V: RedbKey + 'static> MultimapValue<'a, V> { - fn new_subtree(inner: BtreeRangeIter) -> Self { + fn new_subtree(inner: BtreeRangeIter, guard: Arc) -> Self { Self { inner: Some(ValueIterState::Subtree(inner)), freed_pages: None, free_on_drop: vec![], + _transaction_guard: guard, mem: None, _value_type: Default::default(), } @@ -587,22 +591,25 @@ impl<'a, V: RedbKey + 'static> MultimapValue<'a, V> { inner: BtreeRangeIter, freed_pages: Arc>>, pages: Vec, + guard: Arc, mem: Arc, ) -> Self { Self { inner: Some(ValueIterState::Subtree(inner)), freed_pages: Some(freed_pages), free_on_drop: pages, + _transaction_guard: guard, mem: Some(mem), _value_type: Default::default(), } } - fn new_inline(inner: LeafKeyIter<'a, V>) -> Self { + fn new_inline(inner: LeafKeyIter<'a, V>, guard: Arc) -> Self { Self { inner: Some(ValueIterState::InlineLeaf(inner)), freed_pages: None, free_on_drop: vec![], + _transaction_guard: guard, mem: None, _value_type: Default::default(), } @@ -661,6 +668,7 @@ impl<'a, V: RedbKey + 'static> Drop for MultimapValue<'a, V> { pub struct MultimapRange<'a, K: RedbKey + 'static, V: RedbKey + 'static> { inner: BtreeRangeIter>, mem: Arc, + transaction_guard: Arc, _key_type: PhantomData, _value_type: PhantomData, _lifetime: PhantomData<&'a ()>, @@ -669,11 +677,13 @@ pub struct MultimapRange<'a, K: RedbKey + 'static, V: RedbKey + 'static> { impl<'a, K: RedbKey + 'static, V: RedbKey + 'static> MultimapRange<'a, K, V> { fn new( inner: BtreeRangeIter>, + guard: Arc, mem: Arc, ) -> Self { Self { inner, mem, + transaction_guard: guard, _key_type: Default::default(), _value_type: Default::default(), _lifetime: Default::default(), @@ -690,7 +700,14 @@ impl<'a, K: RedbKey + 'static, V: RedbKey + 'static> Iterator for MultimapRange< let key = AccessGuard::with_owned_value(entry.key_data()); let (page, _, value_range) = entry.into_raw(); let collection = AccessGuard::with_page(page, value_range); - Some(DynamicCollection::iter(collection, self.mem.clone()).map(|iter| (key, iter))) + Some( + DynamicCollection::iter( + collection, + self.transaction_guard.clone(), + self.mem.clone(), + ) + .map(|iter| (key, iter)), + ) } Err(err) => Some(Err(err)), } @@ -706,7 +723,14 @@ impl<'a, K: RedbKey + 'static, V: RedbKey + 'static> DoubleEndedIterator let key = AccessGuard::with_owned_value(entry.key_data()); let (page, _, value_range) = entry.into_raw(); let collection = AccessGuard::with_page(page, value_range); - Some(DynamicCollection::iter(collection, self.mem.clone()).map(|iter| (key, iter))) + Some( + DynamicCollection::iter( + collection, + self.transaction_guard.clone(), + self.mem.clone(), + ) + .map(|iter| (key, iter)), + ) } Err(err) => Some(Err(err)), } @@ -1065,14 +1089,14 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> MultimapTable<'db, ' collection, pages, self.freed_pages.clone(), + self.transaction.transaction_guard(), self.mem.clone(), )? } else { - MultimapValue::new_subtree(BtreeRangeIter::new::>( - &(..), - None, - self.mem.clone(), - )?) + MultimapValue::new_subtree( + BtreeRangeIter::new::>(&(..), None, self.mem.clone())?, + self.transaction.transaction_guard(), + ) }; Ok(iter) @@ -1087,14 +1111,14 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadableMultimapTabl where K: 'a, { + let guard = self.transaction.transaction_guard(); let iter = if let Some(collection) = self.tree.get(key.borrow())? { - DynamicCollection::iter(collection, self.mem.clone())? + DynamicCollection::iter(collection, guard, self.mem.clone())? } else { - MultimapValue::new_subtree(BtreeRangeIter::new::>( - &(..), - None, - self.mem.clone(), - )?) + MultimapValue::new_subtree( + BtreeRangeIter::new::>(&(..), None, self.mem.clone())?, + guard, + ) }; Ok(iter) @@ -1107,7 +1131,11 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadableMultimapTabl KR: Borrow> + 'a, { let inner = self.tree.range(&range)?; - Ok(MultimapRange::new(inner, self.mem.clone())) + Ok(MultimapRange::new( + inner, + self.transaction.transaction_guard(), + self.mem.clone(), + )) } fn stats(&self) -> Result { @@ -1236,6 +1264,7 @@ impl<'txn> ReadOnlyUntypedMultimapTable<'txn> { pub struct ReadOnlyMultimapTable<'txn, K: RedbKey + 'static, V: RedbKey + 'static> { tree: Btree<'txn, K, &'static DynamicCollection>, mem: Arc, + transaction_guard: Arc, _value_type: PhantomData, } @@ -1243,11 +1272,13 @@ impl<'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadOnlyMultimapTable<'tx pub(crate) fn new( root_page: Option<(PageNumber, Checksum)>, hint: PageHint, + guard: Arc, mem: Arc, ) -> Result> { Ok(ReadOnlyMultimapTable { tree: Btree::new(root_page, hint, mem.clone())?, mem, + transaction_guard: guard, _value_type: Default::default(), }) } @@ -1262,13 +1293,12 @@ impl<'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadableMultimapTable>( - &(..), - None, - self.mem.clone(), - )?) + MultimapValue::new_subtree( + BtreeRangeIter::new::>(&(..), None, self.mem.clone())?, + self.transaction_guard.clone(), + ) }; Ok(iter) @@ -1280,7 +1310,11 @@ impl<'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadableMultimapTable> + 'a, { let inner = self.tree.range(&range)?; - Ok(MultimapRange::new(inner, self.mem.clone())) + Ok(MultimapRange::new( + inner, + self.transaction_guard.clone(), + self.mem.clone(), + )) } fn stats(&self) -> Result { diff --git a/src/table.rs b/src/table.rs index a457421a..959d0614 100644 --- a/src/table.rs +++ b/src/table.rs @@ -1,3 +1,4 @@ +use crate::db::TransactionGuard; use crate::sealed::Sealed; use crate::tree_store::{ AccessGuardMut, Btree, BtreeDrain, BtreeDrainFilter, BtreeMut, BtreeRangeIter, Checksum, @@ -224,7 +225,9 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbValue + 'static> ReadableTable> + 'a, { - self.tree.range(&range).map(Range::new) + self.tree + .range(&range) + .map(|x| Range::new(x, self.transaction.transaction_guard())) } fn stats(&self) -> Result { @@ -412,6 +415,7 @@ impl<'txn> ReadOnlyUntypedTable<'txn> { pub struct ReadOnlyTable<'txn, K: RedbKey + 'static, V: RedbValue + 'static> { name: String, tree: Btree<'txn, K, V>, + transaction_guard: Arc, } impl<'txn, K: RedbKey + 'static, V: RedbValue + 'static> ReadOnlyTable<'txn, K, V> { @@ -419,11 +423,13 @@ impl<'txn, K: RedbKey + 'static, V: RedbValue + 'static> ReadOnlyTable<'txn, K, name: String, root_page: Option<(PageNumber, Checksum)>, hint: PageHint, + guard: Arc, mem: Arc, ) -> Result> { Ok(ReadOnlyTable { name, tree: Btree::new(root_page, hint, mem)?, + transaction_guard: guard, }) } } @@ -443,7 +449,9 @@ impl<'txn, K: RedbKey + 'static, V: RedbValue + 'static> ReadableTable K: 'a, KR: Borrow> + 'a, { - self.tree.range(&range).map(Range::new) + self.tree + .range(&range) + .map(|x| Range::new(x, self.transaction_guard.clone())) } fn stats(&self) -> Result { @@ -581,14 +589,16 @@ impl< pub struct Range<'a, K: RedbKey + 'static, V: RedbValue + 'static> { inner: BtreeRangeIter, + _transaction_guard: Arc, // TODO: replace with TransactionGuard? _lifetime: PhantomData<&'a ()>, } impl<'a, K: RedbKey + 'static, V: RedbValue + 'static> Range<'a, K, V> { - pub(super) fn new(inner: BtreeRangeIter) -> Self { + pub(super) fn new(inner: BtreeRangeIter, guard: Arc) -> Self { Self { inner, + _transaction_guard: guard, _lifetime: Default::default(), } } diff --git a/src/transactions.rs b/src/transactions.rs index 815d9473..79151bd2 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -194,6 +194,7 @@ pub struct SystemTable<'db, 's, K: RedbKey + 'static, V: RedbValue + 'static> { name: String, namespace: &'s mut SystemNamespace<'db>, tree: BtreeMut<'s, K, V>, + transaction_guard: Arc, } impl<'db, 's, K: RedbKey + 'static, V: RedbValue + 'static> SystemTable<'db, 's, K, V> { @@ -201,6 +202,7 @@ impl<'db, 's, K: RedbKey + 'static, V: RedbValue + 'static> SystemTable<'db, 's, name: &str, table_root: Option<(PageNumber, Checksum)>, freed_pages: Arc>>, + guard: Arc, mem: Arc, namespace: &'s mut SystemNamespace<'db>, ) -> SystemTable<'db, 's, K, V> { @@ -208,6 +210,7 @@ impl<'db, 's, K: RedbKey + 'static, V: RedbValue + 'static> SystemTable<'db, 's, name: name.to_string(), namespace, tree: BtreeMut::new(table_root, mem, freed_pages), + transaction_guard: guard, } } @@ -223,7 +226,9 @@ impl<'db, 's, K: RedbKey + 'static, V: RedbValue + 'static> SystemTable<'db, 's, K: 'a, KR: Borrow> + 'a, { - self.tree.range(&range).map(Range::new) + self.tree + .range(&range) + .map(|x| Range::new(x, self.transaction_guard.clone())) } pub fn insert<'k, 'v>( @@ -261,6 +266,7 @@ impl<'db, 's, K: RedbKey + 'static, V: RedbValue + 'static> Drop for SystemTable struct SystemNamespace<'db> { table_tree: TableTree<'db>, + transaction_guard: Arc, } impl<'db> SystemNamespace<'db> { @@ -283,6 +289,7 @@ impl<'db> SystemNamespace<'db> { definition.name(), root.get_root(), transaction.freed_pages.clone(), + self.transaction_guard.clone(), transaction.mem.clone(), self, )) @@ -414,7 +421,7 @@ pub struct WriteTransaction<'db> { db: &'db Database, transaction_tracker: Arc, mem: Arc, - _transaction_guard: Arc, + transaction_guard: Arc, transaction_id: TransactionId, // The table of freed pages by transaction. FreedTableKey -> binary. // The binary blob is a length-prefixed array of PageNumber @@ -440,6 +447,7 @@ impl<'db> WriteTransaction<'db> { transaction_tracker: Arc, ) -> Result { let transaction_id = guard.id(); + let guard = Arc::new(guard); let root_page = db.get_memory().get_data_root(); let system_page = db.get_memory().get_system_root(); @@ -453,13 +461,14 @@ impl<'db> WriteTransaction<'db> { }; let system_tables = SystemNamespace { table_tree: TableTree::new(system_page, db.get_memory(), freed_pages.clone()), + transaction_guard: guard.clone(), }; Ok(Self { db, transaction_tracker, mem: db.get_memory(), - _transaction_guard: Arc::new(guard), + transaction_guard: guard, transaction_id, tables: Mutex::new(tables), system_tables: Mutex::new(system_tables), @@ -518,6 +527,10 @@ impl<'db> WriteTransaction<'db> { Ok(savepoint.get_id().0) } + pub(crate) fn transaction_guard(&self) -> Arc { + self.transaction_guard.clone() + } + pub(crate) fn next_persistent_savepoint_id(&self) -> Result> { let mut system_tables = self.system_tables.lock().unwrap(); let next_table = system_tables.open_system_table(self, NEXT_SAVEPOINT_TABLE)?; @@ -1136,7 +1149,7 @@ impl<'a> Drop for WriteTransaction<'a> { pub struct ReadTransaction<'a> { mem: Arc, tree: TableTree<'a>, - _guard: TransactionGuard, + transaction_guard: Arc, } impl<'db> ReadTransaction<'db> { @@ -1145,7 +1158,7 @@ impl<'db> ReadTransaction<'db> { Self { mem: mem.clone(), tree: TableTree::new(root_page, mem, Default::default()), - _guard: guard, + transaction_guard: Arc::new(guard), } } @@ -1163,6 +1176,7 @@ impl<'db> ReadTransaction<'db> { definition.name().to_string(), header.get_root(), PageHint::Clean, + self.transaction_guard.clone(), self.mem.clone(), )?) } @@ -1198,6 +1212,7 @@ impl<'db> ReadTransaction<'db> { Ok(ReadOnlyMultimapTable::new( header.get_root(), PageHint::Clean, + self.transaction_guard.clone(), self.mem.clone(), )?) }