fix(http): make /health answer in one probe and admit when llama is down - #140
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
GET /healthblocked for 15.5 seconds when llama-server was down, then answeredHTTP 200 with
status: "ok"— its own body admittingreachable: falsein the samebreath. 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
checkLlamaServer({ retries: 0 })). The 15.5 s camefrom 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.
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.
GET /health?strict=1returns 503 in the degraded case, for orchestratorsthat 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 withlocalModels.urlpointed at a just-released port / a stub llama: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
degradedwhen down; 200 +okwhen upokwith and withoutstricttest-harness.tsgained an optionallocalModelsUrlso llama-probe tests aredeterministic instead of depending on whatever occupies 127.0.0.1:8080 on the test
machine. Full
src/httpsuite: 49/49.npm run lintclean.