From bff1e43f8ca34f623b5b7abd6b01534bb2b3f1e8 Mon Sep 17 00:00:00 2001 From: Michael Constant Date: Thu, 24 Oct 2024 16:57:28 -0700 Subject: [PATCH] Move finalize_dirty_checksums() out of flush_table_root_updates() Refactoring in preparation for quick-repair, which needs to be able to call these separately --- src/multimap_table.rs | 8 ++++---- src/transactions.rs | 29 +++++++++++++++++------------ src/tree_store/btree.rs | 13 ++++++------- src/tree_store/table_tree.rs | 12 +++++++----- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/multimap_table.rs b/src/multimap_table.rs index 5c24c3a3..ac42a7cc 100644 --- a/src/multimap_table.rs +++ b/src/multimap_table.rs @@ -308,8 +308,8 @@ pub(crate) fn finalize_tree_and_subtree_checksums( value_size, <()>::fixed_width(), ); - subtree.finalize_dirty_checksums()?; - sub_root_updates.push((i, entry.key().to_vec(), subtree.get_root().unwrap())); + let subtree_root = subtree.finalize_dirty_checksums()?.unwrap(); + sub_root_updates.push((i, entry.key().to_vec(), subtree_root)); } } } @@ -327,10 +327,10 @@ pub(crate) fn finalize_tree_and_subtree_checksums( Ok(()) })?; - tree.finalize_dirty_checksums()?; + let root = tree.finalize_dirty_checksums()?; // No pages should have been freed by this operation assert!(freed_pages.lock().unwrap().is_empty()); - Ok(tree.get_root()) + Ok(root) } fn parse_subtree_roots( diff --git a/src/transactions.rs b/src/transactions.rs index 2a6cc962..63c9e8e6 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -1095,14 +1095,16 @@ impl WriteTransaction { .lock() .unwrap() .table_tree - .flush_table_root_updates()?; + .flush_table_root_updates()? + .finalize_dirty_checksums()?; let system_root = self .system_tables .lock() .unwrap() .table_tree - .flush_table_root_updates()?; + .flush_table_root_updates()? + .finalize_dirty_checksums()?; self.process_freed_pages(free_until_transaction)?; // If a savepoint exists it might reference the freed-tree, since it holds a reference to the @@ -1113,10 +1115,7 @@ impl WriteTransaction { self.store_freed_pages(savepoint_exists)?; // Finalize freed table checksums, before doing the final commit - // user & system table trees were already finalized when we flushed the pending roots above - self.freed_tree.lock().unwrap().finalize_dirty_checksums()?; - - let freed_root = self.freed_tree.lock().unwrap().get_root(); + let freed_root = self.freed_tree.lock().unwrap().finalize_dirty_checksums()?; self.mem.commit( user_root, @@ -1146,23 +1145,23 @@ impl WriteTransaction { .lock() .unwrap() .table_tree - .flush_table_root_updates()?; + .flush_table_root_updates()? + .finalize_dirty_checksums()?; let system_root = self .system_tables .lock() .unwrap() .table_tree - .flush_table_root_updates()?; + .flush_table_root_updates()? + .finalize_dirty_checksums()?; // Store all freed pages for a future commit(), since we can't free pages during a // non-durable commit (it's non-durable, so could be rolled back anytime in the future) self.store_freed_pages(true)?; // Finalize all checksums, before doing the final commit - self.freed_tree.lock().unwrap().finalize_dirty_checksums()?; - - let freed_root = self.freed_tree.lock().unwrap().get_root(); + let freed_root = self.freed_tree.lock().unwrap().finalize_dirty_checksums()?; self.mem .non_durable_commit(user_root, system_root, freed_root, self.transaction_id)?; @@ -1350,7 +1349,13 @@ impl WriteTransaction { pub(crate) fn print_debug(&self) -> Result { // Flush any pending updates to make sure we get the latest root let mut tables = self.tables.lock().unwrap(); - if let Some(page) = tables.table_tree.flush_table_root_updates().unwrap() { + if let Some(page) = tables + .table_tree + .flush_table_root_updates() + .unwrap() + .finalize_dirty_checksums() + .unwrap() + { eprintln!("Master tree:"); let master_tree: Btree<&str, InternalTableDefinition> = Btree::new( Some(page), diff --git a/src/tree_store/btree.rs b/src/tree_store/btree.rs index 991bfad6..ac3bc784 100644 --- a/src/tree_store/btree.rs +++ b/src/tree_store/btree.rs @@ -152,7 +152,7 @@ impl UntypedBtreeMut { } // Recomputes the checksum for all pages that are uncommitted - pub(crate) fn finalize_dirty_checksums(&mut self) -> Result { + pub(crate) fn finalize_dirty_checksums(&mut self) -> Result> { let mut root = self.root; if let Some(BtreeHeader { root: ref p, @@ -162,14 +162,14 @@ impl UntypedBtreeMut { { if !self.mem.uncommitted(*p) { // root page is clean - return Ok(()); + return Ok(root); } *checksum = self.finalize_dirty_checksums_helper(*p)?; self.root = root; } - Ok(()) + Ok(root) } fn finalize_dirty_checksums_helper(&mut self, page_number: PageNumber) -> Result { @@ -353,7 +353,7 @@ impl BtreeMut<'_, K, V> { .verify_checksum() } - pub(crate) fn finalize_dirty_checksums(&mut self) -> Result { + pub(crate) fn finalize_dirty_checksums(&mut self) -> Result> { let mut tree = UntypedBtreeMut::new( self.get_root(), self.mem.clone(), @@ -361,9 +361,8 @@ impl BtreeMut<'_, K, V> { K::fixed_width(), V::fixed_width(), ); - tree.finalize_dirty_checksums()?; - self.root = tree.get_root(); - Ok(()) + self.root = tree.finalize_dirty_checksums()?; + Ok(self.root) } #[allow(dead_code)] diff --git a/src/tree_store/table_tree.rs b/src/tree_store/table_tree.rs index fc964563..fdef1ced 100644 --- a/src/tree_store/table_tree.rs +++ b/src/tree_store/table_tree.rs @@ -389,7 +389,7 @@ impl<'txn> TableTreeMut<'txn> { Ok(true) } - pub(crate) fn flush_table_root_updates(&mut self) -> Result> { + pub(crate) fn flush_table_root_updates(&mut self) -> Result<&mut Self> { for (name, (new_root, new_length)) in self.pending_table_updates.drain() { // Bypass .get_table() since the table types are dynamic let mut definition = self.tree.get(&name.as_str())?.unwrap().value(); @@ -418,8 +418,7 @@ impl<'txn> TableTreeMut<'txn> { fixed_key_size, fixed_value_size, ); - tree.finalize_dirty_checksums()?; - *table_root = tree.get_root(); + *table_root = tree.finalize_dirty_checksums()?; *table_length = new_length; } InternalTableDefinition::Multimap { @@ -440,8 +439,11 @@ impl<'txn> TableTreeMut<'txn> { } self.tree.insert(&name.as_str(), &definition)?; } - self.tree.finalize_dirty_checksums()?; - Ok(self.tree.get_root()) + Ok(self) + } + + pub(crate) fn finalize_dirty_checksums(&mut self) -> Result> { + self.tree.finalize_dirty_checksums() } // root_page: the root of the master table