Move commit() out of do_repair()

Instead of actually making a commit, do_repair() should just return the
new roots that would be committed. This makes check_integrity() much
faster, since it no longer needs to verify the checksums twice, and
it means commit() can always trim the database without worrying about
whether it'll change the allocator hash.

It also allows check_integrity() to avoid making an unnecessary commit
if the database is clean, which is nice with Durability::ParanoidPlus.
It means a successful check_integrity() won't invalidate the allocator
state table, so you can still do a quick-repair afterwards

src/db.rs

@@ -386,17 +386,34 @@ impl Database {
             .unwrap()
             .clear_cache_and_reload()?;
 
-        if !Self::verify_primary_checksums(self.mem.clone())? {
-            was_clean = false;
-        }
+        let old_roots = [
+            self.mem.get_data_root(),
+            self.mem.get_system_root(),
+            self.mem.get_freed_root(),
+        ];
 
-        Self::do_repair(&mut self.mem, &|_| {}).map_err(|err| match err {
+        let new_roots = Self::do_repair(&mut self.mem, &|_| {}).map_err(|err| match err {
             DatabaseError::Storage(storage_err) => storage_err,
             _ => unreachable!(),
         })?;
-        if allocator_hash != self.mem.allocator_hash() {
+
+        if old_roots != new_roots || allocator_hash != self.mem.allocator_hash() {
             was_clean = false;
         }
+
+        if !was_clean {
+            let next_transaction_id = self.mem.get_last_committed_transaction_id()?.next();
+            let [data_root, system_root, freed_root] = new_roots;
+            self.mem.commit(
+                data_root,
+                system_root,
+                freed_root,
+                next_transaction_id,
+                false,
+                true,
+            )?;
+        }
+
         self.mem.begin_writable()?;
 
         Ok(was_clean)
@@ -592,7 +609,7 @@ impl Database {
     fn do_repair(
         mem: &mut Arc<TransactionalMemory>, // Only &mut to ensure exclusivity
         repair_callback: &(dyn Fn(&mut RepairSession) + 'static),
-    ) -> Result<(), DatabaseError> {
+    ) -> Result<[Option<BtreeHeader>; 3], DatabaseError> {
         if !Self::verify_primary_checksums(mem.clone())? {
             // 0.3 because the repair takes 3 full scans and the first is done now
             let mut handle = RepairSession::new(0.3);
@@ -662,19 +679,7 @@ impl Database {
         // by storing an empty root during the below commit()
         mem.clear_read_cache();
 
-        let transaction_id = mem.get_last_committed_transaction_id()?.next();
-        mem.commit(
-            data_root,
-            system_root,
-            freed_root,
-            transaction_id,
-            false,
-            true,
-            // don't trim the database file, because we want the allocator hash to match exactly
-            false,
-        )?;
-
-        Ok(())
+        Ok([data_root, system_root, freed_root])
     }
 
     fn new(
@@ -705,7 +710,16 @@ impl Database {
             if handle.aborted() {
                 return Err(DatabaseError::RepairAborted);
             }
-            Self::do_repair(&mut mem, repair_callback)?;
+            let [data_root, system_root, freed_root] = Self::do_repair(&mut mem, repair_callback)?;
+            let next_transaction_id = mem.get_last_committed_transaction_id()?.next();
+            mem.commit(
+                data_root,
+                system_root,
+                freed_root,
+                next_transaction_id,
+                false,
+                true,
+            )?;
         }
 
         mem.begin_writable()?;

src/transactions.rs

@@ -1103,7 +1103,6 @@ impl WriteTransaction {
             self.transaction_id,
             eventual,
             two_phase,
-            true,
         )?;
 
         // Mark any pending non-durable commits as fully committed.

src/tree_store/page_store/page_manager.rs

@@ -466,7 +466,6 @@ impl TransactionalMemory {
         transaction_id: TransactionId,
         eventual: bool,
         two_phase: bool,
-        allow_trim: bool,
     ) -> Result {
         let result = self.commit_inner(
             data_root,
@@ -475,7 +474,6 @@ impl TransactionalMemory {
             transaction_id,
             eventual,
             two_phase,
-            allow_trim,
         );
         if result.is_err() {
             self.needs_recovery.store(true, Ordering::Release);
@@ -492,7 +490,6 @@ impl TransactionalMemory {
         transaction_id: TransactionId,
         eventual: bool,
         two_phase: bool,
-        allow_trim: bool,
     ) -> Result {
         // All mutable pages must be dropped, this ensures that when a transaction completes
         // no more writes can happen to the pages it allocated. Thus it is safe to make them visible
@@ -503,11 +500,7 @@ impl TransactionalMemory {
 
         let mut state = self.state.lock().unwrap();
         // Trim surplus file space, before finalizing the commit
-        let shrunk = if allow_trim {
-            Self::try_shrink(&mut state)?
-        } else {
-            false
-        };
+        let shrunk = Self::try_shrink(&mut state)?;
         // Copy the header so that we can release the state lock, while we flush the file
         let mut header = state.header.clone();
         drop(state);
@@ -1011,7 +1004,6 @@ impl Drop for TransactionalMemory {
                         non_durable_transaction_id,
                         false,
                         true,
-                        true,
                     )
                     .is_err()
                 {