fix(state): doctor's holder count enumerates macOS holders via libproc (#109641)
`count_db_holders` returned None on every non-Linux host, so `hermes doctor` on macOS (every reporter in the deleted-WAL cluster) printed no "N process(es) holding the DB open" row. The libproc enumeration that #110544 landed for the sidecar scan already yields `(pid, fd, path, (st_dev, st_ino))`; count distinct PIDs whose fd identity matches state.db's inode. Identity, not pathname: libproc reports the path as the opener spelled it (case, symlinked prefix), which is what the sidecar leg had to case-fold around. Carries the surviving delta from #110023 (@kshitijk4poor), whose libproc leg otherwise landed via #110544.
This commit is contained in:
+10
-4
@@ -765,13 +765,19 @@ def collect_state_db_stats(db_path: Path) -> Dict[str, Any]:
|
|||||||
|
|
||||||
|
|
||||||
def count_db_holders(db_path: Path) -> Optional[int]:
|
def count_db_holders(db_path: Path) -> Optional[int]:
|
||||||
"""Best-effort count of distinct PIDs holding ``db_path`` open (``/proc/*/fd`` scan); ``None``
|
"""Best-effort count of distinct PIDs holding ``db_path`` open (``/proc/*/fd`` on Linux, libproc
|
||||||
on any error or non-Linux host, never raises. Unreadable fd dirs (other users' processes
|
on macOS); ``None`` on any error or other host, never raises. Uninspectable processes (other
|
||||||
without root) are skipped, so this is a lower bound."""
|
users' without root) are skipped, so this is a lower bound."""
|
||||||
try:
|
try:
|
||||||
|
target = os.path.realpath(str(db_path))
|
||||||
|
if sys.platform == "darwin":
|
||||||
|
# Identity, not pathname: libproc reports the vnode's last name as the opener spelled it
|
||||||
|
# (case, symlinked prefix), which is exactly what the sidecar leg had to case-fold around.
|
||||||
|
st = os.stat(target)
|
||||||
|
identity = (st.st_dev, st.st_ino)
|
||||||
|
return len({pid for pid, _fd, _path, ident in _iter_darwin_fd_targets() if ident == identity})
|
||||||
if not sys.platform.startswith("linux"):
|
if not sys.platform.startswith("linux"):
|
||||||
return None
|
return None
|
||||||
target = os.path.realpath(str(db_path))
|
|
||||||
return len({pid for pid, link, _fd_path in _iter_proc_fd_targets() if link == target})
|
return len({pid for pid, link, _fd_path in _iter_proc_fd_targets() if link == target})
|
||||||
except Exception:
|
except Exception:
|
||||||
return None
|
return None
|
||||||
|
|||||||
@@ -4,8 +4,8 @@ Covers:
|
|||||||
- ``hermes_state_dbfile.collect_state_db_stats``: read-only, best-effort stats
|
- ``hermes_state_dbfile.collect_state_db_stats``: read-only, best-effort stats
|
||||||
(page_count, freelist, WAL size, journal mode, row counts, FTS presence,
|
(page_count, freelist, WAL size, journal mode, row counts, FTS presence,
|
||||||
pending v23 FTS-rebuild bookkeeping).
|
pending v23 FTS-rebuild bookkeeping).
|
||||||
- ``hermes_state_dbfile.count_db_holders``: /proc-based best-effort probe for how
|
- ``hermes_state_dbfile.count_db_holders``: best-effort probe for how many processes
|
||||||
many processes hold the DB file open (Linux only; None elsewhere/on error).
|
hold the DB file open (/proc on Linux, libproc on macOS; None elsewhere/on error).
|
||||||
- ``hermes_cli.doctor_state._render_state_db_stats``: formatting/threshold helper
|
- ``hermes_cli.doctor_state._render_state_db_stats``: formatting/threshold helper
|
||||||
the doctor state.db section prints from.
|
the doctor state.db section prints from.
|
||||||
"""
|
"""
|
||||||
@@ -14,7 +14,6 @@ import hermes_state_dbfile
|
|||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import sqlite3
|
import sqlite3
|
||||||
import sys
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
@@ -138,19 +137,28 @@ def test_collect_stats_rebuild_pending_flag(populated_db):
|
|||||||
# ── count_db_holders ────────────────────────────────────────────────────
|
# ── count_db_holders ────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
def test_count_db_holders_sees_open_connection(populated_db):
|
def _assert_sees_own_open_connection(db_path):
|
||||||
conn = sqlite3.connect(str(populated_db))
|
conn = sqlite3.connect(str(db_path))
|
||||||
try:
|
try:
|
||||||
holders = count_db_holders(populated_db)
|
holders = count_db_holders(db_path)
|
||||||
if sys.platform.startswith("linux"):
|
assert isinstance(holders, int)
|
||||||
assert isinstance(holders, int)
|
assert holders >= 1
|
||||||
assert holders >= 1
|
|
||||||
else:
|
|
||||||
assert holders is None
|
|
||||||
finally:
|
finally:
|
||||||
conn.close()
|
conn.close()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.linux_only
|
||||||
|
def test_count_db_holders_sees_open_connection_linux(populated_db):
|
||||||
|
_assert_sees_own_open_connection(populated_db)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.macos_only
|
||||||
|
def test_count_db_holders_sees_open_connection_macos(populated_db):
|
||||||
|
# #109641: the doctor's holder count was Linux-only, so `hermes doctor` on the platform with
|
||||||
|
# every reporter in the deleted-WAL cluster printed no holder row at all.
|
||||||
|
_assert_sees_own_open_connection(populated_db)
|
||||||
|
|
||||||
|
|
||||||
def test_count_db_holders_missing_path_no_raise(tmp_path):
|
def test_count_db_holders_missing_path_no_raise(tmp_path):
|
||||||
holders = count_db_holders(tmp_path / "absent.db")
|
holders = count_db_holders(tmp_path / "absent.db")
|
||||||
assert holders is None or isinstance(holders, int)
|
assert holders is None or isinstance(holders, int)
|
||||||
|
|||||||
Reference in New Issue
Block a user