diff --git a/python/src/errors.rs b/python/src/errors.rs index 4d6136a..ab0557a 100644 --- a/python/src/errors.rs +++ b/python/src/errors.rs @@ -1,7 +1,8 @@ // errors.rs — defines the Python exception hierarchy and converts a // chisel::ChiselError into the appropriate PyErr. The two-tier split // (OperationalError / FatalError) mirrors ChiselError::is_fatal() in -// src/error.rs exactly, so `except chisel.FatalError` in Python +// src/error.rs (with one deliberate exception, PoisonedError — see the +// Fatal block below), so `except chisel.FatalError` in Python // captures the same set of "drop-and-reopen" conditions that would // poison the Rust TransactionManager. // @@ -130,7 +131,10 @@ create_exception!(_chisel, ClosedError, OperationalError); // circuits without raising) so context-manager usage is unaffected. create_exception!(_chisel, AlreadyFinishedError, OperationalError); -// Fatal — matches ChiselError::is_fatal() in src/error.rs exactly. +// Fatal — matches ChiselError::is_fatal() in src/error.rs, plus PoisonedError: +// it is a FatalError subclass (a drop-and-reopen condition for the caller) even +// though ChiselError::Poisoned is classified non-fatal by is_fatal() — Poisoned +// just means the manager is already dead, not a fresh fatal. // IoError is NOT declared here: it needs two bases (FatalError + OSError) and is // built in `register` via `build_io_error_class` / cached in `IO_ERROR_CLASS`. create_exception!(_chisel, ChecksumMismatchError, FatalError); diff --git a/src/defrag.rs b/src/defrag.rs index 58887ed..d337d8a 100644 --- a/src/defrag.rs +++ b/src/defrag.rs @@ -138,8 +138,10 @@ pub struct DefragStats { /// `txm.sparse_data_pages(sparse_threshold)`. A page is sparse if /// its live-slot count is at or below `threshold × max_observed`. /// Dense pages are left alone. -/// 3. Record the initial data-page count so we can report -/// `pages_freed` at the end as the net drop (I17). +/// 3. Snapshot the initial set of data-page IDs (not a count) so we can +/// report `pages_freed` at the end as the set difference against the +/// final set — a net count delta is the wrong metric here, see the +/// header (I17). /// 4. Snapshot the handle list up front so the iteration walks a /// stable set rather than a handle table we are concurrently /// rewriting via update(). @@ -149,8 +151,9 @@ pub struct DefragStats { /// cursor, which is a fresh, densely-packed page. Repeat until /// the max-work cap (if any) is reached. /// 6. Track UNIQUE sparse pages touched via a HashSet (for accurate -/// `pages_examined`) and compute `pages_freed` as the net drop -/// in data-page count after the sweep. +/// `pages_examined`) and compute `pages_freed` as the set difference +/// between the initial and final data-page-ID sets (NOT a net count +/// delta — see header). /// 7. Reclaim freemap pages orphaned by a prior crash that lost the /// in-memory structural recycle pool, via /// `txm.reclaim_freemap_orphans` (reported as diff --git a/src/freemap.rs b/src/freemap.rs index cd43ee0..526b61d 100644 --- a/src/freemap.rs +++ b/src/freemap.rs @@ -9,9 +9,10 @@ // handle-table / membership-index COW paths prefer `FreeMap::allocate_first` // (reusing a page freed by a prior committed transaction) and fall back to // extending the file only when the freemap has no free id to hand out. -// Reclamation happens during commit via `persist_freemap`, which merges -// `txn_freed_pages` into `current_freemap` BEFORE writing the new freemap -// snapshot (I18 ordering) so allocation cannot reuse a page the last-durable +// Reclamation happens during commit via `persist_freemap` (now +// `FreemapRecycle::persist`), which marks `txn_freed_pages` free in a COW of +// the committed freemap tree BEFORE the new snapshot is durable +// (I18 ordering) so allocation cannot reuse a page the last-durable // superblock still references. The handle table and membership index both feed // their COW-superseded pages into `txn_freed_pages` and allocate through // `cow_alloc`, so they reach a bounded steady-state page count rather than diff --git a/src/freemap_tree.rs b/src/freemap_tree.rs index 158903b..9d0ea02 100644 --- a/src/freemap_tree.rs +++ b/src/freemap_tree.rs @@ -232,12 +232,11 @@ impl FreeMapTree { /// Is `id` currently free? Absent subtree (or past reach) reads as in-use. /// /// Read-only query: production allocation/free goes through `allocate_first` - /// / `mark_free_growing`, so the only consumers are the in-crate tests and - /// the transaction layer's C1-invariant assertions (both `#[cfg(test)]`). - /// Kept on the non-test build as a legitimate diagnostic accessor; the - /// targeted allow (covering its sole helper `find_leaf`) documents that it - /// is intentionally retained rather than dead. - #[allow(dead_code)] + /// / `mark_free_growing`. The production consumer of this accessor is + /// `FreemapRecycle::reclaim_orphans` (the defrag orphan-sweep), which calls + /// it to confirm a freemap-typed dead page is genuinely free before + /// reclaiming it; the in-crate tests and the transaction layer's + /// C1-invariant assertions are the other (`#[cfg(test)]`) consumers. pub fn is_free(&self, cache: &mut PageCache, id: u64) -> Result { let Some(leaf) = self.find_leaf(cache, id)? else { return Ok(false); diff --git a/src/page_cache.rs b/src/page_cache.rs index a44c75a..2069a89 100644 --- a/src/page_cache.rs +++ b/src/page_cache.rs @@ -325,13 +325,17 @@ impl PageCache { /// page past the current high-water mark, so no committed page is ever /// overwritten during the transaction. On commit, `flush()` writes the /// page to disk (implicitly extending the file) and the superblock - /// swap makes it visible; on rollback, `discard()` drops the in-memory + /// swap makes it visible; on rollback, the watermark truncate + /// (`discard_all_dirty()` + `truncate()`, I3) drops the in-memory /// buffer and the on-disk bytes (if any) become orphaned garbage that /// the next `truncate()` or freemap reclaim can recover. /// /// Known v1 simplification (per ARCHITECTURE.md): this allocator never - /// consults the freemap. It always extends past EOF, so freed pages - /// from previous transactions remain unreclaimed until a defrag pass. + /// consults the freemap — it always extends past EOF. Freed pages from + /// previous transactions are still reclaimed, but via the freemap-aware + /// reuse path (`cow_alloc` -> `claim_page`, reuse-before-extend), which + /// is where steady-state allocation goes; `new_page` is only the + /// fallback when the freemap has no id to hand out. /// /// The page is inserted BEFORE `maybe_evict()` runs, so the new page /// itself is never the eviction victim (it is MRU and dirty anyway). @@ -428,7 +432,8 @@ impl PageCache { // - evict back to max_pages if the insertions over-filled the cache // // No per-batch fsync is issued. The single trailing fsync in Phase 2 - // covers every write here, preserving the two-fsync commit cost. + // covers every write here, preserving the commit's fsync count (the + // three-fsync protocol: I28 pre-drain + data flush + superblock). // A crash before Phase 2's fsync is a rolled-back transaction — // no main-file bytes are committed without the superblock swap that // follows flush(). diff --git a/src/page_io.rs b/src/page_io.rs index 8619951..639b187 100644 --- a/src/page_io.rs +++ b/src/page_io.rs @@ -326,11 +326,12 @@ impl PageIo { /// on Apple hardware — a plain `fsync` on macOS does NOT flush the /// drive's own write cache. Rust's `sync_all` does the right thing. /// - /// Ordering invariant: the transaction manager calls this twice per - /// commit — once after writing all data pages, once after writing the + /// Ordering invariant: the transaction manager calls this three times + /// per commit — the I28 pre-drain flush, then after writing all data + /// pages, then after writing the /// new superblock into its inactive slot (slot index = - /// `txn_counter % superblock_count`). Reversing or dropping either - /// fsync breaks durability: a superblock that reaches the platter + /// `txn_counter % superblock_count`). Reversing or dropping the data or + /// superblock fsync breaks durability: a superblock that reaches the platter /// before its referenced data pages can point into garbage, and the /// crash-recovery path has no WAL to replay. /// @@ -358,8 +359,8 @@ impl PageIo { file.sync_all()?; } // No durable storage to flush. The commit protocol still calls - // fsync twice per commit; that overhead (two method calls and - // two matches) is preserved for benchmark fidelity. + // fsync three times per commit; that overhead (three method calls + // and three matches) is preserved for benchmark fidelity. Backing::Memory { .. } => {} } // Increment AFTER the operation succeeds. A failed fsync is fatal