Skip to content

perf(ollama): defer and bound the chat-model enumeration (#427) - #428

Open
pjdoland wants to merge 1 commit into
plmbr:mainfrom
pjdoland:perf/427-ollama-startup-enumeration
Open

pjdoland wants to merge 1 commit into
plmbr:mainfrom
pjdoland:perf/427-ollama-startup-enumeration

Conversation

@pjdoland

Copy link
Copy Markdown
Collaborator

Summary

#370 took litellm, openai, and anthropic out of the server-extension import path. Ollama is the one provider SDK still loading eagerly: OllamaLLMProvider.__init__ calls update_chat_model_list(), and since AIServiceManager.__init__ builds every provider, each Jupyter server start imports the ollama SDK and calls the Ollama host whatever the configured provider is. A Claude-only user pays an import, a connection attempt, and a Failed to update supported Ollama models warning on every start, which is the tail end of what #368 reported.

This continues that work and adds a bound on the requests, which turned out to matter more than the deferral.

Solution

The chat-model list is now built on first access of the chat_models property, which the capabilities response and the readiness preflight already read, and the update-provider-models route still refreshes it explicitly.

Being precise about what that buys, since the obvious framing overstates it:

  • Server start: saves roughly 37ms and one connection attempt. The frequently quoted ~0.24s is import ollama standalone; the marginal cost once Jupyter has loaded httpx and pydantic is ~35ms.
  • First page load: an interactive user pays that back inside the first /capabilities GET, since chat_model_ids iterates every registered provider. On a healthy host this is close to a wash for that user.
  • Headless servers (papermill, nbclient, CI) and servers whose users never open the NBI frontend: a genuine removal.
  • An Ollama-configured user still enumerates during startup, because update_models_from_config resolves the persisted selection through get_chat_model. That is what that user needs, and it means the startup-latency benefit is specifically for users of other providers.

Deferring on its own would have made the worst case worse, so the enumeration is now bounded:

  • ollama passes timeout=None to httpx, so a host that drops packets rather than refusing them waits out the OS connect timeout, measured at 75s on macOS. GetCapabilitiesHandler.get is a synchronous handler, so unbounded that wait moves off startup and onto the event-loop thread serving every other request in the process, freezing a live server instead of slowing a boot.
  • Each request now carries a 5s bound, deliberately generous rather than tight: /api/show is a metadata read that a loaded host still answers slowly, and a model whose metadata times out drops out of the list entirely.
  • A 6s wall-clock budget covers the enumeration as a whole, because a per-request bound alone scales with the model count (one /api/show apiece). 20 models against a host that answers the listing then stalls measured 40s with only per-request bounds and 10s with the budget, and the warning names how many models were left out.
  • The list is built locally and rebound once, only on success, so a reader on another thread cannot serialize a half-built list (readiness reads this property on a pool thread while a capabilities GET reads it on the event loop), and a failed refresh keeps the models the dropdown already had.

Testing

pytest tests/ --ignore=tests/test_claude_client.py: 1762 passed. tsc --noEmit, eslint, stylelint, prettier clean; jest: 423 passed.

New coverage in tests/test_lazy_provider_imports.py, alongside #370's existing import test. The tests pin ordering rather than a call count, because "one call in total" is also true of the constructor version: not enumerated after construction, enumerated on first access, cached thereafter. The ollama stub implements both the module-level and Client surfaces so enumeration cannot hide on the one it omits. Also covered: the timeout value (a timeout that merely exists could be an hour), both entries of OLLAMA_EMBEDDING_FAMILIES, two chat families so a hardcoded context-window key cannot pass, a per-model metadata failure not costing the rest of the list, budget truncation and its warning, recovery through the explicit refresh, and last-good retention when a refresh fails. Verified by mutation: these fail against the pre-change provider, and the manager-construction test fails against a constructor that enumerates inline.

No Playwright run: the change touches no TypeScript, LabIcon registration, shell-area iteration, or @jupyterlab/* runtime call. The only user-visible surface is the settings dropdown's contents, so I exercised that path in-process instead and confirmed a stubbed two-model host produces the expected chat_model_ids row and that get_chat_model round-trips.

Risks and follow-ups

  • On a host that answers /api/tags and then stalls on every /api/show, the list can come back empty rather than partial (measured: 0 of 20 models, in 10s). That is bounded and logged with a count, and the explicit refresh retries, but it is a real behavior difference from the old unbounded call, which would eventually populate.
  • A failed first enumeration stays empty for the process lifetime until Refresh models. This is parity with the previous behavior, where the constructor also ran once, and the settings panel already offers that affordance in exactly this state.
  • The single atomic rebind closes the torn-read window, but the concurrency property itself is not unit-tested; a deterministic test would need to interleave threads mid-enumeration.
  • Out of scope, noticed nearby: chat_model_ids iterates providers unfiltered, so disabled_providers does not suppress this enumeration for an admin who disabled Ollama; docs/admin-guide.md advertises an ollama.base_url config block that nothing in the tree consumes; and the two new constants have no NBI_* env override, though claude.py has an idiom for that if it is wanted.
  • The completion calls in the same file stay unbounded on purpose: they run on the threaded request path, and capping a streaming generation would break it.

Closes #427

OllamaLLMProvider built its chat-model list in the constructor, and
AIServiceManager builds every provider, so every Jupyter server start
imported the ollama SDK and called the Ollama host whatever the configured
provider was. A Claude-only user paid the import, one connection attempt,
and a "Failed to update supported Ollama models" warning on each start.

The list is now built on first access of the chat_models property, which the
capabilities response and the readiness preflight already read, and the
update-provider-models route still refreshes it explicitly. Be precise about
the saving: the marginal import is around 37ms once Jupyter has loaded httpx
and pydantic, so an interactive user pays it back on the first capabilities
GET. The real wins are headless servers, servers whose users never open the
NBI frontend, and the stalled-host case below. An Ollama-configured user
still enumerates during startup through get_chat_model, which is what that
user needs.

Deferring alone would have made the worst case worse, so the requests are
bounded too. ollama passes timeout=None to httpx, so a host that drops
packets rather than refusing them waits out the OS connect timeout, 75s on
macOS, and the capabilities handler reading this property is synchronous:
left unbounded, that wait moves off startup and onto the event-loop thread
serving every other request in the process. Each request now carries a 5s
bound, deliberately generous because /api/show is a metadata read that a
loaded host still answers slowly and a model whose metadata times out drops
out of the list entirely. A 6s wall-clock budget covers the enumeration as a
whole, because the per-request bound alone scales with the model count: 20
models against a host that answers the listing and then stalls measured 40s
before the budget and 10s after, and the warning now names how many models
were left out.

Building into a local list and rebinding once, only on success, keeps a
reader on another thread from serializing a half-built list, since readiness
reads this property on a pool thread while a capabilities GET reads it on the
event loop, and keeps the models the dropdown already had when a refresh
fails.

The tests pin ordering rather than a call count, because "one call in total"
held for the constructor version too: not enumerated after construction,
enumerated on first access, cached thereafter. The ollama stub implements
both the module-level and Client surfaces so enumeration cannot hide on the
one it omits, and the suite covers the timeout value, both embedding
families, a per-model failure not costing the rest of the list, budget
truncation, recovery through the explicit refresh, and last-good retention
when a refresh fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(startup): Ollama provider enumerates local models during server-extension startup

1 participant