fix: disable SQLite mmap on all connections to prevent 2^30 FTS truncation - #69759
fix: disable SQLite mmap on all connections to prevent 2^30 FTS truncation#69759mr-september wants to merge 2 commits into
Conversation
…tion Pin mmap_size=0 on every SessionDB connection (read-only and read-write paths). mmap is OFF by default, but a runtime PRAGMA issued against state.db in a past session enabled it; combined with a long unattended FTS optimize on the multi-GB monolith this caused the 2026-07-03 DB truncation/corruption. Pinning makes the safe state explicit and self-healing even if a future code path raises mmap_size. mmap is never needed here (search is sub-5ms at 4.2GB); disabling removes the only code-path-independent way the 2^30 truncation hazard can recur. Read-only/ attached connections may reject the PRAGMA; wrapped in try/except (non-fatal).
mr-september
changed the title
alt-glitch
added
type/bug
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused defensive change. Current main has moved materially since this PR, so the policy needs a current-main repro and the implementation needs to cover the new connection architecture.
Problems
- The reported unattended FTS mechanism is no longer current: routine maintenance uses bounded merges at
hermes_state.py:1766-1780(df841d342c), and v23 storage optimization is opt-in athermes_state.py:1984-1990(9acc4b47f5). The current tree has nommap_sizepragma/configuration to establish a current runtime enablement path. - Current
SessionDBcreates a separate per-thread WAL reader athermes_state.py:2030-2036; this PR predates it. The changed test only checksdb._connattests/test_hermes_state.py:7176-7181, so it would not prove the claimed all-connection guarantee after salvage.
Suggested changes
- Provide a current-main reproduction for the mmap hazard, then wire and test the policy for the constructor's read-only/writer paths and
_get_read_conn().
Automated hermes-sweeper review.
| # connection, regardless of any prior runtime PRAGMA. | ||
| row = db._conn.execute("PRAGMA mmap_size").fetchone() | ||
| cur = row[0] if row else None | ||
| assert cur == 0, f"SessionDB connection mmap_size={cur}, expected 0" |
There was a problem hiding this comment.
This only verifies the writer db._conn. Current main also opens a per-thread WAL mode=ro connection in SessionDB._get_read_conn() (hermes_state.py:2030-2036); when salvaging, please force that path and assert its mmap_size, plus the SessionDB(read_only=True) constructor path.
teknium1
added
sweeper:risk-compatibility
|
Closing as superseded — upstream's own changes have addressed the concerns since this PR was filed: PR #71755 (perf(session): set SQLite PRAGMA for large state.db performance) added config-gated database.mmap_size via apply_database_pragmas(), which covers all three connection types (writer, read-only constructor, and the per-thread WAL reader via _get_read_conn()) — addressing the coverage gap @teknium1 pointed out, and doing so in a more flexible, user-configurable way. The bounded incremental FTS merges (optimize_fts_storage / _merge_fts_incrementally) replaced the full-table optimize cadence that was the original trigger for the corruption, substantially reducing the likelihood of the hazard on current main. Given these changes, the hard mmap pin proposed here would now conflict with the config-gated approach. Closing in favor of upstream's mechanism. The remaining hazard surface (large configured mmap + manually-triggered optimize_fts()) is narrow enough that a defensive cap is not warranted. |
3 participants
What does this PR do?
Pins
PRAGMA mmap_size=0on everySessionDBconnection (read-only and read-write paths) so SQLite never memory-mapsstate.db.Related Issue
Fixes #
Type of Change
Changes Made
hermes_state.py: addpin_mmap_off(conn)helper that executesPRAGMA mmap_size=0, reads the value back, and logs a warning if it is not0/NULL(some SQLite builds / read-only connections report no mmap support — treated as the safe unmapped state, never fatal).pin_mmap_off(self._conn)on bothSessionDBconnection paths: the read-only?mode=roattach and the read-write path (beforeapply_wal_with_fallback).tests/test_hermes_state.py: addTestMmapPin— asserts the helper setsmmap_size=0on a bare connection, that a freshly openedSessionDBconnection reports0, and that the helper is non-fatal on a read-only connection.How to Test
python -m pytest tests/test_hermes_state.py::TestMmapPin -q→ 3 passed.python -m pytest tests/test_hermes_state.py -q -k "MmapPin or corruption or fts or malformed or reindex or cjk" -q→ all upstream FTS/corruption probes still pass (no regression).state.db;PRAGMA mmap_sizereturns0on every connection.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
pytest tests/test_hermes_state.py::TestMmapPin→ 3 passed.