[lawrencecchen]

Summary

Follow-up 4 of the third review of #34901 (S2): the open's syncs per replayed chat, measured against the daemon's 8 s start budget, then cut. Its base is now feat-cmux-next: #35483, #35562 and #35895 have landed, and the S3b head (#35565) is merged in. Behind the same switch, store.journal (default off), which stays off until this PR and the data-loss follow-ups (#35905, #35915) land.

An acpmux daemon that a kill or a crash ended replays the journal at its next start, before it binds its socket. Each replayed chat cost a few syncs, and on macOS a sync of a file is a drive cache flush (F_FULLFSYNC), so the daemon served nothing for seconds.

Open-time bench: distributions, base and head on the same Mac

open_time_after_a_kill_with_replayed_chats (scratch, never lands; branches fix/r-durability-openbench-dist2-{base,head}-nomerge): N chats each with 5 lines and a metadata image past the checkpoint, a kill, then the open. 10 runs per size, sizes interleaved, real files. Both on owned Mac aws-m4pro-8, base first, then head:

• base: fleet 29724350 at 86441c46001 (S3b head 66e1df5ec5e + the bench);
• head: fleet 28d76c7f at 237017ea1ae (this branch at b8159d903c8 + the bench).

Measures, in ms (median of the 10 runs / max):

• ready: the store's open (the daemon binds its socket after it) plus the person's first change after it (a line to a replayed chat) until the S2c gate passes it (its record durable): the time the person waits at a start.
• durable: the start until a checkpoint after that change and a new chat's ends (every replayed record durable and retired).

Chats	Ready, base	Ready, head	Durable, base	Durable, head
10	225 / 249	9 / 82	264 / 284	168 / 390
100	2,035 / 2,118	23 / 28	2,076 / 2,159	764 / 1,150
300	6,224 / 9,000	39 / 48	6,262 / 9,122	2,160 / 3,369

The parts, median / max (base, then head):

• open alone: 10 chats 221 / 245, 7 / 76; 100 chats 2,028 / 2,112, 21 / 26; 300 chats 6,221 / 8,980, 38 / 45.
• first acknowledged change (its record through the gate): 10 chats 3.6 / 3.9, 1.8 / 6.4; 100 chats 3.8 / 9.4, 1.8 / 2.4; 300 chats 3.8 / 20.5, 1.9 / 2.4.
• a new chat (its record and its folder's sync through the gate): 10 chats 12.1 / 15.8, 11.7 / 32.5; 100 chats 14.6 / 29.8, 3.8 / 10.9; 300 chats 13.1 / 25.1, 4.3 / 5.8.

The head is faster in every measure. The 10-chat maxima on the head (ready 82 ms, open 76 ms, new chat 32.5 ms, durable 390 ms) all come from run=0 chats=10, the first open in the test process; every later 10-chat run opened in 5.8 to 11.3 ms, and run=0 of 100 and 300 chats (in the same process right after) is at their medians (19.2 and 36.1 ms). So it is a cost of the first open in a process (about 65 ms here), not host noise. A daemon start is a first open in its process, so the start to expect with 10 chats is that run's: 76 ms open, 82 ms ready (the base's first run: 113 ms open, 114 ms ready). Where those 65 ms go is not measured (not a split by step); follow-up R-DURABILITY.

Does a visible action wait for "durable"?

No, after the fix below. On the base, the open itself made everything durable (open = durable), and the daemon answered nothing until then. On the head, the open returns at once; the checkpoint that makes the replayed records durable runs on the checkpointer's thread:

• Replies and events wait only for their own record (one group commit, the S2c gate): the first acknowledged change above, at most 2.4 ms with 100 or 300 chats replaying.
• A new chat waits for its record and its folder's sync (S3a): the new chat above.
• The durable time is the checkpoint's, after the daemon serves; the base had the same work before it served (100 chats: 2,076 ms before any answer, against the head's 23 ms to the first answer and 764 ms to durable).
• No request path calls Store::flush (only the shutdown does, after the agents stop, within its 5 s budget).
• One path did wait for the checkpoint, found while answering this: the retention worker's first pass runs right after a start, and commit_compaction ran a checkpoint under the chat's append lock (hub/retention.rs), so a person's first message to that chat waited for the open's checkpoint. Fix f9c6a16d1b8: Order::log_lsn keeps each chat's newest log record in this process; a chat whose records are all checkpointed (retention takes chats untouched for days) runs no checkpoint; any other still checkpoints first. Red d860bef9d32: a_retention_rewrite_of_a_checkpointed_chat_does_not_wait_for_a_checkpoint; guards a_retention_rewrite_of_a_chat_with_records_past_the_checkpoint_checkpoints_first, a_retention_rewrite_of_a_replayed_chat_checkpoints_first.

Until the checkpoint ends, the journal keeps every replayed record, so a second crash replays them again; nothing acknowledged depends on the checkpoint.

What changes

• The checkpoint and the recovery sync in write order (log_sync::sync_segment_ordered, sync_directory_ordered: macOS F_BARRIERFSYNC, Linux fdatasync/fsync) and flush the drive once, with the checkpoint file (sync_full), before they retire any record. cmux_journal::sync_file is public for this.
• The open syncs nothing per replayed chat:
  • A log that already holds every merged line, in order, with nothing torn, keeps its bytes. The open writes the bytes it just read and checked against the journal back over themselves (one write per segment, no sync) and lists the segment (Order::read_segments). The next checkpoint syncs each listed segment before it retires their records, through the handle the open wrote with (on Unix, up to a quarter of the open-file limit) or else by path (past that cap, and on Windows): a killed daemon's writes may be in the page cache only, and a previous run's failed sync may have dropped pages that still read back. The write marks those pages dirty again, so the sync puts them on the disk either way, with no need for a durable note of the failure (delta review 2, D2). The write happens at the open, before any other thread uses the logs, and the checkpoint writes nothing, so a file that a delete, a folder replace or a retention rewrite put at the path since is never changed (delta review 3, D3). A write error at the open takes the slow path for that chat (journal, cut, write again); a delete drops its chat's listed segments.
  • The replay puts each chat's newest metadata image in the overlay; the checkpoint writes session.json.
  • The checkpoint after the replay runs on the checkpointer's thread at once, not on the open's thread.
• A retention rewrite of a chat whose records are all checkpointed runs no checkpoint (Order::log_lsn, a per-chat upper bound of its newest log record in this process): it no longer waits, under the chat's append lock, for a checkpoint in flight such as the open's.
• A freeze (a failed sync) is a frozen journal record (kind 6), a control record that a full queue never refuses (Journal::append_control); the journal closes only after the shutdown's last checkpoint. The open that replays it writes every replayed chat's lines again. A record the journal still refuses (its writer stopped) leaves a durable marker, <home>/journal-frozen, which the open reads the same way and the first checkpoint that succeeds after it removes (never while the store is frozen). A chat whose lines come from a repair file (acpmux: event journal follow-ups: a failed repair keeps its records, a hole's last zero splits, an idle checkpointer sleeps #35562) is written again too.

Risk

risk:high, the label the risk classifier set at 02:35Z for the push that added Journal::append_control (it had set risk:normal before; agents do not change it). It lands on an independent SHIP at the exact head and greens on macOS and Linux at that head, both stated below. Behind the default-off switch. It changes when the store's files reach the disk after a crash: the open returns before the replayed state is synced, and the checkpoint on its own thread makes it durable before any record is retired. Every record stays in the journal until then, so a second crash replays the same records again.

Testing

acpmux store::journal_meta_tests and store::journal_store_tests:

• an_open_that_replays_many_chats_flushes_the_drive_a_bounded_number_of_times (40 chats; at most 8 full syncs)
• an_open_after_a_kill_syncs_nothing_per_chat_and_the_checkpoint_makes_it_durable (40 chats; at most 4 syncs on the opening thread; then an OS crash after the checkpoint keeps every line)
• a_freeze_makes_the_next_open_write_the_lines_again_and_only_that_open (the freeze record), and the review tests listed in the review sections below
• the S2 restart tests now flush after the reopen (the checkpoint runs on its own thread)
• a_retention_rewrite_of_a_checkpointed_chat_does_not_wait_for_a_checkpoint; guards a_retention_rewrite_of_a_chat_with_records_past_the_checkpoint_checkpoints_first, a_retention_rewrite_of_a_replayed_chat_checkpoints_first
• the open-time bench (scratch, never lands): open_time_after_a_kill_with_replayed_chats (10, 100 and 300 chats, 10 runs each; it prints ACPMUX-PERF openbench)

Red:

• b2240f1b050: fleet 86535ce1 (aws-m4pro-4): "the open flushed the drive 283 times for 40 replayed chats".
• 6def18f92c5: fleet 90d87e10 (aws-m4pro-5): "the open synced 283 times for 40 replayed chats before it returned".

Red, the freeze marker (a mutant at 07dc13192c8: this branch at 05cb007eeba with the open ignoring journal-frozen; branch fix/r-durability-rsync-nomarker-nomerge, never merged): an_os_crash_after_a_restart_from_a_failed_sync_keeps_every_acked_line fails, in the full run and alone, "the restart's checkpoint retired records whose lines a failed sync lost": macOS fleet cbd6e808, Linux fleet e4e9634c (hq run 41899881551). Nothing else fails.

Red, the retention wait, at d860bef9d32: a_retention_rewrite_of_a_checkpointed_chat_does_not_wait_for_a_checkpoint fails, in the full run and alone, "the retention rewrite of a checkpointed chat waited 1.72083425s for a checkpoint in flight" (macOS fleet 80f5ee16) and 1.70 s (Linux fleet 623bf9af, hq run 41908393200); the two guards pass.

Green at the head f9c6a16d1b8447f40274186314a64f67b80253c8:

• macOS: fleet eb0777e8 (owned Mac aws-m4pro-3), journal_ 356 of 356; fleet a93736f5 (owned Mac aws-m4pro-6), retention 29 of 29 (the hub's retention tests and the three new ones).
• Linux: fleet e109a17e (hq run 41908624629), all store::: fmt, clippy and 357 of 357 tests (acpmux 159, cmux-tui 41, cmux-tui-core 154, cmux-worktree-care 3).
• At b0aee122fe2 (before the retention fix; the merge of the S3a N1 fix): macOS fleet eeaf9997, journal_ 353 of 353; Linux fleet 1e12de8d (hq run 41907479654), all store::: fmt, clippy and 354 of 354.

At 05cb007eeba (the merge of the review fixes, before the retention fix): macOS fleet ea82537d (journal_ 345 of 345), ee6bc8a4 (a_failed_log_sync_stops_the_daemon_for_a_restart 1 of 1); Linux fleet c44c6512 (hq run 41898799949, all store::: clippy and 350 of 350 tests; fmt red only on the base files #35845 fixed). fmt at b8159d903c8 after #35845: fleet e56e4c55, green.

Earlier, at b80145ec8a6: fleet bfb0a243 (macOS, journal_ 328 of 328); fleet 0577ac62 (Linux, store:: 331 of 332, with one failure from the idle checkpointer race that #35562 fixed at 5279af4bbf7; see that PR).

• a_chat_deleted_and_made_again_never_takes_the_old_chats_verified_bytes, a_folder_replaced_outside_the_store_never_takes_the_old_chats_verified_bytes, a_failed_write_back_at_the_open_writes_the_lines_again (store::journal_write_back_tests, delta review 3)

Observed / Not run

• Observed: the fleet steps above, each log read.
• Not run: a dogfood daemon with the switch on and a real kill; a power-loss model of the barrier order (the argument is in the code comments and DESIGN.md 3.3e); the bench on Linux after this PR; the bench on another mini class (both distributions are from aws-m4pro-8; the M6 minis were not used).

Review of f9c6a16d1b8: what changed (P1-1, P2-1, P2-2)

Red tests first (the reviewer's) in 05493cad342. They fail at that commit, in the full run and alone, each for its reason (macOS fleet a3bd72b0, 3 of 27; Linux fleet 0f24f011):

• a_failed_checkpoint_after_the_open_keeps_the_segments_it_read (P1-1).
• a_freeze_during_a_checkpoint_still_makes_the_restart_write_the_lines_again (P2-1).
• a_freeze_in_a_home_that_takes_no_new_file_still_writes_the_lines_again (P2-2).

The fixes:

• f0ebe9e0ac4 (P1-1): checkpoint_once took Order::read_segments at its start and put the set back only when sync_read_segments itself failed. A checkpoint that failed before it (a journal sync slower than its wait, an image that could not be written) dropped the set, and the next checkpoint retired records whose log bytes no sync of this process covered. Now the checkpoint reads a copy and removes those paths only after their sync succeeds.
• 101232eeb4d (P2-1 and P2-2): both came from one root. The restart learned of a freeze from a marker file whose write and removal the journal did not order. Now freeze appends a frozen journal record (kind 6) under the appends' lock, after every line it covers. The open that replays it writes every replayed chat's lines again. No checkpoint can take it back, because a checkpoint in flight read its LSN before the record. The record needs no write that can fail apart from the journal's own. The marker file is gone. A chat whose lines come from a repair file is written again too (Order::rewrite).
• 24cc4ae39e4: a compile fix for 101232eeb4d (E0027: apply did not name the replay's new frozen field).
• Green at 24cc4ae39e4: macOS journal_ (fleet 8095cfdd), Linux all store:: (fleet 65386a18), macOS restart tests (fleet ea9d4c48).

Merged down (head 60cce5e745b)

• 0befb17bc02 merges the S3b head 6a5a6d138c9 (acpmux: a tags or rules change logs its line before its metadata (R-DURABILITY S3b) #35565: feat with acpmux: event journal follow-ups: a failed repair keeps its records, a hole's last zero splits, an idle checkpointer sleeps #35562 squashed). The merge base is the S3b head this branch had (7fc15b776ef). One conflict, in journal_store_tests.rs: both sides appended tests at the end. Both blocks are kept whole (45 tests = 41 + 40 - 36).
• 60cce5e745b merges feat b536ff861c5, which has acpmux: a checkpoint never writes an LSN past a sync that failed on another thread (R-DURABILITY; culprit 71bc09a1e63, S3a) #35895 (a checkpoint never writes its LSN past a sync that failed on another thread). Conflicts in 3 files, all resolved by keeping both sides:
  • checkpoint.rs: acpmux: a checkpoint never writes an LSN past a sync that failed on another thread (R-DURABILITY; culprit 71bc09a1e63, S3a) #35895's counted_sync, then this PR's journal-record freeze(). checkpoint_once keeps this PR's start (P1-1) and acpmux: a checkpoint never writes an LSN past a sync that failed on another thread (R-DURABILITY; culprit 71bc09a1e63, S3a) #35895's end: the closes' note read under the writers' lock, the wait for the syncs under way, then frozen.
  • log_sync.rs: acpmux: a checkpoint never writes an LSN past a sync that failed on another thread (R-DURABILITY; culprit 71bc09a1e63, S3a) #35895's close_sync_failed beside this PR's ordered sync_log, with acpmux: a checkpoint never writes an LSN past a sync that failed on another thread (R-DURABILITY; culprit 71bc09a1e63, S3a) #35895's test seam before its segment sync.
  • journal_store_tests.rs: both appended blocks (49 tests = 45 + 45 - 41).
• One effect of the two together, fixed in this PR (lead's review): a failed counted sync froze the store inside its counted region, and this PR's freeze() appends the frozen record. When the journal's queue is full, that append waits for the writer (up to 5 s) and appends again, so a checkpoint in flight waited for that too. Now freeze() has two steps:
  • freeze_now, inside the count and under the appends' lock: frozen set and the record appended. The journal's append never blocks (it checks its queue and copies the record), so this step is as short as before, and a seal still cannot come between frozen and the record.
  • freeze_later, after the ticket ends: only when the record found the queue full, the wait for the writer and the second append; then the ask to stop, so the shutdown's seal still comes after the record.
  • The panic path takes the same two steps around the ticket's end.
• The checkpoint's wait, with this: it ends when the syncs under way at its start have ended, each with its freeze's first step (one lock and one queue copy) when it failed. It never waits for a freeze's retry, nor for a sync that begins later. This is the bound of acpmux: a checkpoint never writes an LSN past a sync that failed on another thread (R-DURABILITY; culprit 71bc09a1e63, S3a) #35895 again.
• Which threads freeze, and who waits: the freeze runs on the thread whose counted sync failed. That is a request's own thread for flush_session (some callers are async functions on a runtime worker, such as fork_with_boundary and transfer_finish; fork_before and commit_watch_page call it through spawn_blocking), the caller of session_dir, the checkpointer (its own session.json writes; its ticket ends before its wait, so it never waits for itself), or the folder syncer. No request path runs a checkpoint, so before this fix no request waited for the checkpointer. One indirect wait existed: the retention pass holds the chat's append lock while commit_compaction runs a checkpoint (when the chat is not covered yet), so an append to that chat could wait for that checkpoint's extra wait. That chat is closed and untouched, so this was rare. The shutdown's flush could also wait the extra 5 s. Both now wait only for the syncs.
• Red: a_checkpoint_does_not_wait_for_a_frozen_record_on_another_thread fails at df1815e7fd4 ("a checkpoint waited for another thread's frozen record, not only for its failed sync", macOS fleet 63217ea0), and in its queue-full form at 9e90c42de24 (macOS fleet c20b3ace, same message, in the full run and alone). Handshakes only: the checkpoint is held after its start; the failing thread's record finds the queue full (FORCE_FROZEN_BACKLOG) and it stops before its wait for the writer (HOLD_BEFORE_FROZEN_RECORD); the checkpoint must return, refused, before that thread goes on. After the release, the restart must still write the lines again.
• At 60cce5e745b (before the freeze split): fleet clippy -D warnings green (fleet a8038a8a, hq run 41939067327); macOS journal_ 368 of 368 (fleet 2e614ff0); Linux all store:: 369 of 369 with fmt and clippy (fleet 4332aaf1, hq run 41940083373).
• Green at the head f62feba6e3d: macOS journal_ (fleet d7884d24) 369 of 369; Linux all store:: with fmt and clippy (fleet 1172395c) 370 of 370 (Blacksmith; hq run 41946086686).

Delta review of f62feba6e3d (FIX): what changed (D1)

D1 (P2): when the journal refused the store's frozen record, the freeze only logged an error, and a restart in the same boot trusted the log pages the failed sync dropped; an OS crash then lost acknowledged lines. Three ways, one test each, red at 4a883c4f006 (and test 1 armed before the seal at 32fd1f845f4) on macOS (fleet c07e8938, ce3f21e3) and Linux (fleet 5234e655, hq run 41954644923), each in the full run and alone, each lost lines 2 to 5:

• a_log_sync_that_fails_in_the_shutdowns_last_checkpoint_keeps_the_rewrite (the reviewer's): Hub::seal sealed the store, then ran its last checkpoint; a log sync that failed there found the journal closed.
• a_frozen_record_while_the_journals_queue_stays_full_keeps_the_rewrite (the reviewer's, no seam in the store): a slow writer and other chats' lines kept the queue full, so the record was refused twice.
• a_seal_while_a_freeze_is_under_way_keeps_the_rewrite: a failing flush_session held right before its freeze while another thread seals; handshakes only. (The reviewer's third test held the freeze between the two steps of f62feba6e3d, which the fix removes.)

The fix, three commits:

1. 1617013d136: cmux-journal gets Journal::append_control, an append that a full queue never refuses (still Closed when the journal is closed, TooLarge past MAX_PAYLOAD_LEN; unit test a_control_record_is_taken_while_the_queue_is_full). The frozen record uses it (one per store, its reason cut to 4 KiB). So the freeze is one step again, inside the failed sync's ticket, with no retry and no wait: a checkpoint's wait is bounded by the syncs under way (the bound of acpmux: a checkpoint never writes an LSN past a sync that failed on another thread (R-DURABILITY; culprit 71bc09a1e63, S3a) #35895). The two-step freeze of f62feba6e3d, its seams, and the test of its retry are gone.
2. 7b27d29b8be: JournalStore::seal seals the logs, runs the shutdown's last checkpoint while the journal is still open, then seals the journal. A failure in that checkpoint, or in a sync it waits for, journals its record before the journal closes.
3. b3988138ef9 (red at 510cade7cf0: a_refused_frozen_record_falls_back_to_a_durable_marker, macOS fleet 8f90a826, fails, in the full run and alone: "a refused frozen record left no marker"): fail closed. A record the journal still refuses (its writer stopped), or a frozen store whose journal does not close with every record durable, leaves a durable marker <home>/journal-frozen (written whole, fully synced, renamed, the home folder synced), after the failed sync's ticket ends. The open reads it as it reads the record. The first checkpoint that succeeds after that open removes it, never while the store is frozen (so the P2-1 race cannot come back). Guard: a_checkpoint_does_not_wait_for_a_freezes_fallback_marker.

P3s: the module docs, the moved doc comment, and the two P2 review tests' text now describe the freeze as it is; the P2-1 test orders its threads by handshakes (a2475c77387). DESIGN.md 3.3c has the new bound and the rollback note: a build without this PR cannot open a journal with a frozen record (unknown kind 6); with the switch off, its catch-up fails and only logs, and the logs open alone.

Green at the head a2475c77387: macOS journal_ (fleet eca8d85e) 374 of 374; macOS cmux_journal (fleet b0970653) 42 of 42; Linux all store:: with fmt and clippy -D warnings (fleet 296eb354) 374 of 374 (Blacksmith; hq run 41957521689).

Delta review 2 of a2475c77387 (FIX): what changed (D2)

D2 (P2): after a failed sync, the restart trusted a whole log unless it found a frozen record or a marker. A kill before either was durable broke that. Two windows: the record still waited in the journal's queue (its append succeeded, so no marker), or a log's close only set its flag and no checkpoint had read it yet. The base wrote every replayed chat's lines again at each open, so it had neither window; this PR's fast path brought them.

• Red, window 1, at 9125b99c745: a_kill_while_the_frozen_record_waits_in_the_queue_keeps_the_lines, the reviewer's repro (red on fleet e150f624 and e2ba4960), rebuilt with a hold in place of a 2 s sync and a 300 ms sleep (new SimStorage::hold_next_sync). It fails at journal_meta_tests.rs:1353, in the full run and alone, left: [1]: macOS fleet e8626223, Linux fleet a5d401ae (hq run 41965232344).
• Red, window 2, at dcf9ed7a8aa: a_kill_after_a_failed_log_close_before_any_checkpoint_keeps_the_lines (the reviewer found it by reading): it fails at journal_meta_tests.rs:1393 after a clean compile, in the full run and alone, left: [1]: macOS fleet 11761812, Linux fleet e8e5017f (hq run 41966752715).
• Fix at 2a065d25cd3, the reviewer's direction: the checkpoint writes back the bytes the open verified, then syncs (above, "What changes"). It needs no note, so neither window matters. A segment that is gone, replaced (another file id) or shorter is passed; a failed write or sync freezes the store. The crash model learns the write-back (sync_model::rewritten). The frozen record, the seal order and the marker stay for the chats the fast path does not take.
• P3 4: JournalStore::seal's doc comment no longer starts with the old text. P3 3: "What changes" and "Testing" above describe the record, the write-back and the marker. P3 1 (apply_metas does not read rewrite_all, so a session.json from a failed sync is trusted) is R-DURABILITY: four older durability gaps from the reviews of #35895 and #35562 #35915 item 2. P3 2 (at shutdown only, a freeze can wait up to 5 s while the seal holds the appends' lock) is noted here; the wait is the journal's own seal wait, and nothing visible waits behind it.

Green at 6b0547afc43 (before the merge with #35963): macOS journal_ (fleet 1ef76c60) 376 of 376; macOS cmux_journal (fleet d9ac896f) 42 of 42; Linux all store:: with fmt and clippy -D warnings (fleet b33baa1b) 376 of 376 (Blacksmith; hq run 41967437029).

Merged down after #35963 landed (head 7f3e036d277)

#35963 (the #35905 fix) landed on feat and changed the same store files, so this PR conflicted with feat. 2e36ab0db02 merges feat 1b585e3ddca, with conflicts in 2 files, resolved by keeping both sides:

• JournalStore::commit_compaction: this PR's "covered" check (a chat whose newest log record is at or below the checkpoint runs no checkpoint, Order::log_lsn) runs first, then acpmux: a failed sync of a retention rewrite or a kept-aside image freezes the store (R-DURABILITY, #35905) #35963's counted_sync around the commit. A frozen store is not covered, so its checkpoint runs, fails and stops the rewrite, as on both sides.
• counted_sync's doc comment: acpmux: a failed sync of a retention rewrite or a kept-aside image freezes the store (R-DURABILITY, #35905) #35963's list of counted syncs, plus this PR's note that the freeze never waits and that a fallback marker comes after the count.

Checked, with no textual conflict: write_through counts the kept-aside image, which keep_aside_meta now writes through write_kept (a new file each time); the ordered folder sync honors the same test seams, so #35963's FAIL_DIR_SYNC_WHEN tests name the same syncs; and the D2 write-back passes a segment that a retention rewrite renamed over (another file id). 7f3e036d277 is a clean merge-main of feat cd9b4166785.

Green at the head 7f3e036d277: macOS journal_ (fleet c03d8a73) 380 of 380 (the 376 of 6b0547afc43 plus the 4 journal_sync_tests from #35963); macOS cmux_journal (fleet ed5a03c6) 42 of 42; Linux all store:: with fmt and clippy -D warnings (fleet 86498e8b) 380 of 380, fmt and clippy pass (Blacksmith; hq run 41969667583).

Delta review 3 of 7f3e036d277 (FIX): what changed (D3)

• D3 (P2): the checkpoint's write-back found its segment by path, inode number and length (open_verified), and an inode number is not a file's identity. A chat deleted while its write-back was pending and made again under the same id, or a folder that code outside the store replaced (a transfer-in), could put a file with the old number at the path: ext4 hands a freed inode out again (the reviewer's Linux repro got the freed inode 13893637 back), and on Windows nothing was compared. The checkpoint then wrote the old chat's bytes over the new chat's acknowledged lines, synced them and retired their records.

• Red b53a750bcd1: a_chat_deleted_and_made_again_never_takes_the_old_chats_verified_bytes (the reviewer's test, rsync_review5_a_deleted_chats_write_back_never_lands_in_a_new_chats_log, renamed; a second link stands in for the reused inode) and a_folder_replaced_outside_the_store_never_takes_the_old_chats_verified_bytes (the reviewer's request: no delete, the folder renamed aside and a new one put in, as a transfer-in does). Both fail at their own assertions after a clean compile, in the full run and alone: macOS fleet 6b7b8fa2, Linux fleet 55b36bf4 (hq run 41976946052), "the checkpoint wrote the old chat's verified bytes into a new chat's log" (line 179) and "... into a folder put in its place outside the store" (line 229). The reviewer's ext4 test is not taken: on a file system that does not give the inode out again it fails as "inconclusive".

• Fix 4846ec2b09e, the reviewer's direction: the open writes the verified bytes back (Inner::write_back), and the checkpoint only syncs (sync_read_segments). A write error at the open neither fails the open nor freezes: that chat takes the slow path, whose cut also removes a range written back in part (new test a_failed_write_back_at_the_open_writes_the_lines_again). The write opens the segment as every read does, plus the lost-home check (O_NOFOLLOW, never made, owner, mode and macOS ACL checks; Windows owner_only::open_rw_existing): the reviewer's P3 2. Store::delete drops its chat's listed segments.

• 4846ec2b09e kept the handle the open wrote through until the checkpoint synced it. Before its review, the reviewer named two costs of that, and 3452068e332 removes them: the checkpoint now syncs each listed segment by path and holds no handle.

  • Descriptors: one per replayed segment until the first checkpoint that succeeds, longer while checkpoints fail. The daemon raises its soft limit to min(hard, 65536), but when the raise fails the limit stays (256 under launchd), and 300 replayed chats would use them up during the open.
  • Windows: a folder whose file is open may refuse a rename or a removal (a transfer-in's rename aside, Store::delete).
  • What the sync by path gives back: a folder moved aside during the window between the open and its checkpoint (the reviewer's P3 1, a transfer-in that fails and renames its backup back) is passed and its records retired, as before D2. That is now R-DURABILITY: four older durability gaps from the reviews of #35895 and #35562 #35915 item 6 (issuecomment-6652305673), the class of item 4.

• ff6922678dd: clippy -D warnings failed the Linux step at 4846ec2b09e on an unused std::io::Write import in the new tests (fleet bd0548b2, hq run 41979974134; every test passed there).

• Open-time A/B (the reviewer asked for it), the PR's bench, 10 runs per size, all on owned Mac aws-m4pro-8, in ms, median / p90 / max:

  Measure, chats	A: 7f3e036d277 (fleet e5daffbe)	B: 4846ec2b09e (fleet c69599be)	C: 3452068e332 (fleet 93887e54)
  open, 10	5.0 / 9.7 / 9.7	6.1 / 7.6 / 9.3	5.9 / 8.7 / 9.7
  open, 100	17.1 / 20.0 / 21.4	17.1 / 22.8 / 31.4	34.5 / 37.9 / 37.9
  open, 300	35.6 / 43.5 / 47.8	39.1 / 42.3 / 44.9	115.8 / 120.2 / 126.3
  ready, 10	9.0 / 13.5 / 14.1	8.0 / 9.6 / 12.1	8.3 / 14.9 / 16.5
  ready, 100	19.8 / 22.3 / 25.4	18.8 / 25.2 / 34.1	50.4 / 57.6 / 57.7
  ready, 300	37.7 / 45.5 / 49.6	40.9 / 44.9 / 47.1	137.2 / 148.2 / 163.9
  first acknowledged change, 100	2.4 / 4.0 / 4.5	1.8 / 2.4 / 2.6	16.0 / 19.8 / 20.1
  first acknowledged change, 300	2.1 / 2.6 / 2.8	1.9 / 2.2 / 2.6	22.6 / 28.3 / 37.6
  durable, 100	1,323 / 1,939 / 2,166	993 / 1,582 / 2,164	922 / 1,229 / 1,235
  durable, 300	3,364 / 5,407 / 13,054	2,882 / 3,654 / 4,301	2,438 / 3,376 / 3,532

  The three ran one after the other (06:36Z, 07:01Z, 07:31Z), not interleaved. B against A is inside the same-arm spread on the open. C is worse than A and B on the open, ready and the first acknowledged change at 100 and 300 chats, beyond the same-arm spread (C's fastest open with 300 chats, 113.8 ms, is slower than A's slowest, 47.8 ms); with 10 chats nothing changes. In the open, B and C differ in one thing only: B closes each written-back segment after the checkpoint's sync, C right after the write.

  The rerun confirmed it on two more Macs and in both orders: median open with 300 chats, B 52.8 against C 130.8 ms on aws-m4pro-6 (the two run at the same time, fleet dab0d89f and 15f9cd66), B 42.0 against C 121.3 ms on aws-m4pro-9 (C first, then B: fleet 69c4e412, d741646f). The first acknowledged change: 2.3 against 29.4 ms, and 2.4 against 30.4 ms. So on macOS the close right after the write costs the open about a quarter of a millisecond per replayed chat.

• 29967f9f5f7: the open keeps the handle it wrote each segment through (repair::WrittenBack::Held), up to repair::held_cap: a quarter of the open-file limit (which the daemon raises before the store opens), at most 4096, and 64 when the limit cannot be read. The checkpoint syncs a held segment through its handle and closes it on its own thread, after the sync. A segment past the cap, and every segment on Windows, is closed at the open and synced by path, as at 3452068e332. The checkpoint still writes nothing, so D3 stays closed. Within the cap on Unix, a held handle also covers a folder moved aside and put back before the checkpoint (R-DURABILITY: four older durability gaps from the reviews of #35895 and #35562 #35915 item 6), since a rename keeps the file the handle holds. Store::delete takes its chat's entries out under the appends' lock and closes their handles after it lets the lock go. New test: the_written_back_segments_past_the_cap_are_synced_by_path (a cap of 1: one segment held, two closed, then every acknowledged line after the checkpoint and an OS crash).

• Open-time A/B at 29967f9f5f7 (arm E), median / min to max, ms:

  Measure, chats	A 7f3e036d277, aws-m4pro-6 (fleet c74909f1)	E 29967f9f5f7, aws-m4pro-6 (fleet 1021ba81)	E 29967f9f5f7, aws-m4pro-8 (fleet cfcb5ffd)
  open, 10	10.2 / 5.3 to 13.6	10.0 / 6.3 to 12.2	9.9 / 5.2 to 17.8
  open, 100	19.1 / 16.0 to 55.0	22.4 / 17.9 to 33.9	20.9 / 18.8 to 28.1
  open, 300	40.6 / 38.4 to 44.0	41.6 / 36.0 to 61.7	48.3 / 40.4 to 68.9
  ready, 300	43.2 / 40.3 to 46.2	44.0 / 37.5 to 63.5	50.7 / 43.0 to 73.4
  first acknowledged change, 300	2.5 / 1.6 to 2.7	2.4 / 1.6 to 3.5	2.6 / 1.8 to 5.3
  durable, 300	4,541 / 4,435 to 5,143	4,358 / 4,305 to 6,222	4,360 / 4,282 to 5,977

  A and E on aws-m4pro-6 ran at the same time, so they share the host's load: every difference is inside the same-arm spread, so the head is not worse than the code before D3 by the floor rule. E on aws-m4pro-8 ran an hour and a half after the D draft there (09:32Z against 07:59Z) (18612dc, which has the same open path: it differs only in the delete), whose open with 300 chats was 40.8 ms; that gap is the host between the two times, not the code.

Green at the head 29967f9f5f72b3fc668a1d20c6d5211edbc92f81: macOS journal_ (fleet dbb06014) 384 of 384; macOS cmux_journal (fleet 31ef1527) 42 of 42; Linux all store:: with fmt and clippy -D warnings (fleet fc960a45, Blacksmith, hq run 42003274872) 384 of 384. Each has the four journal_write_back_tests. (At 3452068e332: 383, 42 and 383 of 383, fleet 01ee21d5, 4fff42b7, e125df6c.)

Changelog

none

Proof

No capture: a daemon storage change behind a default-off setting, with no UI.

[lawrencecchen]

Review (r-durability-rsync-reviewer): FIX at 7f3e036

Delta review 3: a2475c77387..7f3e036d277 in cmux-tui/crates/acpmux/src/store/ and cmux-tui/crates/cmux-journal/. I read the D2 reds and fix, the seal doc fix, and both merges (2e36ab0db02 with git show --remerge-diff, and the clean 7f3e036d277). Line numbers are at 7f3e036d277.

Repro tests (never merge): branch fix/r-durability-rsync-review-inode-reuse-nomerge at 19ddd526b58, which is this head plus store/journal_review_tests.rs (prefix rsync_review5_). Fleet cmux-tui-rust-check.sh test rsync_review5, each log read:

• macOS: fleet 857c0030 (aws-m4pro-5). The deterministic test is red, in the full run and alone. The real-inode test is Linux only.
• Linux: fleet 878ccf7a (hq run 41971869419, Blacksmith Ubuntu 24.04). Both tests are red, in the full run and alone.

D2 from my last review: fixed

• The open keeps each verified range with its bytes (repair.rs:213-234). The checkpoint writes them back and syncs (checkpoint.rs:160-166, :210-236). This needs no note of the failure, so both windows close. The reds and the greens are as stated: e8626223 and a5d401ae for window 1, 11761812 and e8e5017f for window 2.
• The range is exactly the bytes the open verified. In the whole case every piece is a whole line equal to a merged line, so the range runs from the repair start to the segment's end at the read. Later appends go after it (O_APPEND). Within one file, nothing in this process changes those bytes: only the open's own repair cuts, and a compaction renames a new file over the path.
• Window 2 is also covered for a chat that this run did not replay. Its lines since the last checkpoint are journaled, so the next open replays them, verifies them and writes them back.

Finding

D3 (P2). The write-back finds its file by path, inode number and length. An inode number is not a file's identity, so the write-back can write a deleted chat's bytes into a new chat's log.

• Where:
  • open_verified (checkpoint.rs:598-622) accepts the file at the path when its inode equals the one read at the open and the file is long enough. On Windows it checks no identity at all (same = true, :618).
  • Store::delete (journal_store.rs:787-816) does not drop the chat's entries from Order::read_segments.
• Scenario:
  1. A restart replays chat X on the fast path, so its segment range waits for the write-back.
  2. Before the write-back, X is deleted (a purge) and made again under the same id. The trigger bridge's fixed ids allow this once the folder is gone (refuse_stored_fixed_id, hub/lifecycle.rs:1812).
  3. The new chat's first segment is a new file at the same path. ext4 and XFS give a freed inode number out again, and Windows compares nothing.
  4. When the new log is at least as long as the old range's end, the checkpoint pwrites the deleted chat's bytes over the new chat's lines and syncs. Then it retires the new chat's records.
  5. The new chat's acknowledged lines are gone, and its log shows the deleted chat's lines in their place.
• The same happens with no Store::delete on Windows: a transfer-in that replaces the chat's folder removes the old folder itself (hub/transfer_in.rs:669), and any file at the path passes. So dropping the entries at Store::delete alone does not close D3.
• The window is the open's first checkpoint. That is seconds with many chats (the PR's bench: 2,160 ms median to durable with 300 chats), or minutes while checkpoints fail before the write-back (the entries stay listed, by the P1-1 fix).
• The codebase already treats an inode number alone as no identity: FolderId is the inode and the birth time (meta.rs:64-70), and "another folder made under the same path does not [keep it]".
• Evidence (every red is at 19ddd526b58, this head plus the tests):
  • rsync_review5_a_deleted_chats_write_back_never_lands_in_a_new_chats_log is a stand-in for a reused inode, deterministic on any file system. The deleted segment is kept by a second link, then linked back emptied as the new chat's first segment, so it has the old file id and the new chat's content. Red on macOS (857c0030) and Linux (878ccf7a): "the checkpoint wrote the deleted chat's verified bytes into the new chat's log (another file with the same file id)".
  • rsync_review5_a_reused_inode_never_takes_a_deleted_chats_bytes is Linux only and uses no second link. Fillers first take every free inode below the freed one, as ext4 hands out the lowest free inode of the folder's group. On Blacksmith's ext4 the new segment got the freed inode 13893637. Red on Linux (878ccf7a): "the checkpoint wrote the deleted chat's verified bytes into the new chat's log (a reused inode 13893637, file system ext4)".
  • In both, the new chat's log after the checkpoint starts with its line 1 cut short, then holds the deleted chat's lines 2 to 5 ("queued") in place of its own, then a fragment of its own line 5, then its lines 6 to 8.
• Fix direction:
  • Do the write at the open, not in the checkpoint. The open has just read and verified the file, it is single-threaded, and no other thread touches the folder yet. Write the verified bytes back there: one pwrite per segment, no sync. The checkpoint then only syncs the listed segments by path, as sync_read_segments did before 2a065d25cd3. Verified no longer needs its bytes or the file id.
  • A sync of a file that replaced the segment is harmless; a write is not. The pages stay dirty until a sync writes them, so the D2 argument still holds, and the open still syncs nothing per chat.
  • Also drop a chat's entries in Store::delete, under the order lock it already holds. This is hygiene, not the fix (see the transfer-in case above).
  • A failed pwrite at the open: take the existing slow path for that chat (journal the lines only the log holds, cut, write the merged lines). Its errors already go to the chat's repair file, and the open goes on. Do not fail the open, and do not freeze the store for a write error.
  • Keep refuse_lost_home and the owner and mode checks of private_state_file on that open for writing. open_verified has neither (P3 2).
  • Tests: the deterministic test above passes with the delete hygiene alone. Add one that replaces the folder outside the store (rename it aside and install a new folder, as a transfer-in does), so that only the open-time write makes it pass. The ext4 test depends on the allocator, and its "inconclusive" branch panics when the inode is not given out again. Do not land it in that form.
  • The open now writes each replayed tail once. Measure the open again (the PR's bench) rather than only argue it.

P3

1. A folder that code outside the store moves aside during the write-back window and then puts back. An example is a transfer-in that replaces a durable chat, fails, and renames its backup back (hub/transfer_in.rs:598, :638-639). The write-back passes the segment while it is aside (NotFound), drops the entry and retires the records. The folder then comes back with pages that no sync of this process covered. sync_read_segments had the same gap. The same class is in R-DURABILITY: four older durability gaps from the reviews of #35895 and #35562 #35915 item 4. With the write at the open, the pages are at least dirty again, so the kernel writes them back.
2. open_verified opens a segment for writing with neither refuse_lost_home nor the owner, mode and ACL checks of private_state_file, which sync_read_segments used. Every other write under the home checks them.
3. Unchanged from my last review and accepted in the PR body: P3 1 is now R-DURABILITY: four older durability gaps from the reviews of #35895 and #35562 #35915 item 2, and P3 2 happens at shutdown only.

Checked and found correct

• Both merges lose nothing. For each store and journal file, I compared each side's changed lines from the merge base with what the merge commit holds against the other parent. The only differences are in the two conflicted files, journal_store.rs and checkpoint.rs: comment wording and the inner binding. No code line of either side is lost. 7f3e036d277 is clean (no differences).
• The merged commit_compaction (journal_store.rs:766-779) runs the covered check, then the checkpoint when the chat is not covered, then acpmux: a failed sync of a retention rewrite or a kept-aside image freezes the store (R-DURABILITY, #35905) #35963's counted_sync around the commit.
  • A covered chat has no write-back entries. Covered means its newest LSN is at or below checkpointed, which only a checkpoint that wrote back every entry from the open can reach.
  • So a retention rename never comes before its chat's write-back. A rename between open_verified and the write sends the same bytes to the old inode, which is harmless (also when the archive holds a second link).
  • No deadlock: the commit holds its ticket while it takes the writers' lock. A checkpoint takes the writers' lock (close_sync_failed) and lets it go before it waits for tickets, and it holds neither that lock nor order while it waits.
• The write-back and a log close or an eviction write different ranges of the same file, so neither changes the other's bytes. A close's failed sync is read under the writers' lock after the checkpoint's own syncs (acpmux: a checkpoint never writes an LSN past a sync that failed on another thread (R-DURABILITY; culprit 71bc09a1e63, S3a) #35895).
• The seal doc (P3 4) and the PR body's "What changes" and "Testing" (P3 3) now describe the code.
• The head greens, each log read at 7f3e036d277: macOS journal_ 380/380 (c03d8a73), macOS cmux_journal 42/42 (ed5a03c6), Linux all store:: with fmt and clippy, 380/380 (86498e8b, hq run 41969667583).

Not run

• No fix was written or run.
• The deterministic repro uses a second link as a stand-in for inode reuse. The real-inode repro got a reused inode on Blacksmith's ext4 this time. On another file system or a busier inode group it can miss, and it then panics as "inconclusive".
• Not run on Windows, where no identity is checked. No real failing disk, no real kill under load.

[lawrencecchen]

Review (r-durability-rsync-reviewer): SHIP at 29967f9

Delta review 4: 7f3e036d277..29967f9f5f7 in cmux-tui/crates/acpmux/src/store/, owner_only.rs and config.rs. I read the five commits: b53a750bcd1 (reds), 4846ec2b09e (the fix), ff6922678dd (an import), 3452068e332 (sync by path, no handle) and 29967f9f5f7 (handles held up to a cap). Line numbers are at 29967f9f5f7.

D3 from my last review: fixed

• The open writes the verified bytes back (repair.rs, Inner::write_back), so the write lands in the file the open just read and checked. The checkpoint only syncs (checkpoint.rs, sync_read_segments), never writes, so no file that a delete, a folder replace or a retention rewrite put at the path can take another chat's bytes. A sync of the wrong file is harmless.
• Store::delete takes its chat's entries out under the appends' lock, after the folder's removal. It closes the handles after it lets the lock go. When a checkpoint in flight has cloned the list, that checkpoint syncs the unlinked inode, which is harmless, and closes it at its end.
• A failed write at the open takes the slow path: journal the lines only the log holds, cut, write the merged lines. The cut also removes a range written back in part. The open neither fails nor freezes.
• The write and the sync open the segment with the lost-home, link, owner, mode and ACL checks (repair::open_segment_to_write; Windows owner_only::open_rw_existing, OPEN_EXISTING). This answers my P3 2.
• The reds are as stated: 6b7b8fa2 and 55b36bf4. I did not read those two logs again.

The held handles (29967f9f5f7), the five points you asked about

1. A held handle and a retention rewrite. Order::log_lsn puts every replayed chat above checkpointed until a checkpoint succeeds. That checkpoint syncs every listed segment first. So a retention rewrite of a replayed chat checkpoints before its rename, and the handle syncs the inode the open wrote. After the rename, the chat has no entries left. A checkpoint that fails before sync_read_segments keeps the entries, so covered stays false and the rewrite's own checkpoint fails too. Correct.
2. Descriptors. The cap is a quarter of the soft limit at the open, at most 4096, so held handles use at most a quarter of the descriptors. The daemon raises the limit at daemon.rs:188, before the store opens at :266. On macOS it falls back to OPEN_MAX when the first raise fails, so the limit stays 256 only when both raises fail. In that case the store can hold 64 log writers (OPEN_LOGS) plus 64 written-back segments until the first checkpoint succeeds. That is half the limit, against a quarter before this PR. The window ends with the first successful checkpoint, or with the stop after 10 failed checkpoints. I accept this, but it is the reason the cap must stay a fraction of the limit.
3. The delete order is correct (see above). A delete whose folder removal fails keeps the entries, and the next checkpoint syncs them, which is harmless.
4. Windows. held_cap is 0, so every segment is closed at the open and synced by path through open_rw_existing. That open asks for read and write access and shares read, write and delete. LocalStore's append handle also shares write, so the two opens do not conflict. A held handle could block a folder's rename on Windows, and there is none.
5. R-DURABILITY: four older durability gaps from the reviews of #35895 and #35562 #35915 item 6 (a folder moved aside and put back). Within the cap on Unix it is covered: a rename keeps the inode the handle holds, and the sync writes its dirty pages. Past the cap and on Windows it stays open, tracked in R-DURABILITY: four older durability gaps from the reviews of #35895 and #35562 #35915.

Also checked:

• The checkpoint clones the list under the order lock. The retain after a successful sync drops only the map's reference, because the clone still holds the other. So each close runs when the clone drops, on the checkpoint's thread and off the lock, and after the sync, when the pages are clean.
• A checkpoint that fails before the sync keeps every handle in the map for the next one.
• Rust opens files with O_CLOEXEC, so agent processes do not inherit the held handles.

Your question: a better way to avoid the close at the open, without held handles

I see none. The A/B is consistent with this cause: on macOS, the last close of a written file starts its write-back. C's durable time is shorter than B's, so C's writes start earlier. Any close before the first journal sync puts that I/O in front of the sync, and the first acknowledged change went from about 2 ms to 22-30 ms.

• The pages must be dirty (D2), and the write must happen at the open (D3).
• A close on another thread moves the CPU off the open, but not the I/O in front of the first sync.
• A close after a sync is cheap, because the pages are clean then. Holding the handle until the checkpoint's sync does exactly that.
• One optional saving: open each segment once with read and write access, for both the open's read and its write-back. That saves one open per segment; it is not a different design.

P3

1. The PR body's "What changes" still says "The next checkpoint syncs each listed segment, by path". At this head that is true only past the cap and on Windows. Within the cap on Unix, the checkpoint syncs through the handle the open kept.
2. The open-time numbers: I read both aws-m4pro-6 runs (A c74909f1 at 59c0c9552de, E 1021ba81 at 2382fe6b9c7; the bench diff is the same in both, and both ran at the same time). At 300 chats the medians are within the spread: open 40.6 against 41.6 ms, ready 43.2 against 44.0 ms, first acknowledged change 2.5 against 2.4 ms. E's tail is wider: open p90 57.8 and max 61.7 ms, against 43.6 and 44.0 ms. A has one spike of the same size at 100 chats (55.0 ms). With 10 runs, and two benches on one host at once, I read this as noise. An interleaved rerun would show it if it is not.

Checked and found correct

• The head greens, each log read at 29967f9f5f72b3fc668a1d20c6d5211edbc92f81:
  • macOS journal_: 384/384 (dbb06014).
  • macOS cmux_journal: 42/42 (31ef1527).
  • Linux all store::: fmt pass, clippy -D warnings pass, 384/384 (fc960a45, hq run 42003274872).
  • Each green ran the four journal_write_back_tests.
• Every test this PR took from my repro branches passes at the head on both systems:
  • P1-1, P2-1 and P2-2;
  • the three D1 tests and the fail-closed marker test;
  • both D2 windows;
  • both D3 tests.
• The Windows jobs run cargo check and filtered tests without -D warnings, so the unix-only test re-export (HELD_CAP) does not break them.
• DESIGN.md 3.3c ("The open's fast path and its write-back") describes the held handles, the cap and the Windows rule.
• The earlier delta-review findings and P3s are fixed or tracked: R-DURABILITY: four older durability gaps from the reviews of #35895 and #35562 #35915 items 2, 4 and 6, and the shutdown-only wait.

Not run

• I did not run my ext4 real-inode repro (rsync_review5_a_reused_inode_never_takes_a_deleted_chats_bytes) at this head. The checkpoint no longer writes, and the delete drops the chat's entries, so the write cannot reach another file. The stand-in tests pass on both systems.
• Nothing on Windows. No real failing disk, no real kill under load, no rollback to an older build.
• I did not run the open-time bench myself. I read the A/B/C and A/E logs named above.