feat: one image for every lane, with the model fetch gated on enabling (4296)
CI / lint (push) Successful in 2s
CI / extension-version (push) Successful in 2s
Build images / sign-extension (push) Successful in 3s
Build images / build-agent (push) Successful in 6s
CI / frontend-build (push) Successful in 20s
extension / lint (push) Successful in 23s
CI / backend-lint-and-test (push) Failing after 31s
CI / integration (push) Successful in 2m16s
Build images / build-ml (push) Successful in 3m8s
Build images / build-web (push) Successful in 3m16s
Build images / smoke-web (push) Skipped
Build images / promote (push) Skipped
CI / lint (push) Successful in 2s
CI / extension-version (push) Successful in 2s
Build images / sign-extension (push) Successful in 3s
Build images / build-agent (push) Successful in 6s
CI / frontend-build (push) Successful in 20s
extension / lint (push) Successful in 23s
CI / backend-lint-and-test (push) Failing after 31s
CI / integration (push) Successful in 2m16s
Build images / build-ml (push) Successful in 3m8s
Build images / build-web (push) Successful in 3m16s
Build images / smoke-web (push) Skipped
Build images / promote (push) Skipped
Milestone 422 step 6. Dockerfile.ml is gone; the main image carries torch,
torchvision, transformers, onnxruntime and opencv, and serves every lane.
WHY IT HAD TO MERGE: step 5 runs every lane in one process tree, so a second
image would mean the `ml` lane could never be enabled from the UI — there
would be no worker in that container to enable. The switch needs something to
switch.
THE MODEL NO LONGER DOWNLOADS AT BOOT. `entrypoint.sh`'s ml-worker role ran
download_models before celery started, so every boot of that role reached
HuggingFace for ~3.5GB — a startup dependency on a third party for a feature
the operator may never use. Rule 164 permits a runtime fetch only for
something "optional and clearly off", so the fetch is now a TASK, enqueued
the moment the lane is ENABLED.
Being a task is what makes it visible: it gets a TaskRun row, so the download
shows in Activity with a duration and a status, and a failure is something an
operator can see and retry rather than a container that quietly never became
useful. Idempotent, so re-enabling a provisioned lane costs one no-op.
Enqueued only when the lane actually came ON (`enabled is True`, not the
resolved value) so re-saving slots does not re-fetch, and only when the
consumer change landed — a task queued onto a queue nothing consumes would
sit pending with no explanation.
`fabledcurator-ml` KEEPS PUBLISHING, from the merged Dockerfile. The
operator's Swarm stack references that name and lives outside this repo;
dropping it would not break their deploy, it would freeze it silently at the
last publish — the exact failure class this milestone keeps finding. Retiring
the NAME is its own task, gated on that stack moving. Same two-phase shape
#406 used for pixiv.
THREE LIVE BREAKAGES from deleting the file, found by grepping for it rather
than assuming the build was the only consumer:
- `docker-compose.override.yml` built the ml service from it (contributor
path would have failed at `docker compose build`).
- `tests/test_artifact_paths.py` pins the ml path set.
- `scripts/artifacts.sh` ML_PATHS named it. A path set naming a deleted file
silently stops contributing to the derived revision — which the reuse check
and the version string both read. That is #3202's recorded shape.
The `--with-ml` flag is gone from the generator and the healthcheck rather
than left defaulting to true. One image carries every lane now, so a flag
that can only be passed one way is a branch pretending to be a choice.
The advisory shipped in ecbd325 is what makes this honest to an adopter: the
lane says it is optional, names the model, and gives its download and
per-slot RAM before the switch is thrown.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
This commit is contained in:
@@ -36,12 +36,16 @@ puts tini in front of supervisord. Neither choice reaches the application —
|
||||
nothing in FC talks to the supervisor — so this is reversible without touching
|
||||
a line of product code.
|
||||
|
||||
## What this does NOT start
|
||||
## Every lane, including ml
|
||||
|
||||
The `ml` lane, unless `--with-ml` is passed. Until step 6 merges the images,
|
||||
torch and the ML requirements live only in `Dockerfile.ml`, so an `ml` program
|
||||
in the web image would fail to import on every restart forever. Step 6 is
|
||||
where that flag turns on.
|
||||
Step 6 merged the images, so this one carries torch and the ML requirements
|
||||
and the `ml` lane gets a program like any other. It starts at one slot with
|
||||
its consumers CANCELLED — `enabled=false` in the seeded settings — so it
|
||||
holds a process and no model. That matters: `add_consumer` needs a running
|
||||
worker to reach, and without one the UI switch would have nothing to switch.
|
||||
|
||||
Nothing is downloaded by starting it. The model fetch is enqueued when the
|
||||
lane is enabled, which is what lets rule 164 permit a runtime fetch at all.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -69,10 +73,6 @@ STOP_WAIT_SECONDS: dict[str, int] = {
|
||||
}
|
||||
DEFAULT_STOP_WAIT = 60
|
||||
|
||||
# Lanes whose code is not in this image yet. See the module docstring.
|
||||
_NEEDS_ML_DEPS = frozenset({"ml"})
|
||||
|
||||
|
||||
def _program(lane: Lane, *, slots: int) -> str:
|
||||
"""One [program:x] block.
|
||||
|
||||
@@ -141,7 +141,7 @@ def _web_program() -> str:
|
||||
])
|
||||
|
||||
|
||||
def render(*, with_ml: bool = False) -> str:
|
||||
def render() -> str:
|
||||
parts = [
|
||||
"\n".join([
|
||||
"[supervisord]",
|
||||
@@ -158,8 +158,6 @@ def render(*, with_ml: bool = False) -> str:
|
||||
]
|
||||
# Lanes after web, in LANES order, so the log reads in a stable sequence.
|
||||
for lane in LANES:
|
||||
if lane.name in _NEEDS_ML_DEPS and not with_ml:
|
||||
continue
|
||||
# A lane configured at zero slots still gets a PROCESS, at one slot
|
||||
# with its consumers cancelled by the reconcile. Without a running
|
||||
# worker there is nothing for `add_consumer` to reach, so enabling the
|
||||
@@ -171,12 +169,8 @@ def render(*, with_ml: bool = False) -> str:
|
||||
|
||||
def main(argv: list[str] | None = None) -> int:
|
||||
ap = argparse.ArgumentParser(description=__doc__)
|
||||
ap.add_argument(
|
||||
"--with-ml", action="store_true",
|
||||
help="include the ml lane (only valid once the ML deps are in this image)",
|
||||
)
|
||||
args = ap.parse_args(argv)
|
||||
sys.stdout.write(render(with_ml=args.with_ml))
|
||||
ap.parse_args(argv)
|
||||
sys.stdout.write(render())
|
||||
return 0
|
||||
|
||||
|
||||
|
||||
@@ -47,14 +47,15 @@ def _web_ok() -> tuple[bool, str]:
|
||||
return False, f"web unreachable: {exc}"
|
||||
|
||||
|
||||
def _lanes_ok(*, with_ml: bool) -> tuple[bool, str]:
|
||||
def _lanes_ok() -> tuple[bool, str]:
|
||||
from ..services.worker_control import inspect_lanes_sync
|
||||
from ..services.worker_lanes import LANES
|
||||
|
||||
expected = {
|
||||
lane.name for lane in LANES
|
||||
if with_ml or lane.name != "ml"
|
||||
}
|
||||
# Every lane, ml included: one image carries them all since step 6, and a
|
||||
# disabled lane still runs a process (consumers cancelled), so it answers
|
||||
# inspect and is healthy. Health is "is the process alive"; whether it
|
||||
# should be consuming is the reconcile's business.
|
||||
expected = {lane.name for lane in LANES}
|
||||
live = inspect_lanes_sync()
|
||||
missing = sorted(n for n in expected if not live[n].present)
|
||||
if missing:
|
||||
@@ -63,15 +64,12 @@ def _lanes_ok(*, with_ml: bool) -> tuple[bool, str]:
|
||||
|
||||
|
||||
def main(argv: list[str] | None = None) -> int:
|
||||
argv = sys.argv[1:] if argv is None else argv
|
||||
with_ml = "--with-ml" in argv
|
||||
|
||||
ok, detail = _web_ok()
|
||||
if not ok:
|
||||
print(detail, file=sys.stderr)
|
||||
return 1
|
||||
|
||||
ok, detail = _lanes_ok(with_ml=with_ml)
|
||||
ok, detail = _lanes_ok()
|
||||
if not ok:
|
||||
print(detail, file=sys.stderr)
|
||||
return 1
|
||||
|
||||
@@ -408,6 +408,20 @@ async def set_lane(
|
||||
if applied and slots is not None:
|
||||
applied, error = await asyncio.to_thread(set_lane_slots_sync, lane, new_slots)
|
||||
|
||||
# Enabling a lane that needs models is what triggers the fetch (milestone
|
||||
# 422 step 6). Never at boot: that made every start of the ML role reach
|
||||
# HuggingFace for ~3.5GB, and rule 164 permits a runtime fetch only for a
|
||||
# feature that is optional and clearly OFF.
|
||||
#
|
||||
# Only when the lane actually came on — `enabled is True` rather than
|
||||
# `new_enabled`, so re-saving slots on an already-enabled lane does not
|
||||
# re-enqueue. And only when the consumer change landed: enqueueing a task
|
||||
# onto a queue nothing is consuming would leave it pending with no
|
||||
# explanation until the lane returns.
|
||||
fetching = False
|
||||
if enabled is True and lane.models and applied:
|
||||
fetching = _enqueue_model_fetch()
|
||||
|
||||
return {
|
||||
"name": lane.name,
|
||||
"slots": row.slots,
|
||||
@@ -416,9 +430,34 @@ async def set_lane(
|
||||
"enabled": row.enabled,
|
||||
"applied": applied,
|
||||
"apply_error": error,
|
||||
# Tells the card to say a download has started rather than leaving the
|
||||
# operator to wonder why a freshly enabled lane is busy.
|
||||
"fetching_models": fetching,
|
||||
}
|
||||
|
||||
|
||||
def _enqueue_model_fetch() -> bool:
|
||||
"""Queue the model download. Returns whether it was accepted.
|
||||
|
||||
Import inside the function: `backend.app.tasks.ml` pulls in torch, and web
|
||||
must not pay that import cost on a module that every settings request
|
||||
touches.
|
||||
|
||||
Never raises. A broker that will not take the task is worth reporting, but
|
||||
the SETTING has already been stored and the lane is already enabled — so
|
||||
failing the whole request here would roll back nothing and tell the
|
||||
operator their change did not happen when it did.
|
||||
"""
|
||||
try:
|
||||
from ..tasks.ml import ensure_models
|
||||
|
||||
ensure_models.delay()
|
||||
return True
|
||||
except Exception: # noqa: BLE001 — reported, never raised at a caller
|
||||
log.warning("worker_control: could not enqueue the model fetch", exc_info=True)
|
||||
return False
|
||||
|
||||
|
||||
def reconcile_lanes_sync(desired: dict[str, tuple[int, bool]]) -> dict:
|
||||
"""Drive every RUNNING lane to its stored slots and enabled flag.
|
||||
|
||||
|
||||
@@ -668,3 +668,31 @@ def scheduled_retract_auto_tags() -> str:
|
||||
with SessionLocal() as session:
|
||||
n_ccip = retract_auto_applied_ccip(session)
|
||||
return f"head={n_head} ccip={n_ccip}"
|
||||
|
||||
|
||||
@celery.task(name="backend.app.tasks.ml.ensure_models", bind=True)
|
||||
def ensure_models(self) -> dict:
|
||||
"""Fetch the models this lane needs, if they are not already present.
|
||||
|
||||
Milestone 422 step 6. This used to run in `entrypoint.sh` before celery
|
||||
started, which made every boot of the ML role reach HuggingFace for
|
||||
~3.5GB — a startup dependency on a third party, for a feature the operator
|
||||
may never use. Rule 164 permits a runtime fetch only for something
|
||||
"optional and clearly off", so it moved here: enqueued the moment the lane
|
||||
is ENABLED, never at boot.
|
||||
|
||||
Being a task rather than a startup step is what makes it visible: it gets
|
||||
a TaskRun row like any other, so the download shows in Activity with a
|
||||
duration and a status, and a failure is something the operator can see and
|
||||
retry rather than a container that quietly never became useful.
|
||||
|
||||
Idempotent — `download_models` fetches only what is missing — so enabling
|
||||
an already-provisioned lane costs one no-op task rather than a re-download.
|
||||
That matters because the reconcile may enqueue it again.
|
||||
"""
|
||||
from ..scripts.download_models import main as download
|
||||
|
||||
rc = download()
|
||||
if rc != 0:
|
||||
raise RuntimeError(f"model download failed with exit code {rc}")
|
||||
return {"ok": True}
|
||||
|
||||
Reference in New Issue
Block a user