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

@@ -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<T: Page>(

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),

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<Option<BtreeHeader>> {
         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<Checksum> {
@@ -353,17 +353,16 @@ impl<K: Key + 'static, V: Value + 'static> BtreeMut<'_, K, V> {
         .verify_checksum()
     }
 
-    pub(crate) fn finalize_dirty_checksums(&mut self) -> Result {
+    pub(crate) fn finalize_dirty_checksums(&mut self) -> Result<Option<BtreeHeader>> {
         let mut tree = UntypedBtreeMut::new(
             self.get_root(),
             self.mem.clone(),
             self.freed_pages.clone(),
             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)]

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<Option<BtreeHeader>> {
+    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<Option<BtreeHeader>> {
+        self.tree.finalize_dirty_checksums()
     }
 
     // root_page: the root of the master table