Repository navigation
feat(core): add model loading state machine to prevent routing to loading models - #5032
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a model readiness state tracking mechanism to prevent routing requests to models that are still loading or registering. It defines a new ModelNotReadyError exception, tracks model states (such as registering, loading, ready, error, stopping, and stopped) across ModelActor, Worker, and Supervisor, and raises 503 HTTP exceptions with a retry header when models are not ready. The review feedback highlights several key improvements: enhancing the _require_ready guard in ModelActor to defensively reject any state other than ready and applying it to all inference methods; correcting the decorator on the asynchronous _update_model_state method in worker.py from @log_sync to @log_async; extending the state guard in the worker's get_model method to reject requests when the model is in error, stopping, or stopped states; and ensuring that model loading failures trigger status synchronization via _update_model_state instead of directly modifying the state attribute.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…ding models Introduce an explicit state machine (registering → loading → ready → stopping → stopped) for ModelActor lifecycle, replacing implicit state inference from dict membership. This fixes the "list_models shows model but chat returns 500 AssertionError" race condition where requests are routed to models whose engines are still loading in background threads. Key changes: - New ModelNotReadyError exception for loading-state rejection - LaunchStatus.LOADING enum value and ReplicaStatus.model_state field - ModelActor._model_state with _require_ready() guard on inference methods (generate, chat, create_embedding, rerank) - WorkerActor._update_model_state() helper syncing to StatusGuard - Supervisor: swap wait_for_load before route table insertion in _launch_one_model (both normal and xavier rank0 paths) - API layer: ModelNotReadyError → HTTP 503 with Retry-After header (not written to negative cache since loading is transient) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- _require_ready: defensive reject for any non-ready state (was silently passing stopping/stopped), and add guard to all 13 inference methods that were missing it (transcriptions, translations, speech, text_to_image, txt2img, image_to_image, img2img, inpainting, ocr, infer, text_to_video, image_to_video, flf_to_video) - Fix @log_sync on async _update_model_state — log_sync wrapper doesn't await, so the coroutine was never executed and timing log was meaningless - get_model: reject error/stopping/stopped states instead of returning a dead/dying model ref - Loading failure path: use _update_model_state() instead of direct attribute assignment to sync error state to StatusGuard Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Run black on xinference/core/model.py - Run isort on xinference/core/worker.py Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
c51932e to
7837381
Compare
The model loading state machine (this PR) initializes ModelActor._model_state to "registering" and gates every inference method behind _require_ready(). State only advances to "ready" via load() / wait_for_load(), which the real launch path invokes. test_concurrent_call constructs MockModelActor directly with an already-usable MockModel and never calls load()/wait_for_load(), so the actor stays in "registering" and the first generate() raises ModelNotReadyError. The test only exercises the pending-request queue, not the load lifecycle, so mark the actor ready up front. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…aunching models
When wait_ready=False, supervisor.get_model() raised ValueError
("Model not found") while the model was still launching. This caused
require_model to write the uid into the negative cache as a 404,
poisoning subsequent requests for up to the cache TTL even after
the model became ready.
Fix: when replica_info exists but _replica_model_uid_to_worker has
no entry yet, raise ModelNotReadyError (503, retry-friendly) instead
of ValueError (404, cached as not-found). Applied to both get_model
and describe_model.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
m199369309
left a comment
There was a problem hiding this comment.
wait_ready=False + negative cache race (qinxuye): Fixed.
In supervisor.get_model() and describe_model(), when replica_info exists but _replica_model_uid_to_worker has no entry yet (model is launching with wait_ready=False), now raises ModelNotReadyError (HTTP 503, retry-friendly) instead of ValueError (HTTP 404, cached as not-found).
This prevents the negative cache from being poisoned during the async launch window. Clients get a 503 with Retry-After: 30 header, which is the correct semantic for "model exists but not ready yet".
Other findings (already fixed in prior commits, no changes needed):
- Gemini High ×3:
_require_readyalready checks all non-ready states;_update_model_statealready uses@log_async;get_modelin worker.py already guardserror/stopping/stopped - Gemini Medium: error path already calls
_update_model_state(model_uid, "error")
…pi_options
The supervisor's describe_model() and get_model() now raise
ModelNotReadyError during the wait_ready=False launch window, but the
REST handlers only caught ValueError (->400) and the generic Exception
(->500). As a result, GET /v1/models/{uid} and the SDAPI options
endpoint returned 500 for a transiently loading model, asymmetric with
the inference path which correctly returns 503 + Retry-After via
require_model().
Catch ModelNotReadyError explicitly in:
- REST describe_model handler (GET /v1/models/{model_uid})
- sdapi_options (image model lookup)
- the describe_model call in the chat-completion path
Addressing the last open review finding on PR xorbitsai#5032.
Summary
registering → loading → ready → stopping → stopped(pluserror), replacing implicit state inference from dict membership. This fixes the race condition wherelist_modelsshows a model but chat requests return500 AssertionError: self._engine is Nonebecause the engine is still loading in a background thread.ModelNotReadyErrorexception (xinference/core/exceptions.py) raised byModelActor._require_ready()guard on all inference methods (generate, chat, create_embedding, rerank) when the model is not yet ready.LaunchStatus.LOADINGenum value andReplicaStatus.model_statefield for fine-grained state tracking visible to supervisors and API consumers._launch_one_modelsowait_for_loadcompletes before the model is added to the route table (both normal and xavier rank0 paths). Previously, the route table was written beforewait_for_load, allowing requests to hit a still-loading model.Retry-After: 30header for loading-state requests in the API layer (require_model). Loading is transient soModelNotReadyErroris not written to negative cache.WorkerActor._update_model_state()helper syncs state transitions to StatusGuard, with auto-creation ofModelStatusentries to support registering/loading states (previously only created afterload()completed).Test plan
pytest xinference/core/tests/ -v --timeout=300passesmodel_statetransitions through registering → loading → readyGET /v1/modelsincludesmodel_statefieldreadyreplicas are routed toDesign notes
WorkerActor.ModelStatus.model_stateis the authoritative source (routing decisions).ModelActor._model_stateis a safety net for direct calls bypassing the worker.model_statedefaults to empty string for existing data, treated asready(current behavior).wait_ready=Falseasync launch: model is visible inlist_models(withmodel_state=loading) but not routable until ready. Clients can pollmodel_stateto know when it's available.🤖 Generated with Claude Code