savepoint4/crash2 stay quarantined: no rollback journal, and the commit point is a main-file sync
- Dominant language
- C
- Stars
- 268
- Forks
- 18
- Avg merge
- 2h 27m
- Merged PRs (30d)
- 447
Description
Follow-up to #2045, which is closed: its ask (decide the policy, port the coverage) was delivered by PR #2047. The two gates it justified are still in place and still needed, so per `check_testfixture_inventory_issues_alive.sh` they need a live issue that describes the limitation as it stands now. Filed from #2070.
## The property (unchanged)
DoltLite's chunk-store commit becomes durable at a **sync of the main db file**, and DoltLite writes **no rollback journal**. `crashsql` only crashes on syncs. Stock's commit point (the journal delete) lies strictly after the last main-file sync, which is what makes "any simulated crash recovers the pre-transaction state" true for stock and false here. No commit-protocol change has landed since #2045 (only `9169702773`, a graph-lock handle reuse, which does not move the commit point).
## Current state, measured on master `4ddd48e13b`
| suite | result | breakdown |
|---|---|---|
| `savepoint4` | 1 error / 195 | `savepoint4-2.8.1.2` — a main-db crash recovered the committed state. Layout-dependent (the #2045 coin flip). |
| `crash2` | 60 errors / 84 | `crash2-1.1` raw page-format file-size assertion (3072 bytes); `crash2-2.$i.1` + `crash2-2.$i.2` for i=1..29 (**58 assertions**) the no-rollback-journal class; `crash2-3.1.2` the commit-at-sync coin flip. |
The two suites differ in an important way. `savepoint4-2` starts on `-file test.db-journal` and, when the child does not crash, **falls through** to `-file test.db` and re-baselines its signature — so it adapts to a journal-free engine on its own, and its only failure is the layout-dependent one. `crash2-2` has no such fallback: it crashes on `test.db-journal`, the child completes instead of crashing, and both its "exited abnormally" and signature assertions fail deterministically. Those 58 are the same class already enumerated per-assertion for `crash3-1.odd.3` in `known_testfixture_divergences.txt`.
`crash2-3.$i.1` (main-db crashes) all pass — the crash does fire there.
## Why the exception stands
Both suites hard-code the rollback-journal guarantee, so they cannot be made to pass without a commit-protocol change. The recovered states are consistent and within SQLite's atomic-commit contract (the commit was never acknowledged); replacement coverage that accepts either outcome plus `integrity_check` lives in `test/doltlite_recover_crash_commit.test`, registered for both suites in `known_testfixture_crash_coverage.txt`.
## Work this issue tracks: narrow the quarantine
`known_testfixture_crashes.txt`'s own header says it is for files that "cannot produce a comparable failure list at all -- they abort, time out, or their fault sweep is capped", and warns that "quarantining a whole suite to quiet one assertion costs every other assertion in it". Both suites **do** produce comparable failure lists, so the whole-suite quarantine is over-broad: it silences 84 + 195 assertions to accommodate failures that are mostly deterministic and enumerable.
Proposed direction:
1. Enumerate the deterministic ones as ordinary per-assertion gates in `known_testfixture_divergences.txt`: `crash2-1.1` (raw format) and the 58 `crash2-2.*` no-journal assertions, both `class=intentional`, matching the existing `crash3` treatment.
2. Mark only the genuinely layout-dependent assertions `unstable` with a pointer to this issue — `crash2-3.*.2` and `savepoint4-2.*.1.*`.
3. Drop both suites from `known_testfixture_crashes.txt`, restoring enforcement of everything else in them.
Step 2 is the blocking unknown and the reason this is not a mechanical change: which assertion the coin flip lands on moves with WAL layout and the SQL `random()` stream, so the `unstable` set has to be established **across build flavors and platforms** before it can be trusted (the corrupt4 lesson — raw-outcome sets that vary by build). That needs its own PR and a full CI matrix, not a local run.
An alternative that would remove the limitation outright — making the commit-effective write land after the final sync returns (journal-delete emulation) — was option 2 in #2045; it narrows but cannot eliminate the window, and the durability tradeoffs need their own scrutiny.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with known_testfixture_crashes.txt, known_testfixture_divergences.txt, and the existing crash3 entries, then review test/doltlite_recover_crash_commit.test and the suite registrations. Run the crash2 and savepoint4 coverage across the required build flavors and platforms to establish the unstable assertion set. Done means deterministic divergences are enumerated, only layout-dependent assertions remain unstable, and the whole-suite quarantines are removed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, sqlite
- Domain
- databases, testing
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100