Repository navigation
feat(metal): check free disk space before a model download and report the model store size - #2548
Open
defilan-agent[bot] wants to merge 3 commits into
Open
defilan-agent[bot] wants to merge 3 commits into
defilan-agent[bot] wants to merge 3 commits into
Conversation
… the model store size Model churn filled the Mac disk with nothing warning about it, and a full disk then failed new starts as a generic StartFailed. A download now needs room for its remaining bytes plus --download-free-space-headroom (default 10Gi), checked before anything is written; a disk that fills mid-copy is the same ModelDownloadNoSpace refusal, and the partial is kept to resume. Two gauges report the store size and its free space every minute. Cleanup of unreferenced store entries is left for its own design.
The README said the free-space check runs before every download, but a body of unknown length is only caught when it fills the disk. handleStartProcessError still documented a full disk as StartFailed, and the new option had split WithMaxDownloadBytes from its own comment. The no-space error now unwraps to the filesystem error, opening the partial on a full disk maps to the same refusal, and the controller refusal table lists the new reason.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ction it had split The new README section was inserted in the middle of the --max-download-size section, so that section's plist snippet, startup checks and remembered-refusal note read as describing the headroom flag. The guide no longer lists a full disk among failures that are hidden and only retried on a change; it names StartFailed and the ModelDownloadNoSpace refusal. A store the agent cannot read keeps the used-bytes gauge at its last value, and the refusal says less than 1 GiB rather than about 0 GiB.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The metal agent now checks for disk space before a model download and reports the model store's size. This covers two of the three things #2409 asks for:
--download-free-space-headroom(new flag, default10Gi). If the model store's filesystem has less free than that, the start is refused with the new reasonModelDownloadNoSpacebefore a byte is written. A body of unknown length is not checked up front.ENOSPC) is the same refusal, and the partial is kept so the download resumes.StartFailedon the restart backoff.llmkube_metal_agent_model_store_used_bytes(regular files under the store; a symlink is neither followed nor counted) andllmkube_metal_agent_model_store_free_bytes(free space on its filesystem), refreshed every minute.Not in this PR: the opt-in cleanup of store entries no Model references, the issue's third item. Deciding what is unreferenced is not mechanical:
<store>/<model name>/<file>with no namespace;A wrong answer deletes model weights, so it deserves its own design. Hence
Refs, notFixes.Why
Model churn fills the Mac's disk with nothing warning about it, and a full disk then fails new starts silently (#2409).
--max-download-sizebounds one download, not the store.Refs #2409
How
pkg/agent/executor.go.copyToFileResume, the one copy every agent download goes through (http,https,hf,s3), runscheckFreeSpaceforcontentLength - resumeFrombefore it opens the partial. It does this only when the length is known.errors.Issyscall.ENOSPCbecomesModelDownloadNoSpaceError{DuringCopy: true}.WithDownloadFreeSpaceHeadroomsets the headroom.pkg/agent/diskspace_unix.go.unix.Statfs,f_bavail × f_bsize: the space an unprivileged writer can still use.diskspace_other.goreports it as unknown off Unix.pkg/agent/refusal.go.handleStartProcessErrorrefuses aModelDownloadNoSpaceErrorwithEventReasonModelDownloadNoSpace.ModelDownloadTooLarge. Freeing disk writes nothing to the cluster, so the refusal re-check (publishRefusal's requeue) is what retries it.publishRefusalrewrites status and emits an Event whenever the message changes, so exact byte counts would churn on every re-check.api/v1alpha1/constants.go.ReasonModelDownloadNoSpaceis added toMetalAgentRefusalReasons, so the controller keeps it instead of overwriting it withWaitingForMetalAgent. The repo'srefusal_reasons_scan_testcaught the missing entry in its own table, which I added.pkg/agent/modelstore_metrics.go. The gauges andwatchModelStore, started fromStartwhen--model-storeis set.cmd/metal-agent/main.go. The flag, parsed byparseByteQuantitylike--max-download-size, passed throughMetalAgentConfig.DownloadFreeSpaceHeadroom. Both download flags now resolve inresolveDownloadLimits, which keepsmainunder the linter's complexity limit; each error still names its flag.pkg/cli/metal_join_args.go. The flag is added tollmkube metal join's table of agent flags, so--agent-arg download-free-space-headroom=20Giis accepted.TestMetalAgentFlagsMatchTheAgentcaught it missing on the first full run.deployment/macos/README.md: a "--download-free-space-headroomflag (disk preflight)" section, the refusal in "What the agent checks before an engine starts" and in the Events list, aModelDownloadNoSpacetroubleshooting entry, and the two gauges in the metrics table.docs/site/guides/macos-metal.md: the troubleshooting entry.docs/site/reference/metrics.md: regenerated.Tests (
pkg/agent/diskspace_test.go):copyToFileResume error = <nil>, want a ModelDownloadNoSpaceErrorerror = write model.gguf.partial: no space left on device, want a ModelDownloadNoSpaceError during the copyschedulingStatus = "StartFailed", want ModelDownloadNoSpaceENOSPC;used bytes = 3270, want 3150;executor headroom = 0.cmd/metal-agent:TestParseByteQuantity, andTestResolveDownloadLimits(each error names its flag).make gate. Passed every step on the final tree: regen, drift, fmt, copyright, vet, lint-all and the full test suite. The first run failed two steps, and both are fixed:main's complexity and two long lines;TestMetalAgentFlagsMatchTheAgent, which caught thejointable entry I had missed.Review round 1 (qwen38-flash-next)
All three must-fixes were correct, fixed in f8093e7:
WithMaxDownloadBytesfrom its doc comment. godoc gave its "zero leaves downloads uncapped" to the headroom option. Restored, and the headroom comment now says zero still refuses a download that does not fit.handleStartProcessError's comment still listed a full disk under StartFailed. Rewritten to name the new refusal and why it is not memoized.Nits:
errors.Is(err, syscall.ENOSPC)holds. The disk-full test checks it, and returningnilfromUnwrapfails it.ENOSPCwhile opening the partial maps to the same refusal.ensureModelwraps it.ModelDownloadNoSpaceentry.--model-store. That does not happen. An empty--model-storeresolves to the default store under the agent's data directory (resolveModelStorePath), and the agent exits if it cannot prepare one, so a running agent always has a store to measure.ENOSPCis not mapped (a stamp is tens of bytes, written after the model).Open questions for a maintainer:
10Gidefault. On a Mac with less free space than the headroom, downloads that succeed today are refused. That is the point, but it is a visible change; lower it if you prefer.Review round 2 (qwen38-flash-next)
Both must-fixes were correct; fixed in 1150805.
--max-download-size. Three of that section's paragraphs (the plist snippet, the startup checks, and "the refusal is remembered") read as describing the headroom flag. The memo claim in particular is false forModelDownloadNoSpace. The new section now follows the whole cap section.StartFailedand retries on the restart backoff, fromReasonStartFailed's own documentation, and that a full store is refused asModelDownloadNoSpaceand retried once space is freed.Nits:
TestReportModelStore_KeepsTheLastValueWhenTheStoreIsUnreadablebites without it:used bytes = 0 ..., want the last value 700.enospcReaderinjects throughRead. ProductionENOSPCcomes from write or close. All of them go through the samenoSpacemapping, which the test pins.Open questions:
statfscounts inf_bsizeunits, which is what this uses. On Linux,statvfscounts inf_frsizeunits, which usually but not always equalsf_bsize. The Linux agent is experimental ([Raspberry Pi] Run the metal-agent on Linux hosts (experimental) #2520), so I left Linux as is rather than change it without checking it on a target filesystem; worth a follow-up there.10Gidefault. Still yours to call.Checklist
make testpasses locally (make gate, full suite)make lintpasses locally (make lint-allin the gate)git commit -s) per DCO (a maintainer signs off with signoff-pr before approving)Assisted-by: Claude Code (Claude Opus 5.5) (traced the agent's download paths to the shared copy, the refusal re-check and the refusal-reason list, wrote the tests first and watched them fail, implemented the preflight, the ENOSPC mapping, the refusal, the gauges and the flag and the docs, mutation-checked each test, ran
make gate; scoped the store cleanup out for its own design; verified and fixed the review findings)Assisted-by: qwen38-flash-next (unsloth/Qwen3.8-Flash-Next-GGUF UD-Q4_K_XL, generic (llmkube-gufo-rocm-gfx1151) runtime, self-hosted with LLMKube on AMD Strix Halo (Ryzen AI Max, Radeon iGPU); roles: read-only review, two rounds. Round 1's three must-fixes (doc comment placement, the README's overclaim, the stale routing comment) were correct and are fixed in f8093e7 with four of its nits; round 2's two docs must-fixes (the README section placement, the guide's stale full-disk line) were correct and are fixed in 1150805 with two nits)