145 lines
7.3 KiB
Markdown
145 lines
7.3 KiB
Markdown
---
|
||
id: sqlite-commit-error-arm
|
||
name: SQLite engine — commit-error-arm coverage (wave-3 review's deferred test)
|
||
status: completed
|
||
depends_on: []
|
||
scope: narrow
|
||
risk: medium
|
||
impact: isolated
|
||
level: implementation
|
||
tags: [wave-5, sqlite-engine, tests]
|
||
---
|
||
|
||
## Description
|
||
|
||
Land the wave-3 review gate's deferred coverage item
|
||
(`tasks/review-wave-3.md` Notes: "Coverage of the commit-error arm —
|
||
no test induces a failing `COMMIT` ... natural home is wave 5's suite
|
||
hardening"): a test that drives a failing `COMMIT` through the SQLite
|
||
engine's `commit` path and pins the fix the review verified by
|
||
code-read — the writer slot is replenished via the `reopen` closure on
|
||
the failed `COMMIT` (no stranding), the error surfaces as the opaque
|
||
`Database`, and the store remains fully usable afterward (subsequent
|
||
`begin_tx`/auto-commit ops work).
|
||
|
||
The injection mechanism is this task's to find — candidates: a
|
||
`PRAGMA max_page_count` squeeze forcing `SQLITE_FULL` on a
|
||
space-consuming commit, or a `cfg(test)` fault seam in the seam layer
|
||
if the PRAGMA route cannot hit the COMMIT arm deterministically. The
|
||
wave-4 fix batch's `cfg(test)` config-seam precedent
|
||
(`pg-fix-forwarder-reconnect`) is the house pattern if a seam is
|
||
needed. Prefer the least-invasive mechanism that deterministically
|
||
reaches the arm; document the choice.
|
||
|
||
If the same session has cheap room for it, the waves-1–2 review's
|
||
optional add — the watcher reconnect-*success* path test (upstream's
|
||
W-1 test drives only the failure path; "a reconnect success test would
|
||
require a file appearing mid-run") — rides here **only if** the
|
||
engine kept the current watcher shape and the test is genuinely cheap;
|
||
otherwise leave it recorded as not-taken in Notes (it was a
|
||
suggestion, not an order).
|
||
|
||
## Acceptance Criteria
|
||
|
||
- [ ] A test induces a failing `COMMIT` on the SQLite engine and pins:
|
||
error surfaces (`Database`), writer slot replenished (a
|
||
subsequent `begin_tx` succeeds), no partial-commit residue
|
||
- [ ] The injection mechanism documented in Notes (PRAGMA vs seam, and
|
||
why)
|
||
- [ ] The watcher reconnect-success test either landed or explicitly
|
||
recorded as not-taken with the reason
|
||
- [ ] `cargo test -p alkstore-sqlite` green server-less; clippy
|
||
`-D warnings`; fmt clean
|
||
|
||
## References
|
||
|
||
- tasks/review-wave-3.md (Notes: the deferred commit-error-arm coverage)
|
||
- tasks/sqlite-engine-seam-tx.md (the `commit` replenish fix the test pins)
|
||
- tasks/pg-fix-forwarder-reconnect.md (the `cfg(test)` seam precedent)
|
||
- docs/reviews/001-waves-1-2-general-review.md §2 (the watcher
|
||
reconnect-success suggestion)
|
||
|
||
## Notes
|
||
|
||
> Decisions of record the implementation made that the description
|
||
> didn't pin:
|
||
|
||
- **Injection mechanism: the `cfg(test)` commit-fault seam, not the
|
||
PRAGMA route — decided by live probe.** Both PRAGMA candidates were
|
||
tested against real SQLite before choosing: in *both* WAL and DELETE
|
||
journal modes, `PRAGMA max_page_count` squeezed to the current page
|
||
count fails the first growth statement (`SQLITE_FULL` /
|
||
`DiskFull` at page-allocation time), leaving "no transaction is
|
||
active" for the subsequent `COMMIT` — the commit arm is unreachable
|
||
through PRAGMA-space. (Also probed: `max_page_count` is
|
||
per-connection, so arming on the writer conn pre-`begin_tx` would
|
||
have ridden into the tx connection — the route failed on placement
|
||
of the error, not on delivery.) The seam substitutes a
|
||
`SQLITE_FULL`-shaped `rusqlite::Error` for **one** commit and feeds
|
||
it into the *production* commit error arm — the test pins the real
|
||
drop + `reopen` replenish code, not a replica.
|
||
- **Scoping design** (the seam's cross-test-safety shape): the fault
|
||
flag is a per-`SqliteStore` `Arc<AtomicBool>` (born disarmed, armed
|
||
via the `#[cfg(test)]` `arm_commit_fault`, disarmed on first
|
||
consumption — exactly one commit faults and every later commit is
|
||
real, which the test also pins). Parallel test stores can never
|
||
interfere. Plumbing follows the `pg-fix-forwarder-reconnect`
|
||
precedent exactly: `#[cfg(test)]` field on `SqliteStore` and on
|
||
`SqliteTxHandle`, `#[cfg(test)]` param on `SqliteTxHandle::begin`,
|
||
cfg'd statement branches in `store.rs::begin_tx` and
|
||
`src/tx.rs::commit` — production builds compile the plain path.
|
||
- **Replay-proofed live**: with the commit error arm's
|
||
`writer_reopen` replenish temporarily removed, the new test fails
|
||
(the next `begin_tx` parks past its 5 s timeout — the stranding the
|
||
review identified by code-read); reverted, it passes.
|
||
- **The watcher reconnect-success add rides here — taken.** The
|
||
vanished-file route the waves-1–2 review sketched could not reach
|
||
the success body (on file reappearance the identity check reads a
|
||
fresh `(dev, ino)` and the watcher dies through the dead-man's
|
||
switch before the reconnect would matter). Instead the test drives
|
||
`run_poll_loop` directly through the `open_conn_fn` seam the loop
|
||
already takes — the same instrument as the W-1 failure test — with
|
||
the db file present throughout (identity stable): initial open and
|
||
first two reconnects injected as failures, third reconnect
|
||
succeeds, and a commit from a writer connection then wakes
|
||
`on_change` (proving the success arm re-baselines `data_version`
|
||
and restores delivery). Watcher shape untouched.
|
||
- `cargo build`; `cargo test -p alkstore-sqlite` green server-less
|
||
(191 lib + 25 suite, both new tests included); workspace `cargo
|
||
test` 399 passed / 0 failed (core 25 + harness 3; postgres 121 +
|
||
25 suite + 9 schema; sqlite 191 + 25 suite); `cargo clippy
|
||
--all-targets -- -D warnings`; `cargo fmt --check` — all green.
|
||
|
||
## Summary
|
||
|
||
> What landed, verified how:
|
||
|
||
- **`alkstore-sqlite/src/seam.rs`**: the `#[cfg(test)]`
|
||
`commit_fault` module — per-store `Flag` (`Arc<AtomicBool>`),
|
||
`disarmed`/`arm`/`take` (disarm-on-consume), and the
|
||
`SQLITE_FULL`-shaped fabricated `rusqlite::Error` — with the
|
||
mechanism-choice rationale (the PRAGMA probes) in its doc comment.
|
||
- **`alkstore-sqlite/src/tx.rs`**: `#[cfg(test)]` fault field on
|
||
`SqliteTxHandle` + `begin` param; `commit`'s blocking body gains a
|
||
cfg'd branch that feeds the fault into the existing production
|
||
match — the error arm (drop + `reopen` replenish, `sqlite_error`
|
||
mapping) runs unmodified.
|
||
- **`alkstore-sqlite/src/store.rs`**: `#[cfg(test)]`
|
||
`commit_fault` field on `SqliteStore`, the `arm_commit_fault`
|
||
surface, and the cfg-branched `begin_tx` call.
|
||
- **`alkstore-sqlite/src/store/tx_tests.rs`**:
|
||
`failed_commit_replenishes_the_writer_slot` — failed `COMMIT` errs
|
||
opaque `Database` with the SQLITE_FULL source chain; the next
|
||
`begin_tx` proceeds (slot replenished, no stranding); no
|
||
partial-commit residue; the post-failure commit is real and clean
|
||
(fault disarmed); auto-commit `notify` works afterward.
|
||
- **`alkstore-sqlite/src/substrate/watcher.rs`**:
|
||
`reconnect_success_resumes_wake_delivery` — the waves-1–2 review's
|
||
optional reconnect-success add, driven through the loop's
|
||
`open_conn_fn` seam (injected failed opens, then a successful
|
||
reconnect; wake delivery resumes and is pinned). Recorded as
|
||
*taken* per the task's optional clause.
|
||
- **Gates**: `cargo build`, `cargo test` (server-less: core 25 + core
|
||
suite 25 + sqlite 191 + postgres 121 + 9 schema — 399 passed / 0
|
||
failed, postgres harness skipped server-less), `cargo clippy
|
||
--all-targets -- -D warnings`, `cargo fmt --check` — all green. |