fix(gateway): grace window keeps first-call goal persistence on healthy DBs
Review finding on the off-loop bootstrap: returning None on every cold- cache loop-thread call silently dropped the first goal/heartbeat persistence op even when the DB was perfectly healthy. The loop-thread path now waits up to 250ms on the bootstrap event - a healthy init (tens of ms) completes inside the window and the caller gets the real DB; a contended init (the crash-loop scenario) exceeds it and degrades to None with a bounded, watchdog-safe stall.
This commit is contained in:
committed by
Teknium
parent
e997435004
commit
c9dbdbcea7
+23
-7
@@ -664,10 +664,17 @@ def _meta_key(session_id: str) -> str:
|
||||
|
||||
_DB_CACHE: Dict[str, Any] = {}
|
||||
_DB_BOOTSTRAP_LOCK = threading.Lock()
|
||||
_DB_BOOTSTRAP_INFLIGHT: set = set()
|
||||
_DB_BOOTSTRAP_INFLIGHT: Dict[str, threading.Event] = {}
|
||||
|
||||
# How long a loop-thread caller waits for the background bootstrap before
|
||||
# degrading to None. Normal SessionDB init is ~10-100ms, so the common case
|
||||
# still returns a real DB (no silently dropped goal writes); a contended
|
||||
# init (locked state.db mid-migration) blows past this and the caller
|
||||
# degrades, with the loop stalled far under the watchdog's probe window.
|
||||
_DB_BOOTSTRAP_LOOP_WAIT_S = 0.25
|
||||
|
||||
|
||||
def _bootstrap_session_db(home: str) -> None:
|
||||
def _bootstrap_session_db(home: str, done: threading.Event) -> None:
|
||||
"""Construct SessionDB off-loop and populate the cache (worker thread)."""
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
@@ -679,7 +686,8 @@ def _bootstrap_session_db(home: str) -> None:
|
||||
with _DB_BOOTSTRAP_LOCK:
|
||||
if db is not None and home not in _DB_CACHE:
|
||||
_DB_CACHE[home] = db
|
||||
_DB_BOOTSTRAP_INFLIGHT.discard(home)
|
||||
_DB_BOOTSTRAP_INFLIGHT.pop(home, None)
|
||||
done.set()
|
||||
|
||||
|
||||
def _get_session_db() -> Optional[Any]:
|
||||
@@ -727,15 +735,23 @@ def _get_session_db() -> Optional[Any]:
|
||||
cached = _DB_CACHE.get(home)
|
||||
if cached is not None:
|
||||
return cached
|
||||
if home not in _DB_BOOTSTRAP_INFLIGHT:
|
||||
_DB_BOOTSTRAP_INFLIGHT.add(home)
|
||||
done = _DB_BOOTSTRAP_INFLIGHT.get(home)
|
||||
if done is None:
|
||||
done = threading.Event()
|
||||
_DB_BOOTSTRAP_INFLIGHT[home] = done
|
||||
threading.Thread(
|
||||
target=_bootstrap_session_db,
|
||||
args=(home,),
|
||||
args=(home, done),
|
||||
name="goals-sessiondb-bootstrap",
|
||||
daemon=True,
|
||||
).start()
|
||||
return None
|
||||
# Grace window: a healthy init finishes in tens of ms, so waiting
|
||||
# briefly keeps goal/heartbeat persistence working on the very first
|
||||
# loop-thread call instead of silently dropping it. A contended init
|
||||
# (the crash-loop scenario) exceeds the window and we degrade to
|
||||
# None — a bounded stall far below the watchdog's probe timeout.
|
||||
done.wait(_DB_BOOTSTRAP_LOOP_WAIT_S)
|
||||
return _DB_CACHE.get(home)
|
||||
|
||||
try:
|
||||
db = SessionDB()
|
||||
|
||||
@@ -53,9 +53,10 @@ def _patch_sessiondb(monkeypatch):
|
||||
monkeypatch.setattr(hermes_state, "SessionDB", _RecordingDB)
|
||||
|
||||
|
||||
def test_loop_thread_cache_miss_returns_none_without_constructing(monkeypatch):
|
||||
"""On the event-loop thread with a cold cache: no inline construction,
|
||||
immediate None, background population."""
|
||||
def test_loop_thread_cache_miss_constructs_off_loop(monkeypatch):
|
||||
"""On the event-loop thread with a cold cache: construction happens on a
|
||||
background thread (never the loop thread). A fast init is returned via
|
||||
the grace window; either way the loop thread never runs SessionDB()."""
|
||||
_patch_sessiondb(monkeypatch)
|
||||
loop_thread_id = None
|
||||
inline_result = "UNSET"
|
||||
@@ -67,9 +68,10 @@ def test_loop_thread_cache_miss_returns_none_without_constructing(monkeypatch):
|
||||
|
||||
asyncio.run(main())
|
||||
|
||||
# The loop-thread call must not have blocked on construction.
|
||||
assert inline_result is None
|
||||
# Background thread eventually populates the cache, OFF the loop thread.
|
||||
# Fast init: the grace window returns the real instance (no silent
|
||||
# feature loss on the first call).
|
||||
assert isinstance(inline_result, _RecordingDB)
|
||||
# Cache populated for subsequent calls.
|
||||
deadline = time.monotonic() + 5
|
||||
while time.monotonic() < deadline and not goals._DB_CACHE:
|
||||
time.sleep(0.02)
|
||||
@@ -108,7 +110,8 @@ def test_worker_thread_constructs_inline(monkeypatch):
|
||||
|
||||
def test_slow_construction_does_not_block_the_loop(monkeypatch):
|
||||
"""A SessionDB whose init blocks (locked-DB migration) must not stall
|
||||
the event loop past the watchdog probe window."""
|
||||
the event loop past the watchdog probe window: bounded grace wait,
|
||||
then degrade to None."""
|
||||
import hermes_state
|
||||
|
||||
class _BlockingDB:
|
||||
@@ -120,14 +123,16 @@ def test_slow_construction_does_not_block_the_loop(monkeypatch):
|
||||
|
||||
monkeypatch.setattr(hermes_state, "SessionDB", _BlockingDB)
|
||||
elapsed = None
|
||||
result = "UNSET"
|
||||
|
||||
async def main():
|
||||
nonlocal elapsed
|
||||
nonlocal elapsed, result
|
||||
t0 = time.monotonic()
|
||||
goals._get_session_db()
|
||||
result = goals._get_session_db()
|
||||
elapsed = time.monotonic() - t0
|
||||
|
||||
asyncio.run(main())
|
||||
assert elapsed is not None and elapsed < 0.5, (
|
||||
assert result is None, "contended init must degrade to None"
|
||||
assert elapsed is not None and elapsed < 1.0, (
|
||||
f"loop-thread call blocked for {elapsed:.2f}s — watchdog territory"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user