Skip to content

fix(worker): thread-based liveness server so sync Bedrock signing cannot SIGKILL a busy worker#2906

Closed
Fyko wants to merge 1 commit into
vectorize-io:mainfrom
Fyko:fix/worker-liveness-thread-server
Closed

fix(worker): thread-based liveness server so sync Bedrock signing cannot SIGKILL a busy worker#2906
Fyko wants to merge 1 commit into
vectorize-io:mainfrom
Fyko:fix/worker-liveness-thread-server

Conversation

@Fyko

@Fyko Fyko commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Problem

The dedicated worker runs its poller and its async /health HTTP server on the same asyncio event loop. During Bedrock inference, litellm's acompletion performs synchronous botocore credential resolution + SigV4 signing on the loop thread, which starves the loop for several seconds. A Kubernetes livenessProbe pointed at /health then times out and the pod is SIGKILLed (exitCode 137).

A loop blocked by sync signing is busy, not dead — liveness should not kill it.

Fix

Add a tiny liveness server (worker/liveness.py) on a separate daemon thread with its own stdlib http.server:

  • A task on the main loop bumps a monotonic Heartbeat ~1x/sec.
  • The thread server returns 200 while the heartbeat is fresher than a staleness threshold, 503 once it goes stale.
  • The livenessProbe points at this new port; a multi-second sync block goes momentarily stale but the loop resumes and the heartbeat catches up well within the threshold, so the probe stays green. A genuinely wedged loop never catches up and the probe fails, letting Kubernetes restart the pod.

Why liveness-only (not the DB check)

The asyncpg pool is bound to the main event loop and cannot be awaited from another thread. So the thread server serves liveness from a plain in-process signal (the heartbeat), never the DB. The existing async /health keeps the DB check and remains the readiness signal.

Config

  • HINDSIGHT_API_WORKER_LIVENESS_PORT (default 8890) — liveness port, next to the existing metrics/health port 8889.
  • HINDSIGHT_API_WORKER_LIVENESS_THRESHOLD_SECONDS (default 30) — heartbeat staleness budget; comfortably exceeds worst-case sync-signing block while still catching a wedged loop.

Both follow the existing config.py env/default/dataclass/from_env conventions. A --liveness-port CLI flag mirrors --http-port.

Tests

tests/test_worker_liveness.py covers the heartbeat-staleness predicate (fresh / stale / multi-second-block-tolerant) and Heartbeat age reset.

Scope

This is the worker-code side of the fix. It complements the chart-side port wiring in #2904 (which points the livenessProbe at the new port) but is independently mergeable — this PR adds the server and config; the chart PR consumes them.

@Fyko
Fyko marked this pull request as ready for review July 22, 2026 17:51
@nicoloboschi

Copy link
Copy Markdown
Collaborator

Followed up on the root-cause question with a visibility-first approach in #2942 — instead of a separate heartbeat liveness server, it adds two always-on signals so a stalled /health names its own cause:

  • an off-loop watchdog (separate thread, so it isn't frozen by the stall it's watching) that logs the loop thread's blocking stack when the event loop is wedged, and
  • DB-pool acquire instrumentation — a waiting gauge (callers queued for a connection) + slow-acquire logs with pool stats — for the pool-exhaustion case, which fails /health with the loop completely idle.

Context that pushed me toward visibility over a workaround: on the pinned litellm, boto3/Bedrock credential resolution is already offloaded to a thread (run_in_executor), so that specific path didn't reproduce as a loop-blocker — signing runs on the loop but is sub-ms. So rather than guess the culprit, this makes whatever blocks the loop or exhausts the pool next name itself in the logs. Would love your eyes on it.

@Fyko

Fyko commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

thank you @nicoloboschi!

@Fyko Fyko closed this Jul 24, 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