8000 fix(wal): never append after a torn WAL entry (#513) by adityamukho · Pull Request #514 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Queries no longer lose or duplicate facts when more than 65,535 facts are waiting for a checkpoint (#445; also fixed in v2.0.3).
- **A file shorter than its meta page count is refused at open with `STG-044`** (#497). Open never compared the meta page's `page_count` with the file's length. A truncated file (an interrupted copy, a partial download) opened, and its queries and checkpoints then failed with `INT-049` after the WAL had accepted writes. A meta with a `page_count` far past the end opened too: its next checkpoint wrote at that page id, leaving a sparse file of up to 4 EiB, and `verify()` scanned to that count and never finished. Open now refuses both, read-write and read-only, without modifying the file. It does not fall back to the other meta page, because a valid newer meta means that checkpoint happened. A checkpoint now fails with `INT-049` instead of writing a meta that counts pages the backend does not hold, and `verify()` reports a `page_count` past the end as `STG-044` and stops its page scan at the end of the file. `BrowserDb` is not checked: it keeps only live pages (#440), so its store has no length to compare. New code `STG-044`.
- **A file shorter than one page is refused at open with `STG-045` unless it is a torn first write** (#506). Open treated any file under 4 KiB as a new database, because it holds no whole page, and a read-write open wrote a new empty database over it. A database cut below one page by an interrupted copy, or an unrelated file at the path, was silently replaced. Now such a file opens as a new database only when its bytes are zeros or a prefix of the first meta page a new database writes (what a crash during creation can leave, the rule a torn full page 0 already followed); anything else is `STG-045`, read-write and read-only, and the file is not modified. A zero-length file is still a new database. New code `STG-045`.
- **A commit made after a torn WAL entry is no longer lost on recovery** (#513). Recovery replays the WAL up to its first partial or damaged entry, but the writer appended after the end of the file. After a crash during an append, the next session's commits went in after the partial entry, so a second crash before any checkpoint l 8000 ost them, although each had been fsynced and reported as committed. A failed append (short write, full disk, I/O error) did the same to the next commit through that handle. Opening a database for writing now cuts the WAL back to the end of its last valid entry, and syncs it, before anything is appended. After a failed append, every later write through that handle fails with the new `WAL-007` until the database is reopened or a `checkpoint()` succeeds. The transaction whose append failed is not applied; after a reopen it is either there in full (its entry reached the disk) or not at all. Affected every release since v0.5.0.
- **A v7 fact page with a damaged type byte fails the upgrade with `STG-014`** (#496). Opening a v7 file skipped, without an error, any page in its fact-page range whose type byte was not the fact-page type. The read-write open then upgraded the file to v8 and freed the old v7 pages, so that page's facts were lost for good; the read-only open returned the same answer with them missing. A v2.x writer fills that range with fact pages only, so any other type there is damage: open now fails with `STG-014`, read-write and read-only, and leaves the v7 file as it was. v7 fact pages have no checksum of their own, so damage elsewhere in a page can still decode to wrong facts or fail with another code; keep a copy of a v7 file until it has opened under v3.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).

Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ Auto-migrates v7 → v8 on open (spec §9, with a backup meta page). v1–v6 are

## Test Coverage

**1386 tests** (1377 passing, 9 ignored; unit + integration + doc).
**1393 tests** (1384 passing, 9 ignored; unit + integration + doc).
See `docs/TEST_COVERAGE.md` for the full per-file breakdown.

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

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

Expand Down
18 changes: 18 additions & 0 deletions docs/ERROR_REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ with no `CodedError` anywhere in its chain.
| WAL-004 | Fact serialised size exceeds u32 range | WAL |
| WAL-005 | WAL num_facts exceeds platform usize | WAL |
| WAL-006 | Failed to delete WAL file | WAL |
| WAL-007 | Earlier WAL write failed on this handle | WAL |
| API-001 | Write lock poisoned | Database API |
| API-002 | Unexpected command variant in write path | Database API |
| API-003 | Attribute must be a keyword | Database API |
Expand Down Expand Up @@ -2392,6 +2393,23 @@ The WAL is replayed on open and deleted on checkpoint.

---

### WAL-007 Earlier WAL write failed on this handle

**Error text**: `an earlier write to the WAL failed on this handle; reopen the database or call checkpoint() before writing again`

**Cause**: A write or fsync of the `.wal` file failed earlier on this handle (for example a full disk or an I/O error), and that write returned its own error. The WAL may now end in a partial entry. Recovery stops at the first partial entry, so anything appended after it would be lost after a crash. Every later write through this WAL fails with WAL-007 instead.

The transaction whose write failed was not applied in memory. Whether it is durable is unknown: if its entry reached the disk whole, it is replayed when the database is next opened; otherwise it is lost. It is never applied in part.

**Resolution**:
- Fix the cause (free disk space, check the device), then reopen the database. Opening cuts off a partial entry at the end of the WAL.
- Or call `checkpoint()`. A successful checkpoint writes the committed facts to the `.graph` file and deletes the WAL; the next write starts a new one. The failed transaction is then gone for good.
- If the failed transaction matters, check after reopening whether it is there, and write it again if it is not.

**Scenario**: The disk fills during `db.execute("(transact ...)")`, which fails with an I/O error. The next `transact` fails with WAL-007 until the database is reopened or `checkpoint()` succeeds.

---

## API — Database API Errors

API errors indicate a violated contract in how the public `Minigraf` or
Expand Down
4 changes: 2 additions & 2 deletions docs/TEST_COVERAGE.md

Large diffs are not rendered by default.

50 changes: 48 additions & 2 deletions src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -621,8 +621,11 @@ impl Minigraf {
/// replay the facts on next open.
///
/// If the WAL write fails, an error is returned and the in-memory store is
/// left unchanged. The database remains consistent for subsequent in-process
/// operations.
/// left unchanged. The WAL may then end in a partial entry, so later writes
/// through this handle fail with WAL-007 until the database is reopened or
/// a [`checkpoint`](Self::checkpoint) succeeds; reads keep working. Whether
/// the failed transaction is durable is unknown: after a reopen it is there
/// in full or not at all.
///
/// Returns `Err` if called from the same thread that holds an active
/// `WriteTransaction` (use `tx.execute()` instead).
Expand Down Expand Up @@ -2401,6 +2404,49 @@ mod tests {
);
}

// ── #513: a failed WAL append ─────────────────────────────────────────────

/// A WAL append that fails part-way leaves its transaction unapplied and
/// refuses later writes with WAL-007, so nothing lands after the partial
/// entry. A checkpoint deletes the WAL and writes work again.
#[test]
fn test_failed_wal_append_refuses_writes_until_checkpoint() {
fn names(db: &Minigraf) -> usize {
match db
.execute("(query [:find ?n :where [?e :name ?n]])")
.unwrap()
{
QueryResult::QueryResults { results, .. } => results.len(),
_ => 0,
}
}
let dir = tempfile::tempdir().unwrap();
let db_path = dir.path().join("test.graph");
let db = Minigraf::open(&db_path).unwrap();
db.execute("(transact [[:a :name \"A\"]])").unwrap();

crate::wal::tear_next_append(9);
db.execute("(transact [[:b :name \"B\"]])")
.expect_err("the torn append fails");
assert_eq!(names(&db), 1, "the failed transaction is not applied");

let err = db
.execute("(transact [[:c :name \"C\"]])")
.expect_err("later writes are refused");
assert_eq!(err.code(), "WAL-007");
let mut tx = db.begin_write().unwrap();
tx.execute("(transact [[:c :name \"C\"]])").unwrap();
assert_eq!(tx.commit().unwrap_err().code(), "WAL-007");
assert_eq!(names(&db), 1);

db.checkpoint().unwrap();
db.execute("(transact [[:d :name \"D\"]])").unwrap();
assert_eq!(names(&db), 2);
drop(db);
let db = Minigraf::open(&db_path).unwrap();
assert_eq!(names(&db), 2, "A and D survive reopen");
}

// ── file-backed: checkpoint deletes WAL and updates main file ─────────────

#[test]
Expand Down
9 changes: 9 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ pub(crate) enum ErrorCode {
Wal004,
Wal005,
Wal006,
Wal007,
Api001,
Api002,
Api003,
Expand Down Expand Up @@ -1136,6 +1137,12 @@ pub(crate) const REGISTRY: &[(ErrorCode, &str, &str, ErrorCategory)] = &[
"failed to delete WAL file {}: {}",
ErrorCategory::Wal,
),
(
ErrorCode::Wal007,
"WAL-007",
"an earlier write to the WAL failed on this handle; reopen the database or call checkpoint() before writing again",
ErrorCategory::Wal,
),
(
ErrorCode::Api001,
"API-001",
Expand Down Expand Up @@ -2021,6 +2028,7 @@ mod tests {
| ErrorCode::Wal004
| ErrorCode::Wal005
| ErrorCode::Wal006
| ErrorCode::Wal007
| ErrorCode::Api001
| ErrorCode::Api002
| ErrorCode::Api003
Expand Down Expand Up @@ -2244,6 +2252,7 @@ mod tests {
ErrorCode::Wal004,
ErrorCode::Wal005,
ErrorCode::Wal006,
ErrorCode::Wal007,
ErrorCode::Api001,
ErrorCode::Api002,
ErrorCode::Api003,
Expand Down
Loading
Loading
0