From fd6cbd94d702e0457a760bc8bb4a5e99cffc2469 Mon Sep 17 00:00:00 2001 From: Christophe Pettus Date: Wed, 29 Jul 2026 13:49:07 -0700 Subject: [PATCH] docs: make the README a doctest and correct the public-API docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of this issue's four findings were already fixed earlier in the stack — PUBLIC-API-2 (Poisoned vs is_fatal) landed with #98, and PUBLIC-API-4's encrypted-database under-report landed with #94 as the stride fix. What remains: Every Rust snippet in the README was a compile error against the current public API: `use chisel::defrag::DefragOptions` (the module went pub(crate)), struct literals against `#[non_exhaustive]` `DefragOptions` and `Options`, a `max_pages` field that is called `max_values`, and bare integers where the post-I120/I126 `Tag` and `Handle` newtypes are required. The README is the entry point for every downstream user and nothing caught any of it, because the README was not wired into the build at all. Rather than only correcting the snippets — which would rot again on the next API change — the README is now included via `#![doc = include_str!("../README.md")]`, so all ten fences are doctests and `cargo test` fails the next time the docs fall behind. Fences that would touch the filesystem, or that need a live failure to illustrate, are `no_run`: compiled, not executed. The rest run against an in-memory database set up in hidden lines. Verified the wiring is load-bearing, not decorative: reverting one example to its old `allocate_tagged(b"row-a", 42)` form fails the suite with "expected `Tag`, found integer". The prose the snippets sit in is corrected to match — tags are non-zero and untagged means `tag()` returns None (the "tag 0 means untagged" model predates I126), and the API table now names `Handle`/`Tag` rather than `u64`/`u32`. Both `Stats` fields were also described wrongly, in opposite directions. `total_pages` claimed to match `Superblock.total_pages` while being filled from the PHYSICAL page count, which is the more useful of the two to name explicitly since they diverge exactly when a crash leaves orphan pages in the tail. `file_size_bytes` claimed it "may exceed `total_pages * PAGE_SIZE`" when it is computed from that same count and so can never exceed itself. Finally, `get_root_name` and `clear_root_name` both run the same name validation `set_root_name` does, so `""` or a 25-byte name is an `InvalidRootName` error, not a miss. Their `# Errors` sections said "Only on poisoning" and "NoActiveTransaction if no transaction is open", inviting a caller to treat any Err from a lookup as a fatal drop-and-reopen and tear down a healthy handle. `#[warn(missing_errors_doc)]` guarantees the section exists but not that it is complete, which is what makes an omission like this easy to trust. Both sections now list it, and a test pins the contract — including the half that is easy to over-correct: a valid-but-unbound name is still Ok. Closes #99. --- README.md | 101 +++++++++++++++++++++++++++++----------- src/lib.rs | 22 ++++++++- src/stats.rs | 27 ++++++----- tests/api_edge_cases.rs | 40 ++++++++++++++++ 4 files changed, 149 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index e3d4966..0002f92 100644 --- a/README.md +++ b/README.md @@ -19,8 +19,8 @@ Chisel is designed for single-writer embedded use: one process holds the file vi - **Crash durability** — N configurable superblocks (2–16) with round-robin writes ensure committed data survives crashes. Every page carries an XXH3 checksum for torn-write and bit-rot detection. - **Transactions** — begin / commit / rollback with shadow-paging durability (all data pages fsync'd before the superblock fsync; three fsyncs per commit). - **Savepoints** — PostgreSQL-style named savepoints with `rollback_to` (savepoint preserved for retry) and `release` (merges into the enclosing scope). -- **Handles** — store a value, get back a `u64` handle. Read, update, or delete by handle. Handles are stable across updates, defrag, and reopens. -- **Chunk tags** — attach an immutable `u32` tag to a chunk at allocation time; a reverse membership index maps each tag back to its handles. Enumerate a tag's members, delete with a tag-match assertion, or bulk-drop a whole tagged relation in bounded passes. Tag `0` means untagged. +- **Handles** — store a value, get back a `Handle` (a `u64` newtype). Read, update, or delete by handle. Handles are stable across updates, defrag, and reopens. +- **Chunk tags** — attach an immutable `Tag` (a non-zero `u32` newtype) to a chunk at allocation time; a reverse membership index maps each tag back to its handles. Enumerate a tag's members, delete with a tag-match assertion, or bulk-drop a whole tagged relation in bounded passes. Untagged is the absence of a tag, not a zero value: `tag()` returns `None`. - **Value packing** — slotted data pages pack multiple small values per 8 KB page; values over ~8 KB transparently overflow into chained pages. - **Named roots** — a small fixed table in the superblock mapping string names to handles. Survives commit / rollback transactionally. - **Defragmentation** — explicit `defrag()` consolidates sparse pages and returns a count-based stats record. @@ -44,7 +44,7 @@ The published crate is named `chisel-storage` (plain `chisel` was already taken) ## Quick Start -```rust +```rust,no_run use chisel::{Chisel, Options}; use std::path::Path; @@ -133,10 +133,15 @@ A handle is a stable `u64` returned by `allocate()`. It maps through a radix-tre All mutations require an active transaction. `begin()` opens one, `commit()` makes it durable, `rollback()` discards it. Only one transaction is active at a time — Chisel has savepoints, not nested transactions. ```rust +# fn main() -> chisel::Result<()> { +# use chisel::Chisel; +# let mut db = Chisel::open_in_memory()?; db.begin()?; let h1 = db.allocate(b"a")?; let h2 = db.allocate(b"b")?; db.commit()?; // both h1 and h2 become durable atomically +# Ok(()) +# } ``` Rollback is effectively free: pages written during the transaction were never linked from a superblock, so they are simply abandoned. There is no undo log to replay. @@ -150,6 +155,9 @@ Savepoints are named marks within a transaction. - `rollback()` (full rollback) and `commit()` both clear the entire savepoint stack. ```rust +# fn main() -> chisel::Result<()> { +# use chisel::Chisel; +# let mut db = Chisel::open_in_memory()?; db.begin()?; let keep = db.allocate(b"keep")?; db.savepoint("experiment")?; @@ -157,6 +165,8 @@ let _ = db.allocate(b"maybe discard")?; db.rollback_to("experiment")?; // discards the _ handle; keep remains; sp still open db.release("experiment")?; db.commit()?; +# Ok(()) +# } ``` ### Named roots @@ -164,6 +174,9 @@ db.commit()?; A small fixed-size table in the superblock mapping short string names to handles, intended for long-lived entry points such as a meta-B-tree root. Changes are transactional: `set_root_name` takes effect on commit and reverts on rollback. ```rust +# fn main() -> chisel::Result<()> { +# use chisel::Chisel; +# let mut db = Chisel::open_in_memory()?; db.begin()?; let meta = db.allocate(b"meta-root-payload")?; db.set_root_name("meta", meta)?; @@ -171,6 +184,8 @@ db.commit()?; // Later, possibly after reopen: let meta = db.get_root_name("meta")?.expect("meta root should be set"); +# Ok(()) +# } ``` Names are bounded in length and must be valid UTF-8 without embedded NUL; the table has a small fixed capacity. See `TransactionManager::set_root_name` for exact limits. @@ -180,6 +195,9 @@ Names are bounded in length and must be valid UTF-8 without embedded NUL; the ta `defrag()` consolidates sparse data pages: it re-inserts values from pages whose live-slot count falls below a threshold so those pages become fully free and can be reclaimed. It runs inside an active transaction so it composes with other work and commits atomically. ```rust +# fn main() -> chisel::Result<()> { +# use chisel::Chisel; +# let mut db = Chisel::open_in_memory()?; use chisel::DefragOptions; db.begin()?; @@ -191,31 +209,44 @@ let stats = db.defrag( .max_values(0), // 0 = no cap on values relocated )?; db.commit()?; +# Ok(()) +# } ``` ### Chunk tags -`allocate_tagged(value, tag)` attaches an immutable `u32` tag to a chunk and registers it in a reverse membership index. The tag is fixed at allocation: `update` preserves it, and retagging means delete + re-allocate. Tag `0` is the untagged sentinel and is never indexed — use plain `allocate` for untagged values. Plain `delete` is self-maintaining (it drops the handle from the index automatically); `delete_tagged` is the verified variant that errors with `TagMismatch` if the stored tag differs. +`allocate_tagged(value, tag)` attaches an immutable tag to a chunk and registers it in a reverse membership index. The tag is fixed at allocation: `update` preserves it, and retagging means delete + re-allocate. Plain `delete` is self-maintaining (it drops the handle from the index automatically); `delete_tagged` is the verified variant that errors with `TagMismatch` if the stored tag differs. + +A tag is a `Tag`, not a bare `u32` — `Tag` wraps a `NonZeroU32`, so zero is unrepresentable rather than being an "untagged" sentinel you could pass by accident. Untagged means *no tag at all*: allocate with plain `allocate()`, and `tag()` returns `None` for such a handle. ```rust +# fn main() -> chisel::Result<()> { +# use chisel::Chisel; +# let mut db = Chisel::open_in_memory()?; +use chisel::Tag; + +let tag = Tag::new(42).expect("42 is non-zero"); // or Tag::try_from(42u32)? + db.begin()?; -let a = db.allocate_tagged(b"row-a", 42)?; -let _b = db.allocate_tagged(b"row-b", 42)?; +let a = db.allocate_tagged(b"row-a", tag)?; +let _b = db.allocate_tagged(b"row-b", tag)?; db.commit()?; -assert_eq!(db.tag(a)?, 42); -let members = db.handles_with_tag(42)?; // both handles; order unspecified, but repeatable within a session +assert_eq!(db.tag(a)?, Some(tag)); // None for a handle allocated untagged +let members = db.handles_with_tag(tag)?; // both handles; order unspecified, but repeatable within a session assert_eq!(members.len(), 2); // Bounded relation-drop: loop until the tag is fully drained. loop { db.begin()?; - let progress = db.delete_with_tag(42, 256)?; // up to 256 chunks per pass + let progress = db.delete_with_tag(tag, 256)?; // up to 256 chunks per pass db.commit()?; if progress.complete { break; } } +# Ok(()) +# } ``` ### In-memory mode @@ -223,8 +254,12 @@ loop { `Chisel::open_in_memory()` creates a memory-backed database using a `Vec`-backed `PageIo`. Same code path, same guarantees except durability — no filesystem, no `flock`, and all data is lost on drop. ```rust +# fn main() -> chisel::Result<()> { +# use chisel::Chisel; let mut db = Chisel::open_in_memory()?; // ... same API as a file-backed Chisel ... +# Ok(()) +# } ``` For tuned options (cache size, superblock count), use `Chisel::open_in_memory_with_options(options)`. @@ -238,7 +273,8 @@ Encryption is opt-in and driven entirely through `Options::encryption_key`. A ke Both variants zeroize their material on drop. -```rust +```rust,no_run +# fn main() -> chisel::Result<()> { use chisel::{Chisel, Key, Options}; use std::path::Path; use zeroize::Zeroizing; @@ -249,6 +285,8 @@ let mut db = Chisel::open( Path::new("secret.db"), Options::default().encryption_key(key), )?; +# Ok(()) +# } ``` On create, Chisel generates a random data-encryption key (DEK), encrypts every page under it with XChaCha20-Poly1305, and wraps the DEK under a key-encryption key derived from your supplied key. On reopen, the supplied key must unwrap one of the on-disk key slots or `open` fails with `InvalidEncryptionKey`. Supplying a key to a plaintext DB returns `EncryptionNotSupported`; omitting it on an encrypted DB returns `NoEncryptionKey`. @@ -272,13 +310,13 @@ See [ARCHITECTURE.md#on-disk-encryption](ARCHITECTURE.md#on-disk-encryption) for | `savepoint(name)` | Create a named savepoint | | `rollback_to(name)` | Undo to savepoint (savepoint preserved) | | `release(name)` | Merge savepoint into enclosing scope | -| `allocate(value)` | Store a value; returns a `u64` handle | -| `allocate_tagged(value, tag)` | Store a value with an immutable `u32` tag; returns a `u64` handle | +| `allocate(value)` | Store a value; returns a `Handle` | +| `allocate_tagged(value, tag)` | Store a value with an immutable `Tag` (non-zero); returns a `Handle` | | `read(handle)` | Retrieve a value (takes `&self`) | | `update(handle, value)` | Replace a value (handle preserved) | | `delete(handle)` | Remove a handle | | `delete_many(handles)` | Batch-delete in the current transaction | -| `tag(handle)` | Read a handle's tag, `0` if untagged (takes `&self`) | +| `tag(handle)` | Read a handle's tag as `Option`; `None` if untagged (takes `&self`) | | `handles_with_tag(tag)` | Enumerate live handles carrying `tag`; repeatable within a session, order unspecified (takes `&self`) | | `client_byte(handle)` | Read the handle's opaque client byte, `0` if unset (takes `&self`) | | `set_client_byte(handle, byte)` | Set the opaque client byte; mutable, transactional; `update()` preserves it | @@ -298,23 +336,24 @@ See [ARCHITECTURE.md#on-disk-encryption](ARCHITECTURE.md#on-disk-encryption) for ## Options +`Options` is `#[non_exhaustive]`, so a downstream crate cannot build one with a struct literal. Start from `Options::default()` and use the chained setters — every field has one: + ```rust -use chisel::Options; - -let options = Options { - cache_max_bytes: 8 * 1024 * 1024, // in-memory LRU cap, default 8 MiB - spillway_max_bytes: 1024 * 8 * 1024 * 1024, // sidecar overflow file cap, default 8 GiB - drain_insertion: chisel::DrainInsertion::LruTail, // default; use Mru to drain at insertion - create_if_missing: true, - read_only: false, - superblock_count: 2, // 2..=16; only consulted on create - encryption_key: None, // Some(key) to create/open an encrypted DB - argon2_params: None, // None = OWASP defaults; only used on create -}; +# fn main() { +use chisel::{DrainInsertion, Options}; + +let options = Options::default() + .cache_max_bytes(8 * 1024 * 1024) // in-memory LRU cap, default 8 MiB + .spillway_max_bytes(1024 * 8 * 1024 * 1024) // sidecar overflow file cap, default 8 GiB + .drain_insertion(DrainInsertion::LruTail) // default; use Mru to drain at insertion + .create_if_missing(true) + .read_only(false) + .superblock_count(2); // 2..=16; only consulted on create +// .encryption_key(key) to create/open an encrypted DB; +// .argon2_params(params) to override the OWASP defaults (create only). +# } ``` -`Options` is `#[non_exhaustive]`, so build a customized value with the chained setters rather than a struct literal from another crate: `Options::default().cache_max_bytes(N).encryption_key(key)`. Every field has a matching setter, including `Options::encryption_key(key)` and `Options::argon2_params(params)`. - `cache_max_bytes` is a strict cap on the in-memory LRU cache. When the cache is full and a dirty page cannot be evicted, overflow dirty pages spill to a sidecar `Spillway` file rather than returning an error. The spillway file is bounded by `spillway_max_bytes` (default 8 GiB). Setting `spillway_max_bytes = 0` disables the spillway entirely, restoring the pre-spillway `CacheFull` semantics at the strict cache cap: the operational error `CacheFull` fires when the cache is full and no eviction is possible. With the spillway enabled, exhausting both the cache and the spillway returns `SpillwayFull { limit_bytes }` (also operational; caller recovers by committing or rolling back). `read_only = true` still acquires an exclusive `flock` — it only suppresses writes at the application layer. Two read-only opens cannot coexist on the same file. This is a deliberate choice: even a reader must block concurrent writers to keep the shadow-paging invariants intact. @@ -347,7 +386,11 @@ On any fatal error — including a failed commit-protocol fsync — the `Chisel` `commit` is the one method where the operational/fatal split above does not apply. Its only non-poisoning error is `NoActiveTransaction`, checked before the protocol starts. Once `cache.flush()` has run the manager is in a partial-commit state, so *every* error it can then return poisons the handle — including `CacheFull` and `SpillwayFull`, which are operational anywhere else. Do not catch those from `commit` and call `rollback` to recover: the handle is already poisoned and `rollback` will tell you so. -```rust +```rust,no_run +# fn main() -> chisel::Result<()> { +# use chisel::{Chisel, ChiselError, Options}; +# let path = std::path::Path::new("db.chisel"); +# let mut db = Chisel::open(path, Options::default())?; match db.commit() { Ok(()) => (), // `Poisoned` is matched explicitly: is_fatal() is false for it, so it @@ -359,6 +402,8 @@ match db.commit() { } Err(e) => return Err(e), // operational — handle per your caller's policy } +# Ok(()) +# } ``` The poison model is mandatory because Linux `fsync` semantics (post-2018 "fsyncgate") do not permit safely retrying a failed fsync: the kernel may have discarded the dirty pages before reporting the error. macOS `F_FULLFSYNC` has similar semantics. PostgreSQL `PANIC`s on fsync failure for exactly this reason. diff --git a/src/lib.rs b/src/lib.rs index 5d37b46..5e78caf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,16 @@ // `-D warnings`, which promotes this to a hard error — a new public `-> Result` // method without an `# Errors` section fails the build. #![warn(clippy::missing_errors_doc)] +// The README is the entry point for every downstream user, and every Rust +// snippet in it was a compile error against the current API — the `defrag` +// module went `pub(crate)`, `DefragOptions`/`Options` became +// `#[non_exhaustive]`, and tags/handles became `Tag`/`Handle` newtypes, none +// of which the examples followed. Nothing caught it because the README was +// not wired into the build at all. Including it here makes every fence a +// doctest, so `cargo test` fails the next time an API change outruns the +// docs. Fences that would touch the filesystem or need a live failure to +// demonstrate are marked `no_run`: still compiled, just not executed. +#![doc = include_str!("../README.md")] // I35 (ISSUES.md, 2026-05-22): every storage-internals module is // pub(crate). The supported public surface is the curated re-export @@ -859,7 +869,12 @@ impl Chisel { /// inside an active transaction). Takes `&self` (F3). /// /// # Errors - /// Only on poisoning — an unbound `name` returns `Ok(None)`. + /// `InvalidRootName` if `name` could never have been stored in the first + /// place — empty, over `NAMED_ROOT_NAME_LEN` bytes, or containing a NUL. + /// The lookup path runs the same validation `set_root_name` does, so an + /// unrepresentable name is rejected rather than reported as unbound. A + /// merely *unbound* (but valid) name returns `Ok(None)`. Otherwise only + /// on poisoning. pub fn get_root_name(&self, name: &str) -> Result> { Ok(self.txm.get_root_name(name)?.map(Handle::from)) } @@ -868,7 +883,10 @@ impl Chisel { /// active transaction; becomes durable on commit. /// /// # Errors - /// `NoActiveTransaction` if no transaction is open. + /// `NoActiveTransaction` if no transaction is open; `InvalidRootName` if + /// `name` is empty, over `NAMED_ROOT_NAME_LEN` bytes, or contains a NUL — + /// the clear path runs the same validation `set_root_name` does, so an + /// unrepresentable name is rejected rather than treated as a no-op. pub fn clear_root_name(&mut self, name: &str) -> Result<()> { self.txm.clear_root_name(name) } diff --git a/src/stats.rs b/src/stats.rs index dbbbff4..9245376 100644 --- a/src/stats.rs +++ b/src/stats.rs @@ -18,18 +18,23 @@ pub struct Stats { /// Number of live handles (u64 ids currently mapped in the handle table). pub handle_count: u64, - /// Total allocated pages in the file, matching Superblock.total_pages. + /// PHYSICAL page count: how many whole stride-units the file currently + /// holds, from `PageIo::page_count`. This is NOT `Superblock.total_pages`, + /// which is the last-durable *logical* count. The two diverge when a + /// previous crash left orphan pages in the file tail: those pages are + /// counted here but are dead weight the next allocation will overwrite, + /// and the superblock's figure remains the authoritative one for what the + /// database actually contains. pub total_pages: u64, - /// Raw size of the database file on disk: `page_count × stride`, where - /// `stride` is `PAGE_SIZE` for a plaintext database and `ENC_PAGE_SIZE` - /// (8232) for an encrypted one — an encrypted page carries a 24-byte - /// nonce and a 16-byte tag on top of its 8192 plaintext bytes. May exceed - /// `total_pages * stride` when a previous crash left orphan - /// pages in the file tail — the last-durable superblock's - /// `total_pages` is authoritative, anything beyond it is dead - /// weight that the next allocation will overwrite (see I4). - /// Chisel is single-writer, so there is no concurrent commit - /// that could cause a transient divergence. + /// Raw size of the database file on disk, as `total_pages × stride` — + /// `stride` being `PAGE_SIZE` for a plaintext database and + /// `ENC_PAGE_SIZE` (8232) for an encrypted one, whose pages each carry a + /// 24-byte nonce and a 16-byte tag on top of their 8192 plaintext bytes. + /// + /// Since both fields come from the same physical page count, this is + /// exactly `total_pages × stride` and never diverges from it. It is a + /// page-aligned figure rather than a `stat(2)` call, so it will not + /// reflect a trailing partial page mid-extend. pub file_size_bytes: u64, /// I74 (ISSUES.md, 2026-05-22): current spillway logical-bytes in /// flight (`PAGE_SIZE` × LIVE resident spilled pages — a page read back diff --git a/tests/api_edge_cases.rs b/tests/api_edge_cases.rs index 4a97289..b79929a 100644 --- a/tests/api_edge_cases.rs +++ b/tests/api_edge_cases.rs @@ -605,3 +605,43 @@ fn open_still_creates_over_a_zero_length_file() { drop(db); assert!(std::fs::metadata(tmp.path()).unwrap().len() > 0); } + +// PUBLIC-API-5: `get_root_name` and `clear_root_name` both run the same name +// validation `set_root_name` does, so an unrepresentable name is an +// InvalidRootName error rather than a miss. Their `# Errors` sections said +// otherwise ("Only on poisoning" / "NoActiveTransaction if no transaction is +// open"), which invites a caller to treat any Err from a *lookup* as a fatal +// drop-and-reopen condition and tear down a healthy handle over a 25-byte +// name. This pins the corrected contract, including the part that is easy to +// over-correct: a valid-but-unbound name is still Ok, not an error. +#[test] +fn root_name_validation_applies_to_the_read_and_clear_paths_too() { + let tmp = tempfile::NamedTempFile::new().expect("tempfile"); + let mut db = Chisel::open(tmp.path(), Options::default()).expect("open"); + + let unrepresentable = ["", &"x".repeat(25), "nul\0byte"]; + + for name in unrepresentable { + assert!( + matches!(db.get_root_name(name), Err(ChiselError::InvalidRootName)), + "get_root_name({name:?}) must reject an unrepresentable name" + ); + } + assert!( + db.get_root_name("unbound").expect("valid name").is_none(), + "a valid but unbound name is Ok(None), not an error" + ); + + db.begin().expect("begin"); + for name in unrepresentable { + assert!( + matches!(db.clear_root_name(name), Err(ChiselError::InvalidRootName)), + "clear_root_name({name:?}) must reject an unrepresentable name" + ); + } + assert!( + db.clear_root_name("unbound").is_ok(), + "clearing a valid but unbound name is a no-op, not an error" + ); + db.commit().expect("commit"); +}