6 Commits

Include renames

Author	SHA1	Message	Date
bvandeusenandClaude Opus 5	468ff71353	fix: two lane fixtures described a container that cannot exist (4295) ... `pools={"host-a": 0}` — a celery replica running an empty pool. billiard will not run one, and `gen_supervisord` starts every lane at one process for exactly that reason, so no container has ever looked like this. It passed for as long as the code agreed with it. The floor landed, the reconcile's target for a zero-slot lane became one, and the fixture's impossible zero read as a real difference: `changed: ['ml']` on a settled lane, and run 7346 red on the commit that fixed the live bug. Both now say one, which is what ml actually runs. The two tests the fix added that asserted the same properties from the other side are gone with them — one assertion per fact, in the test that already named it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-23 11:03:13 -04:00
bvandeusenandClaude Opus 5	86d6509936	fix: a lane at zero slots tried to empty a pool billiard will not empty (4295) ... Found on the operator's live deploy, not in CI: [scheduler] worker_control: ml reconciled 1 -> 0 slots [ml] pidbox command error: ValueError("Can't shrink pool. All processes busy!") ML ships at 0 stored slots and disabled, and `gen_supervisord` starts every lane at one process so `add_consumer` has something to reach. So the stored value and the running pool disagreed by one, permanently: billiard will not remove the last worker, and `set_lane_slots_sync` returns True on SENDING the control message — the refusal happens later, on the worker. The reconcile logged a successful correction and reported `changed: ['ml']` every tick, forever, on the default configuration of every install. Lesson #4183 in production: an enforcer whose target is unreachable re-does its own work on every pass and says it worked. The floor is now one PROCESS, in one place — `effective_slots()` — applied wherever a target is COMPARED as well as wherever one is sent. Comparing against the unclamped 0 sees a difference no control message can ever close, which is the same non-convergence one layer up. Zero slots still means zero WORK: the lane's consumers are cancelled, and the idle process is what the enable switch lands on. A cross-file guard ties the generator's starting concurrency to the same function, so the two ends of the floor cannot drift apart again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-23 10:58:58 -04:00
bvandeusenandClaude Opus 5	a987ca41ca	perf: the lane read is one broadcast and three targeted, not four broadcasts (4295) ... Found while fixing the roster's budget (f23ab9f) and reported to the operator rather than changed mid-deploy; they asked for it. `inspect_lanes_sync` made FOUR broadcast inspect calls — active_queues, stats, active, reserved — at 2.0s each. A broadcast with no `destination` cannot know how many replies to expect, so each waits out its whole timeout rather than returning on the last one. About eight seconds, and `lane_view` sits on Settings -> Activity -> Worker lanes, so that was the load time of that card every time it was opened. The composite healthcheck paid it too, against a 15s timeout. Now the first read discovers the nodes and the other three name them, so celery stops as soon as those nodes have answered — milliseconds, for workers in this same container. The worst case is unchanged: a node that vanishes between the broadcast and the targeted reads still costs a full timeout waiting for a reply that is not coming, which is why the bound stays four. Nothing answering now costs ONE round trip instead of four. The three later reads exist only to describe what answered, so with an empty roster they described nothing at three full timeouts. That is the broker-down case — exactly when the healthcheck and the card need an answer rather than a wait. `lane_view` also gets a deadline. It awaited `to_thread` with no bound at all, which is rule 156's shape even though every inner call has its own timeout; on expiry it now reports every lane as not answering, because a page that renders "not answering" is a better answer than one that does not render. The budget is derived the same way the roster's now is — round trips times the timeout, plus slack — and a test asserts the relation rather than the number, plus one that reads the source so a fifth call cannot quietly put the deadline back under the work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-23 10:37:20 -04:00
bvandeusenandClaude Opus 5	a01165365b	feat: a saturated lane can grow itself, within the cap the operator set (4297) ... Milestone 422 step 7 — the one sweep in this milestone that decides rather than obeys, so it is off until a lane is opted in, bounded by the operator's cap, floored at the operator's value, and it reports every decision including the ones where it did nothing. Growth needs BOTH halves: all slots busy AND a backlog. Depth alone means celery is about to pick those up and growing would add idle children (#1253 is that bug in the GPU agent); saturation alone means the lane is busy with exactly as much work as exists. The backlog is depth PLUS reserved, because celery prefetches and LLEN reads 0 while a worker holds thirty tasks in memory — the case an LLEN-only autoscaler misses entirely, and the reason step 2 plumbed `reserved` through. The two sweeps had to be taught not to fight. The reconcile drives every lane to its stored slots every five minutes, which would have reverted each grow on the next tick: grow, revert, grow, revert, forever. For an autoscaling lane the stored value is now a FLOOR — restored when a lane falls below it, never taken back above it. The operator's "a task that runs for x concurrent time" idea stays a UI warning rather than a trigger: a long task does not finish sooner because the lane gained a slot, so scaling on it would spend memory to change nothing. Read from `task_run` on our own wall clock, not celery's `time_start`, which is the WORKER's monotonic clock and would produce a duration that is meaningless in the direction that matters — plausible. Caught while reading it back: the first version read the stored slots as the CURRENT pool. The autoscaler never writes that row, so every tick would have proposed floor+1 — resizing nothing, reporting `grew` anyway (a replica already past the target is issued no message and reports success), and capping the lane one slot above its floor forever while claiming otherwise. It now reads the live pool and keeps the stored value purely as the floor, and the tests fix the two to different numbers so an equal-fixture pass cannot hide it again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-22 10:05:43 -04:00
bvandeusenandClaude Opus 5	5f8c63f61b	feat: reconcile every running lane back to its stored slots (4293) ... Milestone 422 step 3. `pool_grow` is not durable: a worker restarted by its supervisor comes back at its ENV concurrency, silently below whatever the operator set, and nothing on step 2's write path would ever notice. Storing the value made it survivable; this makes it survive. A BEAT TASK, NOT A HOOK IN WEB — a deliberate deviation from the step as written, for a reason already recorded in this codebase. Step 3 said "web applies the stored values after it starts". It cannot: service_roster.py documents that hypercorn runs --workers 4, so anything in before_serving becomes four concurrent loops per container hammering the broker forever. service_roster's own answer — refresh on demand from whichever request arrives — was also rejected, because the two solve different problems. A stale ROSTER only misleads someone looking at it, so recomputing when they look is exactly right. A lane running at the wrong size is doing less work than it was told to whether or not anyone is watching, and the case that matters is a deploy at 3am followed by a backlog nobody is awake to see. So: unattended, every 5 minutes, on the quick `maintenance` lane beside the other recovery sweeps. Accepted cost — a dead scheduler stops reconciliation, but a dead scheduler already stops every other sweep and the roster reports it, so this adds no new blind spot. A BUG I WROTE AND CAUGHT BEFORE COMMITTING. The first version called set_lane_enabled_sync unconditionally, so a settled system re-sent add_consumer for every queue on every tick — forever. Harmless per call (add_consumer on an already-consumed queue does nothing), unbounded in aggregate, and completely invisible. That is lesson #4183's failure mode exactly, in the very function whose docstring cites it. Worse, my test would not have caught it: it asserted only on grew/shrank. LaneLiveState now carries `consuming` — which queues a lane is actually serving, distinct from the queues it was configured with — so the reconcile compares before acting. The test now asserts ALL FOUR control families are silent on a settled tick, plus a new case for an already-disabled lane, which is the other half of the same fixed point. An absent lane is SKIPPED, not corrected. present=False means nothing answered, not zero slots; correcting it would be a conclusion from an unswept read (snippet #3969), and there would be nothing to send the message to. One lane failing does not stop the others. One inspect serves every lane: the two setters now take an optional pre-fetched LaneLiveState, so a tick costs one broker round trip rather than one per lane. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-22 08:08:38 -04:00
bvandeusenandClaude Opus 5	a9c1b421a7	feat: change a lane's slots on a running system, over the broker (4292) ... Milestone 422 step 2. `GET /api/system/workers` reports every lane joined to its live pool; `POST /api/system/workers/<name>` changes it. NO DOCKER SOCKET. Milestone 365 deferred "acting on the state" because restarting a dead worker needs a socket the web container deliberately does not have. That holds for restarting a CONTAINER; it does not hold for changing how much work a RUNNING worker does. celery's pool_grow / pool_shrink / add_consumer / cancel_consumer send a message over the Redis the app already uses, and the worker resizes itself. No new privilege, no new surface, and the security question that deferred this is never raised. PERSIST AND PUSH, in one call, in that order. pool_grow is not durable — a restart drops every lane to its env concurrency — so a UI that only pushed would lose the setting on the next deploy with nothing to show for it (lesson #4202). Storing alone would describe nothing until something restarted. A failed PUSH is not a failed setting: 200 with `applied: false` and a reason, so the UI says "saved, not yet live" rather than "that didn't work". Step 3's reconcile carries it when the lane answers again. PER-REPLICA DELTAS. `pool_grow(n, destination=[...])` adds n to EACH destination, so while `worker` runs `replicas: 2` a single delta from an aggregate is wrong for both. `slots` therefore means what CELERY_CONCURRENCY means — one process's pool — and each replica is driven to it from its OWN current size, so replicas that drifted apart converge rather than moving in lockstep. I wrote this wrong first: the docstring claimed per-replica while the code computed one delta from the max across replicas. LaneLiveState now carries `pools` per hostname and exposes `pool` as a property. A replica already at the target is sent nothing at all — the reachable fixed point step 3's periodic reconcile needs, or it re-issues a grow of zero every tick forever (lesson #4183). A replica that answered inspect but not stats is NAMED in the error rather than skipped silently, since otherwise it would run at a size the UI claims it does not. `present=False` is not "zero slots", it is "nothing answered" — kept distinct throughout, because step 3 skips an absent lane rather than correcting it. /workers now also reports pool size (from `insp.stats()`) and RESERVED count. Celery prefetches, so tasks that have left the Redis list but not started are invisible to LLEN: a lane can read depth 0 with thirty tasks held in worker memory. `pending` is depth + reserved. The UI is misleading without this and step 7's autoscaler would be simply wrong. Also kills the THIRD copy of the queue list: system_activity's _QUEUE_NAMES, whose own comment admitted the coupling ("must match celery_app.task_routes") and which sat alongside task_routes and the ROLE_NAMES copy step 1 collapsed. Now derived from LANES. The rendered order changes to lane grouping, which is the better shape for a lane-oriented UI. Separate blueprint rather than folding into system_activity, which states in its first line that it is read-only and answers a different question — its /workers is keyed on celery HOSTNAME and reports which nodes answered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-22 08:01:23 -04:00