Skip to content

Router: reload doesn't reconcile alias_to_name → get_meta mis-resolves removed/moved aliases (stale cache) #150

Description

@marksverdhei

Summary

After a router reload (POST /models/reload, or a preset/source change that triggers load_models()) that changes a model's aliases, get_meta() resolves the affected alias incorrectly:

  • Removed alias → still resolves to the old model (should be not-found).
  • Moved alias (from model foo to model baz) → resolves to foo (the old owner), mis-routing the request to the wrong model.

has_model() is correct in both cases, so the two disagree after a reload — a code path that gates on has_model and one that resolves via get_meta will diverge.

Root cause

load_models() reload rebuilds each model's inst.meta.aliases but never reconciles the global alias_to_name map:

  • The cleanup at tools/server/server-models.cpp:595 only erases alias_to_name entries whose target model is gone (mapping.find(it->second) == end). It does not touch entries whose target still exists but no longer owns the alias.
  • The re-parse loop at :604-635 does inst.meta.aliases.clear() + re-inserts from the new preset, but writes nothing to alias_to_name.

So an alias removed-from or moved-between still-existing models leaves a stale alias_to_name[oldalias] -> oldname.

Why it mis-resolves (resolution ordering)

get_meta() (:760) resolves in this order:

  1. direct name in mapping
  2. alias_to_name.find(name) (:766) → returns that target's meta
  3. fallback scan of every model's meta.aliases (:774)

Step 2 short-circuits on the stale entry before the authoritative step-3 scan runs. By contrast has_model() skips alias_to_name entirely and uses only the meta.aliases scan — which is why it stays correct and the two diverge.

Why not just drop alias_to_name

Per #135 (ecf2de428, fixing #122) the map exists specifically to resolve aliases when a loaded instance's meta.aliases are shadowed/lost — i.e. the meta.aliases scan is not always sufficient. So the fix is to keep alias_to_name in sync on reload, not remove it.

Repro

  1. Preset INI: [foo] with alias = bar; start router.
  2. Edit preset: remove bar from [foo] (or move alias = bar to a new [baz]).
  3. POST /models/reload.
  4. Resolve the model by bar (e.g. GET /models lookup / a request targeting bar): it still resolves to foo via the stale alias_to_name, instead of not-found (removed) or baz (moved). has_model("bar") disagrees.

Suggested fix

After the re-parse loop finishes rebuilding all meta.aliases, rebuild alias_to_name from scratch (clear + repopulate from each model's final aliases), or update it inside the re-parse loop (drop the model's old alias entries, add the new ones). Keep the existing dead-target cleanup. A regression test can extend test_router_reload_models to move an alias between two preset sections and assert resolution follows.

Severity

Medium — requires an alias-changing reload (a real ops action: editing the preset INI + reload), but then silently routes requests to the wrong model or produces get_meta/has_model inconsistency. Same alias-map-consistency class as #135 / #122.

Found via idle bug-hunt; verified against origin/ht (90ae01b7a). Happy to PR the reconcile fix — holding it until #148 lands to avoid conflicts on server-models.cpp.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions