fix: the cap dial waited out a broker round trip it did not need (4295)
CI and images / lint (push) Successful in 3s
CI and images / extension-version (push) Successful in 3s
CI and images / frontend-build (push) Successful in 25s
CI and images / backend-lint-and-test (push) Successful in 33s
CI and images / integration (push) Successful in 2m13s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-agent (push) Successful in 6s
CI and images / build-web (push) Successful in 1m59s
CI and images / smoke-web (push) Successful in 58s
CI and images / promote (push) Skipped
CI and images / lint (push) Successful in 3s
CI and images / extension-version (push) Successful in 3s
CI and images / frontend-build (push) Successful in 25s
CI and images / backend-lint-and-test (push) Successful in 33s
CI and images / integration (push) Successful in 2m13s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-agent (push) Successful in 6s
CI and images / build-web (push) Successful in 1m59s
CI and images / smoke-web (push) Successful in 58s
CI and images / promote (push) Skipped
Operator: "when the number is changed the change should be queued so that
it isn't blocking of the webui or the system itself. we shouldn't have to
wait for the validation live."
Two waits, and 5b6f2ba removed neither — it stopped a Postgres connection
being HELD across them, which is what had been stalling the whole site,
and left the press itself as slow as it was.
1. The store refetched after every write. GET /api/system/workers runs a
celery inspect on an eleven-second budget, so the stepper stayed
disabled through a round trip the press did not need. It now patches
the row from the reply — cap, ceiling, enabled, the three fields that
reply actually decides — and lets the 15s poll bring the live columns,
which are measurements it must not invent.
2. The endpoint pushed to the broker before answering. Turning a lane off
is four cancel_consumer messages; lowering a cap reads the live pool
first. Now it stores the cap, answers `queued`, and hands the push to a
Quart background task. Raising a cap was already free and stays free.
Nothing is lost by not waiting: the stored cap is what the system obeys
and the sizing pass re-reads it every minute. That sweep was already the
backstop for a push that failed, which under `no_live_workers` is every
push in the suite.
Also closes a hole the move exposed: the model fetch was gated on the
consumer change having landed, so raising ML off zero while the lane was
restarting stored the cap, let the sizing pass start the consumers a
minute later, and left the lane running with no model — nothing else ever
asks for one. It now fires on the transition and waits in the ml queue.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
This commit is contained in:
+65
-13
@@ -19,8 +19,9 @@ second into the first would make a read-only module a write one.
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import UTC, datetime
|
||||
from functools import partial
|
||||
|
||||
from quart import Blueprint, jsonify, request
|
||||
from quart import Blueprint, current_app, jsonify, request
|
||||
|
||||
from ..extensions import get_session
|
||||
from ..services.worker_control import (
|
||||
@@ -30,7 +31,7 @@ from ..services.worker_control import (
|
||||
push_lane_cap,
|
||||
store_lane_cap,
|
||||
)
|
||||
from ..services.worker_lanes import LANES_BY_NAME
|
||||
from ..services.worker_lanes import LANES_BY_NAME, Lane, derived_ceiling
|
||||
from ._responses import error_response as _bad
|
||||
|
||||
workers_bp = Blueprint("workers", __name__, url_prefix="/api/system/workers")
|
||||
@@ -63,22 +64,38 @@ async def list_lanes():
|
||||
|
||||
@workers_bp.route("/<name>", methods=["POST"])
|
||||
async def update_lane(name: str):
|
||||
"""Set a lane's cap. Stores it, then makes the live lane obey it.
|
||||
"""Set a lane's cap. Stores it, answers, and makes the lane follow after.
|
||||
|
||||
ONE field, since 2026-09-23. It used to take `slots`, `slots_cap`,
|
||||
`enabled` and `autoscale`; how many workers are running is now a
|
||||
measurement the sizing pass owns, and `enabled` is `cap > 0`.
|
||||
|
||||
Two failure kinds, deliberately different statuses:
|
||||
## The reply does not wait for the lane
|
||||
|
||||
Operator, 2026-09-23: *"when the number is changed the change should be
|
||||
queued so that it isn't blocking of the webui or the system itself. we
|
||||
shouldn't have to wait for the validation live."*
|
||||
|
||||
So the request does exactly one thing that can be slow — a row update —
|
||||
and hands the broker work to a background task. Turning a lane off is
|
||||
four `cancel_consumer` messages and a resize; lowering a cap is an
|
||||
`inspect` on an eleven-second budget. Both used to happen between the
|
||||
click and the response, with the stepper disabled the whole time.
|
||||
|
||||
Nothing is lost by not waiting: the cap in the database is what the
|
||||
system obeys, the sizing pass re-reads it every minute, and the table
|
||||
polls, so the live columns catch up on their own. If the web process dies
|
||||
before the background task runs, that sweep is the backstop — which is
|
||||
the same guarantee the awaited version had, since a push could fail
|
||||
there too.
|
||||
|
||||
Refusals still happen inline, because they are decided from the value and
|
||||
the machine's ceiling alone and never touch the broker:
|
||||
|
||||
* **400** — the value is not allowed (negative, or above what this
|
||||
container can hold). Nothing was stored. The body carries `detail`,
|
||||
which is the sentence the UI shows; a refused control with no reason
|
||||
reads as a bug.
|
||||
* **200 with `applied: false`** — the value WAS stored but could not be
|
||||
pushed, because the lane is not currently answering. Not an error: the
|
||||
sizing pass carries it within a minute, and the UI should say "saved,
|
||||
not yet live" rather than "that didn't work".
|
||||
"""
|
||||
lane = LANES_BY_NAME.get(name)
|
||||
if lane is None:
|
||||
@@ -97,13 +114,48 @@ async def update_lane(name: str):
|
||||
if not isinstance(value, int) or isinstance(value, bool):
|
||||
return _bad("invalid_body", detail="slots_cap must be an integer")
|
||||
|
||||
# Store, close the session, THEN push. Same reason as the GET above, and
|
||||
# more sharply here: a cap change could do three broker round trips, all
|
||||
# of them previously with a connection held.
|
||||
# Store, close the session, THEN hand off. The session must not be held
|
||||
# across broker work — that is what made this page block the whole site
|
||||
# (see `worker_control.LaneSettings`) — and now the request does not wait
|
||||
# for that work either.
|
||||
async with get_session() as session:
|
||||
try:
|
||||
was_cap = await store_lane_cap(session, lane, value)
|
||||
except LaneUpdateRefused as exc:
|
||||
return _bad("refused", detail=str(exc))
|
||||
result = await push_lane_cap(lane, value, was_cap=was_cap)
|
||||
return jsonify(result)
|
||||
|
||||
_schedule_push(lane, value, was_cap)
|
||||
|
||||
return jsonify({
|
||||
"name": lane.name,
|
||||
"slots_cap": value,
|
||||
"ceiling": derived_ceiling(lane),
|
||||
"enabled": value > 0,
|
||||
# The value is stored; the live lane is being told separately. The UI
|
||||
# patches its row from this and lets the next poll bring the live
|
||||
# columns, rather than refetching and paying for an inspect it just
|
||||
# avoided.
|
||||
"queued": True,
|
||||
# Raising the cap off zero is what downloads the model (step 6), and
|
||||
# the background task does it. Reported here so the UI can say a
|
||||
# download has started rather than leaving the operator to wonder why
|
||||
# a lane they just turned on is busy.
|
||||
"fetching_models": value > 0 and was_cap == 0 and bool(lane.models),
|
||||
})
|
||||
|
||||
|
||||
def _schedule_push(lane: Lane, slots_cap: int, was_cap: int) -> None:
|
||||
"""Run the live push after the response has gone out.
|
||||
|
||||
A seam, not an abstraction: it is one call, and it exists so the tests can
|
||||
hold the push still — a background task that outlived a test's patches
|
||||
would reach the real broker during teardown.
|
||||
|
||||
Quart tracks the task on the app and awaits it at shutdown, so an
|
||||
in-flight push survives a graceful restart. `partial` rather than passing
|
||||
`was_cap=` through `add_background_task`, so nothing depends on how that
|
||||
forwards keyword arguments.
|
||||
"""
|
||||
current_app.add_background_task(
|
||||
partial(push_lane_cap, lane, slots_cap, was_cap=was_cap)
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user