fix(state): route session-resume reads through the WAL read-only connection (salvage #73539) - #77803
Merged
kshitijk4poor merged 1 commit intoAug 3, 2026
Conversation
…ection get_messages_as_conversation, get_resume_conversations, and get_ancestor_display_prefix still took self._lock — the same global choke point the read-path split (WAL per-thread read-only connections) was meant to remove from every recall/browse read. These three are the hottest reads in the file: every session resume across the gateway, CLI, and ACP adapter goes through one of them, so a resume racing a burst of concurrent-session writer flushes still convoys behind them exactly like the fixed paths used to. _session_lineage_root_to_tip (the lineage walk shared by all three, plus get_conversation_root) had its own independent self._lock use and needed the same conversion — without it the outer functions still blocked on the very first line. Verified empirically: a reader thread calling all three functions while another thread holds self._lock blocked for the writer's full hold duration before the fix, and returned immediately after (SQLite 3.50.4 in this dev venv falls back to journal_mode=DELETE per the WAL-reset-bug guard, so the requires_wal-marked regression test is exercised via a local WAL-forced script instead; it still runs and passes on any runtime where WAL is actually active).
kshitijk4poor
enabled auto-merge (rebase)
August 3, 2026 15:15
alt-glitch
added
type/perf
4 tasks
This was referenced Aug 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvages #73539 by @pierrenode — commit cherry-picked to preserve authorship; one conflict resolved (main centralized the resume SELECT's column list into
_CONVERSATION_ROW_COLUMNSafter the PR's base — kept main's constant, the PR's connection routing).Context — what this fixes, for whom
Anyone resuming sessions while the writer is busy:
session.resume's read paths (get_resume_conversations, lineage walk, ancestor resolution) still ran on the writer connection underself._lock, so a resume convoyed behind writer flushes/checkpoints. Main already has the per-thread WAL read-connection infrastructure (_read_ctx(), added for dashboard polling in this campaign's #76895 line of work) — this PR routes the four resume-path read sites through it. In DELETE-journal mode_read_ctx()falls back to the locked writer path, so non-WAL runtimes are unchanged.Absorption check (why this is NOT mooted by #76895)
#76895 routed dashboard/read-polling paths; the resume sites at hermes_state.py ~7003/7057/7096 still used
self._lock+self._connon current main (verified pre-pick). This PR is the complementary half for the resume path.Verification (with one honest caveat)
tests/test_hermes_state.py: 178 passed on the merged treetests/test_session_db_read_path_split.py: 3 passed, 5 skipped LOCALLY — the 5 skips (including the PR's newtest_session_resume_reads_do_not_take_writer_lock) arerequires_waltests, and this machine's SQLite 3.46.0 has the WAL-reset bug so Hermes runs journal_mode=DELETE. They exercise on any runtime with WAL active (the PR author built the skip guard correctly). The mutation check is therefore vacuous for the new test locally; the non-WAL fallback path (test_non_wal_uses_locked_path) does run and passes.Closes #73539 (superseded by this salvage — original author credited via cherry-pick authorship).