From 2041427de8e259aa85d54cc993bac41619d3534f Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sun, 11 Oct 2026 22:42:16 +0530 Subject: [PATCH 1/4] test: reproducers for queries seeing partial or mixed database states Ignored: they fail on v3 and main today. A concurrent query sees part of a WriteTransaction commit, and two patterns of one query can read different states. The checkpoint double-read case did not reproduce. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012wXxZguv7PgKQjMMH4yJ5h --- tests/snapshot_atomicity_test.rs | 176 +++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 tests/snapshot_atomicity_test.rs diff --git a/tests/snapshot_atomicity_test.rs b/tests/snapshot_atomicity_test.rs new file mode 100644 index 000000000..6709dc248 --- /dev/null +++ b/tests/snapshot_atomicity_test.rs @@ -0,0 +1,176 @@ +//! Reproducers: a query must see each transaction whole, and each fact once, +//! while other threads commit and checkpoint (#473 spec §2 and §4.1). +//! +//! Each test runs a writer thread against a reader thread for a fixed time and +//! fails on the first answer that breaks the invariant. +#![cfg(not(target_arch = "wasm32"))] + +use minigraf::{Minigraf, OpenOptions, QueryResult}; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::thread; +use std::time::{Duration, Instant}; + +const RUN_FOR: Duration = Duration::from_secs(10); + +fn rows(db: &Minigraf, query: &str) -> Vec> { + match db.execute(query).unwrap() { + QueryResult::QueryResults { results, .. } => results + .iter() + .map(|r| { + r.iter() + .map(|v| match v { + minigraf::Value::Integer(n) => n.to_string(), + minigraf::Value::Keyword(k) => k.clone(), + minigraf::Value::String(s) => s.clone(), + _ => "other".to_string(), + }) + .collect() + }) + .collect(), + _ => panic!("expected query results"), + } +} + +/// Runs `write` in a loop on one thread and `check` in a loop on this one +/// until `RUN_FOR` passes. `check` returns an error message on a violation. +fn race( + db: Arc, + mut write: impl FnMut(&Minigraf, u64) + Send + 'static, + mut check: impl FnMut(&Minigraf) -> Option, +) -> Option { + let stop = Arc::new(AtomicBool::new(false)); + let writer = { + let (db, stop) = (Arc::clone(&db), Arc::clone(&stop)); + thread::spawn(move || { + let mut i = 0; + while !stop.load(Ordering::Relaxed) { + write(&db, i); + i += 1; + } + }) + }; + let start = Instant::now(); + let mut found = None; + while start.elapsed() < RUN_FOR { + if let Some(msg) = check(&db) { + found = Some(msg); + break; + } + } + stop.store(true, Ordering::Relaxed); + writer.join().unwrap(); + found +} + +/// `WriteTransaction::commit` appends its facts one `load_fact` call at a +/// time, so a query in between sees part of the transaction. +fn commit_is_seen_whole(db: Minigraf) { + const N: u64 = 500; + let found = race( + Arc::new(db), + |db, i| { + let mut tx = db.begin_write().unwrap(); + let facts: String = (0..N) + .map(|j| format!("[:b{i}x{j} :batch/id {i}]")) + .collect(); + tx.execute(&format!("(transact [{facts}])")).unwrap(); + tx.commit().unwrap(); + }, + |db| { + let n = rows(db, "(query [:find ?e :where [?e :batch/id ?b]])").len() as u64; + (n % N != 0).then(|| format!("saw {n} facts, not a multiple of {N}")) + }, + ); + if let Some(msg) = found { + panic!("partial transaction visible: {msg}"); + } +} + +#[test] +#[ignore = "reproducer: fails on v3 today; fixed by #473 sub-issue #543"] +fn write_transaction_commit_is_seen_whole_in_memory() { + commit_is_seen_whole(Minigraf::in_memory().unwrap()); +} + +#[test] +#[ignore = "reproducer: fails on v3 today; fixed by #473 sub-issue #543"] +fn write_transaction_commit_is_seen_whole_on_file() { + let dir = tempfile::tempdir().unwrap(); + let opts = OpenOptions::default().wal_checkpoint_threshold(usize::MAX); + commit_is_seen_whole(Minigraf::open_with_options(dir.path().join("w.graph"), opts).unwrap()); +} + +/// A checkpoint sets the new committed reader and clears the pending store +/// under two separate locks, so a query in between can read the checkpointed +/// facts twice. Every fact here has its own entity, so a repeated row is a +/// fact read twice. +#[test] +#[ignore = "reproducer: fails on v3 today; fixed by #473 sub-issue #543"] +fn checkpoint_never_shows_a_fact_twice() { + let dir = tempfile::tempdir().unwrap(); + let opts = OpenOptions::default().wal_checkpoint_threshold(usize::MAX); + let db = Minigraf::open_with_options(dir.path().join("c.graph"), opts).unwrap(); + let found = race( + Arc::new(db), + |db, i| { + db.execute(&format!("(transact [[:c{i} :ck/n {i}]])")).unwrap(); + db.checkpoint().unwrap(); + }, + |db| { + for q in [ + "(query [:find ?e ?n :where [?e :ck/n ?n]])", + "(query [:find ?e ?n :valid-at :any-valid-time :where [?e :ck/n ?n]])", + ] { + let mut r = rows(db, q); + let n = r.len(); + r.sort(); + r.dedup(); + if r.len() != n { + return Some(format!("{n} rows, {} distinct: {q}", r.len())); + } + } + None + }, + ); + if let Some(msg) = found { + panic!("fact seen twice during a checkpoint: {msg}"); + } +} + +/// A query fetches each attribute with its own `get_live_facts` call, each +/// under its own read lock, so a transaction that commits between the two +/// calls is seen by one pattern and not the other. Every transaction writes +/// `:pair/x` and `:pair/y` for one entity together. +fn patterns_see_one_state(db: Minigraf) { + let found = race( + Arc::new(db), + |db, i| { + db.execute(&format!("(transact [[:p{i} :pair/x {i}] [:p{i} :pair/y {i}]])")) + .unwrap(); + }, + |db| { + let r = rows( + db, + "(query [:find ?e :where [?e :pair/x ?x] (not [?e :pair/y ?x])])", + ); + (!r.is_empty()).then(|| format!("{} entities with x but no y", r.len())) + }, + ); + if let Some(msg) = found { + panic!("one query saw two database states: {msg}"); + } +} + +#[test] +#[ignore = "reproducer: fails on v3 today; fixed by #473 sub-issue #543"] +fn patterns_of_one_query_see_one_state_in_memory() { + patterns_see_one_state(Minigraf::in_memory().unwrap()); +} + +#[test] +#[ignore = "reproducer: fails on v3 today; fixed by #473 sub-issue #543"] +fn patterns_of_one_query_see_one_state_on_file() { + let dir = tempfile::tempdir().unwrap(); + patterns_see_one_state(Minigraf::open(dir.path().join("p.graph")).unwrap()); +} From 8247915122d53bf2f61570832f4590e6d6283fcc Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sun, 11 Oct 2026 22:42:27 +0530 Subject: [PATCH 2/4] test: rustfmt the snapshot reproducers Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012wXxZguv7PgKQjMMH4yJ5h --- tests/snapshot_atomicity_test.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/snapshot_atomicity_test.rs b/tests/snapshot_atomicity_test.rs index 6709dc248..7bd27940a 100644 --- a/tests/snapshot_atomicity_test.rs +++ b/tests/snapshot_atomicity_test.rs @@ -114,7 +114,8 @@ fn checkpoint_never_shows_a_fact_twice() { let found = race( Arc::new(db), |db, i| { - db.execute(&format!("(transact [[:c{i} :ck/n {i}]])")).unwrap(); + db.execute(&format!("(transact [[:c{i} :ck/n {i}]])")) + .unwrap(); db.checkpoint().unwrap(); }, |db| { @@ -146,8 +147,10 @@ fn patterns_see_one_state(db: Minigraf) { let found = race( Arc::new(db), |db, i| { - db.execute(&format!("(transact [[:p{i} :pair/x {i}] [:p{i} :pair/y {i}]])")) - .unwrap(); + db.execute(&format!( + "(transact [[:p{i} :pair/x {i}] [:p{i} :pair/y {i}]])" + )) + .unwrap(); }, |db| { let r = rows( From 7225af1ca83a4b07ffc115fa92531ae816c48793 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sun, 11 Oct 2026 22:55:29 +0530 Subject: [PATCH 3/4] fix(storage): add a committed transaction's facts under one lock (#554) A commit wrote its WAL entry, then added the facts to FactStorage one load_fact call at a time, each under its own write lock, so a query on another thread could see part of the transaction. execute's transact and retract path (since v0.6.0, 21b555a) did this as well as WriteTransaction::commit (since v0.5.0). FactStorage::load_facts appends a batch under one write lock. Commits, WAL replay, BrowserDb's commit and the v7 in-memory load use it. The two #554 reproducers are un-ignored, with execute variants added. Refs #554 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012wXxZguv7PgKQjMMH4yJ5h --- CHANGELOG.md | 3 +- CLAUDE.md | 2 +- README.md | 2 +- docs/TEST_COVERAGE.md | 7 +-- src/browser/mod.rs | 11 +++-- src/db.rs | 18 +++----- src/graph/storage.rs | 49 +++++++++++++++++++++ src/storage/persistent_facts.rs | 4 +- tests/snapshot_atomicity_test.rs | 74 ++++++++++++++++++++------------ 9 files changed, 117 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8b8f4ba7..18f1cfb60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Reopening next to a WAL whose entries were all already checkpointed no longer rewinds the transaction counter, so later transactions no longer reuse `tx_count` values and `:as-of` history stays correct (#447; also fixed in v2.0.3). - `checkpoint()` removes a WAL whose entries are all already in the file (#457). When it had no facts to flush, it returned early and left the WAL until a later write was checkpointed. That WAL is left by a crash between a checkpoint's commit and its WAL delete, or by a failed delete (`WAL-006`). Its contents were always skipped on replay, so no data was affected; the sidecar file just stayed. Also fixed on the v2.x line (`main`, #540), not yet released. - Closing a handle opened with `wal_checkpoint_threshold(usize::MAX)` no longer writes its pending facts to the `.graph` file (#476). The sentinel was documented to suppress the close-time checkpoint, and `Minigraf`'s close did skip it, but the storage layer still saved on drop, leaving a WAL whose entries were all checkpointed. The file now stays as the last explicit `checkpoint()` left it, and the WAL keeps every pending fact for the next open to replay, so bulk loaders and benchmarks see the pending WAL they ask for. No data is lost either way. Also fixed on the v2.x line (`main`, #540), not yet released. +- **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. ### Internal @@ -72,7 +73,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Known issues -- Same-transaction multi-valued facts can read back as one value (#371, #287), a fact retracted and re-asserted in one `WriteTransaction` is lost at commit (#477), a later assertion does not replace an earlier valid-time window (#435), and `save()` is not crash-atomic (#374); all are fixed in v3.0.0. All v2.x known issues, with affected versions and workarounds, are listed in the pinned issue #421. +- Same-transaction multi-valued facts can read back as one value (#371, #287), a fact retracted and re-asserted in one `WriteTransaction` is lost at commit (#477), a later assertion does not replace an earlier valid-time window (#435), and `save()` is not crash-atomic (#374); all are fixed in v3.0.0. The patterns of one query can read different database states while another thread commits (#555); not yet fixed. All v2.x known issues, with affected versions and workarounds, are listed in the pinned issue #421. ## v2.0.4 — 2026-10-08 diff --git a/CLAUDE.md b/CLAUDE.md index 9d83947a8..bd21ae65e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -201,7 +201,7 @@ Auto-migrates v7 → v8 on open (spec §9, with a backup meta page). v1–v6 are ## Test Coverage -**1432 tests** (1422 passing, 10 ignored; unit + integration + doc). +**1440 tests** (1427 passing, 13 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. diff --git a/README.md b/README.md index 2368a3ce0..9cdcc6750 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ let r2 = pq.execute(&[("tx", BindValue::TxCount(2)), ("entity", BindValue::Entit ```bash cargo run # interactive Datalog REPL -cargo test # run 1432 tests +cargo test # run 1440 tests cargo run < demos/demo_recursive.txt # recursive rules demo ``` diff --git a/docs/TEST_COVERAGE.md b/docs/TEST_COVERAGE.md index 73160518c..f03c20a88 100644 --- a/docs/TEST_COVERAGE.md +++ b/docs/TEST_COVERAGE.md @@ -4,7 +4,7 @@ **Verified**: 2026-10-11 with `cargo test --quiet` -**Result**: 1,422 passing tests and 10 ignored tests (1,432 total) +**Result**: 1,427 passing tests and 13 ignored tests (1,440 total) This covers the unreleased file-format-v8 work (#287, #371): the index-key value-collision fix, the v7→v8 migration, and the drop of formats v1–v6. It otherwise still covers the public database API, the Datalog engine, storage and recovery paths, v2.0.0's locking and structured-error-code changes, and v2.0.1's regression test for rebuilding indexes on open after several checkpoints (#370). Merged from the v2.x line: `tests/point_query_history_test.rs` and new storage unit tests check that attribute-narrowed bound-entity lookups return exactly what a full scan returns, and that `net_asserted_facts` matches its previous implementation on randomized histories (#323). Attribute-scan tests check that the AEVT range covers exactly the queried attribute, including non-ASCII names such as `:丿` and prefix siblings such as `:ab` for `:a` (#381). Incremental B+tree rebuild tests check that checkpoints which copy untouched index leaves produce exactly the entries and file checksum a full rebuild would, including over repeated checkpoints, that single-fact checkpoints do not fragment the index, and that a damaged slot directory is rejected rather than copied (#315). B+tree cursor tests check that every scan walks the tree, not the leaf `next_leaf` chain: full scans and start positions match the input, `seek` from the current position lands where a fresh descent from the root would (randomized, plus within a leaf, to a sibling, across subtrees, past the end, into the gap after a leaf, and backwards as a no-op), a seek to the sibling leaf reads exactly one page, reads are unchanged with every `next_leaf` zeroed, a child pointer into a non-B+tree page is STG-013, and a cycle in child pointers is an error, not a hang (v8 storage format PR 1, #374). Directory-sync unit tests check that creating the `.graph` file or the WAL, and deleting the WAL, fsync the parent directory after the file operation, and that reopening an existing file does not (#389). Disjunction tests check that `or-join` returns an empty result, not INT-031, when no rows reach it: on an empty database, after a pattern that matches nothing, after a `not` that removes every row, and in a rule body (#405). A stratification unit test builds the same two-rule negative cycle 64 times and checks that the INT-054 message names the same predicate pair every time (#410). Magic-sets tests check that a recursive rule with a literal start that recurses through another rule returns the same results as without the literal (#297). API tests check that `execute()` and `WriteTransaction::execute()` reject `$slot` bind tokens in every position with API-010 (#407). The REPL `run()` test now runs in a child process with null stdin, so it no longer hangs when the test runner's stdin is an open non-TTY stream (#415). v8 page-format tests (PR 2, #374, #388) check the 24-byte page header (a flipped bit, a wrong page id and a future generation each give their own STG code, legacy page types fail before the CRC, and a failed page is never cached), meta-page encoding, the free-list chain, WAL v2's base generation, and every row of the meta-selection table: both slots valid, a torn newest slot with the WAL's base generation (opens at the older one), a WAL newer than the valid meta, pages of the next generation on the free list or past `page_count` with no WAL (all STG-033, with the file unmodified), an unknown feature bit (STG-034), and a pre-release v8 single header (STG-032). Fault-injection tests crash `save()` at every page write (cleanly and with torn pages, including the meta write) and every sync, and crash a v7 migration at every write; each reopen finds exactly the old or the new checkpoint, and every reachable page plus the free list covers the file exactly. Twenty-five consecutive saves never write a page the previous meta references. Covering-key tests (PR 3, #433) check that byte order equals logical order for every key component and value type on random tuples (negative, zero and large integers, NaN and ±0.0, strings with 0x00, 64- and 65-byte strings, FOREVER, newest-transaction-first) and that every component round-trips; that prefix-compressed leaves and shortest-separator internal nodes round-trip, agree with a linear scan, keep `max(left) < s ≤ min(right)`, and reject an overflow; that the byte-key B+tree's builds, incremental rebuilds, cursor and `seek` match their expected sets; that dictionary ids are assigned once, never reused, shared between a keyword attribute and value, and deterministic; that a re-asserted or retracted long value writes no new value page and a forced hash collision still finds the right value; that one transaction with two timestamps is STG-037 and a missing dictionary entry STG-036; that committed reads by entity, entity and attribute, attribute, and in full equal an in-memory model across eight checkpoints and a reopen; that short-valued reads touch no value page; that a v7 file with every value type, long strings, refs and retractions migrates to exactly the same facts; that a pre-covering-key v8 file is STG-032; and that string values over 4,068 bytes and attributes or keywords over 1,024 bytes are WAL-003. A WAL test checks that reopening next to a fully checkpointed WAL does not reuse transaction numbers (#447). Copy-on-write checkpoint tests (PR 4, #434) check that lazy free-list pops and pushes conserve every page across page boundaries, partial head pages and chain exhaustion, never hand out a page freed in the same generation, and share unread chain pages unchanged; that a copy-on-write insert equals a sorted merge for random batches (including keys below the first and above the last, many into one leaf, root growth to depth 3, and 1 KB separators that split internal nodes), leaves the old tree readable and unchanged, frees exactly the replaced nodes, writes only new pages, writes exactly one leaf and two internal nodes for one entry in a depth-3 tree, and does not fragment leaves under single inserts; that a checkpoint after 1 or 100 right-edge facts writes at most the tree-depth difference more pages at 100,000 facts than at 10,000, and at most one path per new entry per tree for random keys; and that four reader threads querying throughout 40 checkpoints never fail and never see the fact count go backwards. Hardening tests (PR 5) damage an index leaf, an internal node, a dictionary leaf, a value page and a free-list page in turn. Every committed read or checkpoint that reaches the damaged page fails with STG-029, never with wrong or fewer rows. A query through `Minigraf` on a file with damaged leaves fails with STG-029, and the file still opens at its last checkpoint. A size test measures at most 200 bytes per fact (142 measured) on 144k facts in #433's shape; its 1.15M-fact variant is ignored locally and runs in the nightly smoke workflow. A shared dictionary-cache test checks that cached ident and timestamp lookups return the same facts and that the cache stays bounded. Merged from the v2.x line: storage and `edge_cases_test.rs` tests check that more than 65,535 uncheckpointed facts each resolve to their own fact through entity lookups and attribute scans, in memory and file-backed (#445). Merged from the v2.x line: `tests/golden_corpus_test.rs` opens a copy of each committed golden file in `tests/golden/` (v7 files written by minigraf 2.0.3: one checkpoint, several checkpoints, indexes rebuilt on open, a crash-left WAL, an already-checkpointed WAL, same-transaction multi-values) and checks its manifest's CRC32, `tx_count` and query results after open, after a checkpoint and reopen, and after one more write; a second test checks that every golden file has a manifest (#391). On `v3` the same test checks that each v7 file has a v8 meta page after its first open and gives the same results after reopening the migrated file, and adds seven v8 golden files (one checkpoint, seven copy-on-write checkpoints, a crash-left v2 WAL, an already-checkpointed v2 WAL, same-transaction multi-values, a v7 file migrated in place, and long values at the size limits); the #371 queries, skipped on v2.x, run and pass. `tests/model_based_test.rs` runs random sequences of transacts, retracts, write transactions, checkpoints, reopens and simulated crashes against a file and, after every step, compares the transaction counter, `verify`, full rows with windows and tx counts, and six query shapes under `:as-of N` and `:valid-at T` with a reference model (#385). `tests/property_test.rs` writes random histories (batched transacts, retracts, write transactions, checkpoints) in memory and to a file, and compares random queries (one to three joined patterns, `not` or `not-join`, a comparison predicate, `:as-of`, `:valid-at` or `:any-valid-time`) with a reference evaluator, on the open file and after a reopen; a query error fails the test (#386). `tests/crash_kill_test.rs` kills a child process running a seeded workload (multi-valued batches, batched retracts, five checkpoint modes) at a random point and checks that the reopened file holds exactly the transactions the child logged as committed, or those plus the one in flight, through full-scan, AEVT, AVET, EAVT and `:as-of` queries and `verify`, again after a second reopen and after a checkpoint plus reopen (#384). Integrity tests (#373) check that intact files verify with no problems and an exact fact and page count, and that `verify` reports each kind of damage with its code: an EAVT, AEVT, AVET or VAET leaf missing an entry (STG-038), keys out of order and a page reached from two trees (STG-039), a rotted index leaf (STG-029), a free page still in use and a leaked page (STG-035), a missing transaction entry (STG-036), and an entity id whose UUID maps to another id (STG-040). After each index or free-list damage, `rebuild_indexes` leaves a file that verifies clean, reads back every fact and accounts for every page. The source is an index that agrees with another one, or the only intact one; three intact indexes that all disagree, or a damaged dictionary, give STG-041 with the file unchanged. A rebuild commits pending facts, including new ids and long values. A crash at every write (clean and torn) and every sync of a rebuild reopens either at the old generation with only its old damage or at the new one with none, never losing a fact. Through `Minigraf` (`tests/integrity_test.rs`), a rebuild commits WAL entries and deletes the WAL, and flipping a byte in each leaf of a file in turn is always reported, then either repaired (index leaves) or refused with STG-041 and the file unchanged (dictionary leaves). Page-count tests (#497) check that a CRC-valid meta whose `page_count` is past the end of the file (one page past, 2^50, `u64::MAX`; with the other slot valid or damaged; with or without a WAL) and a file cut short by one page, three pages or down to three pages are `STG-044` at open, with the file unchanged and not grown, while a `page_count` exactly at the end opens; that a checkpoint whose pages did not all reach the backend fails with `INT-049` before writing its meta, and the file reopens at the previous generation; and that `verify` at a meta counting pages past the end reports `STG-044` without scanning to that count. Through `Minigraf` (`tests/integrity_test.rs`), a file truncated by one or three pages and a file whose meta was rewritten with `page_count` 2^50 and a valid CRC are `STG-044` read-write and read-only, with no WAL created and the file length unchanged. The `file_header` fuzz input from #497 is a corpus seed. A sub-page test (#506, `tests/integrity_test.rs`) checks that a file of 1 to 4,095 bytes is `STG-045` read-write and read-only, 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 writes when it is a prefix of the first meta page or all zeros; a file-backend unit test checks that `partial_page` returns the bytes past the last whole page. The `corrupt_file` fuzzer no longer skips files under one page: they must fail to open or open empty, and the #506 input is a corpus seed. Damaged v7 fact page tests (#496) check that a page in the counted fact range whose type byte is anything but the fact-page type (zero, `0x01`, the v2.x B+tree types, `0xFF`, a zeroed page; first and last page) is `STG-014` rather than skipped, while a range derived from the first index root skips only v2.x B+tree pages; that opening such a v7 file read-write or read-only fails with `STG-014` and writes nothing; and, in `tests/migration_matrix_test.rs`, that the golden `v7_basic.graph` with page 1's type byte changed or page 1 overwritten with noise fails both opens with the file unchanged and no WAL created. Both inputs are `fact_page` corpus seeds. Merged from the v2.x line: `tests/edge_cases_test.rs` checks the same refusal and acceptance through `Minigraf::open` on a database checkpointed twice (#506). Torn-WAL-tail tests (#513) check that opening a WAL writer cuts off a partial last entry (cut in the checksum, the header or the payload) or an entry with a bad checksum and everything after it, then appends where replay reaches, and leaves an intact WAL byte for byte; that an append that fails part-way makes every later append `WAL-007` while a rejected oversize fact (`WAL-003`, nothing written) does not; through `Minigraf`, that the failed transaction is not applied, later `transact` and `commit` are `WAL-007`, and a `checkpoint()` makes writes work again; and, in `tests/wal_test.rs`, that a commit made after reopening a crash-left WAL with each kind of torn tail survives a second crash. The fault matrix (#390, `src/fault_matrix.rs`) drives a test-only fault plan through the real file backend, WAL and directory fsync: an EIO, a short or torn write (1, 40 and 4,095 bytes), a full disk that keeps failing every later write, a failed fsync, and a failed fsync that also loses the writes since the last good one ("fsyncgate"), each at every write or sync call of a transact, a transact that creates the WAL, an automatic checkpoint, `checkpoint()`, the close-time checkpoint and `rebuild_indexes()`, on a database committed over two checkpoints with multi-valued, retracted and long values and entries pending in the WAL. After each fault, the handle holds the old facts or those plus the whole transaction, never part of one; after a failed checkpoint another is `STG-046`; a later write succeeds or is `WAL-007`/`STG-046`; a kill and reopen gives exactly the handle's facts, plus the failed transaction whole or not at all, and `verify` finds nothing; and a commit after recovery survives another kill. Two more tests fill the disk halfway through a checkpoint (`STG-046`, reads unchanged, reopen intact), and fail the fsync after a checkpoint's meta write and then retry the checkpoint, crashing at each of its writes: the retry is refused and every reopen is intact (without the refusal, a reopen hits `STG-029`). Unit tests check the fault plan's counting, and that a `rebuild_indexes` refused with `STG-041` leaves the handle usable. Live-read tests (#379) check that `triple_len` ends exactly where `tx↓` begins for every value type and index; that on random histories (several valid-time windows, retractions, re-assertions, several values per attribute, long values, 12 transactions per checkpoint) the committed live read for every scan kind and `:as-of` point is exactly the net-asserted set, that it stays equivalent with pending facts on top, and after reopen; and that a point read on a triple asserted and retracted 2,000 times reads under a third of the pages a history read does, with or without `:as-of`. `tests/live_reads_test.rs` runs the same history in a checkpointed file database and in memory and checks that point, attribute, full-scan, join, valid-time, `:as-of` and rule queries return the same rows, before and after reopen. One-window tests (#435) check that `net_asserted_facts` keeps each triple's newest transaction only (a later window replaces an earlier one, closing an open window, a retraction in the newest transaction hides it, records with two windows in one transaction keep both, once each) against a per-triple model on random histories; that the on-disk key walk agrees at every `:as-of` before and after reopen; and that `API-011` rejects two windows of one fact in one transaction without taking a `tx_count`. `tests/valid_time_window_test.rs` checks replacing, closing, extending, reopening and retract-then-reassert through entity-bound and attribute queries in memory, on a checkpointed file and after reopen, `API-011` from `transact` and from `WriteTransaction::execute`, and a later statement's window committed when two statements of a `WriteTransaction` give one fact different windows. Window-validation tests (#436) check that `check_valid_windows` rejects an empty window, an inverted one, and a `:valid-to` at or before the transaction time when `:valid-from` is left out, but not retractions; that `execute` rejects each such `transact` (both bounds, equal bounds, `:valid-to` only, a per-fact bound inverting the transaction's window) with `API-019` without writing or taking a `tx_count`, in memory, checkpointed and after reopen; that `tx.execute()` rejects them and the transaction still commits its other statements; that `commit()` rejects a default `valid_from` that has passed the `:valid-to` by commit time; and that `FactStorage::transact_batch` takes no `tx_count` on rejection. Statement-order tests (#477, `tests/statement_order_test.rs`) run every mix of two statements on one fact inside a `WriteTransaction` (retract then assert, assert then retract, two retracts, two windows, after a committed assertion and on an empty database) and check that reads inside the transaction and the committed rows agree, in memory, checkpointed and after reopen; that other facts of an overridden statement survive; that the commit is one transaction without the overridden records; and that two windows in one statement are `API-011` from `tx.execute()` while the rest commits. A unit test checks the commit collapse keeps each fact's newest statement in order. A log-writer test checks that an inverted window is `API-019`, a second window of a fact in one transaction `API-011`, and an assertion and a retraction of one fact in one transaction `API-020` in either order, each leaving the writer usable and the output exact, while duplicate retractions, identical repeats and the same fact in the next transaction are accepted. Cursor tests (#432, `tests/cursor_test.rs`) check that `query()` batches partition the `execute()` answer for several batch sizes (never empty, never over `max_rows`, `max_rows == 0` still progresses, `None` stays `None`), that the iterator and aggregates, rules, `:as-of` and `:any-valid-time` match `execute()`, that a cursor opened before a `transact`, a `WriteTransaction` commit, a retract or a checkpoint returns the pre-write answer to the end (in memory and file-backed), that `transact`/`retract`/`rule` are `API-012` and bind slots `API-010`, that a `WriteTransaction` thread gets `INT-001`, that rule limits give `INT-020` as from `execute()`, that `PreparedQuery::query` matches `PreparedQuery::execute`, and that a cursor is `Send` and outlives its database handle. Fact-log tests (#430, `tests/fact_log_test.rs`) check that `fact_log()` returns exactly the records written (assertions and retractions, with `tx_count` and `tx_id`) in memory, with everything pending, half checkpointed, fully checkpointed and after reopen, in both orders, with windows of 1 to 1,000 records and batch sizes of 1 to 1,000, in non-decreasing `tx_count` in Tx order; that every layout gives identical records, including valid-time windows and long string values; that a transaction larger than the window is split with no loss or duplicate; that attribute, attribute-prefix, entity and tx-range filters and their combinations match a model, and unknown names match nothing; that writes after open are not in the log; that `checkpoint()` and `rebuild_indexes()` are `API-013` while a log is open (a checkpoint with nothing to write is allowed), that automatic checkpoints from `execute()` and `WriteTransaction::commit` wait for the log, and that closing, dropping or finishing the log releases the database; that a `WriteTransaction` thread gets `INT-001`; and that a log is `Send`, outlives its database handle, and the deferred writes replay on the next open. Read-only open tests (#429, `tests/read_only_open_test.rs`) check that a read-only handle on a file with checkpointed facts and a pending WAL returns the same query results, `tx_count` and fact log as a read-write open; that queries, a cursor, a fact log, `verify`, `prepare`, rules and a UDF leave the `.graph` and WAL bytes and the `.graph` mtime unchanged, and no WAL is created; that `transact`, `retract`, `begin_write`, `checkpoint` and `rebuild_indexes` are `API-014` without taking a `tx_count`, also on a read-only in-memory database and through the builder path; that a missing file is `STG-042` and nothing is created; that an empty file is an empty database and stays 0 bytes; that read-only handles share a file while a read-write one is refused with `STG-025`, both ways; that a file without write permission opens read-only; and that small and whole-file page caches work. File-backend unit tests check that a shared-mode backend refuses page writes and syncs with `INT-056`, that the in-process registry counts readers and admits readers or one writer (also without kernel locks), and that a reader in another process shares the file with a reader here and refuses a writer here with `STG-026`. Storage unit tests check that a read-only open of a v7 file at every point of an interrupted migration (clean and torn writes) sees every fact and writes nothing, and that an empty backend stays empty. `tests/golden_corpus_test.rs` opens a copy of every golden file read-only first and checks its `tx_count` and queries, and that neither the file nor its WAL changes. Log-writer tests (#431, `tests/log_writer_test.rs`) copy a source's fact log (assertions, retractions, an explicit valid-time window, long strings, refs, keywords, checkpointed and pending facts) through `LogWriter` and check that the new file has no `.partial` or WAL, verifies clean, and gives the source's `tx_count`, fact log and query results (current, `:valid-at`, `:any-valid-time`, every `:as-of`, and entity-, attribute- and value-bound lookups) read-write, read-only and after reopen (#370); that it takes new writes at the source's next `tx_count`; that 90,000 facts over two batch commits verify clean and count correctly at `:as-of`; that a purged middle transaction is a hole whose `:as-of` equals the one before, and trailing holes keep the counter through `advance_tx_count`; that an empty log is an empty database with the counter; that `tx_count` 0, a lower or closed transaction and a backwards advance are `API-015`, a second `tx_id` `API-016`, a second window `API-011`, a retraction of an asserted fact in one transaction `API-020`, an empty window `API-019` and oversize values or idents `WAL-003`, each leaving the writer usable and the output exact, with repeats written once; that an existing target or WAL is `STG-043`, a second writer `STG-025` and `read_only` `API-014`; that a stale `.partial` is reused; that dropping an unfinished writer leaves nothing; that `finish` refuses a target created during the build and leaves it untouched; and that `LogWriter` is `Send`. Unit tests check that a batch commits only when a transaction opens past the batch size, never splitting one, and that `FileBackend::truncate` empties a file. A unit test in `error.rs` checks that `MinigrafError::invalid_argument` and `MinigrafError::closed`, the constructors for language bindings, give API-017 and API-018 with their text. Query-validation tests (#406, #414) check that a plain `:find` variable or a window function's argument, `:partition-by` or `:order-by` variable that no `:where` clause binds (including `?_` wildcards) is `PRS-080` from both `execute()` and `prepare()`, naming the variable, while variables bound by a pattern, an attribute position, a rule call, an expression binding, an `or-join`, every branch of an `or`, a pseudo-attribute or a window spec are accepted, and bind slots are not mistaken for `:find` variables; and that an unknown aggregate is `QRY-010` and an unknown window function `QRY-011` on an empty database, when no row matches, when rows match, through a rule query and through a prepared query, while a registered UDF in either position returns an empty result on an empty database. Coverage-gate tests (#396) check that replay stops at a WAL entry whose first fact is longer than 10 MB or that claims more than 1M facts, that a WAL writer cannot be opened in a missing directory, and that a failed cut of a torn WAL tail fails the open and leaves the file as it was; that a short v7 page and a slot past the record count are `INT-024`; that a migration backup meta of another generation or file length is not used (`STG-032`, file unchanged), and a torn first meta with data in page 1 is `STG-032`, not a new file; that `save` with nothing pending writes nothing, `into_backend` saves pending facts, and `rebuild_indexes` with nothing checkpointed commits the pending facts; and that unsorted or repeated B+tree input and a stored key inserted with another value are `INT-049`, a tree pointer to a free-list page is `STG-013` from `get`, a leaf walk and a copy-on-write insert, and the read-only `MutexStorageBackend` refuses writes, syncs and close. A page-cache test holds two threads in the backend read so both miss the same page, and checks that both get the one cached copy; before it, that path ran only when the scheduler happened to race the threads, and the gate flapped. Three `BrowserDb` tests (wasm32, run with `wasm-bindgen-test` in a browser, not counted above) check that `query()` batches of 1, 7 and 1,000 give the rows of `execute()`, that a cursor keeps the answer from when it opened and returns the end after `close()`, and that a `transact` is API-012. The wasm32 targets run in CI (`.github/workflows/wasm.yml`). In a browser (`wasm-pack test --headless`, Firefox and Chrome): 32 `wasm_bindgen_test` tests: nineteen for `BrowserDb` (including IndexedDB persistence, the cursor, and the flush returning every dirty page and failing on one it cannot read, from #470), eleven for `BrowserBufferBackend` (which used `#[test]` and never ran before), and two checking that IndexedDB page keys accept only whole numbers from 0 to 2^53. Five of the `BrowserDb` tests (#411, #440) check that opening a v7 store written straight into IndexedDB stores a v8 meta page and only the live pages, with nothing written on the next open; that after each of 60 transacts memory, and at the end IndexedDB, hold exactly the pages not on the free list while the file grows by under one page per transact; that a commit applied in memory but never flushed reopens at the previous one and keeps working; that an export after churn is as long as the meta says and imports back with its free pages dropped; and that `importGraph()` deletes the replaced database's IndexedDB keys. Two buffer tests check that releasing a page drops its bytes and any unflushed write, and that export writes a released page as zeros. A native storage test checks that a file whose free pages are all zeros (as the browser exports them), both just migrated from v7 and after 20 saves, opens, verifies clean and keeps committing, and the free-list test checks that a checkpoint reports exactly the ids it newly lists. On `wasm32-wasip1` under Wasmtime: the unit and integration tests that do not use a file run, 956 passing; ten that spawn threads or a child process are ignored on WASI. These are not counted above. A WAL test checks that a checkpoint with no facts to flush still removes a WAL whose entries are all already in the file, and the golden-corpus test now requires the WAL to be gone after a checkpoint for the `stale_wal` fixtures too (#457). A WAL test checks that closing a handle opened with `wal_checkpoint_threshold(usize::MAX)`, in both sync modes, leaves the `.graph` file byte for byte as it was and keeps the WAL, whose entries replay on the next open, and a unit test closes an in-memory handle under the same setting (#476). A recursive-rule test checks that a rule query on a database with more live facts than `max_results` returns its answer, with a bound and with a free first argument, and that a query deriving more facts than its `:max-results` is still `INT-020` (#528). A log-writer test runs the online backup from `docs/DURABILITY.md`: it copies a read-write handle with an uncheckpointed transaction through `fact_log` and `LogWriter` while another thread writes past the automatic checkpoint threshold, and checks that the copy verifies clean, holds exactly the records the log returned and the counter read after the log opened, and that every concurrent write reaches the source's file once the log is dropped (#398). @@ -13,18 +13,19 @@ This covers the unreleased file-format-v8 work (#287, #371): the index-key value - **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. - **Multi-value and index-key correctness** (`tests/multi_value_test.rs`, 10 tests): same-transaction multi-valued facts surviving query-time dedup, retracting several values of one attribute in one call, valid-time stints (facts differing only in valid time), maximum-value-size boundaries, and format v8 index keys (value bytes + assert/retract flag) (#287, #371). - **Storage correctness**: packed pages, B+tree indexes and range scans, cache behavior, file-header validation, migration (including v7→v8 index rebuild and v1–v6 rejection via `STG-028`), 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, SIGKILL recovery, and queries seeing each concurrent transaction whole (`tests/snapshot_atomicity_test.rs`, #554). - **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. ### Ignored Tests -The ten ignored tests are intentionally excluded from a normal local run: +The thirteen ignored tests are intentionally excluded from a normal local run: - Six rustdoc examples that reference internal types and cannot compile as standalone examples. - One high-contention concurrency stress test intended for scheduled runs. - One long-haul smoke test intended for scheduled runs. - One soak test at 10M facts (`soak`), run weekly by `.github/workflows/soak.yml` (#392); `soak_short` runs the same driver at 20K facts in the normal suite. +- Three snapshot reproducers in `tests/snapshot_atomicity_test.rs`: two for #555 (patterns of one query read different states under concurrent writes), which fail today, and one checking that a checkpoint never shows a fact twice, which has not failed but guards the checkpoint change planned in #543. - One size test at 1.15M facts (`size_per_fact_in_433_shape_1m`), run by the nightly smoke workflow. ### Reproducing diff --git a/src/browser/mod.rs b/src/browser/mod.rs index 6353c3cd1..78047dd8d 100644 --- a/src/browser/mod.rs +++ b/src/browser/mod.rs @@ -343,12 +343,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) + .map_err(|e| JsValue::from_str(&e.to_string()))?; inner.pfs.mark_dirty(); inner diff --git a/src/db.rs b/src/db.rs index cf05bacfe..e595fd0b4 100644 --- a/src/db.rs +++ b/src/db.rs @@ -630,9 +630,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; @@ -756,10 +754,9 @@ 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)?; // Trigger auto-checkpoint AFTER facts are in FactStorage so the // checkpoint captures the newly written facts. An open fact log @@ -1591,10 +1588,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. An open fact log diff --git a/src/graph/storage.rs b/src/graph/storage.rs index 5c9b21af1..dfdf34543 100644 --- a/src/graph/storage.rs +++ b/src/graph/storage.rs @@ -309,6 +309,29 @@ impl FactStorage { Ok(true) } + /// Insert facts with their tx_id and tx_count preserved, all under one + /// write lock, so a query sees either none of them or all of them (#554). + /// + /// Commits and WAL replay use this for a whole transaction. Duplicates are + /// skipped as in [`Self::load_fact`]. Returns how many facts were loaded. + pub(crate) fn load_facts(&self, facts: impl IntoIterator) -> Result { + let mut d = self + .data + .write() + .map_err(|_| err_coded!(ErrorCode::Int050, "data"))?; + let mut loaded = 0usize; + for fact in facts { + if !d.pending_keys.insert(pending_key(&fact)) { + continue; + } + let pos = d.facts.len(); + d.pending_indexes.insert(&fact, pos); + d.facts.push(fact); + loaded = loaded.saturating_add(1); + } + Ok(loaded) + } + /// Raise tx_counter to max(tx_count) across all loaded facts. /// /// Must be called after all `load_fact()` calls complete so that the next @@ -1641,6 +1664,32 @@ mod tests { assert_eq!(storage.fact_count(), 2); } + #[test] + fn load_facts_appends_a_batch_and_skips_duplicates() { + let storage = FactStorage::new(); + let a = Fact::new( + uuid::Uuid::new_v4(), + ":t/a".to_string(), + Value::Integer(1), + 1, + ); + let b = Fact::new( + uuid::Uuid::new_v4(), + ":t/a".to_string(), + Value::Integer(2), + 1, + ); + assert_eq!( + storage + .load_facts(vec![a.clone(), b.clone(), a.clone()]) + .unwrap(), + 2 + ); + assert_eq!(storage.fact_count(), 2); + assert_eq!(storage.load_facts(vec![b]).unwrap(), 0); + assert_eq!(storage.fact_count(), 2); + } + #[test] fn test_load_fact_duplicate_detection_includes_asserted() { let storage = FactStorage::new(); diff --git a/src/storage/persistent_facts.rs b/src/storage/persistent_facts.rs index a995a74d6..d6f157c5c 100644 --- a/src/storage/persistent_facts.rs +++ b/src/storage/persistent_facts.rs @@ -163,9 +163,7 @@ impl PersistentFactStorage { let max_tx = max_fact_tx.max(header.last_checkpointed_tx_count); // Pending facts are kept in tx order (the fact log relies on it). facts.sort_by_key(|f| f.tx_count); - for fact in facts { - let _ = self.storage.load_fact(fact)?; - } + let _ = self.storage.load_facts(facts)?; Ok(MetaPage { last_checkpointed_tx_count: max_tx, ..MetaPage::empty(1) diff --git a/tests/snapshot_atomicity_test.rs b/tests/snapshot_atomicity_test.rs index 7bd27940a..808a03b30 100644 --- a/tests/snapshot_atomicity_test.rs +++ b/tests/snapshot_atomicity_test.rs @@ -11,7 +11,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::thread; use std::time::{Duration, Instant}; -const RUN_FOR: Duration = Duration::from_secs(10); +const RUN_FOR: Duration = Duration::from_secs(3); fn rows(db: &Minigraf, query: &str) -> Vec> { match db.execute(query).unwrap() { @@ -63,42 +63,62 @@ fn race( found } -/// `WriteTransaction::commit` appends its facts one `load_fact` call at a -/// time, so a query in between sees part of the transaction. -fn commit_is_seen_whole(db: Minigraf) { - const N: u64 = 500; - let found = race( - Arc::new(db), - |db, i| { - let mut tx = db.begin_write().unwrap(); - let facts: String = (0..N) - .map(|j| format!("[:b{i}x{j} :batch/id {i}]")) - .collect(); - tx.execute(&format!("(transact [{facts}])")).unwrap(); - tx.commit().unwrap(); - }, - |db| { - let n = rows(db, "(query [:find ?e :where [?e :batch/id ?b]])").len() as u64; - (n % N != 0).then(|| format!("saw {n} facts, not a multiple of {N}")) - }, - ); +const N: u64 = 500; + +fn batch(i: u64) -> String { + let facts: String = (0..N) + .map(|j| format!("[:b{i}x{j} :batch/id {i}]")) + .collect(); + format!("(transact [{facts}])") +} + +/// Every write commits `N` facts in one transaction; a query must never see +/// a count that is not a multiple of `N` (#554). +fn commit_is_seen_whole(db: Minigraf, write: impl FnMut(&Minigraf, u64) + Send + 'static) { + let found = race(Arc::new(db), write, |db| { + let n = rows(db, "(query [:find ?e :where [?e :batch/id ?b]])").len() as u64; + (n % N != 0).then(|| format!("saw {n} facts, not a multiple of {N}")) + }); if let Some(msg) = found { panic!("partial transaction visible: {msg}"); } } +fn write_transaction(db: &Minigraf, i: u64) { + let mut tx = db.begin_write().unwrap(); + tx.execute(&batch(i)).unwrap(); + tx.commit().unwrap(); +} + +fn execute_transact(db: &Minigraf, i: u64) { + db.execute(&batch(i)).unwrap(); +} + +fn file_db(dir: &tempfile::TempDir) -> Minigraf { + let opts = OpenOptions::default().wal_checkpoint_threshold(usize::MAX); + Minigraf::open_with_options(dir.path().join("w.graph"), opts).unwrap() +} + #[test] -#[ignore = "reproducer: fails on v3 today; fixed by #473 sub-issue #543"] fn write_transaction_commit_is_seen_whole_in_memory() { - commit_is_seen_whole(Minigraf::in_memory().unwrap()); + commit_is_seen_whole(Minigraf::in_memory().unwrap(), write_transaction); } #[test] -#[ignore = "reproducer: fails on v3 today; fixed by #473 sub-issue #543"] fn write_transaction_commit_is_seen_whole_on_file() { let dir = tempfile::tempdir().unwrap(); - let opts = OpenOptions::default().wal_checkpoint_threshold(usize::MAX); - commit_is_seen_whole(Minigraf::open_with_options(dir.path().join("w.graph"), opts).unwrap()); + commit_is_seen_whole(file_db(&dir), write_transaction); +} + +#[test] +fn execute_transact_is_seen_whole_in_memory() { + commit_is_seen_whole(Minigraf::in_memory().unwrap(), execute_transact); +} + +#[test] +fn execute_transact_is_seen_whole_on_file() { + let dir = tempfile::tempdir().unwrap(); + commit_is_seen_whole(file_db(&dir), execute_transact); } /// A checkpoint sets the new committed reader and clears the pending store @@ -166,13 +186,13 @@ fn patterns_see_one_state(db: Minigraf) { } #[test] -#[ignore = "reproducer: fails on v3 today; fixed by #473 sub-issue #543"] +#[ignore = "reproducer for #555: fails on v3 today"] fn patterns_of_one_query_see_one_state_in_memory() { patterns_see_one_state(Minigraf::in_memory().unwrap()); } #[test] -#[ignore = "reproducer: fails on v3 today; fixed by #473 sub-issue #543"] +#[ignore = "reproducer for #555: fails on v3 today"] fn patterns_of_one_query_see_one_state_on_file() { let dir = tempfile::tempdir().unwrap(); patterns_see_one_state(Minigraf::open(dir.path().join("p.graph")).unwrap()); From 38c005441e3e6f7351a45b918ff04209a97e8418 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sun, 11 Oct 2026 23:17:27 +0530 Subject: [PATCH 4/4] fix(query): read all of a query's lookups under one lock (#555) selective_fact_fetch called FactStorage::get_live_facts once per entity or attribute lookup, each under its own read lock, so a transaction that committed between two lookups was seen by some patterns of the query and not others. FactStorage::get_live_facts_of_scans reads every scan under one read guard, and selective_fact_fetch uses it. The #555 reproducer was passing only because of #554: execute added :pair/x before :pair/y, and scans run in sorted attribute order, so the query with :pair/x positive could only catch a partial commit. It now also runs the query with :pair/y positive, which fails without this fix (every one of 5 runs) and passes with it. Both are un-ignored. Refs #555 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012wXxZguv7PgKQjMMH4yJ5h --- CHANGELOG.md | 3 +- CLAUDE.md | 2 +- README.md | 2 +- docs/TEST_COVERAGE.md | 8 +- src/graph/storage.rs | 139 +++++++++++++++++++++---------- src/query/datalog/executor.rs | 23 ++--- tests/snapshot_atomicity_test.rs | 27 +++--- 7 files changed, 127 insertions(+), 77 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18f1cfb60..17e9c5918 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `checkpoint()` removes a WAL whose entries are all already in the file (#457). When it had no facts to flush, it returned early and left the WAL until a later write was checkpointed. That WAL is left by a crash between a checkpoint's commit and its WAL delete, or by a failed delete (`WAL-006`). Its contents were always skipped on replay, so no data was affected; the sidecar file just stayed. Also fixed on the v2.x line (`main`, #540), not yet released. - Closing a handle opened with `wal_checkpoint_threshold(usize::MAX)` no longer writes its pending facts to the `.graph` file (#476). The sentinel was documented to suppress the close-time checkpoint, and `Minigraf`'s close did skip it, but the storage layer still saved on drop, leaving a WAL whose entries were all checkpointed. The file now stays as the last explicit `checkpoint()` left it, and the WAL keeps every pending fact for the next open to replay, so bulk loaders and benchmarks see the pending WAL they ask for. No data is lost either way. Also fixed on the v2.x line (`main`, #540), not yet released. - **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. +- **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 (an entity with one attribute but not another that was always written with it). The lookups now run under one lock. Stored data was never affected. Affected every release since v1.1.0. ### Internal @@ -73,7 +74,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Known issues -- Same-transaction multi-valued facts can read back as one value (#371, #287), a fact retracted and re-asserted in one `WriteTransaction` is lost at commit (#477), a later assertion does not replace an earlier valid-time window (#435), and `save()` is not crash-atomic (#374); all are fixed in v3.0.0. The patterns of one query can read different database states while another thread commits (#555); not yet fixed. All v2.x known issues, with affected versions and workarounds, are listed in the pinned issue #421. +- Same-transaction multi-valued facts can read back as one value (#371, #287), a fact retracted and re-asserted in one `WriteTransaction` is lost at commit (#477), a later assertion does not replace an earlier valid-time window (#435), and `save()` is not crash-atomic (#374); all are fixed in v3.0.0. All v2.x known issues, with affected versions and workarounds, are listed in the pinned issue #421. ## v2.0.4 — 2026-10-08 diff --git a/CLAUDE.md b/CLAUDE.md index bd21ae65e..9263ad33b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -201,7 +201,7 @@ Auto-migrates v7 → v8 on open (spec §9, with a backup meta page). v1–v6 are ## Test Coverage -**1440 tests** (1427 passing, 13 ignored; unit + integration + doc). +**1441 tests** (1430 passing, 11 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. diff --git a/README.md b/README.md index 9cdcc6750..b2f021364 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ let r2 = pq.execute(&[("tx", BindValue::TxCount(2)), ("entity", BindValue::Entit ```bash cargo run # interactive Datalog REPL -cargo test # run 1440 tests +cargo test # run 1441 tests cargo run < demos/demo_recursive.txt # recursive rules demo ``` diff --git a/docs/TEST_COVERAGE.md b/docs/TEST_COVERAGE.md index f03c20a88..5e5dbfefc 100644 --- a/docs/TEST_COVERAGE.md +++ b/docs/TEST_COVERAGE.md @@ -4,7 +4,7 @@ **Verified**: 2026-10-11 with `cargo test --quiet` -**Result**: 1,427 passing tests and 13 ignored tests (1,440 total) +**Result**: 1,430 passing tests and 11 ignored tests (1,441 total) This covers the unreleased file-format-v8 work (#287, #371): the index-key value-collision fix, the v7→v8 migration, and the drop of formats v1–v6. It otherwise still covers the public database API, the Datalog engine, storage and recovery paths, v2.0.0's locking and structured-error-code changes, and v2.0.1's regression test for rebuilding indexes on open after several checkpoints (#370). Merged from the v2.x line: `tests/point_query_history_test.rs` and new storage unit tests check that attribute-narrowed bound-entity lookups return exactly what a full scan returns, and that `net_asserted_facts` matches its previous implementation on randomized histories (#323). Attribute-scan tests check that the AEVT range covers exactly the queried attribute, including non-ASCII names such as `:丿` and prefix siblings such as `:ab` for `:a` (#381). Incremental B+tree rebuild tests check that checkpoints which copy untouched index leaves produce exactly the entries and file checksum a full rebuild would, including over repeated checkpoints, that single-fact checkpoints do not fragment the index, and that a damaged slot directory is rejected rather than copied (#315). B+tree cursor tests check that every scan walks the tree, not the leaf `next_leaf` chain: full scans and start positions match the input, `seek` from the current position lands where a fresh descent from the root would (randomized, plus within a leaf, to a sibling, across subtrees, past the end, into the gap after a leaf, and backwards as a no-op), a seek to the sibling leaf reads exactly one page, reads are unchanged with every `next_leaf` zeroed, a child pointer into a non-B+tree page is STG-013, and a cycle in child pointers is an error, not a hang (v8 storage format PR 1, #374). Directory-sync unit tests check that creating the `.graph` file or the WAL, and deleting the WAL, fsync the parent directory after the file operation, and that reopening an existing file does not (#389). Disjunction tests check that `or-join` returns an empty result, not INT-031, when no rows reach it: on an empty database, after a pattern that matches nothing, after a `not` that removes every row, and in a rule body (#405). A stratification unit test builds the same two-rule negative cycle 64 times and checks that the INT-054 message names the same predicate pair every time (#410). Magic-sets tests check that a recursive rule with a literal start that recurses through another rule returns the same results as without the literal (#297). API tests check that `execute()` and `WriteTransaction::execute()` reject `$slot` bind tokens in every position with API-010 (#407). The REPL `run()` test now runs in a child process with null stdin, so it no longer hangs when the test runner's stdin is an open non-TTY stream (#415). v8 page-format tests (PR 2, #374, #388) check the 24-byte page header (a flipped bit, a wrong page id and a future generation each give their own STG code, legacy page types fail before the CRC, and a failed page is never cached), meta-page encoding, the free-list chain, WAL v2's base generation, and every row of the meta-selection table: both slots valid, a torn newest slot with the WAL's base generation (opens at the older one), a WAL newer than the valid meta, pages of the next generation on the free list or past `page_count` with no WAL (all STG-033, with the file unmodified), an unknown feature bit (STG-034), and a pre-release v8 single header (STG-032). Fault-injection tests crash `save()` at every page write (cleanly and with torn pages, including the meta write) and every sync, and crash a v7 migration at every write; each reopen finds exactly the old or the new checkpoint, and every reachable page plus the free list covers the file exactly. Twenty-five consecutive saves never write a page the previous meta references. Covering-key tests (PR 3, #433) check that byte order equals logical order for every key component and value type on random tuples (negative, zero and large integers, NaN and ±0.0, strings with 0x00, 64- and 65-byte strings, FOREVER, newest-transaction-first) and that every component round-trips; that prefix-compressed leaves and shortest-separator internal nodes round-trip, agree with a linear scan, keep `max(left) < s ≤ min(right)`, and reject an overflow; that the byte-key B+tree's builds, incremental rebuilds, cursor and `seek` match their expected sets; that dictionary ids are assigned once, never reused, shared between a keyword attribute and value, and deterministic; that a re-asserted or retracted long value writes no new value page and a forced hash collision still finds the right value; that one transaction with two timestamps is STG-037 and a missing dictionary entry STG-036; that committed reads by entity, entity and attribute, attribute, and in full equal an in-memory model across eight checkpoints and a reopen; that short-valued reads touch no value page; that a v7 file with every value type, long strings, refs and retractions migrates to exactly the same facts; that a pre-covering-key v8 file is STG-032; and that string values over 4,068 bytes and attributes or keywords over 1,024 bytes are WAL-003. A WAL test checks that reopening next to a fully checkpointed WAL does not reuse transaction numbers (#447). Copy-on-write checkpoint tests (PR 4, #434) check that lazy free-list pops and pushes conserve every page across page boundaries, partial head pages and chain exhaustion, never hand out a page freed in the same generation, and share unread chain pages unchanged; that a copy-on-write insert equals a sorted merge for random batches (including keys below the first and above the last, many into one leaf, root growth to depth 3, and 1 KB separators that split internal nodes), leaves the old tree readable and unchanged, frees exactly the replaced nodes, writes only new pages, writes exactly one leaf and two internal nodes for one entry in a depth-3 tree, and does not fragment leaves under single inserts; that a checkpoint after 1 or 100 right-edge facts writes at most the tree-depth difference more pages at 100,000 facts than at 10,000, and at most one path per new entry per tree for random keys; and that four reader threads querying throughout 40 checkpoints never fail and never see the fact count go backwards. Hardening tests (PR 5) damage an index leaf, an internal node, a dictionary leaf, a value page and a free-list page in turn. Every committed read or checkpoint that reaches the damaged page fails with STG-029, never with wrong or fewer rows. A query through `Minigraf` on a file with damaged leaves fails with STG-029, and the file still opens at its last checkpoint. A size test measures at most 200 bytes per fact (142 measured) on 144k facts in #433's shape; its 1.15M-fact variant is ignored locally and runs in the nightly smoke workflow. A shared dictionary-cache test checks that cached ident and timestamp lookups return the same facts and that the cache stays bounded. Merged from the v2.x line: storage and `edge_cases_test.rs` tests check that more than 65,535 uncheckpointed facts each resolve to their own fact through entity lookups and attribute scans, in memory and file-backed (#445). Merged from the v2.x line: `tests/golden_corpus_test.rs` opens a copy of each committed golden file in `tests/golden/` (v7 files written by minigraf 2.0.3: one checkpoint, several checkpoints, indexes rebuilt on open, a crash-left WAL, an already-checkpointed WAL, same-transaction multi-values) and checks its manifest's CRC32, `tx_count` and query results after open, after a checkpoint and reopen, and after one more write; a second test checks that every golden file has a manifest (#391). On `v3` the same test checks that each v7 file has a v8 meta page after its first open and gives the same results after reopening the migrated file, and adds seven v8 golden files (one checkpoint, seven copy-on-write checkpoints, a crash-left v2 WAL, an already-checkpointed v2 WAL, same-transaction multi-values, a v7 file migrated in place, and long values at the size limits); the #371 queries, skipped on v2.x, run and pass. `tests/model_based_test.rs` runs random sequences of transacts, retracts, write transactions, checkpoints, reopens and simulated crashes against a file and, after every step, compares the transaction counter, `verify`, full rows with windows and tx counts, and six query shapes under `:as-of N` and `:valid-at T` with a reference model (#385). `tests/property_test.rs` writes random histories (batched transacts, retracts, write transactions, checkpoints) in memory and to a file, and compares random queries (one to three joined patterns, `not` or `not-join`, a comparison predicate, `:as-of`, `:valid-at` or `:any-valid-time`) with a reference evaluator, on the open file and after a reopen; a query error fails the test (#386). `tests/crash_kill_test.rs` kills a child process running a seeded workload (multi-valued batches, batched retracts, five checkpoint modes) at a random point and checks that the reopened file holds exactly the transactions the child logged as committed, or those plus the one in flight, through full-scan, AEVT, AVET, EAVT and `:as-of` queries and `verify`, again after a second reopen and after a checkpoint plus reopen (#384). Integrity tests (#373) check that intact files verify with no problems and an exact fact and page count, and that `verify` reports each kind of damage with its code: an EAVT, AEVT, AVET or VAET leaf missing an entry (STG-038), keys out of order and a page reached from two trees (STG-039), a rotted index leaf (STG-029), a free page still in use and a leaked page (STG-035), a missing transaction entry (STG-036), and an entity id whose UUID maps to another id (STG-040). After each index or free-list damage, `rebuild_indexes` leaves a file that verifies clean, reads back every fact and accounts for every page. The source is an index that agrees with another one, or the only intact one; three intact indexes that all disagree, or a damaged dictionary, give STG-041 with the file unchanged. A rebuild commits pending facts, including new ids and long values. A crash at every write (clean and torn) and every sync of a rebuild reopens either at the old generation with only its old damage or at the new one with none, never losing a fact. Through `Minigraf` (`tests/integrity_test.rs`), a rebuild commits WAL entries and deletes the WAL, and flipping a byte in each leaf of a file in turn is always reported, then either repaired (index leaves) or refused with STG-041 and the file unchanged (dictionary leaves). Page-count tests (#497) check that a CRC-valid meta whose `page_count` is past the end of the file (one page past, 2^50, `u64::MAX`; with the other slot valid or damaged; with or without a WAL) and a file cut short by one page, three pages or down to three pages are `STG-044` at open, with the file unchanged and not grown, while a `page_count` exactly at the end opens; that a checkpoint whose pages did not all reach the backend fails with `INT-049` before writing its meta, and the file reopens at the previous generation; and that `verify` at a meta counting pages past the end reports `STG-044` without scanning to that count. Through `Minigraf` (`tests/integrity_test.rs`), a file truncated by one or three pages and a file whose meta was rewritten with `page_count` 2^50 and a valid CRC are `STG-044` read-write and read-only, with no WAL created and the file length unchanged. The `file_header` fuzz input from #497 is a corpus seed. A sub-page test (#506, `tests/integrity_test.rs`) checks that a file of 1 to 4,095 bytes is `STG-045` read-write and read-only, 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 writes when it is a prefix of the first meta page or all zeros; a file-backend unit test checks that `partial_page` returns the bytes past the last whole page. The `corrupt_file` fuzzer no longer skips files under one page: they must fail to open or open empty, and the #506 input is a corpus seed. Damaged v7 fact page tests (#496) check that a page in the counted fact range whose type byte is anything but the fact-page type (zero, `0x01`, the v2.x B+tree types, `0xFF`, a zeroed page; first and last page) is `STG-014` rather than skipped, while a range derived from the first index root skips only v2.x B+tree pages; that opening such a v7 file read-write or read-only fails with `STG-014` and writes nothing; and, in `tests/migration_matrix_test.rs`, that the golden `v7_basic.graph` with page 1's type byte changed or page 1 overwritten with noise fails both opens with the file unchanged and no WAL created. Both inputs are `fact_page` corpus seeds. Merged from the v2.x line: `tests/edge_cases_test.rs` checks the same refusal and acceptance through `Minigraf::open` on a database checkpointed twice (#506). Torn-WAL-tail tests (#513) check that opening a WAL writer cuts off a partial last entry (cut in the checksum, the header or the payload) or an entry with a bad checksum and everything after it, then appends where replay reaches, and leaves an intact WAL byte for byte; that an append that fails part-way makes every later append `WAL-007` while a rejected oversize fact (`WAL-003`, nothing written) does not; through `Minigraf`, that the failed transaction is not applied, later `transact` and `commit` are `WAL-007`, and a `checkpoint()` makes writes work again; and, in `tests/wal_test.rs`, that a commit made after reopening a crash-left WAL with each kind of torn tail survives a second crash. The fault matrix (#390, `src/fault_matrix.rs`) drives a test-only fault plan through the real file backend, WAL and directory fsync: an EIO, a short or torn write (1, 40 and 4,095 bytes), a full disk that keeps failing every later write, a failed fsync, and a failed fsync that also loses the writes since the last good one ("fsyncgate"), each at every write or sync call of a transact, a transact that creates the WAL, an automatic checkpoint, `checkpoint()`, the close-time checkpoint and `rebuild_indexes()`, on a database committed over two checkpoints with multi-valued, retracted and long values and entries pending in the WAL. After each fault, the handle holds the old facts or those plus the whole transaction, never part of one; after a failed checkpoint another is `STG-046`; a later write succeeds or is `WAL-007`/`STG-046`; a kill and reopen gives exactly the handle's facts, plus the failed transaction whole or not at all, and `verify` finds nothing; and a commit after recovery survives another kill. Two more tests fill the disk halfway through a checkpoint (`STG-046`, reads unchanged, reopen intact), and fail the fsync after a checkpoint's meta write and then retry the checkpoint, crashing at each of its writes: the retry is refused and every reopen is intact (without the refusal, a reopen hits `STG-029`). Unit tests check the fault plan's counting, and that a `rebuild_indexes` refused with `STG-041` leaves the handle usable. Live-read tests (#379) check that `triple_len` ends exactly where `tx↓` begins for every value type and index; that on random histories (several valid-time windows, retractions, re-assertions, several values per attribute, long values, 12 transactions per checkpoint) the committed live read for every scan kind and `:as-of` point is exactly the net-asserted set, that it stays equivalent with pending facts on top, and after reopen; and that a point read on a triple asserted and retracted 2,000 times reads under a third of the pages a history read does, with or without `:as-of`. `tests/live_reads_test.rs` runs the same history in a checkpointed file database and in memory and checks that point, attribute, full-scan, join, valid-time, `:as-of` and rule queries return the same rows, before and after reopen. One-window tests (#435) check that `net_asserted_facts` keeps each triple's newest transaction only (a later window replaces an earlier one, closing an open window, a retraction in the newest transaction hides it, records with two windows in one transaction keep both, once each) against a per-triple model on random histories; that the on-disk key walk agrees at every `:as-of` before and after reopen; and that `API-011` rejects two windows of one fact in one transaction without taking a `tx_count`. `tests/valid_time_window_test.rs` checks replacing, closing, extending, reopening and retract-then-reassert through entity-bound and attribute queries in memory, on a checkpointed file and after reopen, `API-011` from `transact` and from `WriteTransaction::execute`, and a later statement's window committed when two statements of a `WriteTransaction` give one fact different windows. Window-validation tests (#436) check that `check_valid_windows` rejects an empty window, an inverted one, and a `:valid-to` at or before the transaction time when `:valid-from` is left out, but not retractions; that `execute` rejects each such `transact` (both bounds, equal bounds, `:valid-to` only, a per-fact bound inverting the transaction's window) with `API-019` without writing or taking a `tx_count`, in memory, checkpointed and after reopen; that `tx.execute()` rejects them and the transaction still commits its other statements; that `commit()` rejects a default `valid_from` that has passed the `:valid-to` by commit time; and that `FactStorage::transact_batch` takes no `tx_count` on rejection. Statement-order tests (#477, `tests/statement_order_test.rs`) run every mix of two statements on one fact inside a `WriteTransaction` (retract then assert, assert then retract, two retracts, two windows, after a committed assertion and on an empty database) and check that reads inside the transaction and the committed rows agree, in memory, checkpointed and after reopen; that other facts of an overridden statement survive; that the commit is one transaction without the overridden records; and that two windows in one statement are `API-011` from `tx.execute()` while the rest commits. A unit test checks the commit collapse keeps each fact's newest statement in order. A log-writer test checks that an inverted window is `API-019`, a second window of a fact in one transaction `API-011`, and an assertion and a retraction of one fact in one transaction `API-020` in either order, each leaving the writer usable and the output exact, while duplicate retractions, identical repeats and the same fact in the next transaction are accepted. Cursor tests (#432, `tests/cursor_test.rs`) check that `query()` batches partition the `execute()` answer for several batch sizes (never empty, never over `max_rows`, `max_rows == 0` still progresses, `None` stays `None`), that the iterator and aggregates, rules, `:as-of` and `:any-valid-time` match `execute()`, that a cursor opened before a `transact`, a `WriteTransaction` commit, a retract or a checkpoint returns the pre-write answer to the end (in memory and file-backed), that `transact`/`retract`/`rule` are `API-012` and bind slots `API-010`, that a `WriteTransaction` thread gets `INT-001`, that rule limits give `INT-020` as from `execute()`, that `PreparedQuery::query` matches `PreparedQuery::execute`, and that a cursor is `Send` and outlives its database handle. Fact-log tests (#430, `tests/fact_log_test.rs`) check that `fact_log()` returns exactly the records written (assertions and retractions, with `tx_count` and `tx_id`) in memory, with everything pending, half checkpointed, fully checkpointed and after reopen, in both orders, with windows of 1 to 1,000 records and batch sizes of 1 to 1,000, in non-decreasing `tx_count` in Tx order; that every layout gives identical records, including valid-time windows and long string values; that a transaction larger than the window is split with no loss or duplicate; that attribute, attribute-prefix, entity and tx-range filters and their combinations match a model, and unknown names match nothing; that writes after open are not in the log; that `checkpoint()` and `rebuild_indexes()` are `API-013` while a log is open (a checkpoint with nothing to write is allowed), that automatic checkpoints from `execute()` and `WriteTransaction::commit` wait for the log, and that closing, dropping or finishing the log releases the database; that a `WriteTransaction` thread gets `INT-001`; and that a log is `Send`, outlives its database handle, and the deferred writes replay on the next open. Read-only open tests (#429, `tests/read_only_open_test.rs`) check that a read-only handle on a file with checkpointed facts and a pending WAL returns the same query results, `tx_count` and fact log as a read-write open; that queries, a cursor, a fact log, `verify`, `prepare`, rules and a UDF leave the `.graph` and WAL bytes and the `.graph` mtime unchanged, and no WAL is created; that `transact`, `retract`, `begin_write`, `checkpoint` and `rebuild_indexes` are `API-014` without taking a `tx_count`, also on a read-only in-memory database and through the builder path; that a missing file is `STG-042` and nothing is created; that an empty file is an empty database and stays 0 bytes; that read-only handles share a file while a read-write one is refused with `STG-025`, both ways; that a file without write permission opens read-only; and that small and whole-file page caches work. File-backend unit tests check that a shared-mode backend refuses page writes and syncs with `INT-056`, that the in-process registry counts readers and admits readers or one writer (also without kernel locks), and that a reader in another process shares the file with a reader here and refuses a writer here with `STG-026`. Storage unit tests check that a read-only open of a v7 file at every point of an interrupted migration (clean and torn writes) sees every fact and writes nothing, and that an empty backend stays empty. `tests/golden_corpus_test.rs` opens a copy of every golden file read-only first and checks its `tx_count` and queries, and that neither the file nor its WAL changes. Log-writer tests (#431, `tests/log_writer_test.rs`) copy a source's fact log (assertions, retractions, an explicit valid-time window, long strings, refs, keywords, checkpointed and pending facts) through `LogWriter` and check that the new file has no `.partial` or WAL, verifies clean, and gives the source's `tx_count`, fact log and query results (current, `:valid-at`, `:any-valid-time`, every `:as-of`, and entity-, attribute- and value-bound lookups) read-write, read-only and after reopen (#370); that it takes new writes at the source's next `tx_count`; that 90,000 facts over two batch commits verify clean and count correctly at `:as-of`; that a purged middle transaction is a hole whose `:as-of` equals the one before, and trailing holes keep the counter through `advance_tx_count`; that an empty log is an empty database with the counter; that `tx_count` 0, a lower or closed transaction and a backwards advance are `API-015`, a second `tx_id` `API-016`, a second window `API-011`, a retraction of an asserted fact in one transaction `API-020`, an empty window `API-019` and oversize values or idents `WAL-003`, each leaving the writer usable and the output exact, with repeats written once; that an existing target or WAL is `STG-043`, a second writer `STG-025` and `read_only` `API-014`; that a stale `.partial` is reused; that dropping an unfinished writer leaves nothing; that `finish` refuses a target created during the build and leaves it untouched; and that `LogWriter` is `Send`. Unit tests check that a batch commits only when a transaction opens past the batch size, never splitting one, and that `FileBackend::truncate` empties a file. A unit test in `error.rs` checks that `MinigrafError::invalid_argument` and `MinigrafError::closed`, the constructors for language bindings, give API-017 and API-018 with their text. Query-validation tests (#406, #414) check that a plain `:find` variable or a window function's argument, `:partition-by` or `:order-by` variable that no `:where` clause binds (including `?_` wildcards) is `PRS-080` from both `execute()` and `prepare()`, naming the variable, while variables bound by a pattern, an attribute position, a rule call, an expression binding, an `or-join`, every branch of an `or`, a pseudo-attribute or a window spec are accepted, and bind slots are not mistaken for `:find` variables; and that an unknown aggregate is `QRY-010` and an unknown window function `QRY-011` on an empty database, when no row matches, when rows match, through a rule query and through a prepared query, while a registered UDF in either position returns an empty result on an empty database. Coverage-gate tests (#396) check that replay stops at a WAL entry whose first fact is longer than 10 MB or that claims more than 1M facts, that a WAL writer cannot be opened in a missing directory, and that a failed cut of a torn WAL tail fails the open and leaves the file as it was; that a short v7 page and a slot past the record count are `INT-024`; that a migration backup meta of another generation or file length is not used (`STG-032`, file unchanged), and a torn first meta with data in page 1 is `STG-032`, not a new file; that `save` with nothing pending writes nothing, `into_backend` saves pending facts, and `rebuild_indexes` with nothing checkpointed commits the pending facts; and that unsorted or repeated B+tree input and a stored key inserted with another value are `INT-049`, a tree pointer to a free-list page is `STG-013` from `get`, a leaf walk and a copy-on-write insert, and the read-only `MutexStorageBackend` refuses writes, syncs and close. A page-cache test holds two threads in the backend read so both miss the same page, and checks that both get the one cached copy; before it, that path ran only when the scheduler happened to race the threads, and the gate flapped. Three `BrowserDb` tests (wasm32, run with `wasm-bindgen-test` in a browser, not counted above) check that `query()` batches of 1, 7 and 1,000 give the rows of `execute()`, that a cursor keeps the answer from when it opened and returns the end after `close()`, and that a `transact` is API-012. The wasm32 targets run in CI (`.github/workflows/wasm.yml`). In a browser (`wasm-pack test --headless`, Firefox and Chrome): 32 `wasm_bindgen_test` tests: nineteen for `BrowserDb` (including IndexedDB persistence, the cursor, and the flush returning every dirty page and failing on one it cannot read, from #470), eleven for `BrowserBufferBackend` (which used `#[test]` and never ran before), and two checking that IndexedDB page keys accept only whole numbers from 0 to 2^53. Five of the `BrowserDb` tests (#411, #440) check that opening a v7 store written straight into IndexedDB stores a v8 meta page and only the live pages, with nothing written on the next open; that after each of 60 transacts memory, and at the end IndexedDB, hold exactly the pages not on the free list while the file grows by under one page per transact; that a commit applied in memory but never flushed reopens at the previous one and keeps working; that an export after churn is as long as the meta says and imports back with its free pages dropped; and that `importGraph()` deletes the replaced database's IndexedDB keys. Two buffer tests check that releasing a page drops its bytes and any unflushed write, and that export writes a released page as zeros. A native storage test checks that a file whose free pages are all zeros (as the browser exports them), both just migrated from v7 and after 20 saves, opens, verifies clean and keeps committing, and the free-list test checks that a checkpoint reports exactly the ids it newly lists. On `wasm32-wasip1` under Wasmtime: the unit and integration tests that do not use a file run, 956 passing; ten that spawn threads or a child process are ignored on WASI. These are not counted above. A WAL test checks that a checkpoint with no facts to flush still removes a WAL whose entries are all already in the file, and the golden-corpus test now requires the WAL to be gone after a checkpoint for the `stale_wal` fixtures too (#457). A WAL test checks that closing a handle opened with `wal_checkpoint_threshold(usize::MAX)`, in both sync modes, leaves the `.graph` file byte for byte as it was and keeps the WAL, whose entries replay on the next open, and a unit test closes an in-memory handle under the same setting (#476). A recursive-rule test checks that a rule query on a database with more live facts than `max_results` returns its answer, with a bound and with a free first argument, and that a query deriving more facts than its `:max-results` is still `INT-020` (#528). A log-writer test runs the online backup from `docs/DURABILITY.md`: it copies a read-write handle with an uncheckpointed transaction through `fact_log` and `LogWriter` while another thread writes past the automatic checkpoint threshold, and checks that the copy verifies clean, holds exactly the records the log returned and the counter read after the log opened, and that every concurrent write reaches the source's file once the log is dropped (#398). @@ -13,19 +13,19 @@ This covers the unreleased file-format-v8 work (#287, #371): the index-key value - **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. - **Multi-value and index-key correctness** (`tests/multi_value_test.rs`, 10 tests): same-transaction multi-valued facts surviving query-time dedup, retracting several values of one attribute in one call, valid-time stints (facts differing only in valid time), maximum-value-size boundaries, and format v8 index keys (value bytes + assert/retract flag) (#287, #371). - **Storage correctness**: packed pages, B+tree indexes and range scans, cache behavior, file-header validation, migration (including v7→v8 index rebuild and v1–v6 rejection via `STG-028`), 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, SIGKILL recovery, and queries seeing each concurrent transaction whole (`tests/snapshot_atomicity_test.rs`, #554). +- **Reliability and concurrency**: concurrent reads and writes, rollback behavior, same-process handle exclusion, cross-process locking, PID namespaces, NFS and container lock behavior, SIGKILL recovery, and queries seeing each concurrent transaction whole and reading one state for all their patterns (`tests/snapshot_atomicity_test.rs`, #554, #555). - **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. ### Ignored Tests -The thirteen ignored tests are intentionally excluded from a normal local run: +The eleven ignored tests are intentionally excluded from a normal local run: - Six rustdoc examples that reference internal types and cannot compile as standalone examples. - One high-contention concurrency stress test intended for scheduled runs. - One long-haul smoke test intended for scheduled runs. - One soak test at 10M facts (`soak`), run weekly by `.github/workflows/soak.yml` (#392); `soak_short` runs the same driver at 20K facts in the normal suite. -- Three snapshot reproducers in `tests/snapshot_atomicity_test.rs`: two for #555 (patterns of one query read different states under concurrent writes), which fail today, and one checking that a checkpoint never shows a fact twice, which has not failed but guards the checkpoint change planned in #543. +- One snapshot test in `tests/snapshot_atomicity_test.rs` checking that a checkpoint never shows a fact twice; it has not failed, and guards the checkpoint change planned in #543. - One size test at 1.15M facts (`size_per_fact_in_433_shape_1m`), run by the nightly smoke workflow. ### Reproducing diff --git a/src/graph/storage.rs b/src/graph/storage.rs index dfdf34543..2f78411df 100644 --- a/src/graph/storage.rs +++ b/src/graph/storage.rs @@ -689,63 +689,90 @@ impl FactStorage { scan: crate::storage::Scan<'_>, as_of: Option, ) -> Result> { - use crate::storage::Scan; - use crate::storage::index::{AevtKey, EavtKey}; + self.get_live_facts_of_scans(&[scan], as_of) + } + + /// [`Self::get_live_facts`] for several scans, all read from one state of + /// the store: no commit lands between two of them (#555). The results are + /// concatenated in `scans` order and not deduplicated. + pub(crate) fn get_live_facts_of_scans( + &self, + scans: &[crate::storage::Scan<'_>], + as_of: Option, + ) -> Result> { let d = self.data.read().unwrap_or_else(|e| e.into_inner()); let mut facts = Vec::new(); - let mut push = |pos: usize| -> Result<()> { - let f = pending_fact(&d, pos)?; - if as_of.is_none_or(|n| f.tx_count <= n) { - facts.push(f); - } - Ok(()) - }; - match scan { - Scan::All => { - for pos in 0..d.facts.len() { - push(pos)?; - } + for &scan in scans { + live_facts_of_scan(&d, scan, as_of, &mut facts)?; + } + Ok(facts) + } +} + +/// The pending facts of `scan` up to `as_of`, then its committed live facts, +/// appended to `facts`. The caller holds `d`'s lock for the whole read. +fn live_facts_of_scan( + d: &FactData, + scan: crate::storage::Scan<'_>, + as_of: Option, + facts: &mut Vec, +) -> Result<()> { + use crate::storage::Scan; + use crate::storage::index::{AevtKey, EavtKey}; + let mut push = |pos: usize| -> Result<()> { + let f = pending_fact(d, pos)?; + if as_of.is_none_or(|n| f.tx_count <= n) { + facts.push(f); + } + Ok(()) + }; + match scan { + Scan::All => { + for pos in 0..d.facts.len() { + push(pos)?; } - Scan::Entity(entity) => { - for (key, &pos) in d - .pending_indexes - .eavt - .range(EavtKey::entity_start(*entity)..) - { - if key.entity != *entity { - break; - } - push(pos)?; + } + Scan::Entity(entity) => { + for (key, &pos) in d + .pending_indexes + .eavt + .range(EavtKey::entity_start(*entity)..) + { + if key.entity != *entity { + break; } + push(pos)?; } - Scan::EntityAttribute(entity, attribute) => { - let start = EavtKey::entity_attribute_start(*entity, attribute); - for (key, &pos) in d.pending_indexes.eavt.range(start..) { - if key.entity != *entity || key.attribute != attribute { - break; - } - push(pos)?; + } + Scan::EntityAttribute(entity, attribute) => { + let start = EavtKey::entity_attribute_start(*entity, attribute); + for (key, &pos) in d.pending_indexes.eavt.range(start..) { + if key.entity != *entity || key.attribute != attribute { + break; } + push(pos)?; } - Scan::Attribute(attribute) => { - for (key, &pos) in d - .pending_indexes - .aevt - .range(AevtKey::attribute_start(attribute)..) - { - if key.attribute != attribute { - break; - } - push(pos)?; + } + Scan::Attribute(attribute) => { + for (key, &pos) in d + .pending_indexes + .aevt + .range(AevtKey::attribute_start(attribute)..) + { + if key.attribute != attribute { + break; } + push(pos)?; } } - if let Some(reader) = &d.committed { - facts.extend(reader.live_facts(scan, as_of)?); - } - Ok(facts) } + if let Some(reader) = &d.committed { + facts.extend(reader.live_facts(scan, as_of)?); + } + Ok(()) +} +impl FactStorage { /// History read (every record); tests compare it with live reads and models. #[cfg(test)] /// Get all facts for a specific entity (index-driven). @@ -1664,6 +1691,28 @@ mod tests { assert_eq!(storage.fact_count(), 2); } + #[test] + fn live_facts_of_scans_concatenates_each_scan_in_order() { + use crate::storage::Scan; + let storage = FactStorage::new(); + let e = uuid::Uuid::new_v4(); + let x = Fact::new(e, ":t/x".to_string(), Value::Integer(1), 1); + let y = Fact::new(e, ":t/y".to_string(), Value::Integer(2), 1); + storage.load_facts(vec![x, y]).unwrap(); + let scans = [ + Scan::Attribute(":t/y"), + Scan::Attribute(":t/x"), + Scan::Entity(&e), + ]; + let attrs: Vec = storage + .get_live_facts_of_scans(&scans, None) + .unwrap() + .into_iter() + .map(|f| f.attribute) + .collect(); + assert_eq!(attrs, [":t/y", ":t/x", ":t/x", ":t/y"]); + } + #[test] fn load_facts_appends_a_batch_and_skips_duplicates() { let storage = FactStorage::new(); diff --git a/src/query/datalog/executor.rs b/src/query/datalog/executor.rs index 252e2c60f..711701f92 100644 --- a/src/query/datalog/executor.rs +++ b/src/query/datalog/executor.rs @@ -383,7 +383,8 @@ impl DatalogExecutor { /// /// Results are not deduplicated: the sole caller feeds them to `net_asserted_facts`, /// which is idempotent under duplicated records. Each lookup returns the pending facts - /// up to `as_of` and the committed facts live at `as_of` (`FactStorage::get_live_facts`). + /// up to `as_of` and the committed facts live at `as_of`, and all lookups read one + /// state of the store (`FactStorage::get_live_facts_of_scans`, #555). fn selective_fact_fetch( &self, patterns: &[Pattern], @@ -442,25 +443,19 @@ impl DatalogExecutor { return Ok(None); } - let mut all_facts: Vec = Vec::new(); - + // Every scan reads one state of the store, so a commit cannot land + // between two patterns of this query (#555). + let mut scans: Vec> = Vec::with_capacity(total); for (uid, attrs) in &entity_attrs { match attrs { Some(attrs) if narrow => { - for attr in attrs { - all_facts.extend( - self.storage - .get_live_facts(Scan::EntityAttribute(uid, attr), as_of)?, - ); - } + scans.extend(attrs.iter().map(|a| Scan::EntityAttribute(uid, a))); } - _ => all_facts.extend(self.storage.get_live_facts(Scan::Entity(uid), as_of)?), + _ => scans.push(Scan::Entity(uid)), } } - - for attr in &attributes { - all_facts.extend(self.storage.get_live_facts(Scan::Attribute(attr), as_of)?); - } + scans.extend(attributes.iter().map(|a| Scan::Attribute(a))); + let all_facts = self.storage.get_live_facts_of_scans(&scans, as_of)?; Ok(Some(all_facts)) } diff --git a/tests/snapshot_atomicity_test.rs b/tests/snapshot_atomicity_test.rs index 808a03b30..060490dfa 100644 --- a/tests/snapshot_atomicity_test.rs +++ b/tests/snapshot_atomicity_test.rs @@ -159,10 +159,12 @@ fn checkpoint_never_shows_a_fact_twice() { } } -/// A query fetches each attribute with its own `get_live_facts` call, each -/// under its own read lock, so a transaction that commits between the two -/// calls is seen by one pattern and not the other. Every transaction writes -/// `:pair/x` and `:pair/y` for one entity together. +/// A query fetched each attribute with its own `get_live_facts` call, each +/// under its own read lock, so a transaction that committed between the two +/// calls was seen by one pattern and not the other (#555). Every transaction +/// writes `:pair/x` and `:pair/y` for one entity together, atomically (#554). +/// Attributes are fetched in sorted order, so only the query whose positive +/// pattern is `:pair/y` can catch a commit between the fetches; both are run. fn patterns_see_one_state(db: Minigraf) { let found = race( Arc::new(db), @@ -173,11 +175,16 @@ fn patterns_see_one_state(db: Minigraf) { .unwrap(); }, |db| { - let r = rows( - db, - "(query [:find ?e :where [?e :pair/x ?x] (not [?e :pair/y ?x])])", - ); - (!r.is_empty()).then(|| format!("{} entities with x but no y", r.len())) + for q in [ + "(query [:find ?e :where [?e :pair/x ?v] (not [?e :pair/y ?v])])", + "(query [:find ?e :where [?e :pair/y ?v] (not [?e :pair/x ?v])])", + ] { + let n = rows(db, q).len(); + if n != 0 { + return Some(format!("{n} entities with one attribute of the pair: {q}")); + } + } + None }, ); if let Some(msg) = found { @@ -186,13 +193,11 @@ fn patterns_see_one_state(db: Minigraf) { } #[test] -#[ignore = "reproducer for #555: fails on v3 today"] fn patterns_of_one_query_see_one_state_in_memory() { patterns_see_one_state(Minigraf::in_memory().unwrap()); } #[test] -#[ignore = "reproducer for #555: fails on v3 today"] fn patterns_of_one_query_see_one_state_on_file() { let dir = tempfile::tempdir().unwrap(); patterns_see_one_state(Minigraf::open(dir.path().join("p.graph")).unwrap());