diff --git a/src/defrag.rs b/src/defrag.rs index dabd9b1..58887ed 100644 --- a/src/defrag.rs +++ b/src/defrag.rs @@ -406,8 +406,8 @@ mod tests { // Run defrag with a generous sparse threshold (0.5) so the source pages // definitely qualify. Steps 5 and 7 execute inside the SAME transaction. // - // Step 5 triggers at least one `allocate_data_page` → `allocate_first` - // call, which COWs a freemap leaf and pushes its old id into + // Step 5 triggers at least one data-page allocation → `cow_alloc` → + // `allocate_first` call, which COWs a freemap leaf and pushes its old id into // `structural_superseded`. Step 7 must then exclude that id so only the // forged orphan is reclaimed. tm.begin().unwrap(); diff --git a/src/page_cache.rs b/src/page_cache.rs index 7a3780f..a44c75a 100644 --- a/src/page_cache.rs +++ b/src/page_cache.rs @@ -624,7 +624,7 @@ impl PageCache { /// directly; any dirty entries at id >= watermark are pages /// allocated AFTER the savepoint, exactly the ones we want /// gone. Savepoint-bearing transactions disable freemap reuse - /// (see `allocate_data_page`) so there are no dirty reused-id + /// (see `cow_alloc`) so there are no dirty reused-id /// pages to worry about. /// /// If a future caller needs "truncate without dropping any dirty @@ -736,7 +736,7 @@ impl PageCache { pub fn claim_page(&mut self, page_id: u64) -> Result<()> { // ISSUES.md I20: enforce the "freemap never returns an already-dirty // id" invariant in debug builds. The only legitimate caller is - // `allocate_data_page` via the freemap, which — post-I18 — keeps the + // `cow_alloc` (the shared freemap-aware allocator), which — post-I18 — keeps the // at-risk id sets out of the in-commit free pool. A violation here // would silently drop the caller's pending writes on `page_id`; an // assertion surfaces the bug at its source rather than hours later @@ -1325,8 +1325,8 @@ mod tests { // Regression test for ISSUES.md I20. claim_page previously silently // dropped any prior dirty writes on the claimed id: it unconditionally // removed the existing cache entry and inserted a fresh zeroed one. - // The only legitimate caller is `allocate_data_page` via the freemap, - // which must never return an id already dirty in the current txn — + // The only legitimate caller is `cow_alloc` (the shared freemap-aware + // allocator), which must never return an id already dirty in the current txn — // but the invariant was unenforced. I20 adds a debug_assert so the // rule is checked in debug builds; a violation fires immediately // rather than surfacing hours later as silent data loss. diff --git a/src/transaction/freemap.rs b/src/transaction/freemap.rs index 5eb8538..4c01f02 100644 --- a/src/transaction/freemap.rs +++ b/src/transaction/freemap.rs @@ -123,9 +123,12 @@ impl TransactionManager { // --- Freemap-aware page allocation (ISSUES.md R2) --- // - // `allocate_data_page` is the single entry point for allocating a - // fresh data page during a transaction. It first tries to reuse an - // id from `current_freemap` and falls back to extending the file. + // `cow_alloc` (free function above) is the shared freemap-aware allocator + // for a fresh page during a transaction: it first tries to reuse an id from + // the committed freemap tree and falls back to extending the file. Data-page + // allocation reaches it through the freemap dance hoisted into + // `SlotPacker::insert`'s caller (`insert_into_data_page` in packing.rs); the + // handle-table / membership COW paths reach it through `ht_insert` below. // // Two important scoping rules: // @@ -183,27 +186,6 @@ impl TransactionManager { self.freemap_session_owned = std::mem::take(&mut tree.session_owned); } - pub(super) fn allocate_data_page(&mut self) -> Result { - let reuse = self.savepoints.is_empty(); - let mut tree = self.take_freemap_tree(); - let id = { - let mut cache = self.cache.borrow_mut(); - cow_alloc( - &mut cache, - &mut tree, - &mut self.freemap_hint, - &mut self.structural_reuse, - reuse, - ) - }; - // Write back tree growth + drain supersedes even on error: the freemap - // pages were extended (never freed), so on a non-fatal failure they are - // harmless above-watermark scratch, and the session set must still be - // returned so a retry/commit in the same transaction stays consistent. - self.put_freemap_tree(tree); - id - } - /// COW `handle`'s handle-table entry to `entry`, installing the new root /// and queuing the superseded spine pages for freemap reclamation at /// commit. Shared by `allocate`, `update`, and `set_client_byte`. diff --git a/src/transaction/lifecycle.rs b/src/transaction/lifecycle.rs index 03a1cfc..3659e74 100644 --- a/src/transaction/lifecycle.rs +++ b/src/transaction/lifecycle.rs @@ -109,8 +109,7 @@ impl TransactionManager { // R1: clone the live-slot counts and reset the insert cursor. // The cursor is always None at begin — it only tracks pages // allocated during the current transaction. - self.current_live_slots = self.committed_live_slots.clone(); - self.insert_cursor = None; + self.packer.begin(); self.active_txn = true; self.savepoints.clear(); self.txn_freed_pages.clear(); @@ -230,7 +229,8 @@ impl TransactionManager { } // I28: drain the page cache BEFORE persist_freemap runs. Without - // this, `persist_freemap`'s own `allocate_data_page` can trip + // this, `persist_freemap`'s own freemap-page allocation + // (`structural_extend` → `new_page`) can trip // `maybe_evict`'s spill-or-CacheFull decision (every existing entry // dirty, nothing evictable, and either spillway disabled or full) // and return `ChiselError::CacheFull` or `ChiselError::SpillwayFull`. @@ -331,8 +331,7 @@ impl TransactionManager { // separate in-memory freemap copy to advance. // R1: promote the live-slot counts. The cursor is per-transaction // and gets reset for the next begin(). - self.committed_live_slots = self.current_live_slots.clone(); - self.insert_cursor = None; + self.packer.commit(); self.active_txn = false; self.savepoints.clear(); // txn_freed_pages were already marked free in the new committed freemap @@ -446,8 +445,7 @@ impl TransactionManager { self.structural_reuse.clear(); self.freemap_session_owned.clear(); // R1: revert the live-slot counts and drop the insert cursor. - self.current_live_slots = self.committed_live_slots.clone(); - self.insert_cursor = None; + self.packer.rollback(); self.active_txn = false; self.savepoints.clear(); self.txn_freed_pages.clear(); diff --git a/src/transaction/mod.rs b/src/transaction/mod.rs index 06a6c3a..3b6e9f1 100644 --- a/src/transaction/mod.rs +++ b/src/transaction/mod.rs @@ -29,8 +29,9 @@ // only in memory. A crash mid-transaction discards all dirty pages from cache // and the on-disk superblock still references the prior committed snapshot. // - NOTE: `new_page()` (file extension) extends the underlying file immediately; -// `allocate_data_page` prefers reuse from `current_freemap` but also calls -// through to `new_page()` when the freemap is empty. Either way, any pages +// data-page allocation (via `cow_alloc`) prefers reuse from the committed +// freemap tree but also calls through to `new_page()` when the freemap is +// empty. Either way, any pages // extended-but-uncommitted before a crash are harmless because nothing in the // committed superblock references them, and the rollback path // (`cache.truncate(committed_roots.total_pages)` — I3) actively shrinks the @@ -46,7 +47,8 @@ use std::cell::{Cell, RefCell}; // I127 (ISSUES.md, 2026-06-21): FxHashMap (not std SipHash) for the per-op -// slot-accounting maps below (current/committed_live_slots, Savepoint.live_slots). +// slot-accounting maps (the SlotPacker live-slot maps in packing.rs, +// Savepoint.live_slots below, and the open-time scan map in recovery.rs). // Keys are trusted local u64 page ids — no DoS surface — so SipHash is pure cost, // exactly the I77 rationale; that pass converted the page cache/LRU but missed // these. FxHashMap is a drop-in std HashMap with a faster non-DoS-resistant hasher. @@ -242,34 +244,13 @@ pub struct TransactionManager { // COW and mutate a live committed page in place), which is exactly why it is // transaction-scoped, not cross-transaction. freemap_session_owned: FxHashSet, - // Live-slot count per data page (ISSUES.md R1). Tracks how many - // handle-table entries currently point at each data page — this - // is the information needed to decide when a page is fully empty - // and can be returned to the freemap. `committed_live_slots` is - // the durable state (rebuilt at open time by scanning the handle - // table); `current_live_slots` is the in-transaction working copy. - // - // Kept in memory rather than on disk because updating a slot count - // on a committed data page would require COW, and COWing a data - // page would require rewriting every handle_table entry that - // points into it — an O(live-slots-in-page) amplification per - // delete that shadow paging does not handle well. - committed_live_slots: FxHashMap, - current_live_slots: FxHashMap, - // Per-transaction "insert cursor" (ISSUES.md R1). The id of a data - // page allocated earlier in the current transaction that still has - // free space. New values pack into it until it fills, at which - // point a new page is allocated and becomes the new cursor. - // - // `None` at the start of each transaction. Only set for pages - // allocated during THIS transaction (so they're dirty in the cache - // and safe to modify). A committed data page is never the cursor — - // that would require COW, which is prohibitively expensive for data - // pages (every handle_table entry pointing at the page would need - // to be rewritten). Disabled entirely when savepoints are active, - // same as freemap reuse (R2): the savepoint-snapshot cost becomes - // manageable when only one code path interacts with packing state. - insert_cursor: Option, + // R1 slot-packing state (live-slot counts per data page + the insert + // cursor), extracted into its own owned unit. No code outside `packing.rs` + // touches the inner fields; `TransactionManager` reaches packing through + // the narrow `SlotPacker` interface (the `insert_into_data_page` / + // `release_data_slot` wrappers plus the lifecycle/savepoint/stats hooks). + // See the `packing` module header for the live-slot and cursor models. + packer: packing::SlotPacker, // Poison flag (ISSUES.md I1). Once set, every public entry point returns // ChiselError::Poisoned until the manager is dropped. Set by commit() on // any error in the commit protocol, and by `poison_on_fatal()` for any diff --git a/src/transaction/packing.rs b/src/transaction/packing.rs index 56dec7d..8f8bc12 100644 --- a/src/transaction/packing.rs +++ b/src/transaction/packing.rs @@ -1,81 +1,86 @@ -//! transaction::packing — R1 data-page slot packing: releasing a data -//! slot, lazily materializing the handle table, and inserting a value into -//! a data page. Split out of `transaction.rs` verbatim; see the parent -//! module for the type and fields. +//! transaction::packing — R1 data-page slot packing as an owned unit. +//! +//! `SlotPacker` owns the three fields that together implement the R1 +//! live-slot / insert-cursor model: `committed_live_slots`, +//! `current_live_slots`, and `insert_cursor`. No code outside this file +//! touches those fields directly; everything goes through the narrow +//! interface below. `TransactionManager` holds one `SlotPacker` (the +//! `packer` field) and delegates via thin wrappers (`insert_into_data_page` +//! / `release_data_slot`) plus the lifecycle/savepoint/stats hooks. +//! +//! Live-slot model (ISSUES.md R1): `current_live_slots[page_id]` counts how +//! many handle-table entries currently point at each data page — the +//! information needed to decide when a page is fully empty and can be +//! returned to the freemap. `committed_live_slots` is the durable state +//! (rebuilt at open time by scanning the handle table); `current_live_slots` +//! is the in-transaction working copy. Both are kept purely in memory: +//! storing a slot count ON the data page would force a COW (and a +//! handle-table rewrite for every entry pointing into it) on every delete — +//! an O(live-slots-in-page) amplification shadow paging does not handle well. +//! +//! Packing cursor: a data page allocated earlier in THIS transaction that +//! still has free space. New values pack into it until it fills, at which +//! point a new page is allocated and becomes the new cursor. `None` at the +//! start of each transaction (only pages allocated during the current +//! transaction — dirty in the cache and safe to modify — are ever the +//! cursor; a committed data page is never the cursor, since that would +//! require COW). Packing is disabled entirely when savepoints are active +//! (the `packing_enabled` gate below): the savepoint-snapshot cost becomes +//! manageable when only one code path interacts with packing state. The +//! packer itself does NOT know about savepoints — the caller passes the gate. +//! +//! Split out of the historical `transaction.rs`; see the parent module for +//! `TransactionManager` and the freemap fields the wrappers borrow. use super::*; -impl TransactionManager { - // --- Private helpers --- +/// Owned R1 slot-packing state. See the module header for the live-slot and +/// cursor models. Constructed empty (`new`) by `create_new`, or seeded from a +/// scanned committed map (`from_committed`) by `open_existing`. +pub(super) struct SlotPacker { + // Durable live-slot counts (promoted from `current_live_slots` at commit; + // rebuilt at open time by scanning the handle table). + committed_live_slots: FxHashMap, + // In-transaction working copy of the live-slot counts. + current_live_slots: FxHashMap, + // The current in-progress insert cursor; see the module header. + insert_cursor: Option, +} - /// Release one slot from a data page (ISSUES.md R1). Decrements - /// `current_live_slots[page_id]`; if the count reaches zero, the - /// whole page becomes unreferenced and is pushed to - /// `txn_freed_pages` so commit can return it to the freemap. - /// Otherwise the slot becomes a tombstone: dead weight inside a - /// still-live page, reclaimable only via defrag. - /// - /// If the page is somehow not tracked in `current_live_slots` (a - /// bug; open-time scan should catch every live data page), this is - /// a no-op — we prefer leaking to a spurious free. - /// - /// NOTE: a stray orphaned line "Lazily create a handle table root - /// on first insert. A fresh database has" previously sat at the - /// top of this doc block (an interleaved remnant of - /// `ensure_handle_table`'s docstring); removed 2026-04-17 during - /// the commenting pass. The counterpart ("root_handle_table_page - /// == PAGE_ID_NONE; we don't materialize...") still sits above - /// `ensure_handle_table` below — both belong together. - pub(super) fn release_data_slot(&mut self, page_id: u64) { - let Some(count) = self.current_live_slots.get_mut(&page_id) else { - return; - }; - if *count > 0 { - *count -= 1; - } - if *count == 0 { - self.current_live_slots.remove(&page_id); - // If this page is the active insert cursor, clear the - // cursor — it's about to become free space, and we don't - // want future inserts to pack into it and then find it - // disappearing at commit time. - if self.insert_cursor == Some(page_id) { - self.insert_cursor = None; - } - self.txn_freed_pages.push(page_id); +impl SlotPacker { + /// Empty packer: no live slots, no cursor. Used for a freshly created + /// database (`create_new`), which has no data pages yet. + pub(super) fn new() -> Self { + SlotPacker { + committed_live_slots: FxHashMap::default(), + current_live_slots: FxHashMap::default(), + insert_cursor: None, } } - /// Lazily create a handle table root on first insert. A fresh - /// database has `root_handle_table_page == PAGE_ID_NONE`; we don't - /// materialize the root until there is a handle to put in it, so - /// empty databases never pay for a handle-table page. No per-page - /// rollback bookkeeping — the watermark rollback mechanism (I3) - /// handles any page allocated here automatically. - pub(super) fn ensure_handle_table(&mut self) -> Result<()> { - if self.current_roots.handle_table_page == PAGE_ID_NONE { - let root = { - let mut cache = self.cache.borrow_mut(); - self.handle_table.create_root(&mut cache)? - }; - self.current_roots.handle_table_page = root; + /// Seed from a committed live-slot map scanned at open time. The working + /// copy starts equal to the committed map; the cursor starts None (it only + /// ever tracks pages allocated during a live transaction). + pub(super) fn from_committed(committed: FxHashMap) -> Self { + let current_live_slots = committed.clone(); + SlotPacker { + committed_live_slots: committed, + current_live_slots, + insert_cursor: None, } - Ok(()) } /// Place a value in a data page and return (page_id, slot_index). /// - /// Post-R1 packing model: the transaction maintains an "insert - /// cursor" — a data page allocated earlier in THIS transaction - /// that still has space — and packs successive small-value inserts - /// into it until it fills. When the cursor is absent/full, a new - /// page is allocated (via `allocate_data_page`, which prefers - /// freemap reuse over file extension — R2) and becomes the new - /// cursor. Packing is disabled while savepoints are active: the - /// cursor is force-cleared by `savepoint()` and is NOT set when a - /// new page is allocated inside a savepoint scope, so each insert - /// under a savepoint gets its own page (the pre-R1 behavior). This - /// keeps the per-savepoint snapshot cheap to restore. + /// Post-R1 packing model: try to reuse the current cursor page if it has + /// room; when the cursor is absent/full, call `alloc` for a fresh page and + /// (when `packing_enabled`) install it as the new cursor. `alloc` is the + /// data-page allocator (formerly `TransactionManager::allocate_data_page`), + /// hoisted to the call site so the freemap dance it performs does not + /// borrow-conflict with `&mut self` here; it receives `cache` as a param. + /// `packing_enabled` is the caller's `savepoints.is_empty()` check: under a + /// savepoint the cursor stays None so each insert gets its own page (the + /// pre-R1 behavior), keeping per-savepoint snapshots cheap to restore. /// /// Checksum is stamped eagerly after every mutation so the page carries a /// valid internal checksum before any path could write it to the main @@ -97,20 +102,22 @@ impl TransactionManager { /// main-file write. See ISSUES.md. /// /// Live-slot bookkeeping: every successful insert increments - /// `current_live_slots[page_id]`. `delete`/`update` consult this - /// map (via `release_data_slot`) to decide when a page is fully - /// empty and can be freed back to the freemap on commit. The map - /// is kept purely in memory — storing a slot count ON the data - /// page would force a COW (and a handle-table rewrite for every - /// entry pointing into it) on every delete. - pub(super) fn insert_into_data_page(&mut self, value: &[u8]) -> Result<(u64, u16)> { - // Packing path: try to reuse the current cursor page if it - // has room. The cursor only exists when savepoints are empty - // (see savepoint_inner) so this branch implicitly respects - // the "no packing under savepoints" rule. + /// `current_live_slots[page_id]`. `delete`/`update` consult this map (via + /// `release`) to decide when a page is fully empty and can be freed back to + /// the freemap on commit. + pub(super) fn insert( + &mut self, + cache: &mut PageCache, + alloc: &mut dyn FnMut(&mut PageCache) -> Result, + packing_enabled: bool, + value: &[u8], + ) -> Result<(u64, u16)> { + // Packing path: try to reuse the current cursor page if it has room. + // The cursor only exists when packing is enabled (savepoints empty), + // so this branch implicitly respects the "no packing under savepoints" + // rule. if let Some(cursor_page_id) = self.insert_cursor { let slot_option = { - let mut cache = self.cache.borrow_mut(); let buf = cache.get_mut(cursor_page_id)?; let result = DataPage::insert(buf, value); if result.is_some() { @@ -126,15 +133,13 @@ impl TransactionManager { // the new page becomes the new cursor. } - // Allocate a fresh data page. Under active savepoints, the - // cursor stays None (set below, then cleared by the savepoint - // check in subsequent calls) so each insert gets its own page — - // matching the pre-R1 "one value per page" behavior within - // savepoint scopes, which is the price of keeping rollback_to + // Allocate a fresh data page. Under active savepoints + // (`packing_enabled == false`) the cursor stays None so each insert + // gets its own page — matching the pre-R1 "one value per page" behavior + // within savepoint scopes, which is the price of keeping rollback_to // semantics simple. - let page_id = self.allocate_data_page()?; + let page_id = alloc(cache)?; let slot = { - let mut cache = self.cache.borrow_mut(); let buf = cache.get_mut(page_id)?; DataPage::init_page(buf); // I46 INVARIANT: DataPage::insert can only return None for @@ -149,13 +154,174 @@ impl TransactionManager { slot }; - // Only install the new page as the cursor if we're outside any - // savepoint scope. During a savepoint scope the cursor stays - // None so packing is effectively disabled. - if self.savepoints.is_empty() { + // Only install the new page as the cursor when packing is enabled + // (outside any savepoint scope). During a savepoint scope the cursor + // stays None so packing is effectively disabled. + if packing_enabled { self.insert_cursor = Some(page_id); } *self.current_live_slots.entry(page_id).or_insert(0) += 1; Ok((page_id, slot)) } + + /// Release one slot from a data page (ISSUES.md R1). Decrements + /// `current_live_slots[page_id]`; if the count reaches zero, the whole + /// page becomes unreferenced. Returns `Some(page_id)` in that case so the + /// caller can push it to `txn_freed_pages` (commit returns it to the + /// freemap); `None` otherwise. Either way, if the freed page was the + /// active insert cursor the cursor is cleared here — it's about to become + /// free space, and we don't want future inserts to pack into it and then + /// find it disappearing at commit time. A non-zero residual count leaves + /// the slot a tombstone: dead weight inside a still-live page, reclaimable + /// only via defrag. + /// + /// If the page is somehow not tracked in `current_live_slots` (a bug; + /// open-time scan should catch every live data page), this is a no-op — we + /// prefer leaking to a spurious free. + pub(super) fn release(&mut self, page_id: u64) -> Option { + let count = self.current_live_slots.get_mut(&page_id)?; + if *count > 0 { + *count -= 1; + } + if *count == 0 { + self.current_live_slots.remove(&page_id); + if self.insert_cursor == Some(page_id) { + self.insert_cursor = None; + } + return Some(page_id); + } + None + } + + /// Begin a transaction: clone the committed counts into the working copy + /// and reset the cursor (it only tracks pages allocated during the current + /// transaction, so it is always None at begin). + pub(super) fn begin(&mut self) { + self.current_live_slots = self.committed_live_slots.clone(); + self.insert_cursor = None; + } + + /// Commit: promote the working counts to committed and reset the cursor + /// (per-transaction state that gets re-established at the next begin). + pub(super) fn commit(&mut self) { + self.committed_live_slots = self.current_live_slots.clone(); + self.insert_cursor = None; + } + + /// Roll back: revert the working counts to the committed baseline and drop + /// the cursor. + pub(super) fn rollback(&mut self) { + self.current_live_slots = self.committed_live_slots.clone(); + self.insert_cursor = None; + } + + /// Read-only snapshot of the working state for a savepoint: the current + /// live-slot map (cloned) and the cursor. The savepoint-CREATE path clears + /// the cursor separately via `clear_cursor` after snapshotting — snapshot + /// itself does not mutate. + pub(super) fn snapshot(&self) -> (FxHashMap, Option) { + (self.current_live_slots.clone(), self.insert_cursor) + } + + /// Restore the working state from a savepoint snapshot. + pub(super) fn restore(&mut self, snap: (FxHashMap, Option)) { + self.current_live_slots = snap.0; + self.insert_cursor = snap.1; + } + + /// Clear the insert cursor. Used by the savepoint-create path: once a + /// savepoint exists the insert path stops packing (same posture as freemap + /// reuse — savepoints disable the optimization to keep rollback_to simple). + pub(super) fn clear_cursor(&mut self) { + self.insert_cursor = None; + } + + /// The in-transaction working live-slot map. Read-only access for stats + /// (sparse-page detection, the defrag page-id snapshot) and tests. + pub(super) fn current_live_slots(&self) -> &FxHashMap { + &self.current_live_slots + } + + /// Whether the working live-slot map is empty (no data page holds a live + /// slot). Convenience accessor used by tests asserting a clean no-op. + #[cfg(test)] + pub(super) fn is_current_empty(&self) -> bool { + self.current_live_slots.is_empty() + } + + /// The current insert cursor. Read-only accessor for tests asserting the + /// packing state after a failed mutation. + #[cfg(test)] + pub(super) fn insert_cursor(&self) -> Option { + self.insert_cursor + } +} + +impl TransactionManager { + /// Place a value in a data page and return (page_id, slot_index). + /// + /// Thin wrapper over `SlotPacker::insert`. The freemap dance that + /// `allocate_data_page` performs (take/put the transient `FreeMapTree`, + /// touching `current_roots`, `freemap_session_owned`, + /// `structural_superseded`, `freemap_hint`, `structural_reuse`) is hoisted + /// HERE, around the closure, exactly like `ht_insert`: those are + /// `&mut self` operations and cannot run inside a closure that the packer + /// borrow (`&mut self.packer`) would also need. The closure captures only + /// the disjoint freemap field refs plus the local `tree`, and takes `cache` + /// as a param. + /// + /// `put_freemap_tree` runs even on the error path — matching the historical + /// `allocate_data_page`, which wrote tree growth back before returning the + /// allocation result. The freemap pages were extended (never freed), so on a + /// non-fatal failure they are harmless above-watermark scratch, and the + /// session set must still be returned for a retry/commit in the same + /// transaction to stay consistent. + pub(super) fn insert_into_data_page(&mut self, value: &[u8]) -> Result<(u64, u16)> { + // `reuse` gates freemap reuse inside cow_alloc; `packing_enabled` gates + // installing the freshly-allocated page as the cursor. Both are the + // same `savepoints.is_empty()` check today, but they are distinct + // concerns (the packer must not know about savepoints), so they are + // named separately. + let reuse = self.savepoints.is_empty(); + let packing_enabled = self.savepoints.is_empty(); + let mut tree = self.take_freemap_tree(); + let result = { + let hint = &mut self.freemap_hint; + let pool = &mut self.structural_reuse; + let mut cache = self.cache.borrow_mut(); + let mut alloc = + |c: &mut PageCache| super::freemap::cow_alloc(c, &mut tree, hint, pool, reuse); + self.packer + .insert(&mut cache, &mut alloc, packing_enabled, value) + }; + self.put_freemap_tree(tree); + result + } + + /// Release one slot from a data page. Thin wrapper over + /// `SlotPacker::release`: when the page hits zero live slots the packer + /// returns its id and we push it to `txn_freed_pages` so commit returns it + /// to the freemap. + pub(super) fn release_data_slot(&mut self, page_id: u64) { + if let Some(freed) = self.packer.release(page_id) { + self.txn_freed_pages.push(freed); + } + } + + /// Lazily create a handle table root on first insert. A fresh + /// database has `root_handle_table_page == PAGE_ID_NONE`; we don't + /// materialize the root until there is a handle to put in it, so + /// empty databases never pay for a handle-table page. No per-page + /// rollback bookkeeping — the watermark rollback mechanism (I3) + /// handles any page allocated here automatically. + pub(super) fn ensure_handle_table(&mut self) -> Result<()> { + if self.current_roots.handle_table_page == PAGE_ID_NONE { + let root = { + let mut cache = self.cache.borrow_mut(); + self.handle_table.create_root(&mut cache)? + }; + self.current_roots.handle_table_page = root; + } + Ok(()) + } } diff --git a/src/transaction/recovery.rs b/src/transaction/recovery.rs index 7d73c06..b1a9457 100644 --- a/src/transaction/recovery.rs +++ b/src/transaction/recovery.rs @@ -95,9 +95,7 @@ impl TransactionManager { structural_superseded: Vec::new(), freemap_session_owned: FxHashSet::default(), // A fresh database has no data pages and no live slots yet. - committed_live_slots: FxHashMap::default(), - current_live_slots: FxHashMap::default(), - insert_cursor: None, + packer: packing::SlotPacker::new(), poisoned: Cell::new(false), #[cfg(test)] fault: fault::FaultInjector::default(), @@ -302,8 +300,6 @@ impl TransactionManager { } } } - let current_live_slots = committed_live_slots.clone(); - Ok(TransactionManager { cache: RefCell::new(cache), committed_roots: roots.clone(), @@ -323,9 +319,7 @@ impl TransactionManager { structural_reuse: Vec::new(), structural_superseded: Vec::new(), freemap_session_owned: FxHashSet::default(), - committed_live_slots, - current_live_slots, - insert_cursor: None, + packer: packing::SlotPacker::from_committed(committed_live_slots), poisoned: Cell::new(false), #[cfg(test)] fault: fault::FaultInjector::default(), diff --git a/src/transaction/savepoints.rs b/src/transaction/savepoints.rs index 7bfbc09..de699f4 100644 --- a/src/transaction/savepoints.rs +++ b/src/transaction/savepoints.rs @@ -29,9 +29,8 @@ impl TransactionManager { // insert path stops packing into the cursor (same posture as // freemap reuse: savepoints disable the optimization so the // rollback_to semantics stay simple). - let live_slots = self.current_live_slots.clone(); - let insert_cursor = self.insert_cursor; - self.insert_cursor = None; + let (live_slots, insert_cursor) = self.packer.snapshot(); + self.packer.clear_cursor(); self.savepoints.push(Savepoint { name: name.to_string(), roots: self.current_roots.clone(), @@ -96,8 +95,11 @@ impl TransactionManager { // created, so this sets the cursor back to whatever value it // held BEFORE the savepoint was taken (typically also None, // since savepoint-bearing transactions disable packing). - self.current_live_slots = self.savepoints[idx].live_slots.clone(); - self.insert_cursor = self.savepoints[idx].insert_cursor; + let snap = ( + self.savepoints[idx].live_slots.clone(), + self.savepoints[idx].insert_cursor, + ); + self.packer.restore(snap); self.savepoints.truncate(idx + 1); self.txn_freed_pages.clear(); diff --git a/src/transaction/stats.rs b/src/transaction/stats.rs index ea41e76..16698c1 100644 --- a/src/transaction/stats.rs +++ b/src/transaction/stats.rs @@ -121,9 +121,9 @@ impl TransactionManager { if threshold_ratio <= 0.0 { return Ok(sparse); } - let page_ids: Vec = self.current_live_slots.keys().copied().collect(); + let page_ids: Vec = self.packer.current_live_slots().keys().copied().collect(); for page_id in page_ids { - let live = match self.current_live_slots.get(&page_id) { + let live = match self.packer.current_live_slots().get(&page_id) { Some(&n) if n > 0 => n, _ => continue, }; @@ -152,7 +152,7 @@ impl TransactionManager { /// creates a dense one; the former should count as "reclaimed" /// even when the latter offsets the net count. pub fn data_page_ids_snapshot(&self) -> std::collections::HashSet { - self.current_live_slots.keys().copied().collect() + self.packer.current_live_slots().keys().copied().collect() } /// Look up the data page id that currently holds `handle`. Returns diff --git a/src/transaction/tests.rs b/src/transaction/tests.rs index 16f344b..a48e0c8 100644 --- a/src/transaction/tests.rs +++ b/src/transaction/tests.rs @@ -1226,9 +1226,9 @@ fn reclaim_freemap_orphans_excludes_live_recycle_pool() { // Regression test for ISSUES.md I28. I19 introduced `CacheFull` as // an **operational** error (documented as "commit or rollback to // recover"), but `commit_inner` runs `persist_freemap` BEFORE -// `cache.flush()` — and `persist_freemap` itself calls -// `allocate_data_page`, which may trip `maybe_evict`'s ceiling -// check when every existing cache entry is dirty. Pre-fix the +// `cache.flush()` — and `persist_freemap` itself allocates a +// freemap page (`structural_extend`), which may trip `maybe_evict`'s +// ceiling check when every existing cache entry is dirty. Pre-fix the // resulting `CacheFull` propagated out of commit_inner and // commit()'s poison wrapper poisoned the manager unconditionally. // The recovery advice ("commit to flush") became impossible to @@ -1303,8 +1303,8 @@ fn commit_does_not_poison_when_cache_is_at_strict_cap() { ); // The actual I28 check. Pre-fix, `persist_freemap`'s internal - // `allocate_data_page` trips the ceiling and propagates - // CacheFull out of commit_inner; commit()'s poison wrapper + // freemap-page allocation (`structural_extend`) trips the ceiling and + // propagates CacheFull out of commit_inner; commit()'s poison wrapper // then sets the poison flag. Post-fix commit drains first. let result = tm.commit(); assert!( @@ -1386,12 +1386,13 @@ fn allocate_membership_failure_leaves_maps_consistent() { // insert cursor survives to skew later packing / defrag density / page // reclamation. (Pre-fix this leaked `{page: 1}` and `Some(page)`.) assert!( - tm.current_live_slots.is_empty(), + tm.packer.is_current_empty(), "failed allocate left a phantom live-slot count: {:?}", - tm.current_live_slots + tm.packer.current_live_slots() ); assert_eq!( - tm.insert_cursor, None, + tm.packer.insert_cursor(), + None, "failed allocate left a ghost insert cursor" ); @@ -1445,11 +1446,11 @@ fn allocate_handle_table_failure_leaves_maps_consistent() { ); assert!(tm.handles_with_tag(7).unwrap().is_empty()); assert!( - tm.current_live_slots.is_empty(), + tm.packer.is_current_empty(), "failed forward step left a phantom live-slot count: {:?}", - tm.current_live_slots + tm.packer.current_live_slots() ); - assert_eq!(tm.insert_cursor, None); + assert_eq!(tm.packer.insert_cursor(), None); // Disarmed retry succeeds and is consistent across both maps. let h = tm.allocate_tagged(b"payload", 7).unwrap(); @@ -1760,10 +1761,10 @@ fn update_handle_table_failure_preserves_old_value_and_releases_new_slot() { // had exactly one live slot (for `old`), and the failed update must // leave precisely that — no phantom count for the abandoned new value. assert_eq!( - tm.current_live_slots.values().sum::(), + tm.packer.current_live_slots().values().sum::(), 1, "failed update left a phantom live-slot: {:?}", - tm.current_live_slots + tm.packer.current_live_slots() ); // Durability: commit, assert C1, force reuse, re-read the old value.