fix(state): stop a waiting FTS detach once the file is quarantined
The FTS fail-open detach now waits up to the caller's write budget (20 s / 60 s) for the write lock, so the one-time quarantine check before the loop left a long window: a sibling that quarantined the file meanwhile still got its triggers dropped and the stale breadcrumb committed on the quarantined handle. Re-check the handle flag and the process-wide storage latch at the top of every attempt, via the same _raise_if_db_corrupt(storage=True) that _execute_write runs per attempt. Classify the retryable lock error with is_sqlite_lock_error (result code first) instead of a locked/busy substring match, matching #120488.
This commit is contained in:
@@ -979,14 +979,7 @@ class SessionDB(
|
|||||||
# mutations, not just idempotent UPSERTs.
|
# mutations, not just idempotent UPSERTs.
|
||||||
ioerr_begin_retried = False
|
ioerr_begin_retried = False
|
||||||
while True:
|
while True:
|
||||||
self._raise_if_db_corrupt()
|
self._raise_if_db_corrupt(storage=True)
|
||||||
if storage_state(self.db_path) == STORAGE_CORRUPT:
|
|
||||||
# Another handle in this process already saw structural damage on this file.
|
|
||||||
# Quarantine this one before it touches SQLite; the error type is the same
|
|
||||||
# StateDbCorruptError, so every transcript-diversion owner handles it unchanged.
|
|
||||||
self._halt_db_corrupt(sqlite3.DatabaseError(
|
|
||||||
"database disk image is malformed (reported earlier in this process: "
|
|
||||||
f"{storage_corrupt_reason(self.db_path)})"))
|
|
||||||
# NOTE: the replaced/generation live probe runs INSIDE the lock below,
|
# NOTE: the replaced/generation live probe runs INSIDE the lock below,
|
||||||
# not here. close() mutates _conn and _db_sidecar_identity under that
|
# not here. close() mutates _conn and _db_sidecar_identity under that
|
||||||
# same lock, ending the WAL generation (SQLite unlinks the -wal/-shm
|
# same lock, ending the WAL generation (SQLite unlinks the -wal/-shm
|
||||||
@@ -1397,9 +1390,16 @@ class SessionDB(
|
|||||||
)
|
)
|
||||||
return retire_without_close
|
return retire_without_close
|
||||||
|
|
||||||
def _raise_if_db_corrupt(self) -> None:
|
def _raise_if_db_corrupt(self, *, storage: bool = False) -> None:
|
||||||
if self._db_corrupt:
|
if self._db_corrupt:
|
||||||
raise self._corrupt_error()
|
raise self._corrupt_error()
|
||||||
|
if storage and storage_state(self.db_path) == STORAGE_CORRUPT:
|
||||||
|
# Another handle in this process already saw structural damage on this file.
|
||||||
|
# Quarantine this one before it touches SQLite; the error type is the same
|
||||||
|
# StateDbCorruptError, so every transcript-diversion owner handles it unchanged.
|
||||||
|
self._halt_db_corrupt(sqlite3.DatabaseError(
|
||||||
|
"database disk image is malformed (reported earlier in this process: "
|
||||||
|
f"{storage_corrupt_reason(self.db_path)})"))
|
||||||
|
|
||||||
def _sleep_before_write_retry(self, deadline: float, patience_s: float) -> bool:
|
def _sleep_before_write_retry(self, deadline: float, patience_s: float) -> bool:
|
||||||
"""Sleep one jitter interval if the budget allows; True = retry, False = deadline passed. Small
|
"""Sleep one jitter interval if the budget allows; True = retry, False = deadline passed. Small
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ from typing import Sequence
|
|||||||
from hermes_constants import get_hermes_home
|
from hermes_constants import get_hermes_home
|
||||||
from hermes_state_common import (FTS_CJK_STALE_KEY, FTS_STALE_KEY, _FTS_CJK_TRIGGERS, _FTS_TRIGGERS,
|
from hermes_state_common import (FTS_CJK_STALE_KEY, FTS_STALE_KEY, _FTS_CJK_TRIGGERS, _FTS_TRIGGERS,
|
||||||
routed_sessions_setting)
|
routed_sessions_setting)
|
||||||
from hermes_state_errors import is_fts_scoped_corruption_error
|
from hermes_state_errors import is_fts_scoped_corruption_error, is_sqlite_lock_error
|
||||||
|
|
||||||
# caplog tests pin the "hermes_state" logger name.
|
# caplog tests pin the "hermes_state" logger name.
|
||||||
logger = logging.getLogger("hermes_state")
|
logger = logging.getLogger("hermes_state")
|
||||||
@@ -364,12 +364,14 @@ class SessionFtsSetupMixin:
|
|||||||
same corrupt index — giving up after 1 s cost that turn's canonical write."""
|
same corrupt index — giving up after 1 s cost that turn's canonical write."""
|
||||||
if not self._fts_enabled or not self._is_fts_write_corruption_error(exc):
|
if not self._fts_enabled or not self._is_fts_write_corruption_error(exc):
|
||||||
return False
|
return False
|
||||||
self._raise_if_db_corrupt()
|
|
||||||
if patience_s is None:
|
if patience_s is None:
|
||||||
patience_s = self._WRITE_PATIENCE_S
|
patience_s = self._WRITE_PATIENCE_S
|
||||||
if deadline is None:
|
if deadline is None:
|
||||||
deadline = time.monotonic() + patience_s
|
deadline = time.monotonic() + patience_s
|
||||||
while True:
|
while True:
|
||||||
|
# Re-checked every attempt: a sibling may quarantine the file while we wait for the
|
||||||
|
# lock, and nothing may be committed on a quarantined handle.
|
||||||
|
self._raise_if_db_corrupt(storage=True)
|
||||||
try:
|
try:
|
||||||
with self._lock:
|
with self._lock:
|
||||||
self._raise_if_db_replaced()
|
self._raise_if_db_replaced()
|
||||||
@@ -401,9 +403,8 @@ class SessionFtsSetupMixin:
|
|||||||
raise
|
raise
|
||||||
break
|
break
|
||||||
except sqlite3.Error as detach_exc:
|
except sqlite3.Error as detach_exc:
|
||||||
msg = str(detach_exc).lower()
|
|
||||||
if (
|
if (
|
||||||
isinstance(detach_exc, sqlite3.OperationalError) and ("locked" in msg or "busy" in msg)
|
isinstance(detach_exc, sqlite3.OperationalError) and is_sqlite_lock_error(detach_exc)
|
||||||
and self._sleep_before_write_retry(deadline, patience_s)
|
and self._sleep_before_write_retry(deadline, patience_s)
|
||||||
):
|
):
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ user-facing guidance:
|
|||||||
* an FTS-scoped error that still escapes (detach refused) classifies as ``fts_index`` and
|
* an FTS-scoped error that still escapes (detach refused) classifies as ``fts_index`` and
|
||||||
never quarantines the handle;
|
never quarantines the handle;
|
||||||
* a sibling process holding the write lock when the detach runs is waited out, not a lost write;
|
* a sibling process holding the write lock when the detach runs is waited out, not a lost write;
|
||||||
|
* a quarantine that lands while the detach waits stops it: nothing is committed on the file;
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import sqlite3
|
import sqlite3
|
||||||
@@ -20,7 +21,8 @@ from types import SimpleNamespace
|
|||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from hermes_state import SessionDB
|
from hermes_state import SessionDB, StateDbCorruptError
|
||||||
|
from hermes_state_health import mark_storage_corrupt, reset_storage_state
|
||||||
from run_agent import AIAgent
|
from run_agent import AIAgent
|
||||||
|
|
||||||
|
|
||||||
@@ -178,3 +180,62 @@ def test_detach_waits_out_a_sibling_holding_the_write_lock(tmp_path):
|
|||||||
assert db._db_corrupt is False
|
assert db._db_corrupt is False
|
||||||
finally:
|
finally:
|
||||||
db.close()
|
db.close()
|
||||||
|
|
||||||
|
|
||||||
|
def test_quarantine_while_detach_waits_commits_nothing(tmp_path):
|
||||||
|
"""The detach may now wait up to the write budget for the lock. A sibling that quarantines
|
||||||
|
this file meanwhile (structural corruption latched process-wide) must stop it: the retry
|
||||||
|
drops no triggers, commits no stale breadcrumb, and the corrupt error surfaces."""
|
||||||
|
db_path = tmp_path / "state.db"
|
||||||
|
db = SessionDB(db_path=db_path)
|
||||||
|
try:
|
||||||
|
_seed(db, rows=5)
|
||||||
|
_stomp_fts_shadow(db_path)
|
||||||
|
held, release = threading.Event(), threading.Event()
|
||||||
|
|
||||||
|
def sibling():
|
||||||
|
raw = sqlite3.connect(str(db_path), timeout=30, isolation_level=None)
|
||||||
|
raw.execute("BEGIN IMMEDIATE")
|
||||||
|
held.set()
|
||||||
|
release.wait(10)
|
||||||
|
raw.execute("COMMIT")
|
||||||
|
raw.close()
|
||||||
|
|
||||||
|
real_check, real_sleep = db._is_fts_write_corruption_error, db._sleep_before_write_retry
|
||||||
|
holder = []
|
||||||
|
|
||||||
|
def check_then_contend(exc):
|
||||||
|
hit = real_check(exc)
|
||||||
|
if hit and not holder:
|
||||||
|
holder.append(threading.Thread(target=sibling))
|
||||||
|
holder[0].start()
|
||||||
|
assert held.wait(10)
|
||||||
|
return hit
|
||||||
|
|
||||||
|
def quarantine_then_sleep(deadline, patience_s):
|
||||||
|
mark_storage_corrupt(db_path, "database disk image is malformed (sibling handle)")
|
||||||
|
release.set()
|
||||||
|
return real_sleep(deadline, patience_s)
|
||||||
|
|
||||||
|
db._is_fts_write_corruption_error = check_then_contend
|
||||||
|
db._sleep_before_write_retry = quarantine_then_sleep
|
||||||
|
with pytest.raises(StateDbCorruptError):
|
||||||
|
db.append_message("s1", "user", "must not land on a quarantined file")
|
||||||
|
if not holder:
|
||||||
|
pytest.skip("this SQLite build defers FTS shadow corruption past the insert trigger")
|
||||||
|
holder[0].join(10)
|
||||||
|
|
||||||
|
raw = sqlite3.connect(str(db_path))
|
||||||
|
try:
|
||||||
|
triggers = raw.execute(
|
||||||
|
"SELECT count(*) FROM sqlite_master WHERE type = 'trigger' AND name LIKE 'messages_fts%'"
|
||||||
|
).fetchone()[0]
|
||||||
|
stale = raw.execute("SELECT value FROM state_meta WHERE key LIKE 'fts%stale%'").fetchall()
|
||||||
|
finally:
|
||||||
|
raw.close()
|
||||||
|
assert triggers > 0
|
||||||
|
assert stale == []
|
||||||
|
assert db._fts_stale is False
|
||||||
|
finally:
|
||||||
|
db.close()
|
||||||
|
reset_storage_state(db_path)
|
||||||
|
|||||||
Reference in New Issue
Block a user