Files
hermes-webui/tests/test_5455_agent_health_single_flight.py
T
ai-ag2026andClaude Opus 4.8 b88b922db5 Single-flight remote gateway health probe to stop cold-cache stampede
_probe_remote_gateway held _remote_probe_lock only around the cache
read and cache write; the network probe itself (_http_probe per path,
each up to ~2s) ran unlocked. On a cold cache a dashboard that fans out
to N panels fired N concurrent probe sets at the (possibly dead)
gateway, each blocking up to ~6s (#5455, #2476).

Wrap the existing lock in a Condition and add an in-flight set: the
first 'leader' thread marks the base_url in-flight and runs the probe
(via extracted _run_remote_probe), while latecomers wait on the
Condition for the leader's cached result instead of probing themselves.
The leader clears the marker and notify_all()s in a finally block, so
waiters are woken even if the probe raises (no deadlock); a bounded wait
budget lets a waiter fall back to probing itself if a leader hangs.
_reset_remote_probe_cache_for_tests also clears the in-flight set.

Expire the cache from the probe's COMPLETION time, not the leader's
entry time: walking every path of a hung gateway takes
len(paths) * timeout (~6s), which exceeds the 5s TTL, so
`entry_time + TTL` would store an already-expired cache line. Waiters
woken right after would then miss the cache and each re-probe the dead
gateway — collapsing single-flight and regressing latency to worse-than-
serial. A regression test drives a walk longer than the TTL and asserts
still exactly one probe set (the shipped happy-path test kept the walk
well under the TTL and so never exercised this).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-09 03:54:10 +00:00

174 lines
6.6 KiB
Python

"""Regression tests for single-flight remote-gateway health probing (#5455, #2476).
Pre-fix behaviour: ``_probe_remote_gateway`` held ``_remote_probe_lock`` only
around the cache read and the cache write; the network probe itself
(``_http_probe`` per path, each up to ~2s) ran with no lock held. On a cold
cache a dashboard that fans out to N panels therefore fired N concurrent probe
sets at the (possibly dead) gateway, each blocking for up to ~6s.
Fix: single-flight. The first "leader" thread marks the base_url in-flight and
runs the probe; latecomers wait on ``_remote_probe_cond`` for the leader's
cached result instead of probing themselves. The leader always clears the
marker and wakes waiters (even on error) so no waiter can deadlock.
These tests pin that only ONE probe set runs under concurrency, that all callers
observe the same result, and that a probe exception does not hang waiters.
"""
from __future__ import annotations
import threading
import time
import pytest
from api import agent_health
@pytest.fixture(autouse=True)
def _clear_cache():
agent_health._reset_remote_probe_cache_for_tests()
yield
agent_health._reset_remote_probe_cache_for_tests()
def _run_concurrent(base_url: str, n: int, timeout: float = 20.0) -> list[object]:
"""Fire *n* threads at ``_probe_remote_gateway`` simultaneously.
Returns each thread's result (or the exception it raised). Uses a Barrier
so all threads reach the call together and genuinely contend on a cold cache.
"""
barrier = threading.Barrier(n)
results: list[object] = [None] * n
def worker(idx: int) -> None:
barrier.wait()
try:
results[idx] = agent_health._probe_remote_gateway(base_url)
except BaseException as exc: # noqa: BLE001 - recorded for the assertion
results[idx] = exc
threads = [threading.Thread(target=worker, args=(i,)) for i in range(n)]
for t in threads:
t.start()
for t in threads:
t.join(timeout)
# No thread may still be running — a hang would mean a single-flight deadlock.
assert not any(t.is_alive() for t in threads), "probe threads did not finish"
return results
def test_concurrent_cold_cache_probes_only_once(monkeypatch):
base_url = "http://gw:9999"
call_count = 0
count_lock = threading.Lock()
def fake_http_probe(url, timeout_s, *, api_key=None):
nonlocal call_count
with count_lock:
call_count += 1
# Hold the leader in the probe long enough for latecomers to pile up and
# wait, so a lock-less stampede would show as call_count > 1.
time.sleep(0.3)
return (True, 200, None, b"{}") # 2xx on the first path
monkeypatch.setattr(agent_health, "_http_probe", fake_http_probe)
results = _run_concurrent(base_url, n=8)
# Single-flight: exactly one leader ran the probe (first path returned 2xx).
assert call_count == 1, f"expected 1 probe, got {call_count}"
# Every caller observed the same alive result.
for r in results:
assert isinstance(r, dict), r
assert r["alive"] is True
assert r["details"]["endpoint"] == base_url + "/health/detailed"
def test_concurrent_down_gateway_probes_paths_once_each(monkeypatch):
base_url = "http://gw:9998"
call_count = 0
count_lock = threading.Lock()
def fake_http_probe(url, timeout_s, *, api_key=None):
nonlocal call_count
with count_lock:
call_count += 1
time.sleep(0.2)
return (False, None, "URLError", None) # never ok -> walks all paths
monkeypatch.setattr(agent_health, "_http_probe", fake_http_probe)
results = _run_concurrent(base_url, n=6)
# One leader walked every path exactly once; latecomers did not re-probe.
assert call_count == len(agent_health._REMOTE_PROBE_PATHS), (
f"expected {len(agent_health._REMOTE_PROBE_PATHS)} probes, got {call_count}"
)
for r in results:
assert isinstance(r, dict), r
assert r["alive"] is False
assert r["details"]["reason"] == "remote_gateway_unreachable"
def test_slow_walk_exceeding_ttl_still_single_flight(monkeypatch):
"""A probe walk LONGER than the cache TTL must not collapse single-flight.
Regression for a born-expired cache: the leader wrote
``expires_at = entry_time + TTL``. Walking every path of a hung gateway
takes len(paths) * timeout, which exceeds the TTL, so the cache line was
already stale the instant the leader stored it. Latecomers woken right
after then missed the cache and each re-probed the dead gateway — the exact
#5455/#2476 fan-out the fix is meant to prevent (and worse: serialized).
Here the per-path sleep makes the total walk exceed a shrunk TTL. The fix
expires from the COMPLETION time, so the freshly written result stays valid
long enough for every waiter to read it: still exactly one probe set.
"""
base_url = "http://gw:9996"
# Walk = len(paths) * 0.25s; keep TTL well below that but comfortably above
# thread wake/reacquire latency so the completion-time cache is readable.
monkeypatch.setattr(agent_health, "_REMOTE_PROBE_CACHE_TTL_S", 0.4)
call_count = 0
count_lock = threading.Lock()
def fake_http_probe(url, timeout_s, *, api_key=None):
nonlocal call_count
with count_lock:
call_count += 1
time.sleep(0.25)
return (False, None, "URLError", None) # down -> leader walks all paths
monkeypatch.setattr(agent_health, "_http_probe", fake_http_probe)
results = _run_concurrent(base_url, n=8)
# One leader walked every path once; no waiter re-probed despite walk > TTL.
assert call_count == len(agent_health._REMOTE_PROBE_PATHS), (
f"born-expired cache regressed single-flight: expected "
f"{len(agent_health._REMOTE_PROBE_PATHS)} probes, got {call_count}"
)
for r in results:
assert isinstance(r, dict), r
assert r["alive"] is False
def test_probe_exception_wakes_waiters_without_deadlock(monkeypatch):
"""A crash inside the leader's probe must not leave waiters hung on the cond."""
base_url = "http://gw:9997"
boom = RuntimeError("simulated probe crash")
def fake_http_probe(url, timeout_s, *, api_key=None):
time.sleep(0.2)
raise boom
monkeypatch.setattr(agent_health, "_http_probe", fake_http_probe)
results = _run_concurrent(base_url, n=5)
# Every caller surfaced the error (no result cached); crucially, the
# _run_concurrent join assertion already proved none of them deadlocked.
for r in results:
assert isinstance(r, RuntimeError), r
assert "simulated probe crash" in str(r)