acpmux: the open after a kill syncs nothing per replayed chat; retention never waits for a checkpoint in flight (R-DURABILITY, follow-up 4 of review 3 of #34901)
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 head66e1df5ec5e+ the bench); - head: fleet 28d76c7f at
237017ea1ae(this branch atb8159d903c8+ 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_compactionran 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. Fixf9c6a16d1b8:Order::log_lsnkeeps 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. Redd860bef9d32:a_retention_rewrite_of_a_checkpointed_chat_does_not_wait_for_a_checkpoint; guardsa_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: macOSF_BARRIERFSYNC, Linuxfdatasync/fsync) and flush the drive once, with the checkpoint file (sync_full), before they retire any record.cmux_journal::sync_fileis 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 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 (
- 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
frozenjournal 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 (#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; guardsa_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 printsACPMUX-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),retention29 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_oncetookOrder::read_segmentsat its start and put the set back only whensync_read_segmentsitself 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. Nowfreezeappends afrozenjournal 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 for101232eeb4d(E0027:applydid not name the replay's newfrozenfield).- Green at
24cc4ae39e4: macOSjournal_(fleet 8095cfdd), Linuxall store::(fleet 65386a18), macOS restart tests (fleet ea9d4c48).
Merged down (head 60cce5e745b)
0befb17bc02merges the S3b head6a5a6d138c9(#35565: feat with #35562 squashed). The merge base is the S3b head this branch had (7fc15b776ef). One conflict, injournal_store_tests.rs: both sides appended tests at the end. Both blocks are kept whole (45 tests = 41 + 40 - 36).60cce5e745bmerges featb536ff861c5, which has #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: #35895'scounted_sync, then this PR's journal-recordfreeze().checkpoint_oncekeeps this PR's start (P1-1) and #35895's end: the closes' note read under the writers' lock, the wait for the syncs under way, thenfrozen.log_sync.rs: #35895'sclose_sync_failedbeside this PR's orderedsync_log, with #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 thefrozenrecord. 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. Nowfreeze()has two steps:freeze_now, inside the count and under the appends' lock:frozenset 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 betweenfrozenand 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 #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 asfork_with_boundaryandtransfer_finish;fork_beforeandcommit_watch_pagecall it throughspawn_blocking), the caller ofsession_dir, the checkpointer (its ownsession.jsonwrites; 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 whilecommit_compactionruns 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'sflushcould 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_threadfails atdf1815e7fd4("a checkpoint waited for another thread's frozen record, not only for its failed sync", macOS fleet 63217ea0), and in its queue-full form at9e90c42de24(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 warningsgreen (fleet a8038a8a, hq run 41939067327); macOSjournal_368 of 368 (fleet 2e614ff0); Linuxall store::369 of 369 with fmt and clippy (fleet 4332aaf1, hq run 41940083373). - Green at the head
f62feba6e3d: macOSjournal_(fleet d7884d24) 369 of 369; Linuxall 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::sealsealed 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 failingflush_sessionheld right before its freeze while another thread seals; handshakes only. (The reviewer's third test held the freeze between the two steps off62feba6e3d, which the fix removes.)
The fix, three commits:
1617013d136:cmux-journalgetsJournal::append_control, an append that a full queue never refuses (stillClosedwhen the journal is closed,TooLargepastMAX_PAYLOAD_LEN; unit testa_control_record_is_taken_while_the_queue_is_full). Thefrozenrecord 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 #35895). The two-step freeze off62feba6e3d, its seams, and the test of its retry are gone.7b27d29b8be:JournalStore::sealseals 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.b3988138ef9(red at510cade7cf0: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 (newSimStorage::hold_next_sync). It fails atjournal_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 atjournal_meta_tests.rs:1393after 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_metasdoes not readrewrite_all, so asession.jsonfrom a failed sync is trusted) is #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 #35963'scounted_syncaround 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: #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) anda_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 testa_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; Windowsowner_only::open_rw_existing): the reviewer's P3 2.Store::deletedrops its chat's listed segments. -
4846ec2b09ekept the handle the open wrote through until the checkpoint synced it. Before its review, the reviewer named two costs of that, and3452068e332removes 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 #35915 item 6 (issuecomment-6652305673), the class of item 4.
-
ff6922678dd: clippy-D warningsfailed the Linux step at4846ec2b09eon an unusedstd::io::Writeimport 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 torepair::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 at3452068e332. 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 (#35915 item 6), since a rename keeps the file the handle holds.Store::deletetakes 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) (18612dc8617, 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.