Skip to content

fix: idle work must not starve real requests — the stuck context probe - #144

Merged
REPPL merged 9 commits into
mainfrom
fix/stuck-probe
Sep 21, 2026
Merged

REPPL merged 9 commits into
mainfrom
fix/stuck-probe

Conversation

@REPPL

@REPPL REPPL commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Fix: idle work must not starve real requests — the context probe on a non-chat model

Resolves iss-2609211334563318, iss-2609211334570516, iss-2609211334576018 (moved to resolved/ in this change). Pre-emption — itd-2609211335097114 — stays a draft and is not implemented or approximated here.

The incident, from the live box's logs (v0.9.1, 2026-09-21)

Every chat request was refused 503 for six hours. The context probe queued mlx-community/GLM-OCR-bf16 (pipeline image-to-text, chat: false) at 09:36 and loaded it 32 times, every eleven minutes, until 15:38. The child's log is one traceback: mlx_lm server raised ValueError: Model type glm_ocr not supported (chained from ModuleNotFoundError: No module named 'mlx_lm.models.glm_ocr') in its generate thread on the pool's own readiness completion, while its httpd kept answering, so the pool's one readiness request blocked for the whole ten-minute ReadyTimeout — did not become ready within 10m0s, model failed to load, unloaded reason=load_failed — and the probe read the resulting 503 as "no room", kept its bounds and re-queued the model for the next idle tick. Only the maintainer's Unload at 15:49 ended it.

Why a loading 2.2 GB model was charged 82.46 GB: not a property of loading. The pool charges a model from the moment its entry exists (startLocked: charge: need), and the charge is LoadCostOf(weights, KV per token × served window × concurrency). With no served_context set, the default served window is derived to fill the budget (App.servedWindowAt: (budget − weights and headroom) / (KV per token × sequences), capped at the declared window). GLM-OCR declares a window large enough that the derived one filled the budget, so its charge ≈ the whole budget while loading and would have been the same once loaded. Left as it is, per the brief.

The three fixes (one commit each, test-first)

  1. The probe considers only models the server offers to chat (52dcef7a). probeSources.Candidates() and App.MeasureNow read registry.Model.CanChat with the rule in force — the one home the archtest holds; nothing copied. Measure now on a chat: false model is refused 400 with the reason.
    • red: TestTheProbeConsidersOnlyChatModels — candidates = [org/chat org/ocr], want only the chat model / Measure now queued a model the server does not offer to chat
    • green: ok internal/app 0.243s
  2. A load that cannot succeed fails in seconds, and a failed model is not retried by idle work (d3478a9f, 190d7172, 5ce44e45, eec93f2d).
    • The pool watches the per-model child log (runtime.LoadLogger, which the real launcher's execProcess satisfies via LogPath()) every 500 ms while its readiness request is out, reading a bounded 64 KiB tail; a traceback whose terminal line is ValueError|ModuleNotFoundError|ImportError cancels the wait (context.WithCancelCause) and the NotReadyError carries that line, bounded to 300 bytes with every path-shaped token blanked. BrokenPipeError and friends are deliberately not in the set — the child survives those.
      • red: TestAFatalLineInTheChildLogEndsTheLoadWaitAtOnce — the reason does not carry the child's own line: "org/ocr did not become ready within 30s" (30.3 s)
      • green: ok internal/runtime 0.745s
    • A load that never became ready is recorded on the model — registry.LoadFailure (reason + provenance: runtime, budget, concurrency, served window). While it stands: Candidates() skips it, selfTestServer.Ready() skips it, Probe.Due drops it from the queue, and modelSource.Resolve refuses it with a NotReadyError carrying the reason and the way out (so a client is refused at once, classed not-ready, text to entitled clients only). Lifted by a moved provenance (RefreshStaleness, at start and after every save), by a re-download, and by hand: Load and Measure now. The pool now tells the observer whether a failure was the model's own or interrupted (the entry taken out of the pool by another path — a client that hung up, an unload, an eviction); only the former is recorded. The card shows a did not load pill and the reason.
      • red (registry): build failed — undefined: LoadFailure, SetLoadFailure, LoadFailed; green: ok internal/registry 2.896s
      • red (app): build failed — a.recordLoadFailure undefined, a.ForgetLoadFailure undefined; green: ok internal/app 0.391s (TestALoadFailureIsRecordedWithItsProvenanceAndAnInterruptedLoadIsNot, TestAFailedModelIsSkippedByIdleWorkAndRefusedWithItsReason)
      • red (pool, interrupted): TestTheObserverIsToldWhetherALoadFailedOnItsOwnOrWasInterrupted — a load abandoned by its only waiter was reported as {… interrupted:false …}, want an interrupted failure; green: ok internal/runtime 1.036s
      • red (probe queue): TestAQueuedModelThatIsNoLongerACandidateIsDropped — the queue still holds [org/m]; green: ok internal/contextprobe 4.422s
      • red (card): TestTheCardSaysWhyAModelDidNotLoadAndHowToRetryIt failed on the line's text; green: ok internal/ui
    • The probe's ten-minute step floor is deliberately left (the one item of the brief not done). defaultStepTimeout is the gateway's own prefillBudget (which has the same ten-minute base) plus a minute: the probe's timer must not undercut the gateway's deadline or a step the gateway would have allowed is filed as prefill_deadline, and a step's request includes the cold load the pool allows ReadyTimeout (ten minutes) for — the probe unloads before every step. Lowering the floor to the body-derived figure (~2 min at the calibration size) would fail every large model's cold load as a deadline. With the fail-fast above, the floor no longer holds the server for anyone: a hung load ends in seconds.
  3. The refusal names the holder (c9f75c6b, eec93f2d). selftest.Status gains Since; the gateway is handed Options.IdleJobs (wired in main.go to a.SelfTest.Status); a NoRoomError 503 to an entitled client — loopback, or key-admitted, the same predicate (g.entitled) the models list's residency fields use — gains: ; the memory is held by <model>, loading, which the context probe has been at for 4m12s — the server's own idle work, not the size of the model asked for. It is released when the run ends, or at once with Unload on that model's card — and Unload now does that even while the job's request is in flight (see the reviews). Only when the job's model is in pool.Resident(). The pool's own NoRoomError.Error() is unchanged and still names no model; an unentitled client still gets cannot serve this model right now; the message for a genuinely full budget is byte-for-byte what it was. The card's residency pill now says loading / loading for the context probe / held by the self-test instead of loaded for every resident model.
    • red: TestARefusalNamesTheModelAnIdleJobIsHolding — body = {"error":{"code":503,"message":"not enough memory to load another model, and no model in memory can be freed (limit 41.0 GB)",…}} (three subtests)
    • green: ok internal/gateway 1.196s
    • red (pill): TestTheResidencyPillNamesTheJobHoldingTheModel, TestTheCardIsBuiltFromTheResidencyLabel failed (function absent); green: ok internal/ui 8.920s

Hand check (scratch root, port 11999, loopback; branch build dessau-fix; the live server on 11535 never contacted)

Two copies of the small model were planted beside it: Fake-OCR-bf16 (registry entry with pipeline_tag: image-to-text, its config.json model_type set to glm_ocr so the real mlx_lm child raises the incident's own ValueError) and Fake-Chat-4bit (adopted, chat by template). Settings: context_probe: true, idle_threshold_sec: 60, upstream_header_timeout_sec: 1 (the gateway's own deadline override, to force a step timeout without a burner), decode_concurrency: 8, max_resident_bytes: 30 GB (one model's charge is 16.45 GB, so two do not fit).

  • /v1/models: Fake-OCR-bf16 chat=False, the others chat=True.
  • A client request for Fake-OCR: 503 in 1.005 s — mlx-community/Fake-OCR-bf16 could not load: ValueError: Model type glm_ocr not supported. — log model failed to load at once; no path in the body. The child's log holds the same traceback the live box's did.
  • The second request: 503 in 0.6 ms, … did not load the last time it was tried (could not load: ValueError: Model type glm_ocr not supported); it is not tried again on its own until … press Load or Measure now …; /api/state carries load_failure with reason and provenance.
  • POST /api/models/measure for Fake-OCR: 400 is not offered to chat, and the probe measures through chat completions.
  • A restart with a changed budget and concurrency re-judged the mark and lifted it (measurements re-judged … Fake-OCR-bf16); POST /api/models/load lifted a fresh one (load_failure: null, model loading), the retried load failed fast again and re-marked it.
  • With the probe on: at the first idle minute idle_jobs = {job: context-probe, model: Fake-Chat-4bit, step: calibrating at 1024 tokens, since: …} — never Fake-OCR (probe_queue empty throughout; no probe line for it in the log).
  • While the probe held Fake-Chat (resident: [(Fake-Chat-4bit, loading, 16.45 GB)]), a client request for Qwen was refused in 1 ms with: not enough memory to load another model, and no model in memory can be freed (limit 30.0 GB); the memory is held by mlx-community/Fake-Chat-4bit, which the context probe has been loading for 0s — the server's own idle work, not the size of the model asked for. It is released when the run ends, or at once with Unload on that model's card. The probe then yielded (context probe paused … a request arrived), and the abandoned load left no mark on Fake-Chat (interrupted, not the model's failure).
  • The probe resumed at the next idle minute; with the 1 s deadline three steps came back 504, and it ended context probe measured model=Fake-Chat-4bit window=8880 bound=prefill_deadline with resident: [], resident_bytes: 0 — the step timeout released the budget.
  • After the review fixes, a second run from the rebuilt branch: with the probe holding Qwen with its request in flight (loading, in_flight 1, calibrating at 1024 tokens), POST /api/models/unload answered 200 in 52 ms (context probe paused … a request arrived; resident: []) — where the live box answered 409 twice.
  • Stopped by pid each time; pgrep -f dessau-fix and pgrep -f mlx_lm both 0. Config, registry and models directory restored to their start state.

Reviews

Both ran on the diff before the PR; every finding applied or explicitly rejected, and the fixes landed in eec93f2d with their own tests.

Security review (security-reviewer): FIX_FIRST → applied.

  1. MEDIUM — the traceback reader never reset inTraceback and read an unterminated final segment, so a stray ValueError: line after a survived traceback, or a line still being written, became a persistent verdict; and the timeout/signal verdicts are transient conditions made into a standing, restart-surviving refusal. Applied: the reader takes a terminal line only straight after a traceback's frames (indented lines), resets on any other line, and reads whole lines only (TestOnlyAFatalTracebackLineIsReadAsALoadFailure gained five cases); the timeout and signal-exit classes are recorded as transient — they stand for this process and are dropped at the next start (TestATransientLoadFailureDoesNotOutliveTheProcess); the child's own traceback and a non-signal exit status stand as before.
  2. LOW — a local process can POST a fake traceback to the child's loopback port during the readiness window and the reader would take it. Accepted, no change: same trust class that can already plant registry.json or drive the child directly; no new privilege. Recorded in the decision line.
  3. LOW — the reader's os.Open followed links and could block on a FIFO. Applied: O_RDONLY|O_NONBLOCK|O_NOFOLLOW, fstat kept.
  4. LOW — the sanitiser leaked words of a path with spaces and passed control characters. Applied: control characters dropped; everything from the first path-shaped token to the last blanked as one <path>.
  5. INFO — observer callbacks are unordered, so a late failure could re-mark a model a hand retry had lifted. Applied: recordLoadFailure skips a failure for a model the pool currently holds an entry for (the retry's own report decides).
  6. INFO — provenance read at record time, not load start. Accepted, no change: a budget change during a ten-minute load records the mark under the provenance in force at its end; the next save re-judges it.

Ruthless review (ruthless-reviewer): FIX_FIRST → applied.

  1. MEDIUM — the refusal, two doc pages and the changelog promised that Unload releases a held model, and the pool refused that Unload as busy while the job's request was in flight (the maintainer's two 409s on the live box). Applied (the honest option): Runner.Interrupt(model) ends the run as a yield; handleUnload asks for it first and waits, bounded, for the model to go. TestUnloadFromThePanelTakesTheModelBackFromTheProbe on the real gateway+pool+probe stack — red: Unload on the probe's model got 409: … org/m is serving 1 request(s); try again in a moment: model is busy; green: ok internal/gateway 0.351s. Plus TestInterruptEndsTheRunOnThatModelAsAYield.
  2. MEDIUM — a one-off readiness timeout became a standing, restart-surviving refusal. Applied: the transient class above; recorded in the decision ledger.
  3. LOW — Due could prune a Measure now queued between its candidate snapshot and its lock. Applied: a queue generation counter; pruning only against the queue the snapshot was read for.
  4. LOW — a stale failure written after a person lifted it. Applied: the residency gate above.
  5. LOW — "has been loading for 4m12s" was the run's age. Applied: the sentence now reads "held by , loading, which the context probe has been at for 4m12s".
    Checked clean by the reviewer: the cancellation path (WithCancelCause, watcher lifetime, cause read after Do), the waitReady reorder, the Interrupted distinction (including Close), RefreshStaleness' identity lift, Due vs rescan, the "/" guard, idleHolder's lock cost, the wiring in main.go, and that no test is tautological.

Definition of done

go test -race ./... green, gofmt -l . empty, go vet ./... clean, abcd docs lint 0 findings, abcd identity 0 adrift. Docs: docs/context-probe.md (which models, how long a step, what happens to a model that does not load, the holder on the card), docs/self-test.md, docs/models-list.md (the refusal rule). Changelog under [Unreleased], impact: fix. One decision line in .abcd/work/DECISIONS.md.

The probe measures through chat completions, so a served window is only
meaningful for a model the models list publishes as chat: true. Candidates()
took every Ready() model, and on the live server it picked an image-to-text
model (chat: false), whose server never answers a completion. Candidates()
and Measure now now read the chat verdict from its one home,
registry.Model.CanChat, with the rule in force; a model the server does not
offer to chat is not a candidate and is refused by Measure now with the
reason. Resolves iss-2609211334563318.

Assisted-by: Claude Opus 5 (claude-opus-5)
While the pool waits for a model to answer its readiness completion, a
child whose generate thread raised on the first request keeps its httpd up,
so the one request blocks for the whole ten-minute readiness timeout. On the
live server that was thirty-two ten-minute waits on one model. The pool
already captures each child's output per model; a process that reports
where (LoadLogger, which the real launcher's process does) is now watched
while the request is out, and a Python traceback there whose terminal line
is a ValueError, ModuleNotFoundError or ImportError ends the wait at once
with that line as the reason. The line is bounded and anything path-shaped
in it is blanked before it goes on the NotReadyError, which an entitled
client is told. A request handler's BrokenPipeError is not in the set: the
child survives those. Part of iss-2609211334570516.

Assisted-by: Claude Opus 5 (claude-opus-5)
The probe now measures only models the server offers to chat, and these
two fixtures carried neither a Hub word nor a chat template, so they read
as chat: false and the probe refused them. A chat template is what makes
an adopted model a chat model; the fixtures carry one.

Assisted-by: Claude Opus 5 (claude-opus-5)
The pool's verdict on a load that never became ready is now written onto
the model (registry.LoadFailure: the reason and the provenance in force —
runtime, budget, concurrency, served window). While it stands, the context
probe and the self-test leave the model alone, a queued probe of it is
dropped, and a request for it is refused at once with the recorded reason
and the way out, as a not-ready refusal an entitled client is told the text
of, instead of paying another readiness timeout. It is lifted when any part
of the provenance moves (the same re-judging a measurement gets at start and
after every save), by a re-download, and by hand: Load and Measure now on
the card. A load another path interrupted — a client that hung up, an
unload, an eviction — is reported to the observer as interrupted and leaves
no mark, since it says nothing about the model. The card shows the reason
with a "did not load" pill. On the live server this is what would have
stopped the probe after its first ten-minute wait rather than its
thirty-second. Resolves iss-2609211334570516.

The probe's ten-minute step floor (defaultStepTimeout) is left as it is:
it is the gateway's own prefill base, which the probe's timer must not
undercut or a slow step is filed as the deadline's, and a step's request
includes the cold load the pool allows ten minutes for. With the fail-fast
above the floor no longer holds the server for anyone.

Assisted-by: Claude Opus 5 (claude-opus-5)
The 503 a client got while the context probe held the budget read "not
enough memory to load another model, and no model in memory can be freed":
true, and it named neither what held the memory nor that the holder was the
server's own idle work, so the person read it as their model being too big.
The idle loop's status now records when the run in progress began, the
gateway is handed that status, and a no-room refusal to a client this
server owes an account of itself — loopback, or one the API key admits, the
same clients the models list tells what is resident — says which model the
job holds, which job, for how long, that it is the server's own work and
not the size of the model asked for, and that Unload on that model's card
releases it. The pool's own refusal still names no model, and an unentitled
client still gets the generic sentence. The card's residency pill says
"loading" for a model still loading and names the job holding a model —
"loading for the context probe" — rather than "loaded" for every resident
model. Resolves iss-2609211334576018.

Assisted-by: Claude Opus 5 (claude-opus-5)
…ts holder

The context-probe page says which models the probe measures, how long a
step may take, and what happens to a model that does not load; the
self-test page says it leaves such a model alone; the models-list reference
says when a no-room refusal names a holder and to whom. The changelog entry
under Unreleased, the three issues resolved naming their commits, and one
decision line recording the three fixes and that pre-emption stays the
draft intent.

Assisted-by: Claude Opus 5 (claude-opus-5)
The recorded reason ends in the child's own full stop, so the refusal and
the card's line read "… not supported. the last time it was tried". Both
now put the reason in parentheses, with the stop trimmed. Seen on the
scratch root's hand check.

Assisted-by: Claude Opus 5 (claude-opus-5)
…ransient mark

From the two adversarial reviews of the branch.

The refusal promised that Unload on the held model's card releases it, and
the pool refused that Unload as busy while the job's own request was in
flight — the maintainer's two 409s on the live box. The idle loop now
exposes Interrupt(model): the run on that model yields the way it does for
a client's request, releases its request and unloads the model itself; the
panel's Unload asks for that first and waits, bounded, for it to land.

A load failure that is the pool's own bound — the readiness timeout, an
exit by signal — rather than the child's verdict is recorded as transient:
it stands for this process, so idle work does not loop on a slow load and
a client is told why at once, and is dropped at the next start, where a
slow load on a busy Mac says nothing about the next one. The child's own
traceback and a non-signal exit status still stand until the provenance
moves or a person retries.

The child-log reader takes a fatal line only straight after a traceback's
frames and only from whole lines, so a matching line the child writes on
its own later, or a terminal line still being written, is not the verdict;
its open refuses a link and cannot block on a FIFO; the sanitiser drops
control characters and blanks a path with spaces in it as one path. A
failure reported after a hand retry has started a fresh load is not written
over that load. A Measure now that arrives between Due's candidate snapshot
and its queue pruning is not pruned on the stale snapshot. The holder
sentence gives the run's age as the run's, not the load's.

Assisted-by: Claude Opus 5 (claude-opus-5)
Assisted-by: Claude Opus 5 (claude-opus-5)
@REPPL
REPPL enabled auto-merge September 21, 2026 16:00
@REPPL
REPPL added this pull request to the merge queue Sep 21, 2026
Merged via the queue into main with commit 1ee70ed Sep 21, 2026
6 checks passed
@REPPL
REPPL deleted the fix/stuck-probe branch September 21, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant