Skip to content

fix(server): idempotent /models/unload for already-stopped models#118

Merged
marksverdhei merged 1 commit into
htfrom
fix/idempotent-unload
Jun 15, 2026
Merged

fix(server): idempotent /models/unload for already-stopped models#118
marksverdhei merged 1 commit into
htfrom
fix/idempotent-unload

Conversation

@marksverdhei

Copy link
Copy Markdown

Problem

The heierchat webui surfaces "Failed to unload model: " when a user clicks unload. Root cause traced on centurion-llm (--models-max 2 + LRU): the router evicts/idle-exits model instances on its own, so a client acting off a stale /v1/models snapshot POSTs /models/unload for a model the router has already stopped. That hits the !is_running() branch and returns 400 {"message":"model is not running"}, which the UI prints verbatim.

Reproduced live: POST /models/unload {"model":"qwen3.6-35b-a3b"}400 in 0.3ms while /v1/models shows it unloaded. The happy path (unload a loaded model) works fine — this is purely the already-stopped race.

Fix

Make /models/unload idempotent: when get_meta() finds the model but it is not running, the caller's target state (stopped) is already met — return 200 {success:true, already_unloaded:true} instead of a 400. 400 is now reserved for a genuinely unknown model (no meta).

server-models.cpp (post_router_models_unload), one branch changed. is_running() = LOADED|LOADING|SLEEPING.

Test

Adds test_router_unload_already_unloaded_is_idempotent covering: real unload → 200; second unload of the now-stopped model → 200 idempotent; unknown model → still 400.

Coordination

Complements heierchat's webui-side defense-in-depth (treat a 400 "model is not running" as success + refresh /v1/models on click) so the UX is fixed on un-upgraded backends too. This server fix is the authoritative one — fixes it for every client. snoop-kube is building + rolling titan-llm/centurion-llm on the new image; deploy verification to follow in a comment.

The router evicts model instances on its own (LRU when models-max is
reached, idle exit, or crash). A client that POSTs /models/unload off a
stale /v1/models snapshot would hit the !is_running() branch and get a
400 "model is not running", which the heierchat webui surfaced verbatim
as "Failed to unload model: <id>".

Treat unload of a known-but-stopped model as an idempotent no-op success
(200, {success:true, already_unloaded:true}) since the caller's target
state is already met. 400 is now reserved for a genuinely unknown model
(no meta). Add a regression test covering the already-unloaded and
unknown-model cases.
@marksverdhei
marksverdhei merged commit 4e329db into ht Jun 15, 2026
8 of 9 checks passed
@marksverdhei
marksverdhei deleted the fix/idempotent-unload branch June 15, 2026 13:21
marksverdhai pushed a commit that referenced this pull request Jun 17, 2026
…ing, and -fa off (ggml-org#24503)

* Add interface is_model_splitted() to check the c-graph is splited or not

* Infer and propagate dynamic-dimension indices for all tensors in the GGML graph in api compute_model_outputs()

* Only do this for fallback sub graph

* Move dynamic dims compute in graph missmatch

* ggml-openvino: fix tensor data handling for PERMUTE/VIEW ops in split models

* ggml-openvino:add comments

* ggml-openvino: override VIEW op_case to 0 for split model inputs

* openvino backend: Handle unsupported VIEW shape-mismatch in OpenVINO backend

* Enable additional mul_mat tests and add tensor data saving function (#81)

* ggml-openvino: fix CONT/TRANSPOSE mapping and improve dynamic-dimension handling

* OpenVINO: add NORM/TANH support and rework SOFT_MAX translation

* ggml-openvino: extend VIEW handling

* Enable -fa off (#118)

* Enable --context-shift

* Fix llm param compute error for normal softmax not the softmax in attention

* OpenVINO backend: fix error for attention size compute in llm param

* use tensor->extra in infer_request i/o

* OpenVINO backend: refacter the compute_llm_params() func add get_attention_pattern_case to easy extand

* OpenVINO backend: clean unused code

* 1to1 match op update (#146)

* added translate_1to1_match_1_input function and updated gelu and tanh translations

* Remove unused translation function calls

---------

Co-authored-by: Mustafa Cavus <mustafacavus@intel.com>

* initial gemma4 support

* removed hardcoded names for kv cache slicing

* OpenVINO backend: Add new attention pattern for llm parameters compute

* flash attn Q shape static conversion

* Remove slice in permute translation when n_seq is 1

* return optional in extract_layer_from_name

* OpenVINO backend: refactor VIEW related operation (#148)

* OpenVINO backend: refactor VIEW related operation

* Enable VIEW handling in following ops

* OpenVINO backend does not support GGML_OP_NORM & GGML_OP_L2_NORM with VIEW input accuracy issue from OpenVINO

* OpenVINO backend: Add ops l2_norm & pad

* OpenVINO backend does not support CPY with non-contiguous data or mismatched types

* add op SSM_CONV GATED_DELTA_NET

* OpenVINO backend: fix error for bf16 in OV gpu plugin

* reverted static Q input shape for attention layer

* OpenVINO backend: remove hardcode name inp_tokens, which ignore some leaf case

* Disable remote tensor due to bug in ov gpu

* Disable n_token > 1 GATED_DELTA_NET on gpu

* OpenVINO backend: fix the view op dynamic handling issue in gemma4 & enable view + get_row

* OpenVINO backend: clean code

* OpenVINO backend: enable view + norm/rms_norm

* OpenVINO backend: concat op

* OpenVINO backend: argsort op

* OpenVINO backend: enable unary + view & GGML_UNARY_OP_SOFTPLUS

* Fix issue for test-backend-ops in TOPK_MOE, which compare VIEW ops result, VIEW node in OpenVINO no need compare, the whole graph result is correct

* OpenVINO backend: enable sum_rows

* OpenVINO backend: enable clamp

* OpenVINO backend: enable DIV

* OpenVINO backend: enable GGML_OP_MUL_MAT_ID

* OpenVINO backend: disable MUL_MAT_ID_FUSION case with large mem needed

* OpenVINO backend: Disable GGML_OP_ARGSORT, cause test_backend-ops failed

* OpenVINO backend: fix issue in mul_mat_id

* OpenVINO backend: Disable DIV with broadcast on GPU

* OpenVINO backend: update DIV

* use ov internal op GatedDeltaNet

* OpenVINO backend: enable llama erch test qwen3next

* OpenVINO backend: enable RMS_NORM + VIEW & remove op_case 2 for rope

* OpenVINO backend: fix error

* suggested changes, need review

* suggested changes, need review

* OpenVINO backend: clean unused code & fix build warning

* OpenVINO backend: enable minicpm3 for arch test

* Disable GDN op (ggml-org#177)

* disable gated_delta_net

* update stateful_kv_size correctly in mismatch case

* OpenVINO backend: enable arch test for qwen3vl

* OpenVINO backend: enable cohere2 for arch test

* OpenVINO backend: enable t5 for arch test

* OpenVINO backend: enable jamba for arch test

* OpenVINO backend: remove warning for tmp

* OpenVINO backend: enable kimi-linear for arch test

* Remove unused

* Fix gpt-oss accuracy issue

* OpenVINO backend: enable arctic for arch test

* OpenVINO backend: enable grok for arch test

* Gemma4 initial npu support (ggml-org#179)

* Initiall gemma4 npu support

* temp. fix for gemma4 accuracy bug on npu

* Remove hardcoded names for npu-fold handling

* revert static n tokens for cont translation as it is not needed

* removed unused variable

* ggml-openvino: add GGML_OPENVINO_ENABLE_CACHE env var to control decoder cache. Add environment variable GGML_OPENVINO_ENABLE_CACHE (default: YES). When set to NO, the decoder_cache is bypassed and models are rebuilt from the cgraph on every inference call in both dynamic and static compute paths. This is useful for debugging and verifying correctness without caching interference.

* Revert "Gemma4 initial npu support (ggml-org#179)"

This reverts commit 0d29a9c.

* OpenVINO backend: disable debug log print

* Update TBB discovery. Delegated to OpenVINOs own config.

* OpenVINO backend: GGML_OPENVINO_ENABLE_CACHE YES -> 1

* OpenVINO backend: fallback FLASH_ATTN_EXT in gemma3n to CPU backend

* Add raw ov infer profiling metric

* Add OV raw infer time metric to static compute path

Co-authored-by: virajwad <84867530+virajwad@users.noreply.github.com>

* Modify precision of static profiling

* update to OV 2026.2, add OV windows CI

* fix editorconfig-checks

* Initiall gemma4 npu support

* temp. fix for gemma4 accuracy bug on npu

* Remove hardcoded names for npu-fold handling

* revert static n tokens for cont translation as it is not needed

* removed unused variable

* test-llama-archs fix

* Fix gemma4 flash_attn fallback

* support im2col

* fix code style

* disable add_rope_sin_cos optimization

* stateless boradcast and rope optimizations

* Enable manual gqa attn by default for stateless gpu

* manual gqa: fixed static batch

* gemma4 llama-bench ctx update fix

* Update OV win CI

* stateful rope fusion temp. fix

* OpenVINO backend: Conslolidate supported ops

* Exclude unsupported GGML_OP_SUB cases

* Exclude unsupported TOPK_MOE cases

* OpenVINO Backend: MUL_MAT enhancements

* Update OV CI

* support f16 mask input for npu

* Make GGML_OPENVINO_* env vars usage uniform

Standardize all GGML_OPENVINO_* env flags:
positive integers >0 to enable. Unset, empty, =0, or non-numeric values to disable.
This fixes cases where text values or empty strings enabled features.

* OpenVINO backend: Enhance envvar handling

* more cleanup

* move ggml_openvino_env_flag to appropriate place

* OpenVINO backend: add REPEAT translator, Q5_1 weights, and GLU view-input fix

* ggml-openvino: fix -Werror=cast-qual in extract_q5_1_data

* Update openvino.Dockerfile

Use BuildKit cache mounts for faster Docker rebuilds.
Use apt instead of dpkg, remove unused .ddeb downloads, add DLLAMA_BUILD_TESTS=OFF.

* ggml-openvino: centralize env var access via *getenv_str/getenv_int helpers

Replace getenv and legacy flags with _str and _int helpers.Minor cleanup, doc updates.

* OpenVINO backend: Enable GGML_OP_ADD_ID

* Uptade openvino backend clamg-format

* clang-format

* Update OPENVINO.md (ggml-org#211)

* OpenVINO backend: fix accuracy issue for op CONCAT with i64 precision

* Remove strict concurrency for gpu-openvino-low-perf

* Update openvino CI keynames; add ccache-clear

* Apply suggestions from code review

Co-authored-by: Sigbjørn Skjæret <1629204+CISC@users.noreply.github.com>

* Fix formatting

---------

Co-authored-by: Xuejun Zhai <Xuejun.Zhai@intel.com>
Co-authored-by: Mustafa Cavus <mustafa.cavus@intel.com>
Co-authored-by: Mustafa Cavus <mustafacavus@intel.com>
Co-authored-by: Xuejun <XuejunZhai@intel.com>
Co-authored-by: Wang Yang <yang4.wang@intel.com>
Co-authored-by: Ravi Panchumarthy <ravi.panchumarthy@intel.com>
Co-authored-by: virajwad <84867530+virajwad@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Mostafa Faheem <mostafaaafaheem@gmail.com>
Co-authored-by: Sigbjørn Skjæret <1629204+CISC@users.noreply.github.com>
marksverdhei added a commit that referenced this pull request Jun 18, 2026
#118)

The router evicts model instances on its own (LRU when models-max is
reached, idle exit, or crash). A client that POSTs /models/unload off a
stale /v1/models snapshot would hit the !is_running() branch and get a
400 "model is not running", which the heierchat webui surfaced verbatim
as "Failed to unload model: <id>".

Treat unload of a known-but-stopped model as an idempotent no-op success
(200, {success:true, already_unloaded:true}) since the caller's target
state is already met. 400 is now reserved for a genuinely unknown model
(no meta). Add a regression test covering the already-unloaded and
unknown-model cases.

Co-authored-by: marksverdhei <mark.sverdhei@gmail.com>
marksverdhei added a commit that referenced this pull request Jun 21, 2026
#118)

The router evicts model instances on its own (LRU when models-max is
reached, idle exit, or crash). A client that POSTs /models/unload off a
stale /v1/models snapshot would hit the !is_running() branch and get a
400 "model is not running", which the heierchat webui surfaced verbatim
as "Failed to unload model: <id>".

Treat unload of a known-but-stopped model as an idempotent no-op success
(200, {success:true, already_unloaded:true}) since the caller's target
state is already met. 400 is now reserved for a genuinely unknown model
(no meta). Add a regression test covering the already-unloaded and
unknown-model cases.

Co-authored-by: marksverdhei <mark.sverdhei@gmail.com>
marksverdhei added a commit that referenced this pull request Jun 27, 2026
#118)

The router evicts model instances on its own (LRU when models-max is
reached, idle exit, or crash). A client that POSTs /models/unload off a
stale /v1/models snapshot would hit the !is_running() branch and get a
400 "model is not running", which the heierchat webui surfaced verbatim
as "Failed to unload model: <id>".

Treat unload of a known-but-stopped model as an idempotent no-op success
(200, {success:true, already_unloaded:true}) since the caller's target
state is already met. 400 is now reserved for a genuinely unknown model
(no meta). Add a regression test covering the already-unloaded and
unknown-model cases.

Co-authored-by: marksverdhei <mark.sverdhei@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant