8000 fix(storage): queries wait for a checkpoint instead of reading it half-written (#558) by adityamukho · Pull Request #560 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

Skip to content
  • Merged
    Show file tree
    Hide file tree
    Changes from all commits
    Commits
    File filter

    Filter by extension

    Filter by extension

    Conversations
    Failed to load comments.
    Loading
    Jump to
    Jump to file
    Failed to load files.
    Loading
    Diff view
    Diff view
    12 changes: 12 additions & 0 deletions CHANGELOG.md
    Original file line number Diff line number Diff line change
    Expand Up @@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file.
    The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
    and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

    ## [Unreleased]

    ### Fixed

    - **A query on another thread sees each transaction whole (#554).** A commit wrote its WAL entry, then added the transaction's facts to the in-memory store one fact at a time, each under its own lock, so a query running on another thread could return part of the transaction. `execute("(transact ...)")`, `execute("(retract ...)")` and `WriteTransaction::commit` all did this. They now add the whole transaction under one lock, as does WAL replay. Stored data was never affected. Affected `WriteTransaction` since v0.5.0 and `execute` since v0.6.0. Also fixed in v3.0.0.
    - **All patterns of one query read the same state of the database (#555).** A query whose patterns name entities or attributes read each of them with its own lookup, each under its own lock, so a transaction that committed between two lookups was seen by some patterns and not others, and the query could return rows that match no state of the database. The lookups now run under one lock. Stored data was never affected. Affected every release since v1.1.0. Also fixed in v3.0.0.
    - **A query during a checkpoint no longer misses committed facts (#558).** A checkpoint wrote its new fact pages over the index pages that queries on other threads were still reading, then installed its new readers and cleared the pending facts in three separately locked steps. A query during a checkpoint, including the automatic one every `wal_checkpoint_threshold` WAL entries, could miss facts committed before it started or see part of a transaction. Queries now wait for a checkpoint to finish: it holds the store's write lock from its first page write until the new state is published. Stored data was never affected. Affected every release since v0.9.0; v3.0.0's copy-on-write checkpoints are not affected.

    ### Tests

    - `tests/snapshot_atomicity_test.rs` races a writer against a reader: a query must see each transaction whole, whether written with `execute` or a `WriteTransaction` (#554), and must read one state for all its patterns (#555), in memory and on file, and must not miss committed facts while a checkpoint runs (#558).

    ## v2.0.5 — 2026-10-11

    Patch release on the v2.x line under the [support policy](PHILOSOPHY.md#support-policy): two data-integrity fixes (#506, #513) and three bug fixes backported from v3.0.0 (#528, #457, #476), plus a weekly soak test and working coverage gates. File format is unchanged (v7). Two new error codes, `STG-045` and `WAL-007`, numbered as in v3.0.0; no other API changes. Upgrading is recommended for every v2.x user.
    Expand Down
    2 changes: 1 addition & 1 deletion CLAUDE.md
    Original file line number Diff line number Diff line change
    Expand Up @@ -174,7 +174,7 @@ Auto-migrates v1/v2/v3/v4/v5/v6 → v7 on open/checkpoint.

    ## Test Coverage

    **1241 tests** (1232 passing, 9 ignored; unit + integration + doc).
    **1250 tests** (1241 passing, 9 ignored; unit + integration + doc).
    See `docs/TEST_COVERAGE.md` for the full per-file breakdown.

    **Testing conventions** — see the Testing Conventions section below before writing any tests.
    Expand Down
    2 changes: 1 addition & 1 deletion README.md
    Original file line number Diff line number Diff line change
    Expand Up @@ -105,7 +105,7 @@ let r2 = pq.execute(&[("tx", BindValue::TxCount(2)), ("entity", BindValue::Entit

    ```bash
    cargo run # interactive Datalog REPL
    cargo test # run 1241 tests
    cargo test # run 1250 tests
    cargo run < demos/demo_recursive.txt # recursive rules demo
    ```

    Expand Down
    4 changes: 2 additions & 2 deletions docs/TEST_COVERAGE.md
    Original file line number Diff line number Diff line change
    Expand Up @@ -4,7 +4,7 @@

    **Verified**: 2026-10-11 with `cargo test`

    **Result**: 1,232 passing tests and 9 ignored tests (1,241 total), including the golden-file corpus (#391)
    **Result**: 1,241 passing tests and 9 ignored tests (1,250 total), including the golden-file corpus (#391)

    For v2.0.5, `tests/edge_cases_test.rs` checks that a file of 1 to 4,095 byte 8000 s is `STG-045`, unchanged and with no WAL, when it is a database cut below one page or foreign bytes, and opens as a new database that takes a write and reads it back after reopen when it is a prefix of the initial header or all zeros (#506).

    Expand All @@ -22,7 +22,7 @@ The v2.0.5 suite covers the public database API, the Datalog engine, storage and

    - **Core database behavior**: in-memory and file-backed operation, transactions, checkpoints, retractions, bi-temporal queries, recursive rules, prepared statements, aggregation, expressions, disjunction, window functions, UDFs, and magic-sets evaluation.
    - **Storage correctness**: packed pages, B+tree indexes and range scans, cache behavior, file-header validation, migration, WAL replay, checksums, corruption handling, and fault injection.
    - **Reliability and concurrency**: concurrent reads and writes, rollback behavior, same-process handle exclusion, cross-process locking, PID namespaces, NFS and container lock behavior, and SIGKILL recovery.
    - **Reliability and concurrency**: concurrent reads and writes, rollback behavior, same-process handle exclusion, cross-process locking, PID namespaces, NFS and container lock behavior, and SIGKILL recovery, and queries seeing each concurrent transaction whole and reading one state for all their patterns, also during a checkpoint (`tests/snapshot_atomicity_test.rs`, #554, #555, #558).
    - **Public diagnostics**: structured parser, query, storage, WAL, API, and internal error codes; the error-code registry is checked against [`ERROR_REFERENCE.md`](ERROR_REFERENCE.md).
    - **Compatibility and quality**: golden-file corpus of released file formats, cross-platform format compatibility, property-based tests, long-haul smoke coverage, a weekly soak at 10M facts with a 5-hour churn (#392), XTDB/Datomic semantic compatibility, grammar conformance, and rustdoc examples.

    Expand Down
    11 changes: 5 additions & 6 deletions src/browser/mod.rs
    Original file line number Diff line number Diff line change
    Expand Up @@ -267,12 +267,11 @@ impl BrowserDb {
    })
    .collect();

    for fact in &stamped {
    inner
    .fact_storage
    .load_fact(fact.clone())
    .map_err(|e| JsValue::from_str(&e.to_string()))?;
    }
    // One batch, so no query sees part of the transaction (#554).
    inner
    .fact_storage
    .load_facts(stamped.iter().cloned())
    .map_err(|e| JsValue::from_str(&e.to_string()))?;

    inner.pfs.mark_dirty();
    inner
    Expand Down
    21 changes: 10 additions & 11 deletions src/db.rs
    Original file line number Diff line number Diff line change
    Expand Up @@ -519,9 +519,7 @@ impl Minigraf {
    // Already present in the main file; skip.
    continue;
    }
    for fact in &entry.facts {
    let _ = fact_storage.load_fact(fact.clone())?;
    }
    let _ = fact_storage.load_facts(entry.facts.iter().cloned())?;
    #[allow(clippy::arithmetic_side_effects)]
    {
    replayed += 1;
    Expand Down Expand Up @@ -637,10 +635,12 @@ impl Minigraf {
    &stamped,
    )?;

    // WAL succeeded — now apply facts to shared FactStorage.
    for fact in &stamped {
    let _ = self.inner.fact_storage.load_fact(fact.clone())?;
    }
    // WAL succeeded — now apply facts to shared FactStorage, in one
    // batch so no query sees part of the transaction (#554).
    let _ = self
    .inner
    .fact_storage
    .load_facts(stamped.iter().cloned())?;

    // Trigger auto-checkpoint AFTER facts are in FactStorage so the
    // checkpoint captures the newly written facts.
    Expand Down Expand Up @@ -1249,10 +1249,9 @@ impl<'a> WriteTransaction<'a> {
    &stamped,
    )?;

    // WAL succeeded — now apply facts to shared FactStorage.
    for fact in stamped {
    let _ = self.inner.fact_storage.load_fact(fact)?;
    }
    // WAL succeeded — now apply facts to shared FactStorage, in one
    // batch so no query sees part of the transaction (#554).
    let _ = self.inner.fact_storage.load_facts(stamped)?;

    // Trigger auto-checkpoint AFTER facts are in FactStorage so the
    // checkpoint captures the newly written facts.
    Expand Down
    Loading
    Loading
    0