review: tracked ro-connection for stats, single WAL warning, hedged locked-cause wording
Review follow-ups from the pre-push falsification pass: - collect_state_db_stats now routes through _connect_tracked_db so the module's byte-probe guard sees the read-only connection (consistency with the module's own ro-connection precedent; prevents a raw header probe from cancelling this reader's locks in multi-threaded callers). - Drop the new >256 MiB WAL warning from the stats renderer: doctor's pre-existing 50 MB WAL check (with --fix checkpoint) already covers WAL runaway, and two warnings for one condition is noise. The test now locks in the dedup decision. - Locked-cause explainer says the message 'should already be saved' rather than overclaiming when the early turn-start persist also failed.
This commit is contained in:
committed by
kshitij
parent
64c342c1c9
commit
a24cbaf426
@@ -309,14 +309,11 @@ def _render_state_db_stats(stats: dict, holders=None) -> list:
|
||||
f"({detail})",
|
||||
))
|
||||
|
||||
# Advisory: WAL runaway — checkpoints are not landing.
|
||||
if wal is not None and wal > STATE_DB_WAL_WARN_BYTES:
|
||||
lines.append((
|
||||
"warn",
|
||||
f"state.db WAL is very large ({_human_bytes(wal)})",
|
||||
"(checkpoints may not be completing — a long-lived reader or "
|
||||
"stuck process can block them; see 'hermes doctor --fix')",
|
||||
))
|
||||
# WAL runaway is deliberately NOT warned here: the pre-existing WAL
|
||||
# check later in the state.db section already warns above 50 MB and
|
||||
# offers a checkpoint via --fix; a second warning at a higher threshold
|
||||
# would only duplicate it. STATE_DB_WAL_WARN_BYTES remains for callers
|
||||
# (dashboards) that consume the stats dict without that legacy check.
|
||||
|
||||
return lines
|
||||
|
||||
|
||||
+8
-2
@@ -2220,8 +2220,14 @@ def collect_state_db_stats(db_path: Path) -> Dict[str, Any]:
|
||||
try:
|
||||
# mode=ro refuses to create the file and refuses every write; a
|
||||
# short timeout keeps doctor snappy when a writer holds the lock.
|
||||
conn = sqlite3.connect(
|
||||
f"file:{Path(db_path)}?mode=ro", uri=True, timeout=2.0
|
||||
# Route through the tracked connect so byte-probe helpers
|
||||
# (read_header_bytes_preopen) see this connection and refuse raw
|
||||
# opens that could cancel our POSIX locks mid-read.
|
||||
conn = _connect_tracked_db(
|
||||
f"file:{Path(db_path)}?mode=ro",
|
||||
tracking_path=Path(db_path),
|
||||
uri=True,
|
||||
timeout=2.0,
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.debug("collect_state_db_stats: cannot open %s read-only: %s",
|
||||
|
||||
+2
-2
@@ -3719,8 +3719,8 @@ class AIAgent:
|
||||
prefix
|
||||
+ "the turn was stopped because session storage was busy "
|
||||
"(another Hermes process was writing to the state "
|
||||
"database). Your message was saved — please send it "
|
||||
"again in a moment."
|
||||
"database). Your message should already be saved — "
|
||||
"please send it again in a moment."
|
||||
)
|
||||
if cause == "disk":
|
||||
return (
|
||||
|
||||
@@ -208,14 +208,18 @@ def test_render_large_db_legacy_trigram_suggests_optimize():
|
||||
assert "optimize-storage" in blob
|
||||
|
||||
|
||||
def test_render_warns_on_large_wal():
|
||||
def test_render_does_not_duplicate_legacy_wal_warning():
|
||||
"""A large WAL must NOT warn here: doctor's pre-existing WAL check
|
||||
(50 MB threshold, with a --fix checkpoint) already covers it, and a
|
||||
second warning at a higher threshold would duplicate the output."""
|
||||
from hermes_cli.doctor import STATE_DB_WAL_WARN_BYTES, _render_state_db_stats
|
||||
|
||||
lines = _render_state_db_stats(
|
||||
_base_stats(wal_size_bytes=STATE_DB_WAL_WARN_BYTES + 1), holders=None
|
||||
)
|
||||
blob = " ".join(" ".join(str(p) for p in line) for line in lines).lower()
|
||||
assert "checkpoint" in blob
|
||||
warns = [line for line in lines if line[0] == "warn"]
|
||||
blob = " ".join(" ".join(str(p) for p in line) for line in warns).lower()
|
||||
assert "wal" not in blob
|
||||
|
||||
|
||||
def test_render_handles_all_none_stats():
|
||||
|
||||
Reference in New Issue
Block a user