acpmux: chat metadata through the event journal (R-DURABILITY S3a)

Merged
LA
2d ago
+1628-39

Summary

Stage S3a of the R-DURABILITY rewrite (design: handoff/rewrites/R-DURABILITY/DESIGN.md 3.3e). Stacked on S2c (#35400, landed as f4aa1bf60ec; this branch has merged feat a0d32a7b4e8 after it). Behind the same switch, store.journal (default off).

Today every chat metadata change (draft, name, unread mark, tags, mode, rules and the rest) writes session.json with write_atomic on the request path: the file synced, renamed, its folder synced, for each change. With the journal on, two defects come from writing the metadata and the log at two times:

  • E13: session.json is synced at once while the agent's lines wait for the journal's next sync, so after an OS crash the metadata names lines the log lost (lastSeq past the log).
  • Nothing syncs the sessions folder when a chat's folder is made, and S2's replay never makes a folder again, so an OS crash soon after a new chat can lose the whole acknowledged chat.

With the switch on, a metadata change is now one meta record (kind 4: chat id, flags, the folder's identity, the stamp of the session.json the save saw, the whole metadata image). No file is written on the request path; the reply waits for the record through S2c's gate, one group commit for a burst. A new chat's first record counts as durable for the gate only once its folder's entry in sessions/ is synced too, by a thread at the daemon's disk tier, so the replay never needs to make a folder again.

What changes

  • store/journal_store/meta.rs (new):
    • save: under the appends' lock, check_append, the chat's folder made when missing, the meta record (Ack), and the image kept in an overlay (each chat's newest image that session.json does not hold yet). The record carries the stamp (file id, modification time, length; one lstat) of the session.json there was.
    • Reads: load and load_startup read the overlay first; list reads the overlay, then lists the folders and puts the overlay's images over them (no lock across the two reads: a checkpoint writes a file before it drops the file's overlay entry, so either read has the newest image).
    • Checkpoint: it waits, on its own thread, until every record through its LSN is durable (one group commit), then writes each image it read to session.json (the image's LSN as journalLsn, and beside it the journal's id as journalId: an LSN means something only in the journal that gave it; a new file, synced, renamed, then the chat folder synced), then syncs the logs and the sessions folder as before. An image so names only lines the journal holds (ends E13). A file that already holds that image or a newer one from this journal is not written again. A failed sync freezes the checkpoint and asks the daemon to stop, as a failed log sync does. An overlay entry goes only if no newer image came; a chat deleted, or deleted and made again, meanwhile is passed.
    • Replay: per chat after its last forget, the newest image, written into its own folder when the file is one this journal wrote (its journalId) with an older journalLsn, when there is no file, or when the file is one this journal did not write and one of the chat's records saw it. A file with another journal's id (a chat folder build adoption moved in from another home, or a home whose journal was made again) counts as one this journal did not write. Any other file moved on without the journal (a run with the switch off whose catch-up did not replay, or an older build): it stays, and the image goes to journal-unapplied/<id>.<lsn>.session.json, as S2's repair does for a log that moved on. A folder that is gone or replaced: the image goes there too, and the folder is never made again.
  • store/journal_store/new_folders.rs (new): a save that made a chat's folder notes its record's LSN under the lock the journal writer's durable listener needs; the acpmux-journal-folders thread syncs sessions/ at once; the durable LSN the S2c gate reads stays below that record until then. A crash before the sync loses only a chat no reply showed. A failed sync freezes the checkpoint and asks the daemon to stop.
  • Folders handed out of the store: code outside the store reads and moves chat folders directly (hub/transfer_in.rs puts the live folder aside and installs a transferred chat under the same id). So each record carries its folder's identity (file id and birth time). A read, the checkpoint and the replay use an image only for that same folder and never make a missing folder; the image of a folder removed or replaced since goes to journal-unapplied/.
  • session_dir and flush_session (the trait: "log and metadata to stable storage") write the chat's image to session.json first, once its record is durable. Store::session_path (new) gives only the path, with no lock, wait or write; the callers that need only the path or its parent use it: an agent's start (hub/spawn.rs), the fixed-id check under the hub's sessions lock (hub/lifecycle.rs), the temporary-folder sweep (hub/mod.rs), the host probe (hub/hosts.rs) and the transfer recovery's root (hub/transfer_in.rs).
  • delete drops the chat's overlay entry with its forget, and removes the folder under a lock a checkpoint's write also takes.
  • Without the switch nothing changes: LocalStore::save writes session.json as before, and MetaWriter (hub/meta_save.rs) keeps its order for both.
  • plans/cmux-next/threading-model.md: the folder syncer's row.

Review of cc9a56002ea (comment 6637151698): what changed

  • F1 (P1, a run without the journal): the stamp rule above. The reviewer's test metadata_written_with_the_switch_off_survives_turning_it_on_again and a delete variant a_chat_deleted_with_the_switch_off_after_a_failed_catch_up_is_not_made_again (the real failed catch-up: a checkpoint file that does not read).
  • F4 (P2, a purged chat comes back) and F1's delete case, at the root: the first head made a missing folder again from a created record, so every remover would have had to wait for a durable forget, and runs without the journal write none. Now the replay never makes a folder again, and the folder durability above makes that safe. a_chat_made_after_the_checkpoint_and_deleted_is_not_made_again (the review's sketch) and a_new_chat_is_acknowledged_only_once_its_folder_is_durable (the gate's durability passes a new chat only after a sync that keeps its folder; then an OS crash keeps the chat). The first head's a_chat_made_after_the_checkpoint_survives_an_os_crash_that_loses_its_folder removed the folder after the journal sync and expected it made again; that premise is gone, and the new test replaces it.
  • F2 (P2, a late settle_meta): no write over the same or a newer journalLsn. The reviewer's test and hold, a_late_settle_never_writes_an_older_image_over_a_checkpointed_one.
  • F3 (P2, an ignored folder sync): session.json's own syncs, SyncFailed, the freeze; the replay and keep_aside_meta return their errors. a_failed_folder_sync_after_a_metadata_write_freezes_the_checkpoint (a test seam fails the chat folder's sync).
  • P3 1 (session_dir on request paths): session_path, session_path_never_waits_for_the_journal.
  • P3, the PR text: list holds no lock across its two reads (corrected above); worktree care does decide from session.json (below).
  • P3, power loss (the refuted item): a comment at settle_meta says the image's full sync is what keeps an image from naming a record a power loss took back, and that a weaker sync must first wait for full_durable_lsn (DESIGN 3.3e too).
  • P3 not done here (follow-ups below): the orphan folder of a crashed import, one unwritable chat failing every checkpoint, the 5 s wait on a slow disk, a move frozen after an OS crash, the private_state_dir check on writes, the identity check and the write as one step.

Delta review at 8301994b135 (comment 6640803810): what changed

  • N1 (P2, brought in by the F2 fix): a journalLsn written by another journal (a chat folder build_adopt::acpmux::merge_sessions moved in from another home, a journal made again, a hand copy) hid every later change of that chat: the checkpoint took it for "this image or a newer one is on disk", wrote nothing and dropped the overlay entry. The replay rule had the same root. Fix fc83b326fba: session.json carries journalId beside journalLsn (cmux-journal Journal::journal_id, the id already in each segment header, kept across opens and retirements); both rules compare an LSN only for a file with this journal's id, and any other file follows the stamp rule.
  • Tests: the reviewer's a_change_to_a_chat_moved_in_from_another_home_reaches_session_json, and a_replayed_change_to_a_chat_moved_in_from_another_home_reaches_session_json, a_change_after_the_journal_is_made_again_reaches_session_json; cmux-journal the_journal_id_survives_a_reopen_and_differs_between_journals.
  • The reviewer's four answers: the lock order has no cycle; no path shows a new chat before its folder sync; the stamp rule refuses no realistic legitimate write; the removed test's premise is gone by design.
  • Delta review at fc83b326fba: SHIP (comment 6641769294). A journal id is never 0 and stays the same for the journal's life; no session.json from an earlier head of this PR exists in the field (unmerged, switch default off, no dogfood run), and one would only send an image to journal-unapplied/.

Takes back #35400: not a revert. This PR rewrites the one plans/cmux-next/threading-model.md row #35400 added (the acpmux event journal) to add the S3a folder syncer thread, acpmux-journal-folders; every part of #35400's row stays in it. Same owner (R-DURABILITY).

Risk: high

Behind the default-off switch. It changes what the daemon writes for chat metadata and when. Rollback: turn store.journal off; the next start replays the journal (the logs and session.json) and checkpoints first, then runs as today. An older build (S2, S2c) that reads a journal with meta records stops at the unknown record kind; its catch-up error is only logged, and it then runs without the journal. Run this build once with the switch off before going back to one of those: if that is skipped, the metadata changes that older build makes are kept (the stamp rule), but a chat it deletes whose records are in the journal is not made again either, and the journal's images of it go to journal-unapplied/ (nothing is lost; nothing deleted comes back).

Known gaps

  • S3b (#35565): the line-first order at the sites that log a line with a metadata change (tags, rules).
  • The open's syncs: the replay writes each replayed chat's session.json with its own syncs before the socket is bound (measured and cut in the recovery-syncs follow-up).
  • Off-daemon readers of session.json (the acpmux CLI's worktree and stats, and a daemon's worktree care pass over other homes) see the file as of the last checkpoint (at most 30 s old). Worktree care decides from it (worktree_sessions::on_disk): a chat made or reopened in the last 30 s reads missing or closed, so a worktree idle past recent_hours that it now uses could be archived in that window. Follow-up for the worktree care owner: read a running daemon's home through its socket.
  • Build adoption (from the review at fc83b326fba, P3, owner R-DURABILITY): build_adopt::acpmux::merge_sessions moves a post-update home's chat folders into the adopted home but leaves that home's journal/ in adopted-post-update-<UTC>/ unreplayed. With the switch on and a post-update daemon that crashed (not one that stopped cleanly), its changes since its last checkpoint stay in that journal, and a chat made in that window has no session.json. Fix direction: run that home's catch-up before merge_sessions, or defer the adoption.
  • session_dir's settle is still only logged when it fails (P3 1's second half: a settling accessor that returns a Result for code that reads or moves the files). Follow-up, owner R-DURABILITY.
  • Review P3 follow-ups (owner R-DURABILITY): a crashed import's folder with log lines and no session.json (recover_durable_marker keeps it); one chat whose session.json cannot be written fails every checkpoint (the next start keeps its image aside); the checkpoint's 5 s wait fails it on a disk that slow (S2c's 10 s stall limit stops the daemon first); transfer_commit's removal right after save_meta_answered; the private_state_dir check on the store's writes; the identity check and the write as one step against a swap without session_dir.

Testing

acpmux store::journal_meta_tests (the journal on cmux_journal::SimStorage; the logs' and the sessions folder's OS crash follow the syncs that ran, store/sync_model.rs):

  • last_seq_never_passes_the_log_after_an_os_crash (E13)
  • a_new_chat_is_acknowledged_only_once_its_folder_is_durable
  • a_metadata_change_is_written_to_session_json_by_the_checkpoint (no write on the request path; reads see the change)
  • guards: a_chat_made_before_the_checkpoint_and_deleted_is_not_made_again, the_replay_never_writes_an_older_image_over_session_json
  • folders handed out of the store: a_folder_replaced_outside_the_store_keeps_its_own_metadata, a_replay_never_writes_an_image_into_a_folder_replaced_since, session_dir_hands_out_a_folder_whose_metadata_is_written, flush_session_writes_the_chats_metadata, the_checkpoint_never_makes_a_folder_removed_outside_the_store_again
  • the delta review at 8301994b135 (N1): a_change_to_a_chat_moved_in_from_another_home_reaches_session_json, a_replayed_change_to_a_chat_moved_in_from_another_home_reaches_session_json, a_change_after_the_journal_is_made_again_reaches_session_json; cmux-journal cmux_journal_tests::the_journal_id_survives_a_reopen_and_differs_between_journals
  • the review of cc9a56002ea: metadata_written_with_the_switch_off_survives_turning_it_on_again, a_chat_deleted_with_the_switch_off_after_a_failed_catch_up_is_not_made_again, a_late_settle_never_writes_an_older_image_over_a_checkpointed_one, a_failed_folder_sync_after_a_metadata_write_freezes_the_checkpoint, a_chat_made_after_the_checkpoint_and_deleted_is_not_made_again, session_path_never_waits_for_the_journal

Red, the review tests on cc9a56002ea merged with feat (9d6c45e526d, then 44b1b45e788): journal_meta_tests 7 of 17 fail, each for its reason, in the full run and alone; the other 10 pass:

  • macOS: fleet 5bc434a9 (aws-m4pro, the first 5), fleet 624d5df4 (aws-m4pro-5, all 7).
  • Linux: fleet a2207ff0 (hq run 41885458993, the first 5), fleet 638a44ef (hq run 41892023040, all 7).
  • Reasons: "the replay wrote the journal's older image over a change made with the switch off"; "a chat deleted with the switch off came back"; "a late settle wrote an older image over an acknowledged one"; "the checkpoint went on past a failed sync of a session.json folder"; "a deleted chat came back"; "the new chat was acknowledged while an OS crash could still take back its folder"; "session_path waited 2.006279208s for the journal and the disk".
  • The reviewer's own reds: F1 fleet 7ec267b5 (macOS) and 1352db75 (Linux); F2 fleet fbfc784a (macOS) and 6b0b80ba (Linux).

Red, N1, at 7c8cb957745 (the three N1 tests on 8301994b135): journal_meta_tests 3 of 19 fail, each for its reason, in the full run and alone; the other 16 pass. macOS fleet 47570494, Linux fleet c29b659d (hq run 41904277997): "the checkpoint dropped a change it never wrote"; "the replay dropped a change to a moved-in chat"; "a change after a journal reset was dropped". The reviewer's own red at e944a7c21de: fleet 4d0d98bc (macOS) and 071ad7c4 (Linux); green at cc9a56002ea (fleet 7cb54a3b), so the F2 fix brought the loss in.

Green at the head fc83b326fbac39c52b03b3ca676b7059aa13854c (the fix fc83b326fba):

  • macOS: fleet c637cfb8 (aws-m4pro-6), journal_ 340 of 340, all 19 journal_meta_tests; fleet 0e43502a, cmux_journal 41 of 41 with the new id test.
  • Linux: fleet 17584257 (hq run 41904405338), hub:: 1136 of 1136 (acpmux 1122, cmux-remote 1, cmux-tui 13); fleet 469f7ff8 (hq run 41904405581), all store::: fmt, clippy and 341 of 341 tests pass (acpmux 143, cmux-tui 41, cmux-tui-core 154, cmux-worktree-care 3).
  • fmt after #35845 (merged as 7dd3f2e99015): green at 8301994b135, fleet f075eb9b (hq run 41902929513).

Earlier greens at 0d246364d90 (the F1-F4 fix): macOS fb06d302 (journal_ 332 of 332), 96a61a40 (durable_gate 4 of 4), 151be01c (the daemon stop test); Linux e3538ce6 (all store::: clippy pass, 337 of 337; fmt red only on base files, the finding #35845 fixed), a963a8a3 (hub:: 1136 of 1136).

Earlier, at the first head and its predecessors: red at 0505e9bee04 (hq runs 41833286562 and 41833575112, fleet 7e5265a4) and at 0e617489731 (fleet c0fd079d); green at cc9a56002ea (fleet a5e2aa2b, 50a3a00e, 1acb0685, 3fdab4b9).

Observed / Not run

  • Observed: the fleet steps above, each log read.
  • Not run: a dogfood daemon with the switch on; the burst A/B (100 chats x _acpmux/draft_set, reply p50/p95/p99 with the switch off and on); a new chat's create latency with the folder sync (one sync of sessions/ before its reply, at the daemon's tier, off the request thread); a power-loss model. verify-local.py --affected here: localization and package-groups fail, and both fail the same way on the base a0d32a7b4e8 (this PR changes no Swift, strings or packages).

Changelog

none

Proof

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

Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

Summary by cubic

Routes chat metadata through the event journal behind the store.journal switch (default off): a metadata change is now a single meta journal record, and session.json is written only by the checkpoint once the record is durable, instead of a synced file write on the request path.

This fixes two crash-safety defects: after an OS crash the metadata could name lines the log lost (E13), and a newly acknowledged chat's folder could be lost entirely.

  • The checkpoint waits for its records to be durable before writing each chat's image to session.json; the replay writes each chat's newest image and remakes a folder only for a chat a replayed record created.
  • Each record is bound to its folder's file id and birth time, since code outside the store moves chat folders directly; a read, checkpoint, or replay never writes an image into a replaced folder, and session_dir and flush_session write the chat's image first.
  • With the switch off nothing changes.

Rollback: turn store.journal off; the next start replays the journal and checkpoints first. Run this once with the switch off before reverting to an older build, which stops at the unknown meta record kind.

Known gaps: a change that also logs a line is still two records (S3b makes it one), and off-daemon readers of session.json see the file as of the last checkpoint (at most 30s old).