fix(state): fail closed when a state.db admission lock file cannot be opened
state.db has two cross-process admission authorities gating destructive work on a file several Hermes processes share: fts_rebuild_admission for full structural FTS rebuilds, and _cross_process_repair_lock for writable_schema surgery / VACUUM. Both document themselves as fail-closed, and both honoured that only for a timed-out acquire. When the lock file could not be open()ed at all they yielded True and proceeded "with in-process serialisation only" — which is no cross-process authority whatsoever. That inversion is reachable exactly when it does the most damage. Creating the lock file needs a directory entry and an inode, so on a full disk open() raises ENOSPC — while a sibling that opened ITS handle before the disk filled is still mid-rebuild or mid-surgery. Every process then ran concurrent destructive work on the same live DB: precisely the interleaving PR #93200 added these locks to prevent, and the shape reported in #100368 (disk-full trigger, then a fresh corruption on every boot with other writers alive, and no re-corruption on a boot with zero other writers). Both helpers now yield False on OSError. This routes the error into the outcome the locks already define and every caller already handles: rebuild_fts returns 0, _recover_stale_fts leaves canonical writes plus LIKE search available behind the retryable stale breadcrumb, the startup path detaches FTS triggers, and repair_state_db_schema re-probes and reports. Nothing reachable is lost — on a read-only directory the rebuild's and the repair's own writes could not have committed either. The repair report's error string now names both ways the authority can be missing, since operators read it directly.
This commit is contained in:
+27
-15
@@ -2336,10 +2336,11 @@ def _cross_process_repair_lock(db_path: Path):
|
||||
"""Serialize state.db schema surgery across processes.
|
||||
|
||||
Yields True when this process holds the repair lock for *db_path*, False
|
||||
when the bounded acquire timed out. Unlike the kanban init lock — whose
|
||||
critical section is idempotent, so proceeding without the lock is merely
|
||||
redundant work — proceeding here would be exactly the unsafe interleaving
|
||||
we are trying to prevent, so a caller that gets False must NOT do surgery.
|
||||
when the bounded acquire timed out or the lock file could not be opened at
|
||||
all. Unlike the kanban init lock — whose critical section is idempotent,
|
||||
so proceeding without the lock is merely redundant work — proceeding here
|
||||
would be exactly the unsafe interleaving we are trying to prevent, so a
|
||||
caller that gets False must NOT do surgery.
|
||||
|
||||
``flock`` is the right primitive for this: the kernel drops the lock when
|
||||
the holding process dies, so a crashed repairer cannot leave a stale lock
|
||||
@@ -2357,14 +2358,22 @@ def _cross_process_repair_lock(db_path: Path):
|
||||
lock_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
handle = lock_path.open("a+b")
|
||||
except OSError as exc:
|
||||
# Read-only dir, exhausted fds, exotic filesystem: fall back to the
|
||||
# in-process behaviour that shipped before this lock existed rather
|
||||
# than refusing to repair a DB we could otherwise heal.
|
||||
# Fail closed, exactly as a timed-out acquire does. A lock file we
|
||||
# cannot even open means the filesystem is out of space, inodes or
|
||||
# descriptors — and a sibling that opened ITS handle before the disk
|
||||
# filled is still inside writable_schema surgery or VACUUM. Yielding
|
||||
# True here let two processes run schema surgery on the same live
|
||||
# state.db concurrently, which is itself the corruption source this
|
||||
# lock exists to remove (#100368: the disk-full trigger, then a fresh
|
||||
# corruption on every boot with other writers alive). Callers already
|
||||
# handle False by re-probing and reporting, and on a read-only
|
||||
# directory no repair strategy could have written anyway.
|
||||
logger.warning(
|
||||
"Could not open state.db repair lock %s (%s) — proceeding with "
|
||||
"in-process serialisation only.", lock_path, exc,
|
||||
"Could not open state.db repair lock %s (%s) — skipping schema "
|
||||
"surgery rather than running it without cross-process authority.",
|
||||
lock_path, exc,
|
||||
)
|
||||
yield True
|
||||
yield False
|
||||
return
|
||||
|
||||
acquired = False
|
||||
@@ -3614,16 +3623,19 @@ def repair_state_db_schema(db_path: Path, *, backup: bool = True) -> Dict[str, A
|
||||
result = report
|
||||
with _cross_process_repair_lock(db_path) as holding_lock:
|
||||
if not holding_lock:
|
||||
# Another process is still inside its critical section. It may
|
||||
# nonetheless have healed the file already (long VACUUM after a
|
||||
# successful strategy), so re-probe before reporting failure.
|
||||
# Another process is still inside its critical section, or the
|
||||
# lock file itself could not be opened (full disk / no fds). It
|
||||
# may nonetheless have healed the file already (long VACUUM after
|
||||
# a successful strategy), so re-probe before reporting failure.
|
||||
if _db_opens_cleanly(db_path) is None:
|
||||
report["repaired"] = True
|
||||
report["strategy"] = "repaired_by_other_process"
|
||||
else:
|
||||
report["error"] = (
|
||||
"another process holds the state.db repair lock; skipped "
|
||||
"schema surgery to avoid racing it"
|
||||
"could not obtain the state.db repair lock (held by "
|
||||
"another process, or the lock file was unopenable); "
|
||||
"skipped schema surgery to avoid racing a concurrent "
|
||||
"repairer"
|
||||
)
|
||||
else:
|
||||
# The fast check above avoids taking the lock for a known-exhausted
|
||||
|
||||
+19
-9
@@ -1132,10 +1132,11 @@ def fts_rebuild_admission(db_path):
|
||||
"""Serialize full structural FTS rebuilds on *db_path* across processes.
|
||||
|
||||
Yields True when this process holds the rebuild authority, False when the
|
||||
bounded acquire timed out. A caller that gets False must NOT perform a
|
||||
full rebuild — proceeding is exactly the concurrent-rebuild interleaving
|
||||
this lock exists to prevent (fail closed). The deferred/stale breadcrumb
|
||||
machinery already guarantees a skipped rebuild is retried later.
|
||||
bounded acquire timed out or the lock file could not be opened at all. A
|
||||
caller that gets False must NOT perform a full rebuild — proceeding is
|
||||
exactly the concurrent-rebuild interleaving this lock exists to prevent
|
||||
(fail closed). The deferred/stale breadcrumb machinery already guarantees
|
||||
a skipped rebuild is retried later.
|
||||
|
||||
``db_path`` may be a str or Path; None (in-memory DB / tests without a
|
||||
file path) yields True — a private in-memory DB has no cross-process
|
||||
@@ -1148,13 +1149,22 @@ def fts_rebuild_admission(db_path):
|
||||
try:
|
||||
handle = open(lock_path, "a+b")
|
||||
except OSError as exc:
|
||||
# Read-only dir, exhausted fds, exotic filesystem: fall back to the
|
||||
# pre-lock behaviour rather than refusing a rebuild we could run.
|
||||
# Fail closed, exactly as a timed-out acquire does. A lock file we
|
||||
# cannot even open means the filesystem is out of space, inodes or
|
||||
# descriptors — and a sibling process that opened ITS handle before
|
||||
# the disk filled is still holding the authority and rebuilding.
|
||||
# Yielding True here handed every process on a full disk a concurrent
|
||||
# structural rebuild of the same live state.db with no cross-process
|
||||
# authority at all: the disk-full trigger and the re-corruption on
|
||||
# every multi-writer boot in #100368. Deferring costs nothing that
|
||||
# was reachable anyway — the breadcrumb retries, and on a read-only
|
||||
# directory the rebuild's own writes could not have committed either.
|
||||
logger.warning(
|
||||
"Could not open FTS rebuild lock %s (%s) — proceeding with "
|
||||
"in-process serialisation only.", lock_path, exc,
|
||||
"Could not open FTS rebuild lock %s (%s) — deferring this rebuild "
|
||||
"rather than running it without cross-process authority.",
|
||||
lock_path, exc,
|
||||
)
|
||||
yield True
|
||||
yield False
|
||||
return
|
||||
|
||||
acquired = False
|
||||
|
||||
Reference in New Issue
Block a user