Skip to content

fix(http): make /health answer in one probe and admit when llama is down - #140

Merged
sosidudku1 merged 1 commit into
mainfrom
valeryb/health-fast-honest
Aug 18, 2026
Merged

fix(http): make /health answer in one probe and admit when llama is down#140
sosidudku1 merged 1 commit into
mainfrom
valeryb/health-fast-honest

Conversation

@plombeer31

Copy link
Copy Markdown
Collaborator

What

GET /health blocked for 15.5 seconds when llama-server was down, then answered
HTTP 200 with status: "ok" — its own body admitting reachable: false in the same
breath. Measured three consecutive calls before the fix: 15.55 / 15.55 / 15.56 s.

For the documented liveness probe that fails in both directions at once: too slow to
survive a typical 1–10 s orchestrator timeout (so the sidecar restart-loops exactly when
llama is down), and untruthful when it does answer.

Fix

  • The route probes llama once (checkLlamaServer({ retries: 0 })). The 15.5 s came
    from inheriting the full retry ladder (5 attempts, exponential backoff); a liveness
    endpoint has no such budget, and the orchestrator's next poll is the retry anyway.
  • The body says the truth: status: "ok" only when llama answers, status: "degraded"
    when it does not. HTTP stays 200 by default — the existing comment is right that the
    sidecar being alive is a separate fact from llama being up, and nothing that polls for
    200 today breaks.
  • New: GET /health?strict=1 returns 503 in the degraded case, for orchestrators
    that do want restart-on-down semantics. Opt-in, so default behaviour is unchanged.

Testing

New route-health.test.ts (the route had none), on the real HTTP harness with
localModels.url pointed at a just-released port / a stub llama:

  • degraded: 200, status: "degraded", reachable: false, and elapsed < 5 s
    against the unpatched route these tests literally hit vitest's 15 s timeout, which is
    the bug demonstrating itself
  • strict: 503 + degraded when down; 200 + ok when up
  • healthy: ok with and without strict

test-harness.ts gained an optional localModelsUrl so llama-probe tests are
deterministic instead of depending on whatever occupies 127.0.0.1:8080 on the test
machine. Full src/http suite: 49/49. npm run lint clean.

@sosidudku1
sosidudku1 merged commit 3029870 into main Aug 18, 2026
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.

2 participants