Merge pull request #5 from NodeDB-Lab/fix/durable-freelist-reclamation

fix: durable header-rooted free-list (closes #1, #2)

Author: farhan-syah

.github/workflows/test.yml

@@ -46,6 +46,11 @@ jobs:
         with:
           components: rustfmt, clippy
 
+      # The comparison benchmark links rocksdb (librocksdb-sys), whose build
+      # runs bindgen and needs libclang. clippy --all-targets compiles it.
+      - name: Install LLVM/Clang
+        run: sudo apt-get update && sudo apt-get install -y clang libclang-dev
+
       - uses: Swatinem/rust-cache@v2
         with:
           prefix-key: lint
@@ -90,6 +95,19 @@ jobs:
       - name: Install Rust
         uses: dtolnay/rust-toolchain@stable
 
+      # nextest compiles the dev-dependency graph (incl. rocksdb →
+      # librocksdb-sys → bindgen → libclang). Windows runner images ship LLVM;
+      # Linux and macOS need it installed.
+      - name: Install LLVM/Clang (Linux)
+        if: runner.os == 'Linux'
+        run: sudo apt-get update && sudo apt-get install -y clang libclang-dev
+
+      - name: Install LLVM/Clang (macOS)
+        if: runner.os == 'macOS'
+        run: |
+          brew install llvm
+          echo "LIBCLANG_PATH=$(brew --prefix llvm)/lib" >> "$GITHUB_ENV"
+
       - uses: Swatinem/rust-cache@v2
         with:
           prefix-key: test-${{ matrix.os }}
@@ -198,6 +216,11 @@ jobs:
       - name: Install Rust
         uses: dtolnay/rust-toolchain@stable
 
+      # --benches compiles the comparison bench, which links rocksdb
+      # (librocksdb-sys) — its build runs bindgen and needs libclang.
+      - name: Install LLVM/Clang
+        run: sudo apt-get update && sudo apt-get install -y clang libclang-dev
+
       - uses: Swatinem/rust-cache@v2
         with:
           prefix-key: features
@@ -215,6 +238,11 @@ jobs:
       - name: Install Rust
         uses: dtolnay/rust-toolchain@stable
 
+      # cargo bench builds the dev-dependency graph (rocksdb → librocksdb-sys →
+      # bindgen → libclang), even when only the btree/segment benches are named.
+      - name: Install LLVM/Clang
+        run: sudo apt-get update && sudo apt-get install -y clang libclang-dev
+
       - uses: Swatinem/rust-cache@v2
         with:
           prefix-key: bench

Cargo.toml

@@ -129,17 +129,24 @@ fastrand = "2"
 name = "pagedb-fsck"
 path = "src/bin/pagedb-fsck.rs"
 
+# `test = false`: these are benchmarks, not tests. Without it, `cargo test`
+# (and nextest) compile every `harness = false` bench and run its `main` as a
+# test — pulling heavy dev-deps (rocksdb → libclang) into the test build on
+# every platform and executing benchmarks during the test run.
 [[bench]]
 name = "btree"
 harness = false
+test = false
 
 [[bench]]
 name = "segment"
 harness = false
+test = false
 
 [[bench]]
 name = "comparison"
 harness = false
+test = false
 
 [profile.release]
 lto = "thin"

benches/comparison.rs

@@ -84,12 +84,11 @@ fn open_pagedb_fresh() -> PagedbBench {
         // Fair-comparison: redb has no equivalent commit-history index;
         // disable pagedb's so the bench measures the same feature surface.
         // (`Disabled` is a pagedb extension; not in the architecture spec.)
+        // With history disabled and no readers, the commit path recycles freed
+        // pages through the in-memory allocator cache without persisting a
+        // deferred-free row, so per-commit work matches redb's for a fair
+        // write-latency comparison.
         .with_commit_history_retain(RetainPolicy::Disabled)
-        // Fair-comparison: redb does not persist a deferred-free queue per
-        // commit. Opt into pagedb's fast-free path so write-latency numbers
-        // measure the same per-commit work. Production embedders that turn
-        // this on must compact periodically.
-        .with_skip_freelist_persistence_when_no_readers(true)
         .with_metrics_enabled(false)
         // Large bulk loads need a generous nonce budget per txn.
         .with_anchor_budget(10_000_000);

src/btree/tree/core.rs

@@ -52,14 +52,18 @@ pub struct BTree<V: Vfs> {
     /// parallel-AEAD flush. In-place mutation by subsequent puts targeting
     /// the same leaf is allowed; no further allocation needed.
     pub(super) fresh_leaves: HashMap<u64, Leaf>,
-    /// Cross-commit pool of pre-vetted reusable page IDs, shared (via `Arc`)
-    /// across all `BTree`s opened by the same `Db`. Allocation pops from
-    /// here before bumping `next_page_id`, keeping the file size bounded
-    /// when `OpenOptions::skip_freelist_persistence_when_no_readers` orphans
-    /// would otherwise accumulate. `None` for callers that haven't wired a
-    /// shared cache (compaction, history tree) — those keep bump-only
-    /// allocation.
+    /// Cross-commit pool of reusable page IDs, shared (via `Arc`) across the
+    /// main, catalog, and history `BTree`s of one `Db`. Allocation pops from
+    /// here before bumping `next_page_id`, recycling pages freed by earlier
+    /// commits (once no reader or retained-history root can observe them) so
+    /// the file stays bounded under sustained writes. `None` for callers that
+    /// haven't wired a shared cache (e.g. compaction's repack trees), which
+    /// keep bump-only allocation.
     pub(super) free_page_cache: Option<Arc<parking_lot::Mutex<Vec<u64>>>>,
+    /// Sink recording page ids drawn from `free_page_cache` and reused this
+    /// session. The commit path removes them from the durable free-list (they
+    /// now hold live committed data). Shared (via `Arc`) across the txn's trees.
+    pub(super) free_page_consumed: Option<Arc<parking_lot::Mutex<Vec<u64>>>>,
     /// Last key successfully appended via [`Self::put_append`]. Used to
     /// enforce the monotonic-key invariant on subsequent calls and to
     /// invalidate the cached path when any non-append mutation (regular
@@ -96,6 +100,7 @@ impl<V: Vfs> BTree<V> {
             dirty_parent_paths: HashMap::new(),
             fresh_leaves: HashMap::new(),
             free_page_cache: None,
+            free_page_consumed: None,
             append_last_key: None,
             append_cached_path: None,
         }
@@ -110,22 +115,18 @@ impl<V: Vfs> BTree<V> {
     }
 
     /// Wire in the `Db`'s shared free-page cache. After this call,
-    /// `allocate_page` will pop from the shared pool before bumping
-    /// `next_page_id`. Pages pushed into the pool by an earlier writer
-    /// commit (via [`Self::push_to_shared_cache`]) become reusable here.
+    /// `allocate_page` pops from the shared pool before bumping `next_page_id`.
+    /// The pool is loaded at `begin_write` with the durable free-list's
+    /// floor-safe pages, so recycling from it is always snapshot-safe.
     pub fn set_free_page_cache(&mut self, cache: Arc<parking_lot::Mutex<Vec<u64>>>) {
         self.free_page_cache = Some(cache);
     }
 
-    /// Push `page_ids` into the shared free-page cache, if one is wired.
-    /// Used by the writer commit path when the no-reader fast-free option
-    /// is active: instead of orphaning freed pages, hand them to the next
-    /// txn's allocator.
-    pub fn push_to_shared_cache(&self, page_ids: &[u64]) {
-        if let Some(cache) = &self.free_page_cache {
-            let mut guard = cache.lock();
-            guard.extend(page_ids.iter().copied());
-        }
+    /// Wire the shared sink that records cache pages reused this session, so the
+    /// commit path can remove them from the durable free-list. Set alongside
+    /// [`Self::set_free_page_cache`].
+    pub fn set_free_page_consumed(&mut self, consumed: Arc<parking_lot::Mutex<Vec<u64>>>) {
+        self.free_page_consumed = Some(consumed);
     }
 
     #[must_use]
@@ -148,34 +149,31 @@ impl<V: Vfs> BTree<V> {
     }
 
     pub(super) fn allocate_page(&mut self) -> u64 {
-        // Reuse a freed page only if it is at or above the reuse threshold.
-        // Pages below the threshold may still be live in pinned reader snapshots.
+        // First, recycle a page freed earlier *in this same session*, gated by
+        // the reuse threshold: a page below it may still be live in a pinned
+        // reader's snapshot, so it can't be reused until the durable free-list
+        // clears it (it leaves via `drain_freed` at commit instead).
         if self.reuse_threshold == 0 {
-            // No readers pinned: recycle freely.
             if let Some(id) = self.freed.pop() {
                 return id;
             }
-            // Consult the cross-commit shared cache (pages handed off by
-            // earlier writer commits under the no-reader fast-free option).
-            // Only safe to draw from this when no readers are pinned — the
-            // cache contract is "always safe to immediately reuse."
-            if let Some(cache) = &self.free_page_cache {
-                if let Some(id) = cache.lock().pop() {
-                    return id;
+        } else if let Some(pos) = self
+            .freed
+            .iter()
+            .rposition(|&id| id >= self.reuse_threshold)
+        {
+            return self.freed.remove(pos);
+        }
+        // Then draw from the shared cross-commit cache. It is loaded at txn
+        // begin with *only* free-list pages below the reclamation floor — pages
+        // no live reader and no retained-history root can observe — so reusing
+        // them is safe regardless of `reuse_threshold`. Record each draw so the
+        // commit path removes it from the durable free-list.
+        if let Some(cache) = &self.free_page_cache {
+            if let Some(id) = cache.lock().pop() {
+                if let Some(consumed) = &self.free_page_consumed {
+                    consumed.lock().push(id);
                 }
-            }
-        } else {
-            // Readers pinned: only recycle pages that were allocated during
-            // this session (>= reuse_threshold) and thus cannot be in any
-            // prior snapshot. The shared cache is bypassed here because
-            // pages in it predate this session and may be visible to a
-            // pinned reader at an older commit.
-            if let Some(pos) = self
-                .freed
-                .iter()
-                .rposition(|&id| id >= self.reuse_threshold)
-            {
-                let id = self.freed.remove(pos);
                 return id;
             }
         }

src/btree/tree/scan.rs

@@ -38,6 +38,20 @@ impl<V: Vfs> BTree<V> {
         }
     }
 
+    /// Return the smallest key in the tree, or `None` if the tree is empty.
+    /// Descends the leftmost spine only — O(tree height), not O(tree size).
+    pub async fn first_key(&self) -> Result<Option<Vec<u8>>> {
+        if self.root_page_id == 0 {
+            return Ok(None);
+        }
+        // The empty key sorts below every stored key, so the descent lands on
+        // the leftmost leaf.
+        let path = self.path_to_leaf_for_key(&[]).await?;
+        let leaf_id = *path.last().expect("non-empty path");
+        let leaf = self.read_leaf(leaf_id).await?;
+        Ok(leaf.records.first().map(|(k, _)| k.clone()))
+    }
+
     /// Reverse range scan: `start` inclusive, `end` exclusive. Returns results
     /// in descending key order. Collects matching records forward then reverses.
     pub async fn scan_rev(&self, start: &[u8], end: &[u8]) -> Result<Vec<(Vec<u8>, Vec<u8>)>> {

src/catalog/codec.rs

@@ -23,16 +23,9 @@ pub enum CatalogRowKind {
     /// (singleton; no name suffix). Value is `RekeyState` encoded as 13 bytes:
     /// `target_mk_epoch[8] || main_db_done[1] || segments_remaining_idx[4]`.
     RekeyState = 0x03,
-    /// Persistent free-list entry. Key is `[0x04] || page_id_big_endian[8]`
-    /// (big-endian so lexicographic sort matches numeric order). Value is empty.
-    FreeList = 0x04,
-    /// Deferred-free queue. Singleton row; key is `[0x05]`. Value is a
-    /// length-prefixed list of `(commit_id[8] || page_id[8])` pairs, in
-    /// ascending `commit_id` order. Pages here are not yet safe to reallocate
-    /// because a reader snapshot at or before that `commit_id` may still observe
-    /// them. Pairs are promoted to the free-list once no tracked reader pins
-    /// a `commit_id` ≤ the pair's `commit_id`.
-    DeferredFree = 0x05,
+    // 0x04 and 0x05 are reserved: they were the in-catalog free-list and
+    // deferred-free queue, superseded by the durable free-list chain rooted in
+    // the A/B header (see `crate::pager::freelist`). Do not reuse these bytes.
     /// Durable reader pin. One row per active cross-process read transaction.
     /// Key: `[0x06] || pid_u32_be[4] || lease_id_u64_be[8]` (13 bytes).
     /// Value: `commit_id[8] || root_page_id[8] || catalog_root_page_id[8] ||
@@ -200,56 +193,6 @@ impl Catalog {
         vec![CatalogRowKind::RekeyState as u8]
     }
 
-    /// Free-list entry key: `[0x04] || page_id_be[8]`.
-    /// Big-endian so lexicographic order == numeric order.
-    #[must_use]
-    pub fn free_list_key(page_id: u64) -> [u8; 9] {
-        let mut k = [0u8; 9];
-        k[0] = CatalogRowKind::FreeList as u8;
-        k[1..9].copy_from_slice(&page_id.to_be_bytes());
-        k
-    }
-
-    /// Deferred-free queue key: `[0x05]` (singleton).
-    #[must_use]
-    pub fn deferred_free_key() -> Vec<u8> {
-        vec![CatalogRowKind::DeferredFree as u8]
-    }
-
-    /// Encode a slice of `(commit_id, page_id)` pairs. Empty slice encodes to
-    /// empty bytes.
-    #[must_use]
-    pub fn encode_deferred_free(pairs: &[(u64, u64)]) -> Vec<u8> {
-        let mut out = Vec::with_capacity(pairs.len() * 16);
-        for (commit_id, page_id) in pairs {
-            out.extend_from_slice(&commit_id.to_le_bytes());
-            out.extend_from_slice(&page_id.to_le_bytes());
-        }
-        out
-    }
-
-    /// Decode a deferred-free value. Returns `Err` if length is not a multiple
-    /// of 16.
-    pub fn decode_deferred_free(bytes: &[u8]) -> Result<Vec<(u64, u64)>> {
-        if bytes.len() % 16 != 0 {
-            return Err(PagedbError::corruption(
-                crate::errors::CorruptionDetail::HeaderUnverifiable,
-            ));
-        }
-        let mut out = Vec::with_capacity(bytes.len() / 16);
-        let mut i = 0;
-        while i + 16 <= bytes.len() {
-            let mut b8 = [0u8; 8];
-            b8.copy_from_slice(&bytes[i..i + 8]);
-            let commit_id = u64::from_le_bytes(b8);
-            b8.copy_from_slice(&bytes[i + 8..i + 16]);
-            let page_id = u64::from_le_bytes(b8);
-            out.push((commit_id, page_id));
-            i += 16;
-        }
-        Ok(out)
-    }
-
     /// Encode a `RekeyStateRow` as 13 bytes.
     #[must_use]
     pub fn encode_rekey_state(r: &RekeyStateRow) -> [u8; REKEY_STATE_LEN] {