Skip to content

feat(serverless): coordinate early health checks once per container - #578

Draft
justinwlin wants to merge 14 commits into
mainfrom
justinlin/dr-1409-python-sdk-move-health-checks-at-start-up
Draft

justinwlin wants to merge 14 commits into
mainfrom
justinlin/dr-1409-python-sdk-move-health-checks-at-start-up

Conversation

@justinwlin

@justinwlin justinwlin commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Worker health checks previously waited until serverless.start(), often after expensive model loading. This change runs the built-in hardware checks (RAM, disk, CUDA version, native GPU test) at the first import runpod in a Serverless worker, and keeps network, Python CUDA initialization, GPU compute, and customer checks at worker start.

Eligibility. Early checks require both RUNPOD_ENDPOINT_ID and RUNPOD_WEBHOOK_GET_JOB, and skip RUNPOD_TEST, --test_input, and --rp_serve_api invocations. The trigger lives in the top-level runpod/__init__.py, so it does not depend on eager serverless import.

Once per process tree. A process whose early pass passes sets RUNPOD_EARLY_FITNESS_CHECKS_DONE=1. Child processes inherit it (multiprocessing spawn re-importing the handler, subprocesses, shell wrappers) and skip the pass instead of repeating the GPU probes. A failed check exits before the marker is set. No lock files, procfs identity, or shared state.

Flags. RUNPOD_DEFER_FITNESS_CHECKS=true restores worker-start timing. RUNPOD_SKIP_FITNESS_CHECKS=true disables all checks. Late threshold changes are applied at worker start and only rerun the affected checks.

Also in this PR. Network check is deferred to worker start, targets the worker API host, and retries within one bounded budget. Registration is atomic and setup failures at worker start go through the same force-exit path. Logger redaction fixes. Module moves to runpod/_health/ with import aliases at the old paths.

Not in this PR. Realtime workers keep their main behavior (no fitness checks). feat/apps-sdk will need the one-line run_import_checks() call carried into its lazy __init__ when main is merged in.

Validation: full suite 694 passed, 93% coverage. A real multiprocessing spawn child test confirms the child inherits the marker and runs zero probes.

🤖 Generated with Claude Code

@justinwlin
justinwlin marked this pull request as ready for review September 8, 2026 18:53
@justinwlin
justinwlin requested a review from deanq September 8, 2026 18:53
@deanq
deanq requested a lite review from Copilot September 9, 2026 19:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Auto-registration failures can currently escape the hard-exit failure path, undermining the “must not hang” operational guarantee during worker startup.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Moves most serverless worker fitness checks earlier (at import runpod.serverless) to fail unhealthy workers before model load, while keeping CUDA-context-creating checks deferred to start() and adding env flags to skip/defer behavior.

Changes:

  • Add an import-time startup pass (run_startup_fitness_checks) gated by worker env, with once-per-process deduping and deferred-check support.
  • Introduce global skip/defer env flags and more consistent “truthy” env parsing; add nvidia-smi timeout for CUDA detection.
  • Add/expand tests and docs to cover startup timing, deduping, deferred checks, and late-config warnings.
File summaries
File Description
tests/test_serverless/test_worker.py Asserts worker loop still runs fitness checks.
tests/test_serverless/test_utils/test_cuda.py Updates CUDA availability test expectations for timeout=5.
tests/test_serverless/test_modules/test_fitness/test_startup.py New test suite validating import/start timing, deferral, dedupe, and config warnings.
tests/test_serverless/test_modules/test_fitness/conftest.py Resets new startup/dedupe global state between tests.
runpod/serverless/utils/rp_cuda.py Adds bounded nvidia-smi probe with timeout to avoid hangs.
runpod/serverless/modules/rp_system_fitness.py Marks CUDA-init and benchmark checks as deferred-to-worker-start.
runpod/serverless/modules/rp_gpu_fitness.py Uses shared truthy env flag parsing for skip behavior.
runpod/serverless/modules/rp_fitness.py Implements startup pass, deduping, skip/defer env flags, and late-config warnings.
runpod/serverless/init.py Triggers startup checks at import (worker-only no-op otherwise).
README.md Updates high-level behavior summary for new timing model.
docs/serverless/worker_fitness_checks.md Documents import-time checks, deferral, and new env flags.
ARCHITECTURE.md Updates architecture docs for new timing and hard-exit behavior.
Review details
  • Files reviewed: 12/12 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread runpod/serverless/modules/rp_fitness.py Outdated
Comment on lines 324 to 329
# Defer GPU check auto-registration until fitness checks are about to run
# This avoids circular import issues during module initialization
_ensure_gpu_check_registered()

# Defer system check auto-registration until fitness checks are about to run
_ensure_system_checks_registered()

@deanq deanq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review from /code-review (correctness + cleanup pass). Four findings, most centered on moving os._exit(1)-capable checks to import time. Lines re-anchored to the current diff.

Comment thread runpod/serverless/modules/rp_fitness.py Outdated
if _env_flag(SKIP_FITNESS_CHECKS_ENV) or _env_flag(DEFER_FITNESS_CHECKS_ENV):
return

if not os.environ.get("RUNPOD_WEBHOOK_GET_JOB"):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import-time gate keys only on RUNPOD_WEBHOOK_GET_JOB, but that misses the _is_local / test_input local-mode guard that previously protected these checks. A handler run with rp_args.test_input used to skip all fitness checks (local mode -> run_worker never called). Now import runpod (eager on main) runs run_startup_fitness_checks(), executes the built-in memory/disk/network/gpu checks, and any failure hard-kills the process via os._exit(1).

Same hazard for any auxiliary CLI/process that imports runpod only for the API client while RUNPOD_WEBHOOK_GET_JOB is inherited in the environment -- it will now run worker fitness checks and can be killed.

Suggest gating the import-time pass on the same local-mode/test-input signal that run_worker uses, so local and non-worker imports stay exempt.

Comment thread runpod/serverless/__init__.py Outdated

# Check the environment here rather than in start(), which a handler module
# only reaches after loading its model. No-op outside a real worker.
run_startup_fitness_checks()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole fail-fast benefit assumes import runpod eagerly imports runpod.serverless. That holds on this PR's declared base (main), but the repo's active feat/apps-sdk line lazy-loads serverless via PEP 562 __getattr__. There, a typical handler -- import runpod -> load model -> runpod.serverless.start(...) -- won't trigger serverless/__init__ until the start() line, i.e. after the multi-minute model load.

So on the apps-sdk line the checks fire no earlier than before, silently regressing, while the README/docs added in this PR assert "built-ins at import" / "any import runpod triggers it."

Which branch does this actually merge into? If it's the lazy-import line, either the docs need correcting or the trigger needs an explicit hook that doesn't depend on eager submodule import.

Comment thread runpod/serverless/modules/rp_fitness.py Outdated
# raises RuntimeError on Python 3.10+.
loop = asyncio.new_event_loop()
try:
loop.run_until_complete(run_fitness_checks(include_deferred=False))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running the network check at import time means a transient failure hard-exits the process mid-import runpod (_terminate_unhealthy -> os._exit(1)). On a cold worker container whose network stack isn't up yet when the handler module is first imported, this turns a recoverable warm-up delay into a boot crash-loop.

Previously this ran in run_worker after start(), giving the container time to become ready. Consider keeping network (and other environment-readiness) checks on the post-start() path, or adding a bounded retry before terminating.

Comment thread runpod/serverless/modules/rp_fitness.py Outdated
return

if _config_snapshot:
_warn_late_config()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_startup_fitness_checks populates _config_snapshot microseconds before calling run_fitness_checks, where this if _config_snapshot: _warn_late_config() re-compares all nine env vars against the values just captured -- guaranteed no change, no warning. It's pure overhead on the import path; the late-config warning is only meaningful on the later run_worker pass. Consider skipping _warn_late_config() when invoked from the import-time pass.

@justinwlin
justinwlin marked this pull request as draft September 10, 2026 17:07
@justinwlin justinwlin changed the title feat(serverless): run fitness checks at startup, add skip env var feat(serverless): add safe early fitness checks and skip controls Sep 10, 2026
Comment thread runpod/_logger.py Fixed
Comment thread runpod/_logger.py Fixed
Comment thread runpod/_health/fitness.py Fixed
Comment thread runpod/_startup.py Fixed
@justinwlin justinwlin changed the title feat(serverless): add safe early fitness checks and skip controls feat(serverless): coordinate early health checks once per container Sep 11, 2026
Comment thread tests/test_serverless/test_modules/test_fitness/test_startup.py Fixed
Comment thread tests/test_serverless/test_modules/test_fitness/test_startup.py Fixed
justinwlin and others added 13 commits September 17, 2026 14:39
Built-in GPU/system fitness checks ran in run_worker, which a handler module
only reaches after loading its model. Run them when runpod.serverless is
imported instead, so a broken environment fails in seconds. User-registered
checks still run at start(); checks that already passed are not repeated.

Adds RUNPOD_SKIP_FITNESS_CHECKS to disable all checks and
RUNPOD_DEFER_FITNESS_CHECKS to restore the previous start()-only timing.
_cuda_init_check and _benchmark_check import torch and allocate on the
device. Running them at import would leave a CUDA context in a process the
handler may later fork, which CUDA does not support and vLLM/DeepSpeed trip
over. Mark them @defer_to_worker_start so only subprocess-based and
non-GPU checks run early.
- run startup pass on a dedicated event loop instead of asyncio.run,
  which resets the loop policy and breaks asyncio.get_event_loop() in
  handler code on Python 3.10+
- set RUNPOD_FITNESS_CHECKS_DONE after the startup pass so children
  re-importing this module under multiprocessing 'spawn' skip the checks
- latch check auto-registration state only on success, so a malformed
  RUNPOD_MIN_*/GPU timeout value re-raises loudly in run_worker instead
  of silently disabling all system checks
- compare completed checks by identity, not equality, so distinct
  registrations that compare equal (bound methods) are not skipped
- bound the nvidia-smi call in rp_cuda.is_available with a 5s timeout
- accept 1/true/yes/on for RUNPOD_SKIP_GPU_CHECK and
  RUNPOD_SKIP_AUTO_SYSTEM_CHECKS, matching the new flags
- tests: pin the worker.py and import-time wiring, the full defer
  behavior, the done marker, the real auto-registration path (guard: no
  torch import), and bound-method re-registration; fix an orphaned
  coroutine in test_unexpected_error_does_not_propagate
- docs: thresholds/skip flags must be set before import runpod, realtime
  API mode runs only the import-time checks, refresh stale
  ARCHITECTURE.md execution flow
…touch-ups

- regression test: malformed RUNPOD_MIN_* must re-raise in run_worker,
  never fail open (latch-on-success)
- fix dormant called/calls typo in the done-marker test
- README: checks run once per check, not once at startup
- ARCHITECTURE.md: failure path is os._exit(1), not sys.exit(1)
- docs: GPU benchmark default timeout is 2s, not 100ms
- rp_gpu_fitness docstring: lazy registration + truthy flag values
The import-time pass consumes RUNPOD_MIN_*/RUNPOD_SKIP_*/RUNPOD_GPU_* at
import; values set from the handler afterwards were silently ignored.
run_fitness_checks now diffs the current env against the values snapshot
at the startup pass and warns with the exact fix (set before import, or
RUNPOD_DEFER_FITNESS_CHECKS=true).
@justinwlin
justinwlin force-pushed the justinlin/dr-1409-python-sdk-move-health-checks-at-start-up branch from 3779487 to 3e6e213 Compare September 17, 2026 18:39
The shared /tmp lock file, procfs container identity, and persisted
failure state solved one problem: child processes that re-import the
handler (multiprocessing spawn, subprocesses) repeating the GPU probes.
An environment marker solves the same problem with no new machinery.

A process whose import-time pass passes sets
RUNPOD_EARLY_FITNESS_CHECKS_DONE=1. Every child inherits it and skips
the pass. A failed check exits before the marker is set, so it only
ever means a parent passed. Setup failures leave the registration latch
unset and do not set the marker either.

Dropping the shared state also removes the sticky-failure behavior under
wrapper entrypoints and the bounded lock waits. The realtime lifespan
hook is removed as well; realtime workers keep their main-branch
behavior and can be addressed separately.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@justinwlin

Copy link
Copy Markdown
Contributor Author

bugbot run

@justinwlin

Copy link
Copy Markdown
Contributor Author

Update: lock file replaced with an inherited env marker, realtime hook removed

Summary of what changed since the last round of review, and how the four review points map to the current code.

What the PR does now, in one paragraph

import runpod in a Serverless worker runs the four hardware checks that are safe before the handler exists (RAM, disk, CUDA version, native GPU test). A failure exits the process right there, before any model loads. Network, torch CUDA init, GPU benchmark, and customer checks still run at start(). When the import pass passes, the process sets RUNPOD_EARLY_FITNESS_CHECKS_DONE=1, and every child it launches inherits that and skips the pass.

Why the lock file is gone

The shared /tmp lock, procfs identity, and persisted failure state solved one problem: child processes that re-import the handler (vLLM spawn workers, multiprocessing, subprocesses) repeating the 30-second GPU probe. An inherited env var solves the same problem with no shared state, no lock waits, and no sticky failures. The realtime lifespan hook came out too, since realtime workers never ran fitness checks on main and that should be its own decision.

Review points

Point Status Where
Import gate misses local mode and --test_input Fixed is_early_check_eligible requires both worker env vars and skips RUNPOD_TEST, --test_input, --rp_serve_api
apps-sdk lazy-loads serverless, so the trigger never fires early Fixed Trigger moved to top of runpod/__init__.py; independent of serverless import. apps-sdk will need to carry the one-line call when main is merged in
Network check at import turns cold-boot network delay into a crash loop Fixed _network_check is @defer_to_worker_start and retries within one bounded budget
Late-config comparison is overhead on the import pass Fixed _refresh_late_config only runs on the worker-start pass
Registration failures escape the hard-exit path (Copilot) Fixed Registration is atomic; worker-start setup errors go through _fail_worker

Examples

Healthy worker. Logs from a real RTX 4090 Serverless worker on this branch:

Running 4 fitness check(s)...
GPU binary test passed: 1 GPU(s) healthy (CUDA 12.8)
Memory check passed: 184.90GB available (of 247.52GB total)
Disk space check passed: 19.78GB free (98.9% available)
CUDA version check passed: 12.8 (minimum: 11.8)
All fitness checks passed. (599.98ms)
IMPORT_DONE 3.82s marker=1
--- pretend model load ---
--- Starting Serverless Worker |  Version 1.12.1.dev23 ---
Running 3 fitness check(s)...
Network connectivity passed: Connected to api.runpod.ai:443
CUDA initialization passed: 1 device(s) initialized successfully
GPU compute benchmark passed: Matrix multiply completed in 108ms

Four checks inside the import, three at start(), nothing runs twice.

Broken worker. Same endpoint with RUNPOD_MIN_MEMORY_GB=99999 to force a failure:

Running 4 fitness check(s)...
GPU binary test passed: 1 GPU(s) healthy (CUDA 12.8)
Fitness check failed: _memory_check | RuntimeError: Insufficient memory: 207.61GB available, 99999.0GB required
Worker is unhealthy, exiting.

Exit about one second into import runpod. The model-load line never prints. The platform marked the worker UNHEALTHY within a minute.

Child process. A handler that spawns a multiprocessing child, which re-imports the handler and therefore runpod:

Running 4 fitness check(s)...        <- parent, once
All fitness checks passed. (416.35ms)
PARENT import 2.29s marker=1
PARENT import 1.59s marker=1         <- child re-import, no checks
('child sees marker', '1')

Opt-outs. RUNPOD_DEFER_FITNESS_CHECKS=true restores the old worker-start timing. RUNPOD_SKIP_FITNESS_CHECKS=true disables all checks. Local --test_input runs and RUNPOD_TEST are exempt automatically.

Validation

  • 694 tests pass locally, 93% coverage, CI green on 3.10 through 3.14.
  • Live on a Pod (RTX 4090): healthy import, forced failure, spawn child, and each exemption flag.
  • Live on a Serverless endpoint (ADA_24): healthy cold start with a completed job, then a forced-failure release observed crash-looping at import.

@justinwlin

Copy link
Copy Markdown
Contributor Author

@copilot review

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.

4 participants