diff --git a/.abcd/development/intents/drafts/itd-2610071035233981-bob-asks-for-an-answer-in-a-shape-he-defines-and-gets.md b/.abcd/development/intents/drafts/itd-2610071035233981-bob-asks-for-an-answer-in-a-shape-he-defines-and-gets.md new file mode 100644 index 00000000..11452530 --- /dev/null +++ b/.abcd/development/intents/drafts/itd-2610071035233981-bob-asks-for-an-answer-in-a-shape-he-defines-and-gets.md @@ -0,0 +1,42 @@ +--- +id: itd-2610071035233981 +slug: bob-asks-for-an-answer-in-a-shape-he-defines-and-gets +spec_id: null +kind: null +suggested_kind: null +reclassification_history: [] +builds_on: [] +severity: minor +origin: researcher-authored +production_mode: hand-written +--- + +# Bob gets answers in the exact JSON shape he asks for + +## Press Release + +> Bob asks for an answer in a shape he defines and gets exactly that shape. He sends a JSON schema with his request, the way OpenAI clients already do, and Dessau makes the model answer only with JSON that fits it, so his script can read the answer without checking and retrying. A schema Dessau cannot apply is refused with the reason before the model starts, rather than producing an answer that does not fit. + +## Why This Matters + +> _Why this matters to the user — replace before planning._ + +## Mechanism + +> _Prompted (the claim-recording gradient): why the authors expect this to work, as a falsifiable "we expect X because Y" — not the outcome restated. Replace this line with the claim, or with the exact token `None stated.` alone on its line to record the claim as considered and declined._ + +## Scope Conditions + +> _Required (the claim-recording gradient): the population, platform, scale, or assumptions this claim holds under, one per top-level bullet — `abcd intent plan` stamps each with a persistent identity. Replace this line with those bullets, or with the exact token `None stated.` alone on its line._ + +## Acceptance Criteria + +> _Required (the itd-1 discipline): add at least one Given-When-Then bullet describing the verifiable bar for "shipped" before this draft can be planned._ + +## Open Questions + +_None recorded yet._ + +## Audit Notes + +_Empty. Populated by intent-auditor when intent moves to shipped/._ diff --git a/.abcd/development/intents/drafts/itd-2610071035236103-alice-fits-a-longer-conversation-or-one-more-model-into-the.md b/.abcd/development/intents/drafts/itd-2610071035236103-alice-fits-a-longer-conversation-or-one-more-model-into-the.md new file mode 100644 index 00000000..d72ffd8f --- /dev/null +++ b/.abcd/development/intents/drafts/itd-2610071035236103-alice-fits-a-longer-conversation-or-one-more-model-into-the.md @@ -0,0 +1,42 @@ +--- +id: itd-2610071035236103 +slug: alice-fits-a-longer-conversation-or-one-more-model-into-the +spec_id: null +kind: null +suggested_kind: null +reclassification_history: [] +builds_on: [] +severity: minor +origin: researcher-authored +production_mode: hand-written +--- + +# Alice fits a longer conversation, or one more model, into the same memory + +## Press Release + +> Alice fits a longer conversation, or one more model, into the same memory. In a model's settings she turns on compact conversation memory, and Dessau keeps that model's working memory for each conversation at about half its usual size, so the window it serves can grow or another model can stay loaded beside it. Alice chooses this per model, because a model kept compact answers one request at a time and may answer very long prompts slightly less exactly; the setting tells her both before she turns it on, and nothing turns it on by itself. + +## Why This Matters + +> _Why this matters to the user — replace before planning._ + +## Mechanism + +> _Prompted (the claim-recording gradient): why the authors expect this to work, as a falsifiable "we expect X because Y" — not the outcome restated. Replace this line with the claim, or with the exact token `None stated.` alone on its line to record the claim as considered and declined._ + +## Scope Conditions + +> _Required (the claim-recording gradient): the population, platform, scale, or assumptions this claim holds under, one per top-level bullet — `abcd intent plan` stamps each with a persistent identity. Replace this line with those bullets, or with the exact token `None stated.` alone on its line._ + +## Acceptance Criteria + +> _Required (the itd-1 discipline): add at least one Given-When-Then bullet describing the verifiable bar for "shipped" before this draft can be planned._ + +## Open Questions + +_None recorded yet._ + +## Audit Notes + +_Empty. Populated by intent-auditor when intent moves to shipped/._ diff --git a/.abcd/development/intents/drafts/itd-2610071035237873-bob-s-coding-assistant-picks-up-where-it-left-off-without.md b/.abcd/development/intents/drafts/itd-2610071035237873-bob-s-coding-assistant-picks-up-where-it-left-off-without.md new file mode 100644 index 00000000..fe1e09ba --- /dev/null +++ b/.abcd/development/intents/drafts/itd-2610071035237873-bob-s-coding-assistant-picks-up-where-it-left-off-without.md @@ -0,0 +1,42 @@ +--- +id: itd-2610071035237873 +slug: bob-s-coding-assistant-picks-up-where-it-left-off-without +spec_id: null +kind: null +suggested_kind: null +reclassification_history: [] +builds_on: [] +severity: minor +origin: researcher-authored +production_mode: hand-written +--- + +# Bob's assistant picks up where it left off after Dessau unloads the model + +## Press Release + +> Bob's coding assistant picks up where it left off without waiting for Dessau to reread the whole conversation. When Dessau unloads a model to make room, or restarts, it keeps the work it had already done on Bob's long prompts on this Mac's disk, in Alice's own account, and uses it again when the same conversation returns, so his next answer starts in a second or two rather than after the model has reread tens of thousands of tokens. Nothing from one model's saved work is ever used for another model or another version of it, and Alice can see how much disk it uses and clear it. + +## Why This Matters + +> _Why this matters to the user — replace before planning._ + +## Mechanism + +> _Prompted (the claim-recording gradient): why the authors expect this to work, as a falsifiable "we expect X because Y" — not the outcome restated. Replace this line with the claim, or with the exact token `None stated.` alone on its line to record the claim as considered and declined._ + +## Scope Conditions + +> _Required (the claim-recording gradient): the population, platform, scale, or assumptions this claim holds under, one per top-level bullet — `abcd intent plan` stamps each with a persistent identity. Replace this line with those bullets, or with the exact token `None stated.` alone on its line._ + +## Acceptance Criteria + +> _Required (the itd-1 discipline): add at least one Given-When-Then bullet describing the verifiable bar for "shipped" before this draft can be planned._ + +## Open Questions + +_None recorded yet._ + +## Audit Notes + +_Empty. Populated by intent-auditor when intent moves to shipped/._ diff --git a/.abcd/development/principles/the-users-directory-is-theirs.md b/.abcd/development/principles/the-users-directory-is-theirs.md index f23befde..32c677de 100644 --- a/.abcd/development/principles/the-users-directory-is-theirs.md +++ b/.abcd/development/principles/the-users-directory-is-theirs.md @@ -4,7 +4,7 @@ > The user's directory is theirs: a tool never creates directories beside the > user's projects or anywhere the user did not hand over; session and agent -> scratch (a worktree, a verifier's copy, an export) goes under ~/.abcd/ keyed +> scratch (a worktree, a verifier's copy, an export) goes under ~/.abcd.noindex/ keyed > on the root-commit SHA, or the checkout's .abcd/.work.local/ tier — see > .abcd/development/principles/the-users-directory-is-theirs.md. diff --git a/.abcd/development/research/notes/2026-09-06-decomposition-calibration.md b/.abcd/development/research/notes/2026-09-06-decomposition-calibration.md index 69af5fe4..edd95a86 100644 --- a/.abcd/development/research/notes/2026-09-06-decomposition-calibration.md +++ b/.abcd/development/research/notes/2026-09-06-decomposition-calibration.md @@ -844,3 +844,23 @@ with a switch. Verdict proposed: FILE-AS-IS with both switches. Verdict adopted after the two reviews: one switch, an ADR the routing had not named, and pins and grace protected. Grade: routing survived in part. + +## 2026-10-07 — SOTA research on MLX serving performance, routed to seven records + +Request: "Conduct SOTA on how to optimise performance of MLX models. What do +other servers offer that Dessau doesn't (yet)?" The research note stayed in +the local tier; its gap table was routed. + +| part | type | home | +|---|---|---| +| Prompt cache with no byte limit | defect (runtime) | iss-2610071035130302 | +| Twice as many requests admitted as charged | defect (runtime) | iss-2610071035138788 | +| Batching default never measured here | evidence task | iss-2610071035147427 | +| MTP speculative decoding | waiting on upstream | iss-2610071035141725 | +| Per-model compact KV memory | capability | itd-2610071035236103 (draft) | +| Prompt cache kept on disk across unloads | capability | itd-2610071035237873 (draft) | +| Structured output to a schema | capability | itd-2610071035233981 (draft) | +| Vision, embeddings, rerank; mlx-vlm pin bump | held | not filed | + +Verdict proposed: SPLIT into four issues and three drafts, with the rest held. +Verdict adopted: the same, unchanged. Grade: routing survived. diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 33ecc988..24fc7217 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -452,3 +452,7 @@ - 2026-10-04 — **A context probe's bounds carry the provenance they were made under, and are dropped when it moves** (iss-2610032241096901; branch `fix/probe-resume-clamps-window`; `impact: fix`). The record offered two remedies: clamp `b.hi` to the served window on resume, or drop bounds made under another provenance. Dropping fits the measurement model in `internal/registry/measurement.go`, where a figure holds only under the runtime, budget, decode concurrency and served window it was taken under and moving any one makes it stale; clamping repairs only `hi`, leaves `lo` verified above the new window and a guard reading taken under the old budget, and still stamps the mixed result current. So the bounds record `Sources.Provenance` at creation, and take their ceiling `hi` from that same read's served window rather than from the candidate list, so the bound and the stamp cannot come from two reads either side of a move; a resume under a different one starts over, a run whose provenance moves with no yield between saves nothing and stays queued or due, and a saved figure is stamped with the bounds' provenance and judged against what is in force once it is written (`probeSources.Save` re-judges after `SetMeasurement`), so a settings save landing between the last check and the save marks the figure stale instead of leaving it current; the settings save's own re-judging may already have run by then, so it alone does not catch that move (amended after the independent review). Reproduced by two tests that saved 32,231 and 64,365 tokens under a served window of 8,192 before the change. What would show this wrong: a saved measurement whose window is above the served window it is stamped with. - 2026-10-04 — **A settings save re-judges measurements after the pool takes the new budget** (iss-2610042040451551; branch `fix/probe-provenance-followup`; `impact: fix`). `SetConfig` ran `refreshStaleness` before `Pool.SetMemoryBudget`, and the provenance's `BudgetBytes` reads the pool, so a budget save was judged against the old budget: a figure stamped 82,463,372,083 bytes read current under 41,231,686,041 until the next start. Not every figure, though: the provenance's `ServedContext` comes from `ServedWindow`, which derives an automatic window from the configuration's budget (`memoryBudget`, that is `enforcedBudget(Config().MaxResidentBytes)`), already stored by then, so a model whose automatic window follows the budget did read stale (`served_context`). The miss was a model with a set window, no KV charge, or an automatic window clamped at its declared size. Two remedies: move the re-judging after the budget is applied, or have `provenanceFor` read `BudgetBytes` from the configuration as well. The pool's figure was kept: it is what enforces the budget, the probe's memory guard measures against it (`probeSources.Available`), and `TestTheProvenanceIsWhatThePoolAndTheRuntimeSay` pins the provenance to it. So the provenance reads two homes, the pool for the budget and the configuration for the served window. They agree once a save has finished (the app is the pool's only writer and hands it `enforcedBudget` of the stored figure), but not between the configuration's store and `SetMemoryBudget`. A probe reading the provenance in that interval gets a mixed reading: its check before saving rejects a figure whose bounds were made under another reading, and a figure stamped with the mixed reading itself is judged against the finished settings by the re-judging that now runs last. That is why the reordering is still right: the re-judging moved to after `SetMemoryBudget` and `RefreshCharges`, once every setting the save changed is in force, and never sees a mixed reading. It also lifts a recorded load failure at once when a save moves only the budget, which `docs/context-probe.md` already promised (`TestABudgetOnlySaveLiftsALoadFailure`); before, the mark stood until the next start. What would show this wrong: a save that moves the budget leaving a measurement stamped with the old budget reading current. - 2026-10-04 — **The 0.10.0 cut, codename Prellerhaus**, by hand on the 0.9.3 precedent, on the maintainer's instruction of 2026-10-04 ("autonomously cut a new release", then "fix all four, then cut"). The version is a minor because the release carries `impact: breaking` entries (the refused sampling values, the empty-prompt and non-string prompt refusals, the mlx-lm 0.32.0 runtime, the removal of shared-cache mode among them) and the project is below 1.0; `build/CODENAME` is unchanged, as at 0.8.0 and 0.9.0. Before the roll, the four fixes the 2026-10-04 handoff named as release essentials were built test-first on their own lanes, each reviewed independently (Opus; the gateway change adversarially on Fable) and merged on `integrate/0.10.0`: the non-string completions prompt refusal (iss-2610040805353721, adr-2610042021365934), a bridged history that opens on the user (iss-2610032306154631), the self-test honouring pins (iss-2610032241098944), and a resumed context probe dropping bounds made under other settings (iss-2610032241096901). The reviews' findings were fixed in the same cut rather than stepped over: a stuck Discord channel after an unanswered message (iss-2610042030092101, major), the bridge fallback's byte budget (iss-2610042030098334), the self-test loading a pinned model it could not then unload (fixed by skipping pinned, unloaded models, mirroring the probe), a budget change judged against the old budget (iss-2610042040451551); smaller ones were captured with remedies. itd-2610030857275099 and itd-2610031024247803 ship (specs closed, fidelity reviews ingested; criterion 9 of the first is recorded not met, pending itd-2610030656210408 step 2, iss-2610042101435880). The two open majors are deferred out loud with `deferred_after: "v0.10.0"`: iss-2609190242198542 (four of six live Discord checks passed on 2026-10-04; two postponed by the maintainer) and iss-2609200815308397 (the client builds and its script checks pass; the appearance switch is unchecked by hand). The roll is manual, as `abcd launch` still cannot cut this repository; the receipts protocol does not apply because `release.yml` arms no semantic gate. Retention after verification deletes the v0.9.3 release and keeps its tag. +- 2026-10-07 — **A staged update holds the old version aside until the new one has loaded and answered once, and puts it back when that first load fails** (iss-2610042101430192; branch `fix/staged-update-keeps-old-copy`; `impact: fix`). This narrows the 2026-10-03 reading of "removed after the new one passes readiness" as the directory checks alone: those checks stay what a swap is held to, and readiness for the old copy's removal is now the pool's own readiness — a load whose probe was answered (`LoadFinished` with no error), which is the first successful load, not one Dessau starts for the purpose (an update nobody asked to serve is still not loaded into memory). A first load that fails on its own, transient or not (an interrupted one does not count), puts the old version back through the same swap an update makes, claimed as a download so a Download, a Delete or Close waits for it; its record is rewritten with its own version, no load failure, `update_failed: "load"`, and the newer commit still offered. The held copy is in memory and on disk only, with no new registry field: across a restart the newest complete aside copy beside a model folder that checks out is held again (this supersedes the 2026-10-03 line that removed such a copy at start as stale), but its version is not known any more, so one put back after a restart is recorded as version unknown. A delete removes it with the model, since the next start would otherwise put it back; a later update replaces it. Rejected: a registry field carrying the old version across restarts (a new persisted field for one edge), and restoring only on non-transient failures (a fallback that waits for a hand retry after every timeout). What would show this wrong: a tester whose good update was rolled back by a slow first load, or an aside copy that outlives both a successful load and a start. +- 2026-10-07 — **The update check reads an LFS `config.json` from the content CDN, and a version whose config it cannot hold to the listed hash is `cannot_check`, never offered** (iss-2610042101439623; branch `fix/update-check-lfs-config`; `impact: fix`). The check treated the Hub's hand-off of an LFS `config.json` to its CDN (`ErrCrossOrigin`) as "offer it", so a newer version that names a `model_file` was marked `available`. `SmallFileAt` now takes the listed `File`: a git file still goes through `do` and its origin rule unchanged; a file the listing keeps in LFS with a 64-hex sha256 goes through `doContent`, which follows the CDN hop as the download does — the 2026-09-19 exception, extended to this one read and only where a sha256 anchors the bytes — drops the token from any hop off the Hub's origin, refuses a listed size past the 1 MiB bound before asking, and holds the body to the sha256 (`ErrContentMismatch`). Off-origin with no sha256, a mismatch, or past the bound records the new `cannot_check` status (card "newer version not checked", Update refused in Go and on the panel); a CDN or Hub that does not answer stays "unreachable" as before. Chosen over a status alone because reading the file is what names a version that ships code, which is the mark the card exists to show. What would show this wrong: a CDN that needs the token, which would turn every gated repository's LFS config into `cannot_check`. +- 2026-10-07 — **The idle clock counts client requests at the gateway, beside the pool's per-model figures** (iss-2610041945030758; branch `fix/idle-clock-counts-unreachable`; `impact: fix`). The idle check read only `Pool.Residency()`: each resident entry's `InFlight` and `lastUsed`. An entry that leaves the pool (unloaded, stopped by the health watch, crashed, evicted, reaped) takes its last use with it, and a request the pool never admitted (still uploading, refused, failed before a model was chosen) never touched one; so an unreachable request on a wedged model that was then taken out left no trace, and the remaining models' 17:34 last use was over the hour by 18:45. The pool was left alone: a pool-wide clock there would still miss requests that never reach `Acquire` and would put the change inside `internal/runtime`. Instead `selftest.Clients` counts every request to the completion routes and every `Ask` from the handler's start to its return, the gateway holds it through `Options.Clients` (wired in `cmd/dessau` from `App.Clients`), and `selftest.Activity` carries `Requests` and `LastRequest` to `heldBy`. The context probe drives its model through the gateway, so a last request no later than the loop's own latest release (`touched`) is discounted, as the pool's stamp of that release already is; the cost is that a client request that never reached the pool and ended entirely inside a run is discounted too. What would show this wrong: an idle job starting within the threshold of a client request that ended unreachable, or one held for the threshold after the probe's own run. +- 2026-10-07 — **The decision-model over-charge is folded into the decision-model spec, with no stopgap** (iss-2610041945038826; spc-2610030846273729 steps 2 and 4). Diagnosis showed that the charge is the automatic served window filling the budget, which applies to every model, not a decision-model KV reservation. The maintainer chose the spec over a joint-head stopgap with a guessed figure, and over reopening the 2026-09-20 window derivation for all models. diff --git a/.abcd/work/issues/open/iss-2610041945038826-a-decision-model-is-charged-about-five-times-its-size.md b/.abcd/work/issues/open/iss-2610041945038826-a-decision-model-is-charged-about-five-times-its-size.md index 4aea2ee8..2e7a39f2 100644 --- a/.abcd/work/issues/open/iss-2610041945038826-a-decision-model-is-charged-about-five-times-its-size.md +++ b/.abcd/work/issues/open/iss-2610041945038826-a-decision-model-is-charged-about-five-times-its-size.md @@ -9,7 +9,13 @@ found_during: "tracing an unexplained model load after a peer's run, 2026-10-04, origin: researcher-authored production_mode: hand-written found_at: "internal/runtime/pool.go" -remedy: "Check what chargeLocked charges for a decision model; if it reserves a KV cache for a served chat window that the decision server never uses, charge decision models by their own measured peak instead, before itd-2610030656210408 ships." +remedy: "Fold into itd-2610030656210408: spc-2610030846273729 steps 2 (recognise a decision model) and 4 (charge it at weights plus its measured prefill peak at the build's fixed window) are the fix; resolve this issue in the change that lands them. No interim code." --- A decision model is charged about five times its size against the memory budget. While the self-test loaded mlx-community/clef-4bit on 2026-10-04, /api/state showed bytes 16330982843 (about 16 GB on disk) and charge_bytes 82463242771 (about 82 GB). The charge probably includes a KV cache sized for a served chat window, which a decision model answering short typed questions never fills. An over-charge like this makes the pool refuse or delay other models for memory that is never used. + +## Triage 2026-10-07 + +The cause is not a decision-model KV reservation. Nothing in `internal/` recognises a decision model yet, so clef-4bit was loaded and charged as an ordinary chat model. With no `served_context` set, `App.ServedWindow` derives the largest window the budget allows, (budget − 1.2 × weights) ÷ (per-token charge × batched requests), and `chargeLocked` charges that window back. So every model with an automatic window is charged about the whole budget. The figures fit: 60% of 128 GiB is 82,463,372,083 bytes, and the observed charge is 129,312 bytes below it, less than one token's charge. This is the property the 2026-09-21 decision line notes and leaves unchanged. Since v0.10.0, the self-test, the probe, chat and the panel's Load skip non-chat models, so the observed path is mostly closed; `/v1/completions` can still load one. + +Decided by the maintainer: no stopgap. The spec for itd-2610030656210408 already commits to the right charge, so steps 2 and 4 of spc-2610030846273729 are the remedy. Revisiting how automatic windows are derived for every model was considered and not taken now, because it reopens the 2026-09-20 decision. diff --git a/.abcd/work/issues/open/iss-2610070546301424-kolibri-1-velaia-kolibri-1-mlx-4bit-on-huggingface-aleph.md b/.abcd/work/issues/open/iss-2610070546301424-kolibri-1-velaia-kolibri-1-mlx-4bit-on-huggingface-aleph.md new file mode 100644 index 00000000..22d74c7e --- /dev/null +++ b/.abcd/work/issues/open/iss-2610070546301424-kolibri-1-velaia-kolibri-1-mlx-4bit-on-huggingface-aleph.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610070546301424" +slug: "kolibri-1-velaia-kolibri-1-mlx-4bit-on-huggingface-aleph" +severity: "minor" +category: "future-work-seed" +source: "agent-finding" +found_during: "abcd:intent assessment of Kolibri 1" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/runtime/mlx-requirements.txt" +remedy: "Once an mlx-lm release includes ml-explore/mlx-lm PR 1945, bump mlx-lm in internal/runtime/mlx-requirements.txt (uv pip compile --generate-hashes), then verify a real Kolibri 1 load on a large-memory Mac; never vendor or run the repo's kolibri1.py." +--- + +Kolibri 1 (velaia/Kolibri-1-MLX-4bit on HuggingFace, Aleph Alpha's 78B MoE at 4-bit, 44.3 GB) cannot be served: its config.json names model_type kolibri1, which the pinned mlx-lm 0.32.0 does not ship, so the load fails with an unsupported-model-type error. The repo bundles kolibri1.py and a run.py that registers it, but config.json sets no model_file, so CheckModelCode passes and Dessau correctly never runs that file (iss-2610030709283687). Support is upstream in ml-explore/mlx-lm PR 1945, open and unreviewed as of 2026-10-07. Assessed from the model card, config.json and the code; no load was attempted. reasoning_effort already reaches the chat template through chat_template_kwargs. Expect a Mac with 64 GB or more. diff --git a/.abcd/work/issues/open/iss-2610071035130302-the-model-server-s-prompt-cache-has-no-byte-limit-so-cached.md b/.abcd/work/issues/open/iss-2610071035130302-the-model-server-s-prompt-cache-has-no-byte-limit-so-cached.md new file mode 100644 index 00000000..6c4ee791 --- /dev/null +++ b/.abcd/work/issues/open/iss-2610071035130302-the-model-server-s-prompt-cache-has-no-byte-limit-so-cached.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610071035130302" +slug: "the-model-server-s-prompt-cache-has-no-byte-limit-so-cached" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "SOTA research on MLX serving performance, 2026-10-07" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/runtime/serve.py" +remedy: "Pass max_bytes to LRUPromptCache in serve.py, derived from the model's charged KV allowance (and pass --prompt-cache-size explicitly), with a test that the bound reaches the constructor; verify on a Mac with the launcher's footprint sampling that peak memory stays within the charge." +--- + +The model server's prompt cache has no byte limit, so cached prompts grow outside the memory budget. internal/runtime/serve.py builds server.LRUPromptCache(cli.prompt_cache_size) with the entry count only, so mlx-lm 0.32.0's default of 10 entries and 2^63 bytes applies; upstream's --prompt-cache-bytes flag is parsed but not applied (ml-explore/mlx-lm PR 1392, open, which reports Metal running out of memory on long-running servers). docs/memory-budget-explained.md already says caches stack across requests. Each cached entry can hold a whole served window's KV, so a model serving several long conversations can hold several windows' worth of cache that the charge (one window per sequence) does not count. diff --git a/.abcd/work/issues/open/iss-2610071035138788-the-pool-admits-twice-as-many-requests-per-model-as-it.md b/.abcd/work/issues/open/iss-2610071035138788-the-pool-admits-twice-as-many-requests-per-model-as-it.md new file mode 100644 index 00000000..e09ac7fa --- /dev/null +++ b/.abcd/work/issues/open/iss-2610071035138788-the-pool-admits-twice-as-many-requests-per-model-as-it.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610071035138788" +slug: "the-pool-admits-twice-as-many-requests-per-model-as-it" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "SOTA research on MLX serving performance, 2026-10-07" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/runtime/pool.go" +remedy: "Make the charge and the admission agree: either size the semaphore to DecodeConcurrency or charge 2 x DecodeConcurrency sequences, choosing by measurement of what the batching gains; add a test that the charge covers every request the semaphore admits." +--- + +The pool admits twice as many requests per model as it charges memory for. chargeLocked in internal/runtime/pool.go charges KV for DecodeConcurrency sequences, but each loaded model's semaphore admits 2 x DecodeConcurrency requests at once (the sem in startLocked), and at the default of 1 mlx-lm is launched without --decode-concurrency, so its own batch limit (32) lets both admitted requests decode together. Two long requests on one model can therefore use about twice the KV the budget reserved. diff --git a/.abcd/work/issues/open/iss-2610071035141725-multi-token-prediction-mtp-speculative-decoding-is-waiting.md b/.abcd/work/issues/open/iss-2610071035141725-multi-token-prediction-mtp-speculative-decoding-is-waiting.md new file mode 100644 index 00000000..f01c5d28 --- /dev/null +++ b/.abcd/work/issues/open/iss-2610071035141725-multi-token-prediction-mtp-speculative-decoding-is-waiting.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610071035141725" +slug: "multi-token-prediction-mtp-speculative-decoding-is-waiting" +severity: "minor" +category: "future-work-seed" +source: "agent-finding" +found_during: "SOTA research on MLX serving performance, 2026-10-07" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/runtime/launcher.go" +remedy: "Once an mlx-lm release includes PR 990 with its automatic switch-off under load, offer MTP as an operator setting per model on all three surfaces, after measuring acceptance on this Mac's 4-bit models; do nothing before that release." +--- + +Multi-token prediction (MTP) speculative decoding is waiting on upstream. ml-explore/mlx-lm PR 990 (open, last pushed 2026-09-19) reports about 1.57x decode speed for a dense Qwen3.5-27B 4-bit model on an M4 Pro at 88 percent acceptance, but only 1.09x to 1.11x on mixture-of-experts models, and it switches itself off when several requests arrive at once. MTP uses the model's own head, so no second model is loaded and Dessau's refusal of draft_model in requests can stand. diff --git a/.abcd/work/issues/open/iss-2610071035147427-batching-beyond-the-default-has-never-been-measured-on-this.md b/.abcd/work/issues/open/iss-2610071035147427-batching-beyond-the-default-has-never-been-measured-on-this.md new file mode 100644 index 00000000..07c9c651 --- /dev/null +++ b/.abcd/work/issues/open/iss-2610071035147427-batching-beyond-the-default-has-never-been-measured-on-this.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610071035147427" +slug: "batching-beyond-the-default-has-never-been-measured-on-this" +severity: "minor" +category: "observation" +source: "agent-finding" +found_during: "SOTA research on MLX serving performance, 2026-10-07" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/config/config.go" +remedy: "Run the parallel self-test at decode concurrency 1, 2 and 4 on the models this Mac serves, record total throughput and time to first token per level in a dated research note, and revisit the default only on that evidence." +--- + +Batching beyond the default has never been measured on this Mac's models. The default decode_concurrency of 1 (decided 2026-09-20) admits two requests per model, while published figures report 2.2x total throughput for 4-way chat on a 27B model (LM Studio mlx-engine, M3 Max) and 2.6x to 3.7x for 8B and 0.6B models (vllm-mlx, M4 Max), with smaller gains for large dense models because decode is bandwidth-bound. The decision has no local evidence either way. diff --git a/.abcd/work/issues/open/iss-2610071200183905-a-403-from-the-content-cdn-during-the-update-check-s-config.md b/.abcd/work/issues/open/iss-2610071200183905-a-403-from-the-content-cdn-during-the-update-check-s-config.md new file mode 100644 index 00000000..0d79104e --- /dev/null +++ b/.abcd/work/issues/open/iss-2610071200183905-a-403-from-the-content-cdn-during-the-update-check-s-config.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610071200183905" +slug: "a-403-from-the-content-cdn-during-the-update-check-s-config" +severity: "nitpick" +category: "ux" +source: "review-followup" +found_during: "security review of the LFS update-check fix (iss-2610042101439623), 2026-10-07" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/hub/hub.go" +remedy: "Word SmallFileAt's non-200 error by which host answered: name the content CDN and drop the token advice when the refusal came from off the Hub's origin." +--- + +A 403 from the content CDN during the update check's config fetch is worded as Hugging Face refusing access with the access token in Settings, though the CDN was never sent the token. The update check counts it as unreachable and never shows or logs the text, so no one sees it today; it would mislead anyone who surfaced it. diff --git a/.abcd/work/issues/open/iss-2610071200187548-the-update-check-s-content-fetch-docontent-in-internal-hub.md b/.abcd/work/issues/open/iss-2610071200187548-the-update-check-s-content-fetch-docontent-in-internal-hub.md new file mode 100644 index 00000000..402a96ea --- /dev/null +++ b/.abcd/work/issues/open/iss-2610071200187548-the-update-check-s-content-fetch-docontent-in-internal-hub.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610071200187548" +slug: "the-update-check-s-content-fetch-docontent-in-internal-hub" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "security review of the LFS update-check fix (iss-2610042101439623), 2026-10-07" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/hub/hub.go" +remedy: "In doContent's CheckRedirect, drop Authorization for the rest of the chain once any hop in via is off-origin, and refuse a non-https hop when the base URL is https; add hub tests for leave-then-return and for an https-to-http redirect." +--- + +The update check's content fetch (doContent in internal/hub/hub.go) strips the access token only on the hop that leaves the Hub's origin, so a redirect chain that leaves and then returns sends the token to the Hub again. A CDN on a host net/http treats as the same domain, or the same host on another port, can redirect back to the Hub origin, and the next request is rebuilt from the original headers. The reviewer's probe showed the CDN hop receiving no token and the hop back receiving the bearer token. The token only reaches its own audience and the bytes are still checked against the listed sha256, but a hostile CDN can choose one authenticated Hub GET, which refuseOffOrigin exists to prevent. The same fetch also follows an https-to-http hop: integrity holds through the hash and the token is dropped, but a gated repo's config.json crosses the network unencrypted. diff --git a/.abcd/work/issues/open/iss-2610071200187920-the-model-download-path-sets-no-checkredirect-of-its-own-so.md b/.abcd/work/issues/open/iss-2610071200187920-the-model-download-path-sets-no-checkredirect-of-its-own-so.md new file mode 100644 index 00000000..50724b71 --- /dev/null +++ b/.abcd/work/issues/open/iss-2610071200187920-the-model-download-path-sets-no-checkredirect-of-its-own-so.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610071200187920" +slug: "the-model-download-path-sets-no-checkredirect-of-its-own-so" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "security review of the LFS update-check fix (iss-2610042101439623), 2026-10-07" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/hub/download.go" +remedy: "Give the download client the same redirect policy as doContent, one shared primitive rather than a second copy: strip Authorization off-origin and keep the 10-hop limit, with a download test using a same-host, other-port CDN." +--- + +The model download path sets no CheckRedirect of its own, so it relies on net/http's default header stripping, which keeps Authorization on a redirect to a subdomain of the Hub's host. The update check's newer content fetch (doContent) drops the token on any hop that leaves the Hub's origin, so the path that moves gigabytes is the less strict of the two. Today's CDN hosts are on another domain, so net/http drops the token anyway; the gap opens only if the Hub redirects to one of its own subdomains. diff --git a/.abcd/work/issues/open/iss-2610041945030758-the-idle-clock-appears-not-to-count-client-requests-that-end.md b/.abcd/work/issues/resolved/iss-2610041945030758-the-idle-clock-appears-not-to-count-client-requests-that-end.md similarity index 85% rename from .abcd/work/issues/open/iss-2610041945030758-the-idle-clock-appears-not-to-count-client-requests-that-end.md rename to .abcd/work/issues/resolved/iss-2610041945030758-the-idle-clock-appears-not-to-count-client-requests-that-end.md index 1675896b..81cf3d52 100644 --- a/.abcd/work/issues/open/iss-2610041945030758-the-idle-clock-appears-not-to-count-client-requests-that-end.md +++ b/.abcd/work/issues/resolved/iss-2610041945030758-the-idle-clock-appears-not-to-count-client-requests-that-end.md @@ -10,6 +10,8 @@ origin: researcher-authored production_mode: hand-written found_at: "internal/app/contextprobe.go" remedy: "Read the idle check the self-test and context probe share; if it ignores requests that fail before reaching a model, count every client request toward the idle clock, with a test that a request ending unreachable resets it." +resolution: "The idle clock now counts every client request at the gateway (selftest.Clients), in flight and at its end, so a request that ended unreachable on a model since unloaded holds the idle jobs" +impact: fix --- The idle clock appears not to count client requests that end unreachable. On 2026-10-04 a client's seven requests to a wedged Bonsai each ended as class unreachable after 600 s, the last at 18:44:05. The idle-time jobs (self-test, context probe) share idle_threshold_sec, which was 3600 here, yet the self-test started at 18:45, about a minute later. The last request that reached a model ended at 17:34, so the clock seems to have counted only those. A client that is retrying against a stuck model is not idle, and an idle job loading models around it adds load at the worst moment. Inferred from /api/stats and /api/state timings; the idle check itself was not read. diff --git a/.abcd/work/issues/open/iss-2610042101430192-a-staged-update-removes-the-old-copy-of-a-model-before-the.md b/.abcd/work/issues/resolved/iss-2610042101430192-a-staged-update-removes-the-old-copy-of-a-model-before-the.md similarity index 83% rename from .abcd/work/issues/open/iss-2610042101430192-a-staged-update-removes-the-old-copy-of-a-model-before-the.md rename to .abcd/work/issues/resolved/iss-2610042101430192-a-staged-update-removes-the-old-copy-of-a-model-before-the.md index d4714c95..444e6caf 100644 --- a/.abcd/work/issues/open/iss-2610042101430192-a-staged-update-removes-the-old-copy-of-a-model-before-the.md +++ b/.abcd/work/issues/resolved/iss-2610042101430192-a-staged-update-removes-the-old-copy-of-a-model-before-the.md @@ -10,6 +10,8 @@ origin: researcher-authored production_mode: hand-written found_at: "internal/app/app.go" remedy: "Keep the old copy aside until the new version has been loaded and answered once (or the operator confirms), and restore it automatically when the first load fails, with a test where the new version passes the directory checks but fails to load." +resolution: "A staged update now holds the old copy aside until the new version has loaded and answered once, and puts it back automatically when that first load fails (restoreFallback, held across restarts)." +impact: fix --- A staged update removes the old copy of a model before the new version has loaded successfully. The spec for itd-2610030857275099 says the old copy is removed only after the new one 'passes readiness'. DECISIONS.md reads readiness as the directory checks, so internal/app/app.go (around line 1759) deletes the old copy as soon as the new record is written. A new version that passes those checks but fails at its first load leaves nothing to fall back to: the model is unusable until the operator downloads an older version by hand. diff --git a/.abcd/work/issues/open/iss-2610042101439623-the-update-check-cannot-see-a-newer-version-s-code-when-its.md b/.abcd/work/issues/resolved/iss-2610042101439623-the-update-check-cannot-see-a-newer-version-s-code-when-its.md similarity index 81% rename from .abcd/work/issues/open/iss-2610042101439623-the-update-check-cannot-see-a-newer-version-s-code-when-its.md rename to .abcd/work/issues/resolved/iss-2610042101439623-the-update-check-cannot-see-a-newer-version-s-code-when-its.md index 63fe9d74..ad55fe47 100644 --- a/.abcd/work/issues/open/iss-2610042101439623-the-update-check-cannot-see-a-newer-version-s-code-when-its.md +++ b/.abcd/work/issues/resolved/iss-2610042101439623-the-update-check-cannot-see-a-newer-version-s-code-when-its.md @@ -10,6 +10,8 @@ origin: researcher-authored production_mode: hand-written found_at: "internal/app/updatecheck.go" remedy: "When config.json is an LFS pointer, fetch its real content through the resolve endpoint the download already uses (or mark the version 'cannot check'), so a version that ships its own code is never offered Update." +resolution: "The update check reads an LFS config.json from the content CDN, held to the listed sha256 with no token sent there, and marks a version whose config cannot be held to it cannot_check, never offered" +impact: fix --- The update check cannot see a newer version's code when its config.json is stored in Git LFS. internal/app/updatecheck.go (around line 238) reads config.json to decide whether a newer version names code of its own. When that file is an LFS pointer the check cannot read it, so the version is marked 'available' with Update offered. Only the update's own launch checks refuse it later, after a download. diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b2bf653..7a876562 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,40 @@ GitHub release notes. ## [Unreleased] +### Fixed + +- **An update that will not load no longer leaves a model without a + version to serve.** `impact: fix`. Dessau removed the old version of a + model as soon as the new one passed the checks it makes before starting a + model, so a new version that passed them and then failed to load left + Alice with nothing to fall back to until she downloaded an older version + by hand. The old version now stays aside until the new one has loaded and + answered a request; if that first load fails, Dessau puts the old version + back, the next request is answered by it, and the card says the newer + version did not load. The old version is kept aside across a restart too, + and goes when the model is removed + ([model updates](docs/model-updates.md); iss-2610042101430192). +- **A newer version that ships its own code is no longer offered when its + repository keeps `config.json` in Git LFS.** `impact: fix`. HuggingFace + hands such a file to its content CDN, which the update check did not read + from, so the check could not see whether the newer version names a + `model_file` and marked it as available with **Update** offered; only the + update's own checks refused it, after the download. The check now reads the + file from the CDN, held to the hash HuggingFace lists for it and sending + the CDN no access token, and a version that ships its own code is marked + **newer version not run**. One whose `config.json` cannot be held to that + hash is marked **newer version not checked**, and is not offered either + ([model updates](docs/model-updates.md); iss-2610042101439623). +- **The self-test and the context probe no longer start while a client is + still sending requests that fail.** `impact: fix`. The idle check read each + loaded model's last request, so a request that never reached a model, or + one whose model was unloaded or stopped afterwards, did not count. When + Bob's client kept retrying a stuck model and the model was then unloaded, + the self-test could start a minute after his last request although the idle + threshold was an hour, and load models while he was still trying. Every + client request now counts from the moment it arrives until it ends, however + it ends ([self-test](docs/self-test.md); iss-2610041945030758). + ## [0.10.0] - 2026-10-04 ### Added diff --git a/cmd/dessau/main.go b/cmd/dessau/main.go index 096cfa47..18a48e40 100644 --- a/cmd/dessau/main.go +++ b/cmd/dessau/main.go @@ -369,8 +369,9 @@ func runServer(lns []net.Listener, plan bind.Plan, paths config.Paths, cfg confi // gateway reads the key live (a.Config) so setting one in the control panel // takes effect without a restart. // IdleJobs is the idle loop's run in progress, so a refusal for want of - // memory can name a holder that is the server's own idle work. - g := gateway.New(gateway.Options{ConfigFunc: a.Config, Pool: a.Pool, Models: a.Registry, Log: log, Stats: a.Stats, ServedWindow: a.ServedWindow, IdleJobs: a.SelfTest.Status}) + // memory can name a holder that is the server's own idle work; Clients is + // the idle loop's count of every client request (iss-2610041945030758). + g := gateway.New(gateway.Options{ConfigFunc: a.Config, Pool: a.Pool, Models: a.Registry, Log: log, Stats: a.Stats, ServedWindow: a.ServedWindow, IdleJobs: a.SelfTest.Status, Clients: a.Clients}) // The Discord bridge, wired here because it needs the gateway. SetBridge // puts the stored settings in force — which for an install that has never // touched it means off, and nothing is opened. diff --git a/docs/model-updates.md b/docs/model-updates.md index 031876c8..da6a352c 100644 --- a/docs/model-updates.md +++ b/docs/model-updates.md @@ -50,7 +50,12 @@ you are serving. a README, a licence or other Markdown is not counted as newer. - **A newer version that ships its own code is named as such.** If the newer version's `config.json` names a `model_file`, Dessau records that it will not - run that version. + run that version. A repository that keeps `config.json` in Git LFS has it + handed to HuggingFace's content CDN; the check reads it there and holds it + to the hash HuggingFace lists for it, as a download does, and sends the CDN + no access token. If the file cannot be held to that hash — the bytes differ, + HuggingFace lists no hash it can check, or the file is larger than 1 MiB — + Dessau records that it could not check that version, and does not offer it. ## What the marks mean @@ -60,6 +65,7 @@ Each model's card under **My Models** says what the last check found: | --- | --- | --- | | **newer version** | A newer version changes a file the model server reads. | Offered | | **newer version not run** | The newer version ships its own code, which Dessau does not run. | Not offered | +| **newer version not checked** | A newer version exists, but its `config.json` could not be checked against the hash HuggingFace lists, so whether it ships its own code is unknown. The next check tries again. | Not offered | | Version unknown | Dessau did not record which version it downloaded, so it cannot tell whether a newer one exists. | Offered: it fetches the current version and records it | | Nothing | The last check found nothing newer, or checks are off. | Not offered | @@ -78,7 +84,7 @@ models with is not installed yet, checks the new version's own files, refusing one that ships its own code, and leaves the rest to the check every model has before it starts. Only then does it swap the two: it waits for requests already being answered by the model to finish, moves the new version in, and -removes the old one. While it waits, a new request for that model is answered +keeps the old one aside. While it waits, a new request for that model is answered at once with `503`, since a model in steady use would otherwise never fall idle and the update would never land; a client that is told why reads that the model is being updated and to try again in a moment. The wait lasts at most @@ -90,10 +96,26 @@ stops, a version that ships its own code, or a disk without room for both versions at once — the new version is removed and the old one keeps serving, unchanged. The model's card then says why on its version line — a file that did not download or match, no room on the disk, a version that did not pass -the checks, a model still answering requests when the wait ran out, or a -version Dessau does not run — until an update succeeds. A file both versions +the checks, a model still answering requests when the wait ran out, a +version Dessau does not run, or a newer version that did not load — until an +update succeeds. A file both versions share is not fetched again. +The old version stays aside until the new one has loaded and answered a +request, since a version can pass every check and still fail to load. The +first request for the model after an update loads the new version: once it +answers, the old version is removed. If that first load fails instead, Dessau +puts the old version back and removes the new one; the request that +triggered the load is refused, the next one is answered by the old version, +and the card says the newer version did not load, with the newer version +still offered. The old version is kept aside across a restart of Dessau too, +but Dessau no longer knows which version it was, so after a restart the +version put back shows as version unknown. Until that first load, both +versions take up disk space, however long the model goes unused. A second +update before the first load keeps the version that last served aside, not +the one that never loaded. Removing the model removes the old version with +it. + What Dessau learned about the old files goes with them: the measured context window, whether the model calls tools, and a recorded failure to load. What you set for the model stays: its pin and its served context window. diff --git a/docs/self-test-reference.md b/docs/self-test-reference.md index e8fd6545..5b5924d7 100644 --- a/docs/self-test-reference.md +++ b/docs/self-test-reference.md @@ -92,6 +92,7 @@ for the same model, which times the prompt alone. | --- | --- | | The idle check | once a minute | | Idle means | no request in flight, none waiting for a load, no download running, and the last request older than the idle threshold (`idle_threshold_sec`, five minutes unless set; shared with the context probe) | +| A request counts when | it is a client's request for an answer that the server admits, however it ends — answered, failed or refused by the model's server — and whether or not its model is still loaded; a request turned away for a missing or wrong key, or from an unpaired client, does not count | | A model is loaded only when | it fits beside what is in memory; the self-test never evicts | | A run in progress ends when | a request is in flight on any model, a load is waiting for room, a load has been refused room since the run began, or a client's load needs the memory the run is holding | | A run in progress checks for a client | every quarter of a second | diff --git a/docs/self-test.md b/docs/self-test.md index aa4d09ac..0017bf61 100644 --- a/docs/self-test.md +++ b/docs/self-test.md @@ -29,12 +29,15 @@ are. ## What happens while it is on -Once a minute Dessau asks whether the Mac is idle: no request in flight on -any model, no request waiting for a model to load, no download running, and -the last request older than the idle threshold — five minutes unless you +Once a minute Dessau asks whether the Mac is idle: no request in flight, no +request waiting for a model to load, no download running, and the last +request older than the idle threshold. Every client request the server +admits counts, however it ends — one that failed, or whose model has since +been unloaded, counts the same as one that was answered. A request turned +away for a missing or wrong key, or from an unpaired client, does not count. The threshold is five minutes unless you change it under **Settings → Context probe**, `idle_threshold_sec` in `config.json`; the self-test and the [context probe](context-probe.md) share -it. When it is, Dessau picks the +it. When the Mac is idle, Dessau picks the model measured longest ago — a model never measured comes first, and a model measured within the last day is left alone — and runs it through the same short set every model gets: diff --git a/internal/app/app.go b/internal/app/app.go index cf451bb4..c37c10c1 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -50,6 +50,11 @@ type App struct { // Probe measures each model's servable context window as a job of that // same loop (itd-2609091301112705); see internal/contextprobe. Probe *contextprobe.Probe + // Clients is the idle clock's count of client requests, which the + // gateway keeps (gateway.Options.Clients) and the idle loop reads + // through selfTestServer.Activity: every client request, whatever it + // ended as and wherever its model is now (iss-2610041945030758). + Clients *selftest.Clients // ToolProbe asks each model once, at its first serve under this // runtime, whether it calls tools (itd-2609201445423499); see // internal/toolprobe. It is queued by the pool's observer and reaches @@ -156,7 +161,11 @@ type App struct { // one the pool starts model servers with, whose Precheck a staged // version must pass; drainWait bounds the swap's wait for requests in // flight. - swapping map[string]bool + swapping map[string]bool + // fallbacks holds, for each model whose update has not yet loaded and + // answered once, the version it replaced, waiting aside to be put back + // should the first load fail (staged.go); guarded by dlMu. + fallbacks map[string]fallback launcher runtime.Launcher drainWait time.Duration // freeSpace says how much room the models folder's volume has; a seam @@ -271,8 +280,10 @@ func New(opts Options) (*App, error) { return nil, err } // Before the rescan: a model an interrupted update left aside goes back - // where the rescan will find it, and staged versions are cleared. - recoverStaging(opts.Paths.Models, opts.Log) + // where the rescan will find it, and staged versions are cleared. A + // version an update replaced, and holds until the new one has loaded + // once, is kept and held again. + fallbacks := recoverStaging(opts.Paths.Models, opts.Log) // Adopt whatever is already in this account's models directory — what lets // a reinstall pick up models without re-downloading them — and drop any // entry whose model is not there, whatever path the index stored for it. @@ -322,6 +333,7 @@ func New(opts Options) (*App, error) { updatePause: updateCheckPause, updateTimeout: updateCheckRequestTimeout, swapping: map[string]bool{}, + fallbacks: fallbacks, drainWait: drainWait, freeSpace: capability.FreeDisk, } @@ -385,6 +397,7 @@ func New(opts Options) (*App, error) { MaxLoadWaitersPerSource: 2, }) a.applyStatistics(opts.Config) + a.Clients = &selftest.Clients{} a.Probe = contextprobe.New(contextprobe.Options{ Sources: probeSources{a}, Enabled: func() bool { return a.Config().ContextProbe }, @@ -1731,6 +1744,8 @@ func (a *App) startDownload(repoID, commit string) error { // words: a repo the Hub is silent about is not asked again at the // next start, and one the Hub was not heard for is. var perr error + var superseded fallback + var held bool a.finishDownload(dl, func() { perr = a.Registry.Put(registry.Model{ RepoID: repoID, @@ -1754,9 +1769,23 @@ func (a *App) startDownload(repoID, commit string) error { }) // Inside finishDownload's hold of dlMu, where the mark lives. delete(a.swapping, dlKey(repoID)) + // And the old version is held aside in the same step, so + // no load of the new one can end before it is: it goes + // once the new version has loaded and answered, and comes + // back if that first load fails (iss-2610042101430192). + // A version an earlier update already holds stays held: it + // is the last one known to have served, and the version + // this update replaced never loaded, so that one goes. + if dl.staged.Load() { + if _, held = a.fallbacks[dlKey(repoID)]; held { + superseded = fallback{aside: st.aside} + } else { + a.fallbacks[dlKey(repoID)] = fallback{aside: st.aside, commit: st.priorCommit, hashes: st.priorHashes} + } + } }) - if dl.staged.Load() { - a.removeAside(repoID, st.aside) + if held { + a.removeAside(repoID, superseded.aside) } if perr != nil { // The files are on disk; only the index write failed. Surface it — @@ -1987,7 +2016,13 @@ func (a *App) Delete(repoID string) error { } // The directory comes from the validated id, never from the registry's // stored path (see Registry.Remove). - return a.Registry.Remove(repoID, a.Paths.ModelDir(repoID)) + if err := a.Registry.Remove(repoID, a.Paths.ModelDir(repoID)); err != nil { + return err + } + // The version an update replaced goes with the model, or the next start + // would put it back in the model's place. + a.dropFallback(repoID) + return nil } // Close shuts the app down. diff --git a/internal/app/contextprobe.go b/internal/app/contextprobe.go index 85e82c93..a65e6cde 100644 --- a/internal/app/contextprobe.go +++ b/internal/app/contextprobe.go @@ -239,6 +239,9 @@ func (a *App) AdoptMeasurement(repoID string) error { // one. Called off the pool's lock, on the observer's own goroutine. func (a *App) recordLoadFailure(repoID string, err error) { if err == nil { + // A load that answered is what an update's new version waits for + // before the version it replaced is let go. + a.dropFallback(repoID) a.ForgetLoadFailure(repoID) return } @@ -255,6 +258,12 @@ func (a *App) recordLoadFailure(repoID string, err error) { return } } + // A new version that has never loaded failed: the version its update + // replaced is put back, and its record carries no failure, since the + // failure was not its own (iss-2610042101430192). + if a.restoreFallback(repoID) { + return + } reason := notReady.Reason if reason == "" { reason = "did not become ready" diff --git a/internal/app/selftest.go b/internal/app/selftest.go index 4e84b01a..1d0e0cf6 100644 --- a/internal/app/selftest.go +++ b/internal/app/selftest.go @@ -68,6 +68,7 @@ func (s selfTestServer) Activity() selftest.Activity { Downloading: len(s.a.Downloading()), Refusals: res.Refusals, } + act.Requests, act.LastRequest = s.a.Clients.Snapshot() for _, m := range res.Models { act.Models = append(act.Models, selftest.ModelActivity{ RepoID: m.RepoID, InFlight: m.InFlight, LastUsed: m.LastUsed, diff --git a/internal/app/staged.go b/internal/app/staged.go index 41f2f36d..e8295cd8 100644 --- a/internal/app/staged.go +++ b/internal/app/staged.go @@ -249,8 +249,8 @@ func renameAt(from *os.File, fromName string, to *os.File, toName string) error // being served, and swaps it in only when every file has checked out. A model // whose version Dessau never recorded, or that no check has marked, is brought // to the repository's current version. A model whose newer version ships its -// own code, or waits for review, is refused: there is nothing Dessau would -// run to update it to. +// own code, waits for review, or could not be checked for code, is refused: +// there is nothing Dessau knows it would run to update it to. func (a *App) Update(repoID string) error { m, err := a.Registry.Get(repoID) if err != nil { @@ -264,7 +264,7 @@ func (a *App) Update(repoID string) error { switch u.Status { case registry.UpdateAvailable: commit = u.Commit - case registry.UpdateRunsOwnCode, registry.UpdateAwaitingReview: + case registry.UpdateRunsOwnCode, registry.UpdateAwaitingReview, registry.UpdateCannotCheck: return fmt.Errorf("%s: %w", m.RepoID, ErrUpdateNotOffered) } } @@ -283,17 +283,20 @@ type stagedVersion struct { pipelineTag string tags []string answered bool - // aside is where the swap moved the old version, for the caller to - // remove once the new record is published. - aside string + // aside is where the swap moved the old version, held there until the + // new version has loaded once (fallback); priorCommit and priorHashes + // are the old version's record, for putting it back. + aside string + priorCommit string + priorHashes map[string]string } // stagedDownload fetches repoID at commit (the current one when empty) into // the staging folder, checks it, reads its facts, and swaps it in for the // version being served. On success the model is still marked swapping — // loads refused — until the caller has published the new record and lifted -// the mark, and the old version waits aside for the caller to remove -// (removeAside). On any failure the staging folder is removed and the +// the mark, and the old version waits aside for the caller to hold until the +// new one has loaded once (fallback). On any failure the staging folder is removed and the // version being served is untouched. func (a *App) stagedDownload(ctx context.Context, repoID, commit string, prior registry.Model, onProgress func(hub.Progress)) (stagedVersion, error) { root, err := a.modelsRoot() @@ -377,11 +380,11 @@ func (a *App) stagedDownload(ctx context.Context, repoID, commit string, prior r // moment their record can be written. out := stagedVersion{snap: snap, bytes: a.measureDir(staging), facts: registry.ReadModelFacts(staging)} out.pipelineTag, out.tags, out.answered = a.repoCategory(ctx, repoID) - aside, err := a.swapIn(ctx, root, sorg, repoID) + aside, err := a.swapIn(ctx, root, sorg, repoID, name) if err != nil { return fail(err) } - out.aside = aside + out.aside, out.priorCommit, out.priorHashes = aside, prior.Commit, prior.FileHashes return out, nil } @@ -425,14 +428,16 @@ func (a *App) linkUnchanged(root *os.Root, repoID string, prior registry.Model, return linked } -// swapIn moves the staged version in for the one being served. Loads of the -// model are refused from the start (the swapping mark, which modelSource -// reads), the model is drained from the pool — requests in flight finish, -// for up to drainWait — and then the old directory is moved aside and the -// staged one moved in and checked once more. A failure at any point puts the -// old directory back and lifts the mark; on success the mark stays for the +// swapIn moves from, a directory in the model's staging org folder — the +// staged version, or an old one held aside that is being put back +// (restoreFallback) — in for the one being served. Loads of the model are +// refused from the start (the swapping mark, which modelSource reads), the +// model is drained from the pool — requests in flight finish, for up to +// drainWait — and then the directory being served is moved aside and from +// moved in and checked once more. A failure at any point puts the old +// directory back and lifts the mark; on success the mark stays for the // caller to lift with the new record. -func (a *App) swapIn(ctx context.Context, root *os.Root, sorg *stagingOrg, repoID string) (aside string, err error) { +func (a *App) swapIn(ctx context.Context, root *os.Root, sorg *stagingOrg, repoID, from string) (aside string, err error) { a.dlMu.Lock() a.swapping[dlKey(repoID)] = true a.dlMu.Unlock() @@ -536,7 +541,7 @@ func (a *App) swapIn(ctx context.Context, root *os.Root, sorg *stagingOrg, repoI "model", repoID, "err", rerr) } } - staged, err := sorg.Lstat(name) + staged, err := sorg.Lstat(from) if err != nil || !staged.IsDir() { putBack() return "", errors.New("the staged version is not a directory, so it is not moved in") @@ -544,21 +549,21 @@ func (a *App) swapIn(ctx context.Context, root *os.Root, sorg *stagingOrg, repoI if a.beforeSwapRename != nil { a.beforeSwapRename("in") } - if err := renameAt(stagingDir, name, org.dir, name); err != nil { + if err := renameAt(stagingDir, from, org.dir, name); err != nil { putBack() return "", fmt.Errorf("move the new version in: %w", err) } // The check below reads the model's folder by its path, so the path must // name the folder just moved in, as well as the held folder holding it. if fi, err := os.Stat(a.Paths.ModelDir(repoID)); !sameDir(org.Root, name, staged) || err != nil || !os.SameFile(fi, staged) { - undo(org.dir, name, stagingDir, name) + undo(org.dir, name, stagingDir, from) putBack() return "", errors.New("the model's folder changed as the new version was moved in; the update is abandoned") } if err := validateModelDir(a.Paths.ModelDir(repoID)); err != nil { // Not expected — the staged copy was checked — but a check that // fails here puts the old version back rather than serving neither. - if rerr := renameAt(org.dir, name, stagingDir, name); rerr != nil { + if rerr := renameAt(org.dir, name, stagingDir, from); rerr != nil { a.Log.Error("could not move a failed new version out of the way", "model", repoID, "err", rerr) } else { putBack() @@ -613,10 +618,145 @@ func asideAttempt(attempt string) int64 { return at } -// removeAside removes the old version this attempt's swap left aside. Called -// after the mark is lifted, so loads are not refused for as long as the -// removal takes; the folder is this attempt's own, so a later update's swap -// is never what it removes. +// fallback is the version an update replaced, held aside in the staging +// folder until the new version has loaded and answered once. The checks a +// swap makes are the ones a launch makes before it starts a model server, and +// a version can pass them all and still fail to load; held aside, the version +// it replaced is put back then rather than lost (iss-2610042101430192). +type fallback struct { + // aside is where it waits, relative to the models folder. + aside string + // commit and hashes are its record's version, empty when not known: a + // fallback found at start (recoverStaging), whose record the new + // version's had already replaced when the process ended. + commit string + hashes map[string]string +} + +// dropFallback removes the version an update replaced and held aside for +// repoID, if one is: once the new version has loaded and answered, and when +// the model is deleted, since the next start would otherwise put it back in +// the model's place. +func (a *App) dropFallback(repoID string) { + a.dlMu.Lock() + fb, ok := a.fallbacks[dlKey(repoID)] + delete(a.fallbacks, dlKey(repoID)) + a.dlMu.Unlock() + if ok { + a.removeAside(repoID, fb.aside) + } +} + +// restoreFallback puts back the version an update replaced once the new one +// has failed its first load: the same swap an update makes moves the failed +// version out and the held one in, the record is written for the one put +// back — its version when this process knows it, version unknown when it +// was found at start — with why the update failed, and the failed version is +// removed. It reports whether it did; when it did not, the fallback is held +// as before and the caller records the failure as any other. +// +// It claims the model as a download does, so a Download is refused as one +// already in flight, a Delete cancels it and waits, and Close waits for it, +// rather than any of them reaching the folder mid-swap. It does not start +// while one of those holds the model: a download in flight leaves the +// fallback held, and a delete removes it. +func (a *App) restoreFallback(repoID string) bool { + key := dlKey(repoID) + a.dlMu.Lock() + fb, ok := a.fallbacks[key] + if _, busy := a.downloads[key]; !ok || busy || a.dlClosed || a.deleting[key] { + a.dlMu.Unlock() + return false + } + delete(a.fallbacks, key) + ctx, cancel := context.WithCancel(context.Background()) + dl := &download{repoID: repoID, cancel: cancel, done: make(chan struct{})} + a.downloads[key] = dl + a.dlWG.Add(1) + a.dlMu.Unlock() + defer func() { + cancel() + a.finishDownload(dl, nil) + close(dl.done) + a.dlWG.Done() + }() + keep := func(err error) bool { + a.finishDownload(dl, func() { + if _, held := a.fallbacks[key]; !held { + a.fallbacks[key] = fb + } + }) + a.Log.Error("the new version of a model did not load, and the version it replaced could not be put back; it stays aside", + "model", repoID, "err", err) + return false + } + + m, err := a.Registry.Get(repoID) + if err != nil { + return keep(err) + } + repoID = m.RepoID + root, err := a.modelsRoot() + if err != nil { + return keep(err) + } + defer root.Close() + sorg, err := openStagingOrg(root, repoID) + if err != nil { + return keep(err) + } + defer sorg.Close() + failed, err := a.swapIn(ctx, root, sorg, repoID, filepath.Base(fb.aside)) + if err != nil { + return keep(err) + } + + dest := a.Paths.ModelDir(repoID) + bytes, facts := a.measureDir(dest), registry.ReadModelFacts(dest) + // The newer version is still offered when the one put back is a version + // Dessau knows: it is there to try again, by hand. + var update *registry.UpdateCheck + if fb.commit != "" && m.Commit != "" && m.Commit != fb.commit { + update = ®istry.UpdateCheck{Status: registry.UpdateAvailable, Commit: m.Commit, CheckedAt: time.Now()} + } + var perr error + a.finishDownload(dl, func() { + perr = a.Registry.Put(registry.Model{ + RepoID: repoID, + Path: dest, + Bytes: bytes, + ContextLength: facts.ContextLength, + KVChargePerToken: facts.KVChargePerToken, + ChatTemplate: facts.ChatTemplate, + QuantizationBits: facts.QuantizationBits, + PipelineTag: m.PipelineTag, + Tags: m.Tags, + HubSilent: m.HubSilent, + Commit: fb.commit, + FileHashes: fb.hashes, + Update: update, + UpdateFailed: registry.UpdateFailedLoad, + State: registry.StateReady, + Progress: 100, + AddedAt: m.AddedAt, + }) + delete(a.swapping, key) + }) + if err := sorg.RemoveAll(filepath.Base(failed)); err != nil { + a.Log.Warn("could not remove a new version that did not load", "model", repoID, "err", err) + } + if perr != nil { + a.Log.Error("the version an update replaced is back but could not be recorded", "model", repoID, "err", perr) + } + a.Log.Warn("the new version of a model did not load, so the version it replaced is back", "model", repoID) + a.Pool.RefreshCharges() + return true +} + +// removeAside removes an old version a swap left aside. Called after the +// mark is lifted, so loads are not refused for as long as the removal takes; +// the folder is one attempt's own, so a later update's swap is never what it +// removes. func (a *App) removeAside(repoID, aside string) { if aside == "" { return @@ -705,42 +845,53 @@ func (a *App) isSwapping(repoID string) bool { // aside by a swap the previous process died in the middle of goes back where // it was when nothing took its place, and every staged version is removed — // a download is never resumed into the staging folder. One that cannot go -// back is kept (clearStagingKeepingAside). All of it inside a -// root at the models folder; a link at the staging folder's name, or at an +// back is kept (clearStagingKeepingAside), and so is one an update finished +// with that is still held until the new version loads; those are returned, +// keyed as the in-flight downloads are, for the app to hold. All of it inside +// a root at the models folder; a link at the staging folder's name, or at an // org's inside it, is removed as a link and nothing it names is touched. -func recoverStaging(models string, log interface { - Warn(string, ...any) -}) { +func recoverStaging(models string, log stagingLog) map[string]fallback { + held := map[string]fallback{} root, err := os.OpenRoot(models) if err != nil { - return + return held } defer root.Close() fi, err := root.Lstat(stagingDirName) if errors.Is(err, fs.ErrNotExist) { - return + return held } if err != nil || !fi.IsDir() { if err := root.RemoveAll(stagingDirName); err != nil { log.Warn("could not clear the staging folder", "err", err) } - return + return held } - putBackAside(root, log) - clearStagingKeepingAside(root, models, log) + putBack := putBackAside(root, log) + clearStagingKeepingAside(root, models, log, putBack, held) + return held +} + +// stagingLog is what the start's recovery of the staging folder logs to. +type stagingLog interface { + Info(string, ...any) + Warn(string, ...any) } // clearStagingKeepingAside removes every staged version and everything else -// in the staging folder but an old version left aside that could not be put -// back while nothing the rescan would adopt stands in its model's folder — the -// rescan's own check, so the two never disagree — that may -// be the only copy of the model, and an update never removes the last copy. -// It is kept and named in the log for the person to look at. An aside copy -// beside a model folder that checks out is what a swap that finished but -// died before removing it leaves, and goes like everything else. Each org -// folder is opened and held before anything in it is removed, so a link +// in the staging folder but two kinds of old version left aside. One that +// could not be put back while nothing the rescan would adopt stands in its +// model's folder — the rescan's own check, so the two never disagree — may +// be the only copy of the model, and an update never removes the last copy: +// it is kept and named in the log for the person to look at. And the newest +// complete one beside a model folder that checks out is what an update +// finished with and held until the new version has loaded once (fallback): +// it is kept and added to held, unless a copy of that model was put back at +// this start, which makes it older than the one in place. Any other aside +// copy beside a model folder that checks out goes like everything else. Each +// org folder is opened and held before anything in it is removed, so a link // planted at its name meanwhile is never followed. -func clearStagingKeepingAside(root *os.Root, models string, log interface{ Warn(string, ...any) }) { +func clearStagingKeepingAside(root *os.Root, models string, log stagingLog, putBack map[string]bool, held map[string]fallback) { orgs, err := readDirIn(root, stagingDirName) if err != nil { log.Warn("could not read the staging folder", "err", err) @@ -764,14 +915,35 @@ func clearStagingKeepingAside(root *os.Root, models string, log interface{ Warn( sorg.Close() continue } + isAside := func(e os.DirEntry) (name string, ok bool) { + name, _, ok = strings.Cut(e.Name(), asideSuffix) + return name, ok && e.IsDir() && e.Type()&fs.ModeSymlink == 0 + } + // The newest aside copy of each model, the one an update held. + newest := map[string]string{} for _, e := range entries { - if name, _, isAside := strings.Cut(e.Name(), asideSuffix); isAside && e.IsDir() && e.Type()&fs.ModeSymlink == 0 { + if name, ok := isAside(e); ok { + if cur, seen := newest[name]; !seen || asideAttempt(strings.TrimPrefix(e.Name(), name+asideSuffix)) > asideAttempt(strings.TrimPrefix(cur, name+asideSuffix)) { + newest[name] = e.Name() + } + } + } + for _, e := range entries { + if name, ok := isAside(e); ok { repoID := org.Name() + "/" + name // One whose name gives no model can never go back, and goes. - if config.ValidRepoID(repoID) && !registry.ModelDirComplete(filepath.Join(models, filepath.FromSlash(repoID))) { - log.Warn("an old version an interrupted update left aside could not be put back, because something else stands in its model's folder; it is kept", - "path", filepath.ToSlash(filepath.Join(orgRel, e.Name()))) - continue + if config.ValidRepoID(repoID) { + aside := filepath.Join(orgRel, e.Name()) + switch { + case !registry.ModelDirComplete(filepath.Join(models, filepath.FromSlash(repoID))): + log.Warn("an old version an interrupted update left aside could not be put back, because something else stands in its model's folder; it is kept", + "path", filepath.ToSlash(aside)) + continue + case newest[name] == e.Name() && !putBack[dlKey(repoID)] && registry.ModelDirComplete(filepath.Join(models, aside)): + held[dlKey(repoID)] = fallback{aside: aside} + log.Info("the version an update replaced is kept aside until the new version has loaded once", "model", repoID) + continue + } } } if err := sorg.RemoveAll(e.Name()); err != nil { @@ -785,11 +957,13 @@ func clearStagingKeepingAside(root *os.Root, models string, log interface{ Warn( } // putBackAside moves each old version an interrupted swap left aside back to -// its model's folder, when nothing stands there. -func putBackAside(root *os.Root, log interface{ Warn(string, ...any) }) { +// its model's folder, when nothing stands there, and returns the models it +// put one back for, keyed as the in-flight downloads are. +func putBackAside(root *os.Root, log stagingLog) map[string]bool { + putBack := map[string]bool{} orgs, err := readDirIn(root, stagingDirName) if err != nil { - return + return putBack } for _, org := range orgs { orgRel := filepath.Join(stagingDirName, org.Name()) @@ -828,9 +1002,12 @@ func putBackAside(root *os.Root, log interface{ Warn(string, ...any) }) { } if err := root.Rename(aside, destRel(repoID)); err != nil { log.Warn("could not put back a model left aside by an interrupted update", "model", repoID, "err", err) + continue } + putBack[dlKey(repoID)] = true } } + return putBack } // readDirIn lists a directory inside root. diff --git a/internal/app/staged_fallback_test.go b/internal/app/staged_fallback_test.go new file mode 100644 index 00000000..9b821300 --- /dev/null +++ b/internal/app/staged_fallback_test.go @@ -0,0 +1,276 @@ +package app + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/intentdriven/Dessau/internal/config" + "github.com/intentdriven/Dessau/internal/mlxtest" + "github.com/intentdriven/Dessau/internal/registry" + "github.com/intentdriven/Dessau/internal/runtime" +) + +// cannotLoad marks a version whose files pass every check made before a +// start — the directory checks and the launcher's Precheck — and whose model +// server still exits during startup, as mlx-lm's does for a model it cannot +// load. +const cannotLoad = `"dessau_test_cannot_load":true` + +func brokenV2Config() []byte { + return []byte(`{"model_type":"qwen3","max_position_embeddings":40961,` + cannotLoad + `}`) +} + +// loadFailLauncher starts every model as precheckLauncher does, except one +// whose config.json carries cannotLoad: its server has exited, with an exit +// status, by the time the pool first asks it anything. +type loadFailLauncher struct{ precheckLauncher } + +func (l *loadFailLauncher) Launch(ctx context.Context, spec runtime.Spec) (runtime.Process, error) { + if b, _ := os.ReadFile(filepath.Join(spec.ModelPath, "config.json")); strings.Contains(string(b), cannotLoad) { + done := make(chan struct{}) + close(done) + return exitedProcess{done}, nil + } + return l.precheckLauncher.Launch(ctx, spec) +} + +type exitedProcess struct{ done chan struct{} } + +func (p exitedProcess) Stop(context.Context) error { return nil } +func (p exitedProcess) Done() <-chan struct{} { return p.done } +func (p exitedProcess) Err() error { return errors.New("exit status 1") } +func (p exitedProcess) Pid() int { return 4243 } + +// newFallbackApp is newStagedApp over paths, with a launcher that fails a +// version marked cannotLoad. The caller closes the app. +func newFallbackApp(t *testing.T, paths config.Paths, h *versionedHub) *App { + t.Helper() + l := &loadFailLauncher{precheckLauncher{toolLauncher{servers: map[string]*mlxtest.Server{}}}} + a, err := New(Options{Paths: paths, Config: config.Default(), Launcher: l}) + if err != nil { + t.Fatal(err) + } + a.Hub.BaseURL = h.srv.URL + return a +} + +// updateToV2 downloads v1 of org/repo, then updates it to v2 as the hub +// serves it, and waits for the swap to finish. +func updateToV2(t *testing.T, a *App, h *versionedHub) { + t.Helper() + if err := a.Download("org/repo"); err != nil { + t.Fatal(err) + } + waitFor(t, "v1 to be ready", func() bool { + m, err := a.Registry.Get("org/repo") + return err == nil && m.Ready() && m.Commit == commitV1 + }) + h.set(func(h *versionedHub) { h.current = commitV2 }) + a.Registry.SetUpdate("org/repo", commitV1, registry.UpdateCheck{Status: registry.UpdateAvailable, Commit: commitV2, CheckedAt: time.Now()}) + if err := a.Update("org/repo"); err != nil { + t.Fatalf("Update: %v", err) + } + waitSettled(t, a) + if m, _ := a.Registry.Get("org/repo"); !m.Ready() || m.Commit != commitV2 { + t.Fatalf("the new version was not swapped in: state %s, commit %s", m.State, m.Commit) + } +} + +// asidesOf lists the copies of org/repo a swap holds aside. +func asidesOf(a *App) []string { + entries, _ := os.ReadDir(filepath.Join(a.stagingRoot(), "org")) + var out []string + for _, e := range entries { + if strings.HasPrefix(e.Name(), "repo"+asideSuffix) { + out = append(out, e.Name()) + } + } + return out +} + +// servable reports whether a request for org/repo is answered: the model +// loads, and is released again. +func servable(a *App) error { + _, release, err := a.Pool.Acquire(context.Background(), "org/repo") + if err != nil { + return err + } + release() + return nil +} + +// A new version that passes every check made before a start, and then fails +// its first load, is not left as the only copy: the version it replaced is +// put back, recorded as it was, and serves the next request; the card says +// why the update failed and that the newer version is still there to try +// (iss-2610042101430192). +func TestANewVersionThatFailsItsFirstLoadPutsTheOldOneBack(t *testing.T) { + h := newVersionedHub(t, map[string]map[string][]byte{commitV1: v1Files(), commitV2: v2Files()}, commitV1) + h.set(func(h *versionedHub) { h.versions[commitV2]["config.json"] = brokenV2Config() }) + a := newFallbackApp(t, config.NewPaths(t.TempDir()), h) + t.Cleanup(func() { a.Close() }) + updateToV2(t, a, h) + + if err := servable(a); err == nil { + t.Fatal("the broken new version loaded") + } + waitFor(t, "the old version to be put back", func() bool { + m, err := a.Registry.Get("org/repo") + return err == nil && m.Commit == commitV1 + }) + if got := filesIn(t, a.Paths.ModelDir("org/repo")); !sameFiles(got, v1Files()) { + t.Errorf("the model's folder holds %v, want the old version back", got) + } + m, _ := a.Registry.Get("org/repo") + if !m.Ready() || m.LoadFailed() || len(m.FileHashes) != len(v1Files()) { + t.Errorf("the record put back: state %s, load failure %+v, %d hashes", m.State, m.LoadFailure, len(m.FileHashes)) + } + if m.UpdateFailed != registry.UpdateFailedLoad { + t.Errorf("UpdateFailed = %q, want %q", m.UpdateFailed, registry.UpdateFailedLoad) + } + if m.Update == nil || m.Update.Status != registry.UpdateAvailable || m.Update.Commit != commitV2 { + t.Errorf("Update = %+v, want the newer version still offered", m.Update) + } + if err := servable(a); err != nil { + t.Errorf("the old version put back is not served: %v", err) + } + if got := asidesOf(a); len(got) != 0 { + t.Errorf("left aside after the old version went back: %v", got) + } + if entries, _ := os.ReadDir(filepath.Join(a.stagingRoot(), "org")); len(entries) != 0 { + t.Errorf("the failed new version was left in the staging folder: %d entries", len(entries)) + } +} + +// The old copy is held aside until the new version has loaded and answered +// once, and goes then. +func TestTheOldCopyGoesOnceTheNewVersionHasLoaded(t *testing.T) { + h := newVersionedHub(t, map[string]map[string][]byte{commitV1: v1Files(), commitV2: v2Files()}, commitV1) + a := newFallbackApp(t, config.NewPaths(t.TempDir()), h) + t.Cleanup(func() { a.Close() }) + updateToV2(t, a, h) + + if got := asidesOf(a); len(got) != 1 { + t.Fatalf("held aside before the new version loaded: %v, want the old copy", got) + } + if err := servable(a); err != nil { + t.Fatalf("the new version did not load: %v", err) + } + waitFor(t, "the old copy to go", func() bool { return len(asidesOf(a)) == 0 }) + if got := filesIn(t, a.Paths.ModelDir("org/repo")); !sameFiles(got, v2Files()) { + t.Errorf("the model's folder holds %v, want the new version", got) + } + if m, _ := a.Registry.Get("org/repo"); m.Commit != commitV2 || m.UpdateFailed != "" { + t.Errorf("record after the first load: commit %s, update failed %q", m.Commit, m.UpdateFailed) + } +} + +// A restart between the swap and the first load keeps the old copy aside, +// and a first load that then fails still puts it back. The version it was is +// not kept across the restart, so it comes back as a version Dessau did not +// record, which Update brings to the current one. +func TestTheOldCopyIsKeptAcrossARestartUntilTheFirstLoad(t *testing.T) { + h := newVersionedHub(t, map[string]map[string][]byte{commitV1: v1Files(), commitV2: v2Files()}, commitV1) + h.set(func(h *versionedHub) { h.versions[commitV2]["config.json"] = brokenV2Config() }) + paths := config.NewPaths(t.TempDir()) + first := newFallbackApp(t, paths, h) + updateToV2(t, first, h) + if err := first.Close(); err != nil { + t.Fatal(err) + } + + a := newFallbackApp(t, paths, h) + t.Cleanup(func() { a.Close() }) + if got := asidesOf(a); len(got) != 1 { + t.Fatalf("held aside after the restart: %v, want the old copy", got) + } + if err := servable(a); err == nil { + t.Fatal("the broken new version loaded") + } + waitFor(t, "the old version to be put back", func() bool { + return sameFiles(filesIn(t, a.Paths.ModelDir("org/repo")), v1Files()) + }) + waitFor(t, "the record to describe it", func() bool { + m, err := a.Registry.Get("org/repo") + return err == nil && m.UpdateFailed == registry.UpdateFailedLoad + }) + if m, _ := a.Registry.Get("org/repo"); m.Commit != "" || !m.Ready() { + t.Errorf("record put back after a restart: commit %q, state %s; want ready, version unknown", m.Commit, m.State) + } + if err := servable(a); err != nil { + t.Errorf("the old version put back is not served: %v", err) + } +} + +// Deleting a model removes the old copy its update holds aside too, so the +// next start does not bring the model back from it. +func TestDeletingAModelRemovesTheCopyHeldAside(t *testing.T) { + h := newVersionedHub(t, map[string]map[string][]byte{commitV1: v1Files(), commitV2: v2Files()}, commitV1) + paths := config.NewPaths(t.TempDir()) + first := newFallbackApp(t, paths, h) + updateToV2(t, first, h) + if err := first.Delete("org/repo"); err != nil { + t.Fatal(err) + } + if got := asidesOf(first); len(got) != 0 { + t.Errorf("held aside after the model was deleted: %v", got) + } + if err := first.Close(); err != nil { + t.Fatal(err) + } + a := newFallbackApp(t, paths, h) + t.Cleanup(func() { a.Close() }) + if _, err := a.Registry.Get("org/repo"); err == nil { + t.Error("a deleted model came back at the next start") + } +} + +// A second update before the first one's version has loaded keeps the copy +// the first update held: that is the last version known to have served, and +// the version the second update replaces never loaded. So when the newest +// version fails its first load too, the version that served comes back. +func TestASecondUpdateBeforeAnyLoadKeepsTheVersionThatServed(t *testing.T) { + const commitV3 = "3333333333333333333333333333333333333333" + v3 := map[string][]byte{ + "config.json": []byte(`{"model_type":"qwen3","max_position_embeddings":40962,` + cannotLoad + `}`), + "model.safetensors": []byte("weights-all-v3"), + "tokenizer.json": []byte(`{"v":1}`), + } + h := newVersionedHub(t, map[string]map[string][]byte{commitV1: v1Files(), commitV2: v2Files(), commitV3: v3}, commitV1) + h.set(func(h *versionedHub) { h.versions[commitV2]["config.json"] = brokenV2Config() }) + a := newFallbackApp(t, config.NewPaths(t.TempDir()), h) + t.Cleanup(func() { a.Close() }) + updateToV2(t, a, h) + + h.set(func(h *versionedHub) { h.current = commitV3 }) + a.Registry.SetUpdate("org/repo", commitV2, registry.UpdateCheck{Status: registry.UpdateAvailable, Commit: commitV3, CheckedAt: time.Now()}) + if err := a.Update("org/repo"); err != nil { + t.Fatalf("second Update: %v", err) + } + waitSettled(t, a) + if m, _ := a.Registry.Get("org/repo"); m.Commit != commitV3 { + t.Fatalf("the second update was not swapped in: commit %s", m.Commit) + } + if got := asidesOf(a); len(got) != 1 { + t.Fatalf("held aside after two updates: %v, want one copy", got) + } + + if err := servable(a); err == nil { + t.Fatal("the broken newest version loaded") + } + waitFor(t, "the version that served to be put back", func() bool { + m, err := a.Registry.Get("org/repo") + return err == nil && m.Commit == commitV1 + }) + if got := filesIn(t, a.Paths.ModelDir("org/repo")); !sameFiles(got, v1Files()) { + t.Errorf("the model's folder holds %v, want the version that served", got) + } + if err := servable(a); err != nil { + t.Errorf("the version put back is not served: %v", err) + } +} diff --git a/internal/app/staged_test.go b/internal/app/staged_test.go index 4577da59..c86d2095 100644 --- a/internal/app/staged_test.go +++ b/internal/app/staged_test.go @@ -398,11 +398,11 @@ func TestAnUpdateClearsFileFactsAndKeepsSettings(t *testing.T) { } } -// Update offers nothing for a version Dessau would not run, or one waiting -// for review. +// Update offers nothing for a version Dessau would not run, one waiting for +// review, or one whose code a check could not tell. func TestUpdateIsRefusedWhereNoneIsOffered(t *testing.T) { a, _ := newStagedApp(t) - for _, status := range []string{registry.UpdateRunsOwnCode, registry.UpdateAwaitingReview} { + for _, status := range []string{registry.UpdateRunsOwnCode, registry.UpdateAwaitingReview, registry.UpdateCannotCheck} { a.Registry.SetUpdate("org/repo", commitV1, registry.UpdateCheck{Status: status, Commit: commitV2, CheckedAt: time.Now()}) if err := a.Update("org/repo"); !errors.Is(err, ErrUpdateNotOffered) { t.Errorf("%s: err = %v, want ErrUpdateNotOffered", status, err) diff --git a/internal/app/updatecheck.go b/internal/app/updatecheck.go index b9070249..829b6c08 100644 --- a/internal/app/updatecheck.go +++ b/internal/app/updatecheck.go @@ -233,13 +233,22 @@ func (a *App) checkOne(ctx context.Context, m registry.Model) (registry.UpdateCh if !a.Config().UpdateCheck { return registry.UpdateCheck{}, errChecksOff } - b, err := a.Hub.SmallFileAt(ctx, up, "config.json", maxCheckedConfig) + listed := hub.File{Path: "config.json"} + for _, f := range files { + if f.Path == "config.json" { + listed = f + break + } + } + b, err := a.Hub.SmallFileAt(ctx, up, listed, maxCheckedConfig) switch { - case errors.Is(err, hub.ErrCrossOrigin): - // A config.json the repository keeps in LFS is served from the - // Hub's content CDN, which a check does not read from. The - // newer version is offered; the update's own Precheck refuses - // it if it names a model_file. + case errors.Is(err, hub.ErrCrossOrigin), errors.Is(err, hub.ErrContentMismatch), errors.Is(err, hub.ErrOversizedBody): + // The Hub answered, but with a config.json the check cannot + // hold to the hash it lists — handed off its origin with no + // sha256 to check, not the listed bytes, or past the bound — + // so whether the newer version names a model_file is unknown, + // and it is not offered (iss-2610042101439623). + return registry.UpdateCheck{Status: registry.UpdateCannotCheck, Commit: up.Commit}, nil case err != nil: return registry.UpdateCheck{}, err default: diff --git a/internal/app/updatecheck_test.go b/internal/app/updatecheck_test.go index 4fe7c3e1..5d1152f5 100644 --- a/internal/app/updatecheck_test.go +++ b/internal/app/updatecheck_test.go @@ -3,6 +3,8 @@ package app import ( "bytes" "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "log/slog" @@ -512,26 +514,86 @@ func TestAnUnrecordedConfigIsReadWhenTheVersionMoves(t *testing.T) { } } -// A config.json the repository keeps in LFS is handed to the Hub's content -// CDN, which a check does not read from; the newer version is offered rather -// than the model counted unreachable at every check, and the update's own -// Precheck is what refuses one that names a model_file. -func TestAConfigOnTheContentCDNDoesNotStopTheCheck(t *testing.T) { - cdn := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, `{"model_type":"qwen3"}`) - })) - defer cdn.Close() +// lfsConfigCheck runs one check of a model whose newer config.json the Hub +// hands to a content CDN that answers with serve; listed is the hash the +// listing gives for it, a sha256 for a file the repository keeps in LFS. +func lfsConfigCheck(t *testing.T, listed string, serve http.HandlerFunc) (*registry.UpdateCheck, UpdateRound) { + t.Helper() + cdn := httptest.NewServer(serve) + t.Cleanup(cdn.Close) files := recordedFiles() - files["config.json"] = hConfig2 + files["config.json"] = listed h := newCheckFakeHub(t, map[string]upstreamRepo{"org/m": {commit: newer, files: files}}) h.cdn = cdn.URL a, _ := newCheckApp(t, h, true, map[string]map[string]string{"org/m": recordedFiles()}) round := a.CheckForUpdates(context.Background(), a.updateCheckDue(time.Now())) - if u := update(t, a, "org/m"); u == nil || u.Status != registry.UpdateAvailable { + return update(t, a, "org/m"), round +} + +func sha256Hex(s string) string { + sum := sha256.Sum256([]byte(s)) + return hex.EncodeToString(sum[:]) +} + +// A config.json the repository keeps in LFS is handed to the Hub's content +// CDN; the check reads it there, held to the sha256 the listing gives, so a +// newer version that ships its own code is named as one and never offered +// (iss-2610042101439623). +func TestAConfigInLFSThatNamesCodeIsNotOffered(t *testing.T) { + const body = `{"model_type":"qwen3","model_file":"modeling.py"}` + u, round := lfsConfigCheck(t, sha256Hex(body), func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, body) + }) + if u == nil || u.Status != registry.UpdateRunsOwnCode { + t.Errorf("Update = %+v (round %+v), want runs_own_code", u, round) + } +} + +// One that names no code is offered as any other newer version is. +func TestAConfigInLFSThatNamesNoCodeIsOffered(t *testing.T) { + const body = `{"model_type":"qwen3"}` + u, round := lfsConfigCheck(t, sha256Hex(body), func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, body) + }) + if u == nil || u.Status != registry.UpdateAvailable { t.Errorf("Update = %+v (round %+v), want available", u, round) } } +// Bytes from the CDN that are not the ones the listing describes say nothing +// about the newer version's code: it is marked as one Dessau could not +// check, and not offered. +func TestAConfigFromTheCDNThatIsNotTheListedOneIsNotOffered(t *testing.T) { + u, round := lfsConfigCheck(t, sha256Hex(`{"model_type":"qwen3","model_file":"modeling.py"}`), + func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `{"model_type":"qwen3"}`) }) + if u == nil || u.Status != registry.UpdateCannotCheck { + t.Errorf("Update = %+v (round %+v), want cannot_check", u, round) + } +} + +// A CDN that does not answer leaves the model as a Hub that does not answer +// does: no mark, so nothing is offered, and the next interval asks again. +func TestAConfigTheCDNDoesNotServeIsNotOffered(t *testing.T) { + u, round := lfsConfigCheck(t, sha256Hex(`{"model_type":"qwen3"}`), func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + }) + if u != nil || round.Unreachable != 1 { + t.Errorf("Update = %+v (round %+v), want no mark and the model counted unreachable", u, round) + } +} + +// A config.json the listing keeps in git, so with no sha256, that the Hub +// hands off its own origin anyway has nothing to hold those bytes to: the +// newer version is marked as one Dessau could not check, and not offered. +func TestAGitConfigHandedOffTheHubIsNotOffered(t *testing.T) { + u, round := lfsConfigCheck(t, hConfig2, func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"model_type":"qwen3"}`) + }) + if u == nil || u.Status != registry.UpdateCannotCheck { + t.Errorf("Update = %+v (round %+v), want cannot_check", u, round) + } +} + // A round cut short leaves the models it did not reach due at the next tick, // and a round cut short by the switch says nothing about the Hub. func TestARoundCutShortLeavesTheRestDue(t *testing.T) { diff --git a/internal/gateway/ask.go b/internal/gateway/ask.go index b40bc6af..7b6ca2eb 100644 --- a/internal/gateway/ask.go +++ b/internal/gateway/ask.go @@ -46,6 +46,9 @@ import ( // adr-2609061610102325 already made. func (g *Gateway) Ask(ctx context.Context, req AskRequest) error { started := time.Now() + // Counted for the idle clock like a request over the network + // (Options.Clients). + defer g.clients.Begin()() cfg := g.cfg() obs := g.observe(cfg.Statistics, started) obs.sourced(req.Source) diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index 760d8839..250a9510 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -105,6 +105,11 @@ type Options struct { // Nil means no idle job ever holds anything, and every such refusal is // the plain sentence. IdleJobs func() selftest.Status + // Clients is the idle clock's count of client requests: every request + // to the completion routes and every Ask is counted from its start to + // its end, whatever it ends as (iss-2610041945030758). Nil counts + // nothing. + Clients *selftest.Clients } // Gateway routes OpenAI requests to model servers. @@ -125,6 +130,8 @@ type Gateway struct { transcriptOn func() bool // idleJobs is Options.IdleJobs, never nil. idleJobs func() selftest.Status + // clients is Options.Clients; nil counts nothing. + clients *selftest.Clients } // New builds a Gateway. @@ -159,6 +166,7 @@ func New(opts Options) *Gateway { servedWindow: served, transcriptOn: transcriptOn, idleJobs: idleJobs, + clients: opts.Clients, } } @@ -696,6 +704,9 @@ func (g *Gateway) completions(w http.ResponseWriter, r *http.Request, chat bool) // and the only definition under which time to first token means what a // client thinks it means. started := time.Now() + // Counted for the idle clock from here to the handler's return, whatever + // the request ends as (Options.Clients). + defer g.clients.Begin()() cfg := g.cfg() obs := g.observe(cfg.Statistics, started) defer obs.finish(r.Context()) diff --git a/internal/gateway/idleclock_integration_test.go b/internal/gateway/idleclock_integration_test.go new file mode 100644 index 00000000..5f511c9d --- /dev/null +++ b/internal/gateway/idleclock_integration_test.go @@ -0,0 +1,189 @@ +package gateway + +import ( + "bytes" + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/intentdriven/Dessau/internal/app" + "github.com/intentdriven/Dessau/internal/config" + "github.com/intentdriven/Dessau/internal/mlxtest" + "github.com/intentdriven/Dessau/internal/registry" + "github.com/intentdriven/Dessau/internal/runtime" + "github.com/intentdriven/Dessau/internal/selftest" +) + +// idleLauncher stands a fake model server up for each launch and keeps the +// newest, so a test can take it away underneath a model the pool still holds: +// the process stays, the socket answers nothing, and a request through the +// gateway ends unreachable — what a wedged model server looks like from the +// gateway once its client gives up. +type idleLauncher struct { + mu sync.Mutex + srv *mlxtest.Server +} + +func (l *idleLauncher) Precheck(runtime.Spec) error { return nil } + +func (l *idleLauncher) Launch(_ context.Context, spec runtime.Spec) (runtime.Process, error) { + srv := mlxtest.Start(mlxtest.Options{ModelArg: spec.ModelPath, Socket: spec.Socket}) + l.mu.Lock() + l.srv = srv + l.mu.Unlock() + return &fakeProc{srv: srv, done: make(chan struct{})}, nil +} + +func (l *idleLauncher) server() *mlxtest.Server { + l.mu.Lock() + defer l.mu.Unlock() + return l.srv +} + +// idleStack is an App with a real pool over the idle launcher, one ready chat +// model, the idle loop on a fast tick with a one-hour idle threshold, and a +// gateway wired to the App's client clock the way the server's own start +// wires it. +func idleStack(t *testing.T) (*app.App, *Gateway, *idleLauncher) { + t.Helper() + paths := config.NewPaths(t.TempDir()) + launcher := &idleLauncher{} + a, err := app.New(app.Options{ + Paths: paths, Config: config.Default(), Launcher: launcher, + Log: slog.New(slog.NewTextHandler(io.Discard, nil)), + Idle: app.IdleOptions{Tick: 10 * time.Millisecond, Poll: 5 * time.Millisecond, Quiet: time.Hour}, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { a.Close() }) + dir := paths.ModelDir("org/m") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "model.safetensors"), []byte("weights"), 0o644); err != nil { + t.Fatal(err) + } + // A current tool-calling verdict, so the tool probe queues nothing on the + // load and the only request this test makes is its client's. + if err := a.Registry.Put(registry.Model{ + RepoID: "org/m", Path: dir, State: registry.StateReady, Bytes: 7, ChatTemplate: true, + ToolCalling: ®istry.ToolCalling{Can: false, At: 1, Runtime: runtime.MLXLMVersion()}, + }); err != nil { + t.Fatal(err) + } + g := New(Options{ConfigFunc: a.Config, Pool: a.Pool, Models: a.Registry, Log: a.Log, Clients: a.Clients}) + return a, g, launcher +} + +// selfTestOn switches the self-test on, through the save a person's tick goes +// through. +func selfTestOn(t *testing.T, a *app.App) { + t.Helper() + c := a.Config() + c.SelfTest = true + if err := a.SetConfig(c); err != nil { + t.Fatal(err) + } +} + +// firstVerdict waits for the idle loop's first word on the due model: held, +// and by what, or a run started. It reports the status it saw. +func firstVerdict(t *testing.T, a *app.App) selftest.Status { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + st := a.SelfTest.Status() + if st.HeldBy != "" || st.Model != "" { + return st + } + time.Sleep(2 * time.Millisecond) + } + t.Fatalf("the idle loop said nothing; status %+v", a.SelfTest.Status()) + return selftest.Status{} +} + +func chatBody(t *testing.T) []byte { + t.Helper() + body, err := json.Marshal(map[string]any{ + "model": "org/m", "messages": []map[string]string{{"role": "user", "content": "hello"}}, "max_tokens": 1, + }) + if err != nil { + t.Fatal(err) + } + return body +} + +// A client request that ends unreachable resets the idle clock, and the reset +// outlasts the model it was sent to: on 2026-10-04 a client's requests to a +// wedged model each ended unreachable, the model left the pool, and the +// self-test started a minute after the last of them although the idle +// threshold was an hour (iss-2610041945030758). The clock lived on the +// model's pool entry and went with it. +func TestAClientRequestThatEndsUnreachableHoldsTheIdleJobs(t *testing.T) { + a, g, launcher := idleStack(t) + + // The model loaded by something other than a client, then its server + // taken away underneath the pool's entry. + _, release, err := a.Pool.Acquire(context.Background(), "org/m") + if err != nil { + t.Fatal(err) + } + release() + launcher.server().Close() + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", bytes.NewReader(chatBody(t))) + req.Header.Set("Content-Type", "application/json") + g.Handler().ServeHTTP(rec, req) + if rec.Code != http.StatusBadGateway { + t.Fatalf("the client got %d, want 502 (the model server did not respond): %s", rec.Code, rec.Body) + } + + // The model leaves the pool, as a wedged one does when it is unloaded + // or its server is stopped. + if err := a.Pool.Unload("org/m"); err != nil { + t.Fatal(err) + } + if res := a.Pool.Residency(); len(res.Models) != 0 { + t.Fatalf("the model is still resident: %+v", res.Models) + } + + selfTestOn(t, a) + if st := firstVerdict(t, a); st.HeldBy != selftest.HeldByRecent { + t.Errorf("the idle loop's first word a moment after an unreachable client request: %+v; want held by %q", st, selftest.HeldByRecent) + } +} + +// A client request in flight holds the idle jobs for as long as it runs, even +// where no model shows it: here it is still uploading its body, so the pool +// has not seen it at all. +func TestAClientRequestInFlightHoldsTheIdleJobs(t *testing.T) { + a, g, _ := idleStack(t) + + body, upload := io.Pipe() + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", body) + req.Header.Set("Content-Type", "application/json") + done := make(chan struct{}) + go func() { + defer close(done) + g.Handler().ServeHTTP(httptest.NewRecorder(), req) + }() + t.Cleanup(func() { upload.Close(); <-done }) + if _, err := upload.Write([]byte(`{"model":`)); err != nil { + t.Fatal(err) + } + + selfTestOn(t, a) + if st := firstVerdict(t, a); st.HeldBy != selftest.HeldByInFlight { + t.Errorf("the idle loop's first word with a client request in flight: %+v; want held by %q", st, selftest.HeldByInFlight) + } +} diff --git a/internal/hub/check_test.go b/internal/hub/check_test.go index 0387b90c..fbb200bf 100644 --- a/internal/hub/check_test.go +++ b/internal/hub/check_test.go @@ -2,6 +2,8 @@ package hub import ( "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -187,18 +189,106 @@ func TestACheckReadsASmallFileAtTheCommit(t *testing.T) { if err != nil { t.Fatal(err) } - b, err := c.SmallFileAt(context.Background(), up, "config.json", 1<<20) + b, err := c.SmallFileAt(context.Background(), up, File{Path: "config.json"}, 1<<20) if err != nil { t.Fatalf("SmallFileAt: %v", err) } if string(b) != `{"model_file":"x.py"}` { t.Errorf("body = %q", b) } - if _, err := c.SmallFileAt(context.Background(), up, "config.json", 4); !errors.Is(err, ErrOversizedBody) { + if _, err := c.SmallFileAt(context.Background(), up, File{Path: "config.json"}, 4); !errors.Is(err, ErrOversizedBody) { t.Errorf("a body past the bound: err = %v, want ErrOversizedBody", err) } } +// lfsSmallFileHub is a Hub that hands config.json to a content CDN, as it +// does for a file a repository keeps in LFS; the CDN answers with body and +// records the Authorization header it was sent. +func lfsSmallFileHub(t *testing.T, body string) (c *Client, cdnAuth func() []string) { + t.Helper() + var mu sync.Mutex + var auths []string + cdn := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + auths = append(auths, r.Header.Get("Authorization")) + mu.Unlock() + fmt.Fprint(w, body) + })) + t.Cleanup(cdn.Close) + hub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/org/repo/resolve/"+testCommit+"/config.json" { + http.Redirect(w, r, cdn.URL+"/blob", http.StatusFound) + return + } + http.NotFound(w, r) + })) + t.Cleanup(hub.Close) + return &Client{BaseURL: hub.URL, HTTP: hub.Client()}, func() []string { + mu.Lock() + defer mu.Unlock() + return append([]string(nil), auths...) + } +} + +func lfsFile(path, body string, size int64) File { + sum := sha256.Sum256([]byte(body)) + f := File{Path: path, Type: "file", Size: size} + f.LFS = &struct { + OID string `json:"oid"` + Size int64 `json:"size"` + }{OID: hex.EncodeToString(sum[:]), Size: size} + return f +} + +// A small file the listing says the repository keeps in LFS is read where +// the Hub hands it, its content CDN, as a download reads it: held to the +// sha256 the listing gives and to the bound, and never sent the token off +// the Hub's origin (iss-2610042101439623). +func TestACheckReadsASmallLFSFileFromTheContentCDN(t *testing.T) { + const body = `{"model_file":"x.py"}` + c, cdnAuth := lfsSmallFileHub(t, body) + up := Upstream{RepoID: "org/repo", Commit: testCommit, Authed: true, token: "hf_secret"} + b, err := c.SmallFileAt(context.Background(), up, lfsFile("config.json", body, int64(len(body))), 1<<20) + if err != nil { + t.Fatalf("SmallFileAt: %v", err) + } + if string(b) != body { + t.Errorf("body = %q", b) + } + for _, a := range cdnAuth() { + if a != "" { + t.Errorf("the content CDN was sent Authorization %q", a) + } + } + if _, err := c.SmallFileAt(context.Background(), up, lfsFile("config.json", `{"model_type":"qwen3"}`, int64(len(body))), 1<<20); !errors.Is(err, ErrContentMismatch) { + t.Errorf("bytes that are not the listed ones: err = %v, want ErrContentMismatch", err) + } + if _, err := c.SmallFileAt(context.Background(), up, lfsFile("config.json", body, int64(len(body))), 4); !errors.Is(err, ErrOversizedBody) { + t.Errorf("a body past the bound: err = %v, want ErrOversizedBody", err) + } + before := len(cdnAuth()) + if _, err := c.SmallFileAt(context.Background(), up, lfsFile("config.json", body, 1<<30), 1<<20); !errors.Is(err, ErrOversizedBody) { + t.Errorf("a file listed past the bound: err = %v, want ErrOversizedBody", err) + } + if len(cdnAuth()) != before { + t.Error("a file listed past the bound was fetched") + } +} + +// A file the listing keeps in git has no sha256 to hold the CDN's bytes to, +// so one the Hub hands off its origin is still refused. +func TestASmallGitFileHandedOffTheHubIsRefused(t *testing.T) { + c, cdnAuth := lfsSmallFileHub(t, `{}`) + up := Upstream{RepoID: "org/repo", Commit: testCommit} + _, err := c.SmallFileAt(context.Background(), up, File{Path: "config.json", OID: testCommit}, 1<<20) + if !errors.Is(err, ErrCrossOrigin) { + t.Errorf("err = %v, want ErrCrossOrigin", err) + } + if len(cdnAuth()) != 0 { + t.Error("the content CDN was asked for a file the listing keeps in git") + } +} + // The Hub's rate-limit header is read off every answer, so a round of checks // can stop before it spends what is left. func TestTheRateLimitHeaderIsRead(t *testing.T) { diff --git a/internal/hub/hub.go b/internal/hub/hub.go index 46a13888..0e646528 100644 --- a/internal/hub/hub.go +++ b/internal/hub/hub.go @@ -10,6 +10,8 @@ package hub import ( "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -261,7 +263,9 @@ func (c *Client) refuseOffOrigin(req *http.Request, via []*http.Request) error { // blob id the listing gives, a hash of their content too; one the listing gives // no usable hash for is checked on length alone, so for those the exception is // wider than that justification — iss-2609190151179403 holds the question of -// narrowing it to files the tree gave a hash for. +// narrowing it to files the tree gave a hash for. SmallFileAt's read of a +// file the listing keeps in LFS takes the same exception, through doContent, +// and only for a file the listing gave a sha256 for. func (c *Client) do(req *http.Request) (*http.Response, error) { hc := *c.httpClient() hc.CheckRedirect = c.refuseOffOrigin @@ -376,11 +380,23 @@ func (c *Client) FilesAt(ctx context.Context, up Upstream) ([]File, error) { // SmallFileAt reads one small file of the repository at the commit Latest // found, under the token choice it made, refusing a body past max with -// ErrOversizedBody. It goes through do, so the answer comes from the Hub's -// own origin or not at all: it is for a file the repository keeps in git, -// such as config.json, never for weights the Hub hands to its content CDN. -func (c *Client) SmallFileAt(ctx context.Context, up Upstream, file string, max int64) ([]byte, error) { - u, err := c.ResolveURL(up.RepoID, up.Commit, file) +// ErrOversizedBody. f is the file as the listing at that commit gives it. +// +// A file the repository keeps in git goes through do, so the answer comes +// from the Hub's own origin or not at all. One the listing says it keeps in +// LFS, such as a config.json some repositories store there, the Hub hands to +// its content CDN on another host by design, and it is read there as a +// download reads it: what anchors those bytes is the sha256 the listing +// stated for them, and a body that is not those bytes is refused with +// ErrContentMismatch (iss-2610042101439623). An LFS entry the listing gives +// no sha256 for is read as a git file is, so off the Hub's origin it is +// refused. The token never leaves the Hub's origin on the CDN hop. +func (c *Client) SmallFileAt(ctx context.Context, up Upstream, f File, max int64) ([]byte, error) { + lfs := f.LFS != nil && isHex(f.LFS.OID, 64) + if lfs && f.LFS.Size > max { + return nil, fmt.Errorf("%s of %s is listed at %d bytes, longer than %d: %w", f.Path, up.RepoID, f.LFS.Size, max, ErrOversizedBody) + } + u, err := c.ResolveURL(up.RepoID, up.Commit, f.Path) if err != nil { return nil, err } @@ -388,9 +404,14 @@ func (c *Client) SmallFileAt(ctx context.Context, up Upstream, file string, max if err != nil { return nil, err } - resp, err := c.do(req) + var resp *http.Response + if lfs { + resp, err = c.doContent(req) + } else { + resp, err = c.do(req) + } if err != nil { - return nil, fmt.Errorf("read %s of %s: %w", file, up.RepoID, err) + return nil, fmt.Errorf("read %s of %s: %w", f.Path, up.RepoID, err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { @@ -398,14 +419,40 @@ func (c *Client) SmallFileAt(ctx context.Context, up Upstream, file string, max } b, err := io.ReadAll(io.LimitReader(resp.Body, max+1)) if err != nil { - return nil, fmt.Errorf("read %s of %s: %w", file, up.RepoID, err) + return nil, fmt.Errorf("read %s of %s: %w", f.Path, up.RepoID, err) } if int64(len(b)) > max { - return nil, fmt.Errorf("%s of %s is longer than %d bytes: %w", file, up.RepoID, max, ErrOversizedBody) + return nil, fmt.Errorf("%s of %s is longer than %d bytes: %w", f.Path, up.RepoID, max, ErrOversizedBody) + } + if lfs { + sum := sha256.Sum256(b) + if got := hex.EncodeToString(sum[:]); !strings.EqualFold(got, f.LFS.OID) { + return nil, fmt.Errorf("%s of %s: got sha256:%s, expected sha256:%s: %w", f.Path, up.RepoID, got, f.LFS.OID, ErrContentMismatch) + } } return b, nil } +// doContent issues a request for an LFS object's content, which the Hub +// answers with a redirect to its content CDN. Unlike do it follows that hop +// off the Hub's origin — the caller holds what arrives to the sha256 the +// listing gave — but it drops the access token from any hop that leaves the +// origin, which a pre-signed CDN URL does not need, and keeps the hop limit. +// It runs on a shallow copy of the client, as do does. +func (c *Client) doContent(req *http.Request) (*http.Response, error) { + hc := *c.httpClient() + hc.CheckRedirect = func(next *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return fmt.Errorf("stopped after %d redirects", maxRedirects) + } + if !sameOrigin(c.baseURL(), next.URL.String()) { + next.Header.Del("Authorization") + } + return nil + } + return hc.Do(req) +} + // APIError is a non-2xx response from the Hub. type APIError struct { StatusCode int @@ -806,6 +853,10 @@ func (c *Client) nextPage(pageURL, link string) (string, error) { // them. var ErrCrossOrigin = errors.New("the answer did not come from the hub's origin") +// ErrContentMismatch is SmallFileAt's refusal of a body that is not the bytes +// the listing's sha256 describes. +var ErrContentMismatch = errors.New("the content does not match the hash the hub lists for it") + // sameOrigin reports whether target has the same scheme and host as base. A // parse failure or missing host counts as different, i.e. refuse it. func sameOrigin(base, target string) bool { diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 5973cf3e..a81bbe96 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -174,6 +174,11 @@ const ( // UpdateAwaitingReview: a decision model's newer version has not been // reviewed by a Dessau release, so it is not offered until one has. UpdateAwaitingReview = "awaiting_review" + // UpdateCannotCheck: the Hub handed out the newer version's config.json + // in a form the check could not hold to the hash it lists for it, so + // whether that version ships its own code is unknown and it is not + // offered (iss-2610042101439623). + UpdateCannotCheck = "cannot_check" ) // ErrVersionMoved is SetUpdate's refusal when the model on disk is no longer @@ -198,7 +203,7 @@ func sanitizeUpdate(m Model) Model { switch u.Status { case UpdateCurrent: ok = ok && u.Commit == "" - case UpdateAvailable, UpdateRunsOwnCode, UpdateAwaitingReview: + case UpdateAvailable, UpdateRunsOwnCode, UpdateAwaitingReview, UpdateCannotCheck: ok = ok && validCommit(u.Commit) && u.Commit != m.Commit default: ok = false diff --git a/internal/registry/updatefailure.go b/internal/registry/updatefailure.go index 471faeb8..a45ddc40 100644 --- a/internal/registry/updatefailure.go +++ b/internal/registry/updatefailure.go @@ -5,7 +5,8 @@ import "fmt" // Why a model's last update failed, as a class rather than the error's text: // the text can carry paths and addresses, and the card shows words of its // own for each class (iss-2610031320392078). The version being served is -// untouched in every case. +// untouched in every case but UpdateFailedLoad, where the version served +// before the update is put back. const ( // UpdateFailedDownload: a file did not download, or did not match the // hash the Hub lists for it. @@ -22,11 +23,15 @@ const ( // UpdateFailedNotOffered: the newer version is one Dessau does not run // — it ships its own code, or waits for review. UpdateFailedNotOffered = "not_offered" + // UpdateFailedLoad: the new version passed every check and was swapped + // in, then failed its first load, so the version it replaced was put + // back (iss-2610042101430192). + UpdateFailedLoad = "load" ) func validUpdateFailure(class string) bool { switch class { - case UpdateFailedDownload, UpdateFailedNoSpace, UpdateFailedRefused, UpdateFailedBusy, UpdateFailedNotOffered: + case UpdateFailedDownload, UpdateFailedNoSpace, UpdateFailedRefused, UpdateFailedBusy, UpdateFailedNotOffered, UpdateFailedLoad: return true } return false diff --git a/internal/selftest/clients.go b/internal/selftest/clients.go new file mode 100644 index 00000000..293602d6 --- /dev/null +++ b/internal/selftest/clients.go @@ -0,0 +1,62 @@ +package selftest + +import ( + "sync" + "time" +) + +// Clients is the idle clock's count of client requests, kept where every +// client request passes — the gateway — rather than on the model that serves +// it. +// +// The pool's own view (ModelActivity) is per resident model, and a model's +// figures leave with it: a request that ended unreachable on a wedged model +// that was then unloaded or stopped left no trace the idle loop could read, +// and a request that never reached the pool — still uploading its body, +// refused, or failed before a model was chosen — left none to begin with. On +// 2026-10-04 that let the self-test start a minute after a client's last +// unreachable request although the idle threshold was an hour +// (iss-2610041945030758). This count is kept beside the pool's, not instead +// of it. +// +// The zero value is ready to use, and a nil *Clients counts nothing. +type Clients struct { + mu sync.Mutex + inFlight int + last time.Time +} + +// Begin counts one client request as started and returns the function that +// counts it as ended. The request holds the Mac busy for as long as it is in +// flight, and the clock is stamped at both ends, so a request that ran for +// ten minutes is quiet from its end, not its start. Calling end more than once +// counts it once. +func (c *Clients) Begin() (end func()) { + if c == nil { + return func() {} + } + c.mu.Lock() + c.inFlight++ + c.last = time.Now() + c.mu.Unlock() + var once sync.Once + return func() { + once.Do(func() { + c.mu.Lock() + c.inFlight-- + c.last = time.Now() + c.mu.Unlock() + }) + } +} + +// Snapshot is how many client requests are in flight and when the last one +// started or ended; zero for both when none has been seen. +func (c *Clients) Snapshot() (inFlight int, last time.Time) { + if c == nil { + return 0, time.Time{} + } + c.mu.Lock() + defer c.mu.Unlock() + return c.inFlight, c.last +} diff --git a/internal/selftest/clients_test.go b/internal/selftest/clients_test.go new file mode 100644 index 00000000..a484348b --- /dev/null +++ b/internal/selftest/clients_test.go @@ -0,0 +1,38 @@ +package selftest + +import ( + "testing" + "time" +) + +// A client request counts from the moment it starts until it ends, and the +// clock reads the later of the two: a request in flight for ten minutes is +// ten minutes of a busy Mac, not one moment at its end. +func TestClientsCountARequestFromStartToEnd(t *testing.T) { + var c Clients + if n, last := c.Snapshot(); n != 0 || !last.IsZero() { + t.Fatalf("a fresh clock reads %d in flight, last %v", n, last) + } + before := time.Now() + end := c.Begin() + n, started := c.Snapshot() + if n != 1 || started.Before(before) { + t.Fatalf("after Begin: %d in flight, last %v; want 1, at or after %v", n, started, before) + } + time.Sleep(time.Millisecond) + end() + end() // a second call counts nothing + n, ended := c.Snapshot() + if n != 0 || !ended.After(started) { + t.Errorf("after the end: %d in flight, last %v; want 0, after %v", n, ended, started) + } +} + +// A gateway built without a clock counts nothing and does not fail. +func TestANilClientsCountsNothing(t *testing.T) { + var c *Clients + c.Begin()() + if n, last := c.Snapshot(); n != 0 || !last.IsZero() { + t.Errorf("a nil clock reads %d in flight, last %v", n, last) + } +} diff --git a/internal/selftest/selftest.go b/internal/selftest/selftest.go index 970f3f9b..1c2bfda7 100644 --- a/internal/selftest/selftest.go +++ b/internal/selftest/selftest.go @@ -1,13 +1,14 @@ // Package selftest measures Dessau's own models while nobody is using them. // // When the operator switches it on (config.Config.SelfTest), a loop wakes -// once a minute and asks whether the Mac is idle: no request in flight on any -// model, nobody waiting for a load, no download running, and the last request -// older than a quiet period. When it is, the loop picks the ready model that -// was tested longest ago — never one tested within the last day — acquires it -// through the pool's ordinary path, runs the same short set of tests against -// it, appends one line of figures to a file, and unloads the model if the -// self-test was what brought it in. Then it goes back to sleep. +// once a minute and asks whether the Mac is idle: no client request in flight +// anywhere, nobody waiting for a load, no download running, and the last +// client request — whatever it ended as, and whether or not its model is still +// loaded (Clients) — older than a quiet period. When it is, the loop picks the +// ready model that was tested longest ago — never one tested within the last +// day — acquires it through the pool's ordinary path, runs the same short set +// of tests against it, appends one line of figures to a file, and unloads the +// model if the self-test was what brought it in. Then it goes back to sleep. // // The set is llama-bench's pair plus what the pool can tell: how long the load // took, how fast a long prompt is read (pp512), how fast tokens come out @@ -125,6 +126,10 @@ type Activity struct { // the run to let go. This is what is left — a client refused over a model // the run is not holding, or one the run would not give up in time. Refusals uint64 + // Requests is how many client requests are in flight anywhere, and + // LastRequest when the last one started or ended (Clients.Snapshot). + Requests int + LastRequest time.Time } // ModelActivity is one resident model's share of that view. @@ -571,8 +576,20 @@ func (r *Runner) heldBy(act Activity, now time.Time) string { if act.Downloading > 0 { return HeldByDownloading } + // Every client request, counted at the gateway whatever it ended as and + // wherever its model is now (Clients, iss-2610041945030758). Ticks do not + // run during a run, so a request in flight here is almost never the + // loop's own: a probe step that timed out can leave its gateway handler + // running past the run, which holds the next job for one threshold, as + // the pool's own release already did. + if act.Requests > 0 { + return HeldByInFlight + } r.mu.Lock() defer r.mu.Unlock() + if r.clientRecentLocked(act.LastRequest, now) { + return HeldByRecent + } for _, m := range act.Models { if m.InFlight > 0 { return HeldByInFlight @@ -588,10 +605,28 @@ func (r *Runner) heldBy(act Activity, now time.Time) string { return "" } +// clientRecentLocked reports whether the gateway's last client request is +// within the quiet period. A last request no later than the loop's own most +// recent release is the loop's own — the context probe drives its model +// through the gateway — and is discounted the way the pool's stamp of that +// release is (touched). Callers hold r.mu. +func (r *Runner) clientRecentLocked(last, now time.Time) bool { + if last.IsZero() || now.Sub(last) >= r.opts.Quiet { + return false + } + for _, own := range r.touched { + if !last.After(own) { + return false + } + } + return true +} + // quiet reports whether the Mac is idle enough to start a run: nothing in // flight, nothing waiting, nothing downloading, and the last request older -// than the quiet period. A model never used counts as long idle, and a last -// use that is the self-test's own release is not a request. +// than the quiet period — the last on any resident model and the last client +// request the gateway counted, wherever it went. A model never used counts as +// long idle, and a last use that is the loop's own is not a request. func (r *Runner) quiet(act Activity, now time.Time) bool { return r.heldBy(act, now) == "" } // hold is what the loop itself has taken from the pool right now: the model, diff --git a/internal/selftest/selftest_test.go b/internal/selftest/selftest_test.go index 1a55aa53..0f170cf8 100644 --- a/internal/selftest/selftest_test.go +++ b/internal/selftest/selftest_test.go @@ -37,7 +37,12 @@ type fakeServer struct { downloading int // foreign is an in-flight request the self-test is not holding, on the // model named, standing in for a client's. - foreign string + foreign string + // requests and lastRequest are the gateway's count of client requests + // (Clients), which no resident model need show: one still uploading its + // body, or one whose model has left the pool since it ended. + requests int + lastRequest time.Time acquired []string unloaded []string concurrency int @@ -164,7 +169,8 @@ func (s *fakeServer) Acquire(ctx context.Context, id string) (Upstream, func(), func (s *fakeServer) Activity() Activity { s.mu.Lock() defer s.mu.Unlock() - act := Activity{Waiting: s.waiting, Downloading: s.downloading, Refusals: s.refusals} + act := Activity{Waiting: s.waiting, Downloading: s.downloading, Refusals: s.refusals, + Requests: s.requests, LastRequest: s.lastRequest} for id := range s.resident { n := s.inFlight[id] if s.foreign == id { @@ -577,6 +583,13 @@ func TestNothingRunsWhileTheMacIsBusy(t *testing.T) { {"a load waiting", func(s *fakeServer) { s.waiting = 1 }}, {"a download running", func(s *fakeServer) { s.downloading = 1 }}, {"a recent request", func(s *fakeServer) { s.resident["org/b"] = true; s.lastUsed["org/b"] = time.Now() }}, + // The gateway's count, which no resident model shows: a request in + // flight for longer than the quiet period, as one waiting on a wedged + // model is, and one that ended a moment ago on a model since gone — a + // wedged one unloaded after its client's requests ended unreachable + // (iss-2610041945030758). + {"a long client request no model shows", func(s *fakeServer) { s.requests = 1; s.lastRequest = time.Now().Add(-2 * time.Hour) }}, + {"a client request that ended on a model since gone", func(s *fakeServer) { s.lastRequest = time.Now() }}, } { t.Run(tt.name, func(t *testing.T) { srv := newFakeServer(t, "org/a", "org/b") @@ -955,6 +968,66 @@ func TestTheSelfTestsOwnRunDoesNotCountAsARecentRequest(t *testing.T) { waitFor(t, "both resident models measured despite a one-hour quiet period", func() bool { return len(runsIn(t, r)) == 2 }) } +// gatewayJob stands in for the context probe, the job that drives its model +// through the gateway: every run moves the gateway's client clock, the way +// the probe's own requests do. +type gatewayJob struct { + srv *fakeServer + runs atomic.Int64 +} + +func (j *gatewayJob) Name() string { return "gateway" } +func (j *gatewayJob) Due(_ []string, _ time.Time) string { return "org/a" } +func (j *gatewayJob) Parks() bool { return false } +func (j *gatewayJob) Run(_ *Session, _ string) { + j.srv.mu.Lock() + j.srv.lastRequest = time.Now() + j.srv.mu.Unlock() + j.runs.Add(1) +} + +// A job's own requests through the gateway are not a client's: the clock they +// moved does not keep the next tick from starting, as the pool's stamp of the +// self-test's own release does not. +func TestAJobsOwnGatewayRequestsDoNotCountAsARecentRequest(t *testing.T) { + srv := newFakeServer(t, "org/a") + job := &gatewayJob{srv: srv} + r := New(Options{ + Server: srv, Path: filepath.Join(t.TempDir(), FileName), + Tick: 5 * time.Millisecond, Poll: 2 * time.Millisecond, Quiet: time.Hour, + Jobs: []Job{job}, SelfTest: func() bool { return false }, + }) + t.Cleanup(r.Close) + r.SetEnabled(true) + waitFor(t, "the job to run again despite a one-hour quiet period", func() bool { return job.runs.Load() >= 2 }) +} + +// A client request after the loop's own run is a client's: the clock moving +// past the run's end holds the next tick. +func TestAClientRequestAfterARunHoldsTheNextTick(t *testing.T) { + srv := newFakeServer(t, "org/a") + job := &gatewayJob{srv: srv} + r := New(Options{ + Server: srv, Path: filepath.Join(t.TempDir(), FileName), + Tick: 5 * time.Millisecond, Poll: 2 * time.Millisecond, Quiet: time.Hour, + Jobs: []Job{job}, SelfTest: func() bool { return false }, + }) + t.Cleanup(r.Close) + r.SetEnabled(true) + waitFor(t, "the job's first run", func() bool { return job.runs.Load() >= 1 }) + r.SetEnabled(false) + time.Sleep(2 * time.Millisecond) + srv.mu.Lock() + srv.lastRequest = time.Now() + srv.mu.Unlock() + before := job.runs.Load() + r.SetEnabled(true) + waitFor(t, "the next tick to be held by the client's request", func() bool { return r.Status().HeldBy == HeldByRecent }) + if got := job.runs.Load(); got != before { + t.Errorf("the job ran %d more times after a client's request", got-before) + } +} + // With eviction grace off, a client whose load finds no room is refused at // once and waits nowhere the self-test can see; the pool's refusal count is // the sign it was there, and it ends the run so the client's retry finds the diff --git a/internal/ui/static/app.js b/internal/ui/static/app.js index c6841f39..3cbdc811 100644 --- a/internal/ui/static/app.js +++ b/internal/ui/static/app.js @@ -570,6 +570,7 @@ function updateText(m) { case 'refused': return 'The last update failed: the new version did not pass the checks Dessau makes before it starts a model. This version keeps serving.'; case 'busy': return 'The last update failed: this model was still answering requests when the wait ran out, so it kept serving. Try again when it is quiet.'; case 'not_offered': return 'The last update failed: the newer version is not one Dessau runs. This version keeps serving.'; + case 'load': return 'The last update failed: the newer version did not load, so Dessau put this version back. This version keeps serving.'; default: return ''; } })(); @@ -581,6 +582,7 @@ function updateText(m) { case 'available': return `A newer version is available (${short}).`; case 'runs_own_code': return `A newer version (${short}) ships its own code, which Dessau will not run, so it is not offered.`; case 'awaiting_review': return `A newer version (${short}) exists and will be offered once a Dessau release has reviewed it.`; + case 'cannot_check': return `A newer version (${short}) exists, but HuggingFace did not hand over its configuration in a form Dessau could verify, so Dessau could not tell whether it ships its own code and it is not offered.`; default: return ''; } })(); @@ -594,6 +596,7 @@ function updateMark(m) { case 'available': return 'newer version'; case 'runs_own_code': return 'newer version not run'; case 'awaiting_review': return 'newer version awaiting review'; + case 'cannot_check': return 'newer version not checked'; default: return ''; } } diff --git a/internal/ui/update_test.go b/internal/ui/update_test.go index 9cc637c5..e50b5e5d 100644 --- a/internal/ui/update_test.go +++ b/internal/ui/update_test.go @@ -24,6 +24,8 @@ func TestTheCardSaysWhatACheckFound(t *testing.T) { `ships its own code, which Dessau will not run`, `newer version not run`, false}, {`{"state":"ready",` + c + `,"update":{"status":"awaiting_review","commit":"2222222222222222222222222222222222222222"}}`, `will be offered once a Dessau release has reviewed it`, `newer version awaiting review`, false}, + {`{"state":"ready",` + c + `,"update":{"status":"cannot_check","commit":"2222222222222222222222222222222222222222"}}`, + `could not tell whether it ships its own code`, `newer version not checked`, false}, {`{"state":"ready"}`, `Version unknown`, ``, true}, {`{"state":"ready",` + c + `,"updating":42.7,"update":{"status":"available","commit":"2222222222222222222222222222222222222222"}}`, `Updating to the newer version: 42%`, ``, false}, @@ -81,6 +83,7 @@ func TestTheCardSaysWhyTheLastUpdateFailed(t *testing.T) { {`{"state":"ready",` + c + `,"update_failed":"refused"}`, `did not pass the checks`, ``}, {`{"state":"ready",` + c + `,"update_failed":"busy"}`, `still answering requests`, ``}, {`{"state":"ready",` + c + `,"update_failed":"not_offered"}`, `not one Dessau runs`, ``}, + {`{"state":"ready",` + c + `,"update_failed":"load"}`, `did not load, so Dessau put this version back`, ``}, {`{"state":"ready",` + c + `,` + avail + `,"update_failed":"download"}`, `The last update failed`, `A newer version is available (222222222222).`}, {`{"state":"ready","update_failed":"download"}`, `The last update failed`, `Version unknown`}, }