Notes: a save never overwrites an outside edit that lands mid-save (#222)

kayg commented

2026-10-14 11:59:32 +00:00

Owner

Fixes #222.

The bug

room/save.rs checked the file against our last write, then wrote it. Between the check and the rename() came:

the pending-save history write (SQLite, fsync), and
the temp file's write and fsync.

If another app renamed its version over the note inside that window, our rename() replaced it. The watcher's later event then read our own bytes, matched our hash, and ignored them. The edit was gone and everyone agreed on the result: the perf storm's symptom exactly.

The fix

calternal_fsio::write_unless_changed (new, crates/fsio/src/swap.rs):
1. Checks the file once more right before the write, keeping it open.
2. Swaps our file in with one atomic exchange: renameat2(RENAME_EXCHANGE) through rustix, RENAME_SWAP on macOS.
3. If what it replaced is not the very file it checked (same device and inode), another app wrote in between. That file is swapped straight back and returned.
- On systems without an exchange, it is the check followed by a plain rename: a much smaller window, but not none.
- The temporary name is never deleted after the swap on an error, so another app's file can't be thrown away.
room/save.rs: when the write hands a file back, it is ingested like any outside edit (diffed against the base) and the save loop tries again. The save checks the disk first in the same way.
ADR 0020 gains one line.

Tests

tests/storm.rs, the regression test, runs 3 storms by default. CALTERNAL_STORM_RUNS and CALTERNAL_STORM_SEED set more. Each storm has:
- 3 typists,
- saves on their own plus 20 on request at random moments,
- another app rewriting the file every 0–8 ms (read, append a line, write a temp file, rename).
- It passes when every outside line is in the note exactly once, all clients equal the server, and the file equals the note.
An fsio unit test covers the three cases: no file, someone else's version at the check, and someone else's version renamed in right after the check.
cargo test -p calternal-fsio -p calternal-notes passes (141 + 7), and clippy is clean with -D warnings.

Numbers (one storm per seed, debug build, on a shared machine)

build	storms	lost a line
dev (unfixed)	300	296
this PR	4,400+	1 (seed 292, two lines; see the issue)

Write cost: the extra work is one more read of the note file before the swap. That is noise next to fsync. On a disk-backed folder both writes take 20–60 ms per save on this busy machine; on tmpfs both take 30–300 µs.

🤖 Generated with Claude Code

https://claude.ai/code/session_018Yyy8fQGxPLkLa8v8NXz7T

Fixes #222. ### The bug `room/save.rs` checked the file against our last write, then wrote it. Between the check and the `rename()` came: - the pending-save history write (SQLite, fsync), and - the temp file's write and fsync. If another app renamed its version over the note inside that window, our `rename()` replaced it. The watcher's later event then read our own bytes, matched our hash, and ignored them. The edit was gone and everyone agreed on the result: the perf storm's symptom exactly. ### The fix - `calternal_fsio::write_unless_changed` (new, `crates/fsio/src/swap.rs`): 1. Checks the file once more right before the write, keeping it open. 2. Swaps our file in with one atomic exchange: `renameat2(RENAME_EXCHANGE)` through rustix, `RENAME_SWAP` on macOS. 3. If what it replaced is not the very file it checked (same device and inode), another app wrote in between. That file is swapped straight back and returned. - On systems without an exchange, it is the check followed by a plain rename: a much smaller window, but not none. - The temporary name is never deleted after the swap on an error, so another app's file can't be thrown away. - `room/save.rs`: when the write hands a file back, it is ingested like any outside edit (diffed against the base) and the save loop tries again. The save checks the disk first in the same way. - ADR 0020 gains one line. ### Tests - `tests/storm.rs`, the regression test, runs 3 storms by default. `CALTERNAL_STORM_RUNS` and `CALTERNAL_STORM_SEED` set more. Each storm has: - 3 typists, - saves on their own plus 20 on request at random moments, - another app rewriting the file every 0–8 ms (read, append a line, write a temp file, rename). - It passes when every outside line is in the note exactly once, all clients equal the server, and the file equals the note. - An fsio unit test covers the three cases: no file, someone else's version at the check, and someone else's version renamed in right after the check. - `cargo test -p calternal-fsio -p calternal-notes` passes (141 + 7), and clippy is clean with `-D warnings`. ### Numbers (one storm per seed, debug build, on a shared machine) | build | storms | lost a line | |---|---|---| | dev (unfixed) | 300 | 296 | | this PR | 4,400+ | 1 (seed 292, two lines; see the issue) | Write cost: the extra work is one more read of the note file before the swap. That is noise next to fsync. On a disk-backed folder both writes take 20–60 ms per save on this busy machine; on tmpfs both take 30–300 µs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_018Yyy8fQGxPLkLa8v8NXz7T

kayg added 2 commits

2026-10-14 11:59:32 +00:00

Notes: a save never overwrites an outside edit that lands mid-save (#222 ) 2d95cc5d31

A save checked the file against our last write, then (after the pending
history write and the temp file's fsync) renamed its file over the note.
Another app renaming its version in during that window was overwritten
without a trace: its echo later read our bytes and matched by hash. This
is the line the perf storm lost.

Saves now write through calternal_fsio::write_unless_changed: the file is
checked again just before the write, and our file is swapped in with an
atomic exchange (renameat2 RENAME_EXCHANGE). If what it replaced is not
the file that was checked, it is swapped straight back, ingested like any
outside edit, and the save tries again.

The storm test (tests/storm.rs) reproduces it: 3 typists, saves on their
own and on request, another app rewriting the file every 0-8 ms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Yyy8fQGxPLkLa8v8NXz7T

fsio: keep the checked file open across the swap (#222 )

CI / web (pull_request) Failing after 30s

Details

CI / rust (pull_request) Successful in 2m12s

Details

67dbfcbb10

Its inode number can't then be reused by a newer file and mistaken for it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Yyy8fQGxPLkLa8v8NXz7T

kayg referenced this pull request

2026-10-14 12:09:29 +00:00

Storm: one outside rewrite never landed (possible lost edit) #222

kayg referenced this pull request

2026-10-14 12:13:35 +00:00

Merge train 2026-10-14: #259 #243 #251 #252 #254 #240 #264 #261 #265

kayg commented

2026-10-14 12:13:53 +00:00

Author

Owner

Landed in merge train #265.

kayg closed this pull request