feat(server): answer HTTP during model load; split /livez + /readyz probes #44
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/serve-during-model-load"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Serve HTTP from the moment the port binds instead of letting connections hang in the accept backlog during the (minutes-long) model load, and split the probe endpoints k8s-style.
During the load (
server.Loading, swapped out atomically once the engine is ready)GET /v1/internal/status-> 200{"state": "loading", "model_dir": ..., "elapsed_seconds": ...}-- an orchestrator can tell "starting" from "dead"POST /v1/internal/drain-> aborts the load and exits (same secret gating); a drain racing load completion is honored after the swapRetry-AfterThe loading status deliberately omits the
modelkey:glchat.Proberequires one, so loading instances stay out of the chat picker on old and new glchat builds alike. Pinned by tests on both sides and documented in AGENTS.md as a wire contract.Probe split
GET /livez-- liveness: 200 whenever the process answers HTTP (loading, serving, draining)GET /readyz-- readiness: 200 only while traffic should route here; 503 during load and once a drain triggersGET /healthz-- temporary deprecated alias of/readyz; warns once per probing IP (capped at 100 IPs with a final suppression notice) so the log names every prober left to migrateThe serving status payload now reports
"state": "serving"/"draining", giving pollers one lifecycle field from port-open to exit.Verified
go build/vet/test ./.../gofmtclean/healthzprobes from one IP warn once🤖 Generated with Claude Code
Serve on the already-bound listener immediately instead of letting connections hang in the kernel accept backlog for the minutes a model load takes (where a TCP probe looked "up" while every request stalled). A loading-phase handler (server.Loading) answers until the engine is ready, then the real handler is swapped in atomically: - GET /v1/internal/status: 200 with {"state": "loading", "model_dir", "elapsed_seconds", ...} so an orchestrator can tell "starting" from "dead". The payload deliberately omits the "model" key: glchat's Probe requires one, which keeps loading instances out of its picker on old and new builds alike (pinned by tests on both sides). - POST /v1/internal/drain: same secret gating as the serving handler, but aborts the load and exits, so an orchestrator can retire an instance without waiting out a load it intends to throw away. A drain that races load completion is honored after the swap. - Everything else: 503 with a Retry-After hint. The serving status payload gains "state": "serving" -> "draining", so a poller sees one lifecycle field from port-open to exit. Probes now follow the k8s liveness/readiness split: GET /livez is 200 whenever the process answers HTTP (loading, serving, and draining -- restarting an instance for loading or draining would defeat both); GET /readyz is 200 only while traffic should route here (503 during load and once a drain triggers). GET /healthz stays temporarily as a deprecated alias of /readyz; each IP still probing it is warned about once in the log (capped, with a final suppression notice) so the log names every prober left to migrate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Automated review by pr-reviewer v0.36.0 | Safety Check | Claude Sonnet 5 | tracking id
r-604f45-b5f40aThis is an AI-generated review and may contain mistakes.
Status: ✅ Completed
✅ Verdict: LGTM — The flagged "secret" is just standard drain-secret header-comparison code (already existing pattern), and the rest of the diff is a well-tested feature addition with no obvious bugs or breaking changes.
Checked the flagged file (
internal/server/loading.go) for the "possible secret" hit — it's just the drain-secret gating logic (X-Drain-Secretheader comparison viasubtle.ConstantTimeCompare), same pattern already used inserver.go. Test fixtures use an obvious placeholder"s3cr3t". No real credential leaked.Logic review:
main.go(loading handler -> real handler once engine ready) looks race-safe; usesatomic.Pointer[http.Handler].srv.Shutdown/srv.Closeappropriately, avoiding goroutine leaks/hangs.BuildVersionexport rename is a trivial, backward-compatible change (unexported -> exported), no external callers broken since it's a new export.modelkey while loading) is deliberately tested on both sides (glchat/discover_test.goandserver/loading_test.go), consistent with the AGENTS.md documentation update.handleDrainRequestused by both loading and serving paths — reduces duplication without changing behavior.No exposed secrets, no obvious logic bugs, no breaking API removal (old
/healthzkept as deprecated alias rather than removed). This is a substantial but well-tested feature addition.