Files
FabledCurator/tests/test_service_roster.py
T
bvandeusenandClaude Opus 5 45bb7044f7
CI and images / lint (push) Failing after 3s
CI and images / extension-version (push) Successful in 4s
CI and images / frontend-build (push) Successful in 31s
CI and images / backend-lint-and-test (push) Successful in 35s
CI and images / integration (push) Successful in 2m44s
CI and images / sign-extension (push) Skipped
CI and images / build-web (push) Skipped
CI and images / smoke-web (push) Skipped
CI and images / promote (push) Skipped
CI and images / build-agent (push) Skipped
feat: the System tab reads a stored sample instead of inspecting per load (4295)
Operator: "there is a repull every time this page loads is there a reason
this info isn't being tracked in the background and stored in some way?"

There was a reason and it had expired, and underneath it there was plain
waste.

The expired one: /api/system/workers was deliberately uncached because an
operator dragging the stepper must not be shown a pre-change value. That
stopped being true at 1353d34, when the UI began patching its row from the
write's reply instead of refetching.

The waste: size_worker_lanes already inspected the broker on a timer to
decide pool sizes — computing the pool, active, reserved and queue depth
the page shows, using them, and discarding them. The browser then asked
the broker for the same numbers four times a minute, per open tab.

So one inspect now feeds three things: the sizing decision, a stored
sample (worker_lane_sample, alembic 0107), and the celery roster. No
request path touches the broker at all — the roster refresh comes off
/api/system/health too, where it had been rate-limited to 20s and so made
worker liveness a function of whether anyone had a browser open.

Consequences, stated rather than hidden:

- The live figures are up to one sweep old. measured_at travels with each
  lane and the page says how old, because a stale number presented as
  current is how someone watches a queue "not move" that is moving.
- The sweep is the roster's only writer now, so its period and the
  staleness thresholds are in a relationship. 60s against a 90s stale
  threshold left one missed tick between normal and all-yellow — the
  shape of lesson #4355 — so the period is 30s, named once in
  worker_lanes, and system_health asserts its headroom at import with a
  test stating the same thing in prose.
- An idle lane therefore also gives a worker back twice as fast. That is
  the direction asked for: "idle instances quiet down when not running".

Also bounds the inspect in push_lane_cap, which was an await with no
deadline (rule 156) — harmless while it ran on a request, less so now
that it runs in a background task where a hang would be silent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
2026-09-23 18:52:08 -04:00

194 lines
6.9 KiB
Python

"""The roster's inspect budget must exceed the inspect work.
Both tests here assert a RELATION between constants rather than their values.
The failure they exist for is not a wrong number — it is the budget and the
work drifting apart, which is invisible in each one read on its own.
"""
from __future__ import annotations
from backend.app.services import service_roster as sr
from tests.doubles import RecordingSession
def test_the_wrapper_budget_exceeds_the_work_it_waits_for():
"""The bug the operator's first consolidated deploy surfaced (2026-09-23).
`_inspect_celery_sync` makes INSPECT_ROUND_TRIPS broadcasts, and a
broadcast with no `destination` cannot know how many replies to expect, so
each waits out its full timeout rather than returning on the last reply.
The sync call therefore costs round_trips x INSPECT_TIMEOUT_SECONDS.
The wrapper allowed `INSPECT_TIMEOUT_SECONDS * 2`, which reads like a
safety factor and is exactly that worst case with nothing left over. Live
result: a TimeoutError traceback on every refresh while the two inspect
calls were working perfectly, and a roster that stopped advancing.
Asserted as a RELATION, not as a number. A third inspect call added to the
sync function is the way this silently comes back, and the only thing that
keeps the two honest is deriving one from the other.
"""
budget = (
sr.INSPECT_TIMEOUT_SECONDS * sr.INSPECT_ROUND_TRIPS
+ sr.INSPECT_SLACK_SECONDS
)
work = sr.INSPECT_TIMEOUT_SECONDS * sr.INSPECT_ROUND_TRIPS
assert budget > work, (
f"the wrapper allows {budget}s for {work}s of broadcasts — a budget "
f"equal to the work fails under any load at all"
)
assert sr.INSPECT_SLACK_SECONDS > 0
def test_the_round_trip_count_matches_the_calls_actually_made():
"""INSPECT_ROUND_TRIPS is only true if someone keeps it true, so read the
source rather than trusting the constant: the budget above is derived from
it, and a call added without updating it puts the wrapper back under the
work."""
import inspect as _inspect
src = _inspect.getsource(sr._inspect_celery_sync)
calls = src.count("insp.")
assert calls == sr.INSPECT_ROUND_TRIPS, (
f"_inspect_celery_sync makes {calls} inspect calls but "
f"INSPECT_ROUND_TRIPS says {sr.INSPECT_ROUND_TRIPS}"
)
# --- a lane that is off keeps its own row ------------------------------------
def _stub_inspect(monkeypatch, active_queues):
class _Insp:
def __init__(self, **_):
pass
def active_queues(self):
return active_queues
def active(self):
return {}
class _Control:
inspect = _Insp
import sys
import types
mod = types.ModuleType("backend.app.celery_app")
class _C:
pass
c = _C()
c.control = _Control()
mod.celery = c
monkeypatch.setitem(sys.modules, "backend.app.celery_app", mod)
def test_a_lane_with_no_consumers_keeps_the_row_it_had_while_running(monkeypatch):
"""The phantom, and the reason the operator had two wrong rows at once.
A lane at cap 0 cancels its consumers, so it answers with an EMPTY queue
list. Grouped on that, it landed under the key `celery:` and rendered as a
row called `Worker ()` — reported running — while the real lane's row went
stale beside it because nothing updated it any more.
Keyed on the LANE's queue set now, which is the same string the row
already had while the lane was consuming. So turning a lane off updates
its row instead of minting a second one.
"""
from backend.app.services.worker_lanes import LANES_BY_NAME
_stub_inspect(monkeypatch, {"ml@abc123": []})
grouped = sr._inspect_celery_sync()
assert list(grouped) == [LANES_BY_NAME["ml"].queue_key]
assert () not in grouped, "the empty queue set is the phantom `Worker ()`"
def test_the_row_is_the_same_one_whether_the_lane_is_consuming_or_not(monkeypatch):
"""Stated as an identity rather than as two separate assertions: if these
keys ever differ, turning a lane off silently starts a second roster row
and the first goes stale — which is exactly what happened."""
ml_queues = [{"name": "ml"}]
_stub_inspect(monkeypatch, {"ml@abc123": ml_queues})
on = set(sr._inspect_celery_sync())
_stub_inspect(monkeypatch, {"ml@abc123": []})
off = set(sr._inspect_celery_sync())
assert on == off
def test_a_worker_this_build_did_not_name_is_still_grouped_by_its_queues(
monkeypatch,
):
"""The fallback, and the case the roster exists to report honestly: a
deployment slicing CELERY_QUEUES differently gets its raw queue list
rather than a name this code invented for it."""
_stub_inspect(monkeypatch, {"celery@xyz": [{"name": "odd"}]})
assert list(sr._inspect_celery_sync()) == [("odd",)]
# --- the sweep writes the roster, not the page -------------------------------
#
# It used to be refreshed on the /api/system/health request path, rate-limited
# to once per 20s. So the roster only advanced while someone had a browser
# open: the liveness of the workers was a function of whether anyone was
# looking at them. Operator, 2026-09-23: *"is there a reason this info isn't
# being tracked in the background and stored in some way?"*
def test_the_sync_refresh_writes_a_row_for_everything_that_answered(monkeypatch):
monkeypatch.setattr(
sr, "_inspect_celery_sync",
lambda: {("ml",): {"hostnames": ["ml@a"], "active": 2},
("scan",): {"hostnames": ["scheduler@a"], "active": 0}},
)
session = RecordingSession()
sr.refresh_celery_roster_sync(session)
assert len(session.stmts) == 2
assert session.commits == 1
def test_a_broker_that_will_not_answer_does_not_kill_the_sweep(monkeypatch):
"""The sizing pass runs on a timer and does three things; a roster refresh
that raised would take the other two with it. Rows going stale IS the
correct report about a broker nobody can reach."""
def boom():
raise RuntimeError("no broker")
monkeypatch.setattr(sr, "_inspect_celery_sync", boom)
session = RecordingSession()
sr.refresh_celery_roster_sync(session)
assert session.stmts == []
assert session.commits == 0
def test_both_refreshes_build_the_same_row(monkeypatch):
"""The async path (an agent lease over the API) and the sync one (the
sweep) must not drift. Asserted on the shared mapping rather than by
running both, because the thing that could drift is what a roster row IS —
not which kind of session writes it."""
grouped = {("ml",): {"hostnames": ["ml@a", "ml@b"], "active": 3}}
rows = sr._roster_rows(grouped)
assert rows == [{
"key": "celery:ml",
"kind": "celery",
"display_name": sr.role_display_name(("ml",)),
"details": {
"queues": ["ml"],
"hostnames": ["ml@a", "ml@b"],
"replicas": 2,
"active": 3,
},
}]