fix(dashboard): stop checkpoints from read polling - #67903
Conversation
Related to #60885: both prevent read-only dashboard activity from checkpointing WAL, while this PR also classifies the dashboard's SessionDB read and mutation endpoints explicitly. The overlapping but broader scope needs a maintainer choice. |
|
Thanks, agreed that the two PRs overlap on the read-only close guard. The runtime distinction here is that This PR adds a public-route WAL regression and makes access mode explicit across the session dashboard helpers, while mutation paths assert If #60885 lands first, the endpoint access-mode classification and public |
|
Controlled runtime verification is complete on macOS using this branch with the installed dashboard assets. Before restart, the registered dashboard process wrote 19,677,184 bytes in 30 seconds. Four write intervals were nonzero, and the largest one-second burst was 6,569,984 bytes. The zero-length WAL mtime changed ten times. After restart, a five-minute sample issued 61 remote
The remote dashboard session list remained available throughout the sample. The recurring checkpoint and truncate write pattern did not recur. |
|
Pushed 9b246615d, hardening the read-only open path in
The happy path stays read-only end to end, no writable open and no checkpoint, which PR body updated with the test matrix and a cross-reference to #60885. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the WAL-preservation work. The core premise remains live: current GET /api/sessions opens a writable DB at hermes_cli/web_server.py:4770, while SessionDB.close() performs wal_checkpoint(TRUNCATE) at hermes_state.py:2229.
Problems
- The read-only conversion at diff RIGHT line 4245 conflicts with current auto-archive.
GET /api/sessionsinvokes_maybe_auto_archive_for_profileathermes_cli/web_server.py:4775; that path writes session/archive metadata throughhermes_state.py:7892-7899. The existing contract is covered bytests/hermes_cli/test_web_server.py:2229-2265. - The proposed schema probe does not include
sessions.pinned. Current listing requests pinned backfill athermes_cli/web_server.py:4801, and its query requiress.pinnedathermes_state.py:4956.
Suggested changes
- Run enabled auto-archive through a dedicated writable maintenance open before reopening read-only for the list response.
- Extend stale-schema healing and tests for the current
pinnedcolumn.
This is an automated hermes-sweeper review.
9b24661 to
eb36eb7
Compare
|
Thanks, both findings were valid. I rebased onto current main and pushed Auto-archive now runs through a dedicated writable maintenance connection, which closes before the list query opens read-only. The stale-store probe now covers both The focused state, dashboard, search and event-loop suite is 254 passed, with ruff clean. I also added the repository’s |
|
The shared Vercel fixture fix has now landed on I rebased this PR onto the corrected |
Signed-off-by: joelbrilliant <joelbrilliant1@gmail.com>
Signed-off-by: joelbrilliant <joelbrilliant1@gmail.com>
b8249b4 to
b30d6d9
Compare
|
Merged via #76895 — thank you @joelbrilliant. Both your commits were cherry-picked, so you remain the author in git history. This fixed two real bugs we verified side-by-side before merging: dashboard read polling truncating another writer's pending WAL to 0 bytes on every poll (your read_only connections + checkpoint-skip-on-RO-close fix it exactly), and read-only FTS search silently returning nothing (your capability probing restores both the FTS5 and CJK trigram paths). Your test suite — WAL preservation, auto-archive maintenance writer, fresh/zero-byte stores, stale-schema healing, 8-thread concurrent first-load, and the AST access-mode invariant — all held up. The salvage added one fix commit on top: the new RO FTS probe could raise DatabaseError on a malformed store and leak a tracked connection (blocking the forensic backup in the writable heal that follows); fixed with close-then-reraise mirroring your own _open_probed cleanup, plus a regression test. Everything else is your work verbatim. |
Summary
wal_checkpoint(TRUNCATE)when closing read-onlySessionDBconnectionsRoot cause
The desktop dashboard Sessions page polls
/api/sessionsevery five seconds. That GET path created a writableSessionDB, thenSessionDB.close()unconditionally requested a truncate checkpoint.When another process had pending WAL frames, the read-only dashboard poll forced those frames through a checkpoint even though canonical session data had not changed. This matched the repeatable WAL truncation and disk-write bursts observed from the always-running dashboard backend.
Impact
Read-only desktop polling no longer requests SQLite checkpoints. Dashboard mutation endpoints remain explicitly writable, while writer shutdown retains its existing truncate checkpoint and durability behaviour.
Applicability
This fix is not tied to one operator, database size, profile name, install path, tunnel or service manager. It applies to any Hermes dashboard reading a WAL-backed
state.db. The default store and named profile stores resolve through Hermes paths, while the regression tests run against isolated temporary homes and disposable databases.Verification
scripts/run_tests.sh tests/test_web_server_sessiondb_eventloop.py tests/test_hermes_state.py tests/hermes_cli/test_web_server.py tests/hermes_cli/test_web_server_session_search.py tests/gateway/test_session_api.pyruff check .data_version: 2 before and 2 after(1, 1)before and(1, 1)afterNo WAL, synchronous, FTS, checkpoint full-fsync, or macOS corruption safeguards are disabled.
Update 2026-07-20 - pushed 9b246615d: self-healing for the read-only open path. Stale-schema stores (predating
sessions.archived), zero-byte store files, and racing first-load polls now trigger one writable healing open through the existing init/reconcile/repair machinery, then reopen read-only. The happy path stays read-only end to end with no checkpoint.Testing for the update:
requires-python <3.14; the 4 failures below are environment-related and reproduce identically on the base commit, so they are unrelated to this change. Windows/Linux not exercised locally; nothing in the change is platform-specific (stdlibthreading.Lock,Path.stat, sqlite error-message classification already used elsewhere inhermes_state).python3 -m pytest tests/hermes_cli/test_web_server.py tests/hermes_cli/test_web_server_session_search.py tests/test_web_server_sessiondb_eventloop.py -q: 460 passed, 4 failed (TestPtyWebSocket x3, TestNewEndpoints::test_blueprint_instantiate_creates_job; identical failures on the base commit in this environment).python3 -m pytest tests/test_hermes_state.py -q: 385 passed.no such column: archivedvialist_sessions_rich; zero-byte store and concurrent fresh-store reads 500 withno such table: sessions), then pass with the hardened helper.test_get_sessions_poll_preserves_pending_walpasses unchanged: across three read polls the-walbyte size,PRAGMA data_version, and row counts are identical, which also proves the healing path stays cold on the happy path (a healing open would checkpoint on close and fail the byte check).Overlap with #60885: that PR guards
SessionDB.close()so read-only connections never request a WAL checkpoint, which this PR also carries in itshermes_state.pyhunk. Beyond the shared close() guard, this PR classifies every_open_session_db_for_profilecall site with an explicit read_only access mode, converts the dashboard read endpoints (/api/sessions, stats, search, messages, export, analytics, cron runs) to genuinemode=roopens with self-healing for fresh/stale/racing stores, and adds a WAL-preservation regression test. The two changes are compatible in either order: if #60885 lands first, this PR's close() hunk merges down to a near no-op; if this one lands first, #60885 can drop its overlapping close() change and keep whatever scope remains. No sequencing requirement from this side, maintainers' choice.