Keep a serving model when a runtime reload fails - #158
Open
craig-b wants to merge 1 commit into
Open
Conversation
ModelLifecycleService.LoadModel disposed the current model before attempting the new load, so any failure — a deleted or truncated GGUF, a backend init error, an out-of-memory during load — left the server with no model at all: every subsequent request failed until the process was restarted. The dispose-before-load ordering itself is required (two models generally don't fit in memory at once), so the fix keeps it and adds two guards around it. Pre-flight: before the current model is touched, the target GGUF (and the projector, when one exists at the given path) is opened header-only and checked with ThrowIfTruncated. A missing, non-GGUF, or truncated file — the realistic failure modes for the pinned hosted-model path, e.g. an interrupted re-download — is rejected while the old model keeps serving, with the reader's diagnostic instead of a mid-load short-read error. Rollback: if the load fails past pre-flight, the previous model/projector/backend triple is reloaded best-effort. The original exception still reaches the caller; the rollback outcome is logged. A load failure now also disposes any partially initialized model, so the service always holds either a fully loaded model or none. ModelBase.Create is injected through an internal constructor so the tests can script load outcomes; the public constructor is unchanged. Verified: ModelReloadRollbackTests (8, red on the old ordering, green now); live on a ggml_cpu Qwen3.5-0.8B server — truncating the hosted GGUF in place and POSTing /api/models/load returns 500 with the re-download diagnostic while /v1/chat/completions keeps answering, where main is left with no model and every request failing; restoring the file makes reload work again without a restart. Full suite 1388 green.
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
1 participant
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.
ModelLifecycleService.LoadModeldisposes the current model before loading the new one (required — two models generally don't fit in memory), so any load failure — deleted or truncated GGUF, backend init error, OOM — left the server with no model at all: every request failed until a process restart.Two guards around the existing ordering:
ThrowIfTruncated. A missing, non-GGUF, or truncated file — the realistic failure modes, e.g. an interrupted re-download — is rejected while the old model keeps serving, with the reader's diagnostic instead of a mid-load short-read error.ModelReloadRollbackTestscovers pre-flight rejection, rollback success, and rollback failure. Verified locally: environment-independent lane 1149 passed / 0 failed on current main.