uncommitted => unpersisted

feat: defer persist every 'N' commits

Author: RodrigoVillar

ffi/firewood.go

@@ -113,14 +113,18 @@ type config struct {
 	rootStore bool
 	// expensiveMetricsEnabled controls whether expensive metrics recording is enabled.
 	expensiveMetricsEnabled bool
+	// deferredPersistenceCommitCount determines the maximum number of unpersisted
+	// revisions that can exist at a given time.
+	deferredPersistenceCommitCount uint
 }
 
 func defaultConfig() *config {
 	return &config{
-		nodeCacheEntries:     1_000_000,
-		freeListCacheEntries: 40_000,
-		revisions:            100,
-		readCacheStrategy:    OnlyCacheWrites,
+		nodeCacheEntries:               1_000_000,
+		freeListCacheEntries:           40_000,
+		revisions:                      100,
+		readCacheStrategy:              OnlyCacheWrites,
+		deferredPersistenceCommitCount: 1,
 	}
 }
 
@@ -193,6 +197,15 @@ func WithExpensiveMetrics() Option {
 	}
 }
 
+// WithDeferredPersistenceCommitCount sets the maximum number of unpersisted revisions
+// that can exist at a time. Note: `commitCount` must be greater than 0.
+// Default: 1
+func WithDeferredPersistenceCommitCount(commitCount uint) Option {
+	return func(c *config) {
+		c.deferredPersistenceCommitCount = commitCount
+	}
+}
+
 // A CacheStrategy represents the caching strategy used by a [Database].
 type CacheStrategy uint8
 
@@ -245,20 +258,24 @@ func New(dbDir string, nodeHashAlgorithm NodeHashAlgorithm, opts ...Option) (*Da
 	if conf.freeListCacheEntries < 1 {
 		return nil, fmt.Errorf("free list cache entries must be >= 1, got %d", conf.freeListCacheEntries)
 	}
+	if conf.deferredPersistenceCommitCount == 0 {
+		return nil, fmt.Errorf("deferred persistence commit count must be >= 1, got %d", conf.deferredPersistenceCommitCount)
+	}
 
 	var pinner runtime.Pinner
 	defer pinner.Unpin()
 
 	args := C.struct_DatabaseHandleArgs{
-		dir:                  newBorrowedBytes([]byte(dbDir), &pinner),
-		cache_size:           C.size_t(conf.nodeCacheEntries),
-		free_list_cache_size: C.size_t(conf.freeListCacheEntries),
-		revisions:            C.size_t(conf.revisions),
-		strategy:             C.uint8_t(conf.readCacheStrategy),
-		truncate:             C.bool(conf.truncate),
-		root_store:           C.bool(conf.rootStore),
-		expensive_metrics:    C.bool(conf.expensiveMetricsEnabled),
-		node_hash_algorithm:  C.enum_NodeHashAlgorithm(nodeHashAlgorithm),
+		dir:                               newBorrowedBytes([]byte(dbDir), &pinner),
+		cache_size:                        C.size_t(conf.nodeCacheEntries),
+		free_list_cache_size:              C.size_t(conf.freeListCacheEntries),
+		revisions:                         C.size_t(conf.revisions),
+		strategy:                          C.uint8_t(conf.readCacheStrategy),
+		truncate:                          C.bool(conf.truncate),
+		root_store:                        C.bool(conf.rootStore),
+		expensive_metrics:                 C.bool(conf.expensiveMetricsEnabled),
+		node_hash_algorithm:               C.enum_NodeHashAlgorithm(nodeHashAlgorithm),
+		deferred_persistence_commit_count: C.uint64_t(conf.deferredPersistenceCommitCount),
 	}
 
 	return getDatabaseFromHandleResult(C.fwd_open_db(args))

ffi/firewood.h

@@ -1,7 +1,7 @@
 // Copyright (C) 2025, Ava Labs, Inc. All rights reserved.
 // See the file LICENSE.md for licensing terms.
 
-use std::num::NonZeroUsize;
+use std::num::{NonZeroU64, NonZeroUsize};
 
 use firewood::{
     db::{Db, DbConfig},
@@ -99,6 +99,9 @@ pub struct DatabaseHandleArgs<'a> {
     ///
     /// Opening returns an error if this does not match the compile-time feature.
     pub node_hash_algorithm: NodeHashAlgorithm,
+
+    /// The maximum number of unpersisted revisions that can exist at a given time.
+    pub deferred_persistence_commit_count: u64,
 }
 
 impl DatabaseHandleArgs<'_> {
@@ -151,12 +154,15 @@ impl DatabaseHandle {
     /// If the path is empty, or if the configuration is invalid, this will return an error.
     pub fn new(args: DatabaseHandleArgs<'_>) -> Result<Self, api::Error> {
         let metrics_context = MetricsContext::new(args.expensive_metrics);
+        let commit_count = NonZeroU64::new(args.deferred_persistence_commit_count)
+            .ok_or(api::Error::InvalidConversionToCommitCount)?;
 
         let cfg = DbConfig::builder()
             .node_hash_algorithm(args.node_hash_algorithm.into())
             .truncate(args.truncate)
             .manager(args.as_rev_manager_config()?)
             .root_store(args.root_store)
+            .deferred_persistence_commit_count(commit_count)
             .build();
 
         let path = args

ffi/src/handle.rs

@@ -23,8 +23,9 @@ use firewood_storage::{
     CheckOpt, CheckerReport, Committed, FileBacked, FileIoError, HashedNodeReader,
     ImmutableProposal, NodeHashAlgorithm, NodeStore, Parentable, ReadableStorage, TrieReader,
 };
+use nonzero_ext::nonzero;
 use std::io::Write;
-use std::num::NonZeroUsize;
+use std::num::{NonZeroU64, NonZeroUsize};
 use std::path::Path;
 use std::sync::Arc;
 use thiserror::Error;
@@ -135,6 +136,9 @@ pub struct DbConfig {
     /// Whether to enable `RootStore`.
     #[builder(default = false)]
     pub root_store: bool,
+    /// The maximum number of unpersisted revisions that can exist at a given time.
+    #[builder(default = nonzero!(1u64))]
+    pub deferred_persistence_commit_count: NonZeroU64,
 }
 
 #[derive(Debug)]
@@ -182,6 +186,7 @@ impl Db {
             .create(cfg.create_if_missing)
             .truncate(cfg.truncate)
             .root_store(cfg.root_store)
+            .deferred_persistence_commit_count(cfg.deferred_persistence_commit_count)
             .manager(cfg.manager)
             .build();
         let manager = RevisionManager::new(config_manager)?;
@@ -337,9 +342,9 @@ impl Db {
 
     /// Closes the database gracefully.
     ///
-    /// This method shuts down the background persistence worker. If not called
-    /// explicitly, `Drop` will attempt a best-effort shutdown but cannot report
-    /// errors.
+    /// This method shuts down the background persistence worker and persists
+    /// the last committed revision. If not called explicitly, `Drop` will
+    /// attempt a best-effort shutdown but cannot report errors.
     pub fn close(mut self) -> Result<(), api::Error> {
         self.manager.close().map_err(Into::into)
     }
@@ -423,18 +428,19 @@ mod test {
     use core::iter::Take;
     use std::collections::HashMap;
     use std::iter::Peekable;
-    use std::num::NonZeroUsize;
+    use std::num::{NonZeroU64, NonZeroUsize};
     use std::ops::{Deref, DerefMut};
     use std::path::Path;
 
     use firewood_storage::{
         CheckOpt, CheckerError, HashedNodeReader, IntoHashType, LinearAddress, MaybePersistedNode,
         NodeStore, RootReader, TrieHash,
     };
+    use nonzero_ext::nonzero;
 
     use crate::db::{Db, Proposal, UseParallel};
     use crate::manager::RevisionManagerConfig;
-    use crate::v2::api::{Db as _, DbView, Proposal as _};
+    use crate::v2::api::{Db as _, DbView, HashKeyExt, Proposal as _};
 
     use super::{BatchOp, DbConfig};
 
@@ -1392,6 +1398,194 @@ mod test {
         assert_eq!(value, new_value.as_ref());
     }
 
+    #[test]
+    fn test_deferred_persist_close_with_high_commit_count() {
+        const HIGH_COMMIT_COUNT: NonZeroU64 = nonzero!(1_000_000u64);
+
+        // Set commit count to an arbitrarily high number so persist happens
+        // only on shutdown
+        let dbcfg = DbConfig::builder()
+            .deferred_persistence_commit_count(HIGH_COMMIT_COUNT)
+            .build();
+
+        let db = TestDb::new_with_config(dbcfg);
+
+        // Then, commit once and see what the latest revision is
+        let key = b"foo";
+        let value = b"bar";
+        let batch = vec![BatchOp::Put { key, value }];
+        let proposal = db.propose(batch).unwrap();
+        let root_hash = proposal.root_hash().unwrap().unwrap();
+
+        proposal.commit().unwrap();
+        let db = db.reopen();
+
+        let revision = db.view(root_hash).unwrap();
+        let new_value = revision.val(b"foo").unwrap().unwrap();
+
+        assert_eq!(value, new_value.as_ref());
+    }
+
+    #[test]
+    fn test_deferred_persist_after_reaching_sub_interval() {
+        const COMMIT_COUNT: NonZeroU64 = nonzero!(10u64);
+        const SUB_INTERVAL: u64 = COMMIT_COUNT.get() / 2;
+
+        let dbcfg = DbConfig::builder()
+            .deferred_persistence_commit_count(COMMIT_COUNT)
+            .build();
+
+        let db = TestDb::new_with_config(dbcfg);
+
+        // Commit SUB_INTERVAL proposals to trigger the first persist
+        for i in 0..SUB_INTERVAL {
+            let batch = vec![BatchOp::Put {
+                key: format!("key{i}").as_bytes().to_vec(),
+                value: format!("value{i}").as_bytes().to_vec(),
+            }];
+            let proposal = db.propose(batch).unwrap();
+            proposal.commit().unwrap();
+        }
+
+        // Wait for the background thread to finish persisting
+        db.wait_persisted();
+
+        // Verify the root is now persisted
+        let revision = db.manager.current_revision();
+        let is_persisted = revision
+            .root_as_maybe_persisted_node()
+            .is_some_and(|node| node.unpersisted().is_none());
+
+        assert!(
+            is_persisted,
+            "Root should be persisted after hitting commit count"
+        );
+    }
+
+    /// Verifies that an unpersisted revision which wipes the database is
+    /// persisted when the database closes.
+    #[test]
+    fn test_deferred_persistence_closing_on_empty_trie() {
+        const COMMIT_COUNT: NonZeroU64 = nonzero!(10u64);
+        const SUB_INTERVAL: u64 = COMMIT_COUNT.get() / 2;
+
+        let dbcfg = DbConfig::builder()
+            .deferred_persistence_commit_count(COMMIT_COUNT)
+            .build();
+
+        let db = TestDb::new_with_config(dbcfg);
+
+        // Commit SUB_INTERVAL proposals to trigger the first persist
+        for i in 0..SUB_INTERVAL {
+            let batch = vec![BatchOp::Put {
+                key: format!("key{i}").as_bytes().to_vec(),
+                value: format!("value{i}").as_bytes().to_vec(),
+            }];
+            let proposal = db.propose(batch).unwrap();
+            proposal.commit().unwrap();
+        }
+
+        // Empty the trie
+        let batch: Vec<BatchOp<Vec<u8>, Vec<u8>>> = vec![BatchOp::DeleteRange { prefix: vec![] }];
+        let proposal = db.propose(batch).unwrap();
+        proposal.commit().unwrap();
+
+        let db = db.reopen();
+
+        // Verify that the latest committed revision is empty.
+        let last_committed_hash = db.root_hash().unwrap();
+        assert_eq!(last_committed_hash, TrieHash::default_root_hash());
+    }
+
+    #[test]
+    fn test_deferred_persistence_root_store() {
+        const NUM_COMMITS: usize = 20;
+        const COMMIT_COUNT: NonZeroU64 = nonzero!(10u64);
+        const MAX_IN_MEMORY_REVISIONS: usize = 5;
+
+        // Revisions to verify after reopening (1-indexed commit numbers)
+        const CHECKPOINTS: [usize; 4] = [5, 10, 15, 20];
+
+        let dbcfg = DbConfig::builder()
+            .manager(
+                RevisionManagerConfig::builder()
+                    .max_revisions(MAX_IN_MEMORY_REVISIONS)
+                    .build(),
+            )
+            .deferred_persistence_commit_count(COMMIT_COUNT)
+            .root_store(true)
+            .build();
+
+        let db = TestDb::new_with_config(dbcfg);
+
+        // Track root hashes at checkpoint commits
+        let mut checkpoint_roots: Vec<TrieHash> = Vec::new();
+
+        // Commit NUM_COMMITS proposals
+        let key = b"key";
+        for i in 1..=NUM_COMMITS {
+            let batch = vec![BatchOp::Put {
+                key,
+                value: format!("{i}").as_bytes().to_vec(),
+            }];
+            let proposal = db.propose(batch).unwrap();
+
+            if CHECKPOINTS.contains(&i) {
+                checkpoint_roots.push(proposal.root_hash().unwrap().unwrap());
+            }
+
+            proposal.commit().unwrap();
+        }
+
+        let db = db.reopen();
+
+        // Verify that checkpoint revisions are accessible after reopening
+        // and contain the expected values
+        for (i, root) in checkpoint_roots.into_iter().enumerate() {
+            let view = db.view(root).unwrap();
+            let checkpoint = CHECKPOINTS[i];
+            let expected = format!("{checkpoint}");
+            let actual = view.val(key).unwrap().unwrap();
+            assert_eq!(expected.as_bytes(), actual.as_ref());
+        }
+    }
+
+    /// Verifies that non-persisted revisions are lost after reopening the database.
+    #[test]
+    fn test_deferred_persistence_unpersisted_revisions() {
+        const NUM_COMMITS: usize = 10;
+        const COMMIT_COUNT: NonZeroU64 = nonzero!(10u64);
+
+        let dbcfg = DbConfig::builder()
+            .deferred_persistence_commit_count(COMMIT_COUNT)
+            .root_store(true)
+            .build();
+
+        let db = TestDb::new_with_config(dbcfg);
+
+        // Track root hashes for every commit
+        let mut root_hashes: Vec<TrieHash> = Vec::new();
+
+        let key = b"key";
+        for i in 1..=NUM_COMMITS {
+            let batch = vec![BatchOp::Put {
+                key,
+                value: format!("{i}").as_bytes().to_vec(),
+            }];
+            let proposal = db.propose(batch).unwrap();
+            root_hashes.push(proposal.root_hash().unwrap().unwrap());
+            proposal.commit().unwrap();
+        }
+
+        let db = db.reopen();
+
+        // Commits 1-4 and 6-9 were not persisted and should not be accessible
+        let unpersisted: [usize; 8] = [1, 2, 3, 4, 6, 7, 8, 9];
+        for &i in &unpersisted {
+            assert!(db.view(root_hashes[i - 1].clone()).is_err());
+        }
+    }
+
     // Testdb is a helper struct for testing the Db. Once it's dropped, the directory and file disappear
     pub(super) struct TestDb {
         db: Db,