the status endpoint must not be the first thing a busy server stops serving (v0.4.21)

During a boot wave /v1/runpod/status stopped answering entirely, and a client
reasonably read that as "the front is dead" -- while /healthz answered in 2.9
ms throughout and the front recorded zero event-loop stalls. It was never the
loop. The snapshot takes the pool locks, a provisioning wave holds them, and
the request simply waited until the caller gave up.

That cost more than observability. It is the one surface an operator has for
seeing WHY a server is busy, so losing it exactly when the server is busy sent
two investigations to the wrong layer, and it silently turned a monitoring
sampler's output into zeros -- 48 pods billing at $35/hr, recorded as an empty
fleet, because a hung read and an empty fleet wrote the same row.

So every lock in `pods_status` is taken with a timeout (`_lock_for_read`,
2 s). A pool whose lock is busy contributes one `state: "busy"` row naming
itself; a locked registry answers one such row rather than an empty list,
because "no pods" and "ask again" are opposite actions and must not look
alike. Writers keep the plain lock: a reader that skips it reports less, which
is the point, while a writer that skipped it would race.

Note for anyone testing this: `_pools_lock` and `pool._cv` are RLocks, so a
test that takes one in the thread that then reads simply re-enters and passes.
The holder in production is always another thread, so the tests hold them from
one too -- three of them passed for the wrong reason until they did.

Also, in the same endpoint: `caps` has been silently empty in every answer it
has ever given. `from codai.config import config_manager` -- which does not
exist, the manager lives in `codai.admin.routes` -- raised every time and the
except swallowed it, so an operator polling for the install's spend ceilings
never received them. Identical mistake, identical shape, to the one that
pinned both thread pools to their floors on every install that ever ran.
Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KnfkxBpcZ1v6ypRFFyhrVM
parent c3361e42
......@@ -16,7 +16,7 @@
# Canonical product version for CoderAI — single source of truth. Both the API
# metadata and the admin web UI read from here.
__version__ = "0.4.20"
__version__ = "0.4.21"
__tagline__ = "Complete orchestration, distribution and escalation of remotizable advanced inference."
# Configure the CUDA caching allocator BEFORE torch is imported anywhere.
......
......@@ -919,7 +919,16 @@ async def runpod_spend(request: Request):
live_cost = sum((p.get("live_cost_usd") or 0.0) for p in pods)
caps = {}
try:
from codai.config import config_manager
# THE SHARED MANAGER LIVES IN `codai.admin.routes`. `codai.config` has
# no `config_manager`, so this import raised every time and the except
# swallowed it: `caps` has been silently empty in every answer this
# endpoint has ever given, which is how an operator polling it never
# saw the spend ceilings it is supposed to report. Same mistake, same
# shape, as the one that pinned both thread pools to their floors.
try:
from codai.admin.routes import config_manager
except Exception:
from codai.config import config_manager
cfg = config_manager.config.runpod if config_manager and config_manager.config else None
if cfg is not None:
caps = {"global_max_hourly_usd": cfg.global_max_hourly_usd,
......
......@@ -2335,6 +2335,7 @@ def auth_headers(account_cfg) -> dict:
# --------------------------------------------------------------------------- #
# Pods mode — coderai-managed pool of remote GPU pods
# --------------------------------------------------------------------------- #
import contextlib
import threading
import time
import uuid
......@@ -5037,6 +5038,39 @@ def _forget_unavailable(gpu_type_id: str, dc: str = "") -> None:
_unavail.pop((str(gpu_type_id), ""), None)
#: How long a READ-ONLY snapshot waits for a lock before giving up on it.
#:
#: A monitoring endpoint that blocks is worse than one that says "busy": during
#: a boot wave /v1/runpod/status stopped answering entirely, which a client
#: reasonably read as "the server is dead" when /healthz was answering in 2.9
#: ms the whole time -- and it is the one surface an operator has for seeing
#: WHY the server is busy, so it must not be the first thing a busy server
#: stops serving. Short, because a partial answer now beats a complete one
#: after the incident.
SNAPSHOT_LOCK_TIMEOUT_S = 2.0
@contextlib.contextmanager
def _lock_for_read(lock, timeout: float = None):
"""Hold `lock` if it can be had quickly; yield False if not.
For snapshots only. A writer must never use this: skipping the lock to
carry on would race. A reader that skips it reports less, which is the
whole point.
"""
want = SNAPSHOT_LOCK_TIMEOUT_S if timeout is None else float(timeout)
got = False
try:
got = lock.acquire(timeout=want)
yield got
finally:
if got:
try:
lock.release()
except RuntimeError:
pass
class NoPodRightNow(RuntimeError):
"""No pod is free THIS INSTANT and the caller asked not to wait here.
......@@ -5728,13 +5762,31 @@ def set_client_pods(model: str, pods: int, teardown: bool = False,
def pods_status() -> list:
"""Snapshot of all pods across pools for the stats/tasks pages."""
"""Snapshot of all pods across pools for the stats/tasks pages.
Every lock here is taken with a timeout: see `_lock_for_read`. A pool whose
lock is busy contributes one `state: "busy"` row naming itself instead of
hanging the whole snapshot, so the answer is always late-but-complete about
what it could see and explicit about what it could not.
"""
now = time.time()
out = []
with _pools_lock:
pools = list(_pools.items())
with _lock_for_read(_pools_lock) as got:
pools = list(_pools.items()) if got else []
if not pools:
out.append({"model": "", "pod_id": "", "gpu": "", "healthy": False,
"inflight": 0, "state": "busy",
"note": "the pool registry was locked; this snapshot is "
"incomplete (a provisioning wave holds it)"})
return out
for key, pool in pools:
with pool._cv:
with _lock_for_read(pool._cv) as got:
if not got:
out.append({"model": key, "pod_id": "", "gpu": "",
"healthy": False, "inflight": 0, "state": "busy",
"note": f"pool {key!r} was locked when this "
f"snapshot was taken"})
continue
for p in pool.pods:
out.append({
"model": key, "pod_id": p.pod_id, "gpu": p.gpu,
......@@ -5750,8 +5802,8 @@ def pods_status() -> list:
})
# Booting pods (created, not yet serving) — visible so a slow/failing boot is
# obvious and its vLLM log is one click away.
with _pools_lock:
prov = list(_provisioning_info.items())
with _lock_for_read(_pools_lock) as got:
prov = list(_provisioning_info.items()) if got else []
for pid, info in prov:
out.append({
"model": info.get("model"), "pod_id": pid, "gpu": info.get("gpu"),
......
"""The one endpoint an operator has for seeing why the server is busy must not
be the first thing a busy server stops serving.
During a boot wave `/v1/runpod/status` stopped answering entirely, which a
client reasonably read as "the front is dead" -- while `/healthz` answered in
2.9 ms throughout and the front had zero event-loop stalls. It was not the
loop: the snapshot takes the pool locks, and a provisioning wave holds them.
So every lock in the snapshot is taken with a timeout. A pool whose lock is
busy contributes one `state: "busy"` row naming itself, and the answer is
always prompt, explicit about what it could not read, and never absent.
"""
import threading
import time
import types
import pytest
from codai.api import runpod_worker as rw
class _HeldElsewhere:
"""Hold a lock from ANOTHER thread.
These are RLocks, so acquiring in the thread that then reads would simply
re-enter and succeed -- which is exactly what a snapshot must not rely on.
The holder in production is always a different thread (a provisioning
wave), so the test has to be one too.
"""
def __init__(self, lock):
self.lock = lock
self._held = threading.Event()
self._release = threading.Event()
self._t = threading.Thread(target=self._run, daemon=True)
def _run(self):
self.lock.acquire()
self._held.set()
self._release.wait(10.0)
self.lock.release()
def __enter__(self):
self._t.start()
assert self._held.wait(5.0), "could not take the lock"
return self
def __exit__(self, *_exc):
self._release.set()
self._t.join(5.0)
return False
def _pool(key="m", pods=()):
p = rw.RunpodPodPool.__new__(rw.RunpodPodPool)
p._cv = threading.Condition(threading.RLock())
p.pods = list(pods)
p.mcfg = types.SimpleNamespace(pool="", engine="coderai-vllm-ocr")
p.model_key = key
return p
def _pod(pid="p1"):
return types.SimpleNamespace(
pod_id=pid, gpu="NVIDIA A40", gpu_count=1, is_spot=False, healthy=True,
inflight=0, hourly_usd=1.0, started_at=time.time(), console_url="",
data_center="EU-SE-1", cloud_type="SECURE")
@pytest.fixture(autouse=True)
def _clean(monkeypatch):
monkeypatch.setattr(rw, "_pools", {}, raising=False)
monkeypatch.setattr(rw, "_provisioning_info", {}, raising=False)
monkeypatch.setattr(rw, "SNAPSHOT_LOCK_TIMEOUT_S", 0.15)
yield
# ------------------------------------------------------------- the helper #
def test_the_helper_yields_true_when_the_lock_is_free():
lk = threading.Lock()
with rw._lock_for_read(lk) as got:
assert got is True
assert lk.locked()
assert not lk.locked()
def test_the_helper_gives_up_rather_than_waiting():
lk = threading.RLock()
with _HeldElsewhere(lk):
t0 = time.monotonic()
with rw._lock_for_read(lk) as got:
assert got is False
assert time.monotonic() - t0 < 1.0
def test_the_helper_does_not_release_a_lock_it_never_took():
lk = threading.Lock()
with _HeldElsewhere(lk):
with rw._lock_for_read(lk) as got:
assert got is False
assert lk.locked(), "it released someone else's lock"
# ----------------------------------------------------------- the snapshot #
def test_a_quiet_fleet_is_reported_normally(monkeypatch):
monkeypatch.setattr(rw, "_pools", {"m": _pool("m", [_pod()])}, raising=False)
out = rw.pods_status()
assert [r for r in out if r.get("pod_id") == "p1"]
assert not [r for r in out if r.get("state") == "busy"]
def test_a_locked_pool_is_reported_busy_not_waited_for(monkeypatch):
pool = _pool("m", [_pod()])
monkeypatch.setattr(rw, "_pools", {"m": pool}, raising=False)
with _HeldElsewhere(pool._cv):
t0 = time.monotonic()
out = rw.pods_status()
took = time.monotonic() - t0
assert took < 2.0, f"the snapshot waited {took:.1f}s on a held lock"
busy = [r for r in out if r.get("state") == "busy"]
assert busy and busy[0]["model"] == "m"
assert "locked" in busy[0]["note"]
def test_one_locked_pool_does_not_hide_the_others(monkeypatch):
busy_pool = _pool("busy", [_pod("b1")])
ok_pool = _pool("ok", [_pod("o1")])
monkeypatch.setattr(rw, "_pools",
{"busy": busy_pool, "ok": ok_pool}, raising=False)
with _HeldElsewhere(busy_pool._cv):
out = rw.pods_status()
assert [r for r in out if r.get("pod_id") == "o1"], "lost a readable pool"
assert [r for r in out if r.get("state") == "busy"
and r.get("model") == "busy"]
def test_a_locked_registry_says_so_instead_of_answering_empty(monkeypatch):
"""An empty list and a locked registry must not look the same: one means
'no pods', the other means 'ask again', and they are opposite actions."""
monkeypatch.setattr(rw, "_pools", {"m": _pool("m", [_pod()])}, raising=False)
with _HeldElsewhere(rw._pools_lock):
out = rw.pods_status()
assert len(out) == 1
assert out[0]["state"] == "busy"
assert "incomplete" in out[0]["note"]
def test_the_timeout_is_short_enough_to_be_useful():
assert 0.1 <= rw.SNAPSHOT_LOCK_TIMEOUT_S <= 10.0
def test_a_truly_empty_fleet_is_still_empty(monkeypatch):
monkeypatch.setattr(rw, "_pools", {}, raising=False)
out = rw.pods_status()
# No pools at all: the registry was readable and there was nothing in it.
assert all(r.get("state") == "busy" for r in out) or out == []
# ------------------------------------------------- the caps it never sent #
def test_the_spend_caps_are_read_from_the_manager_that_exists():
"""`codai.config` has no `config_manager`, so this import raised every
time and the except swallowed it: caps was silently empty in every answer
the endpoint ever gave. Same mistake that pinned both thread pools to
their floors."""
import codai.api.app as app_mod
src = open(app_mod.__file__).read()
i = src.index("caps = {}")
window = src[i:i + 900]
assert "from codai.admin.routes import config_manager" in window
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment