feat(openai): env-driven per-worker config + MODEL_NAME recovery - #87
Open
robballantyne wants to merge 4 commits into
Open
feat(openai): env-driven per-worker config + MODEL_NAME recovery#87robballantyne wants to merge 4 commits into
robballantyne wants to merge 4 commits into
Conversation
…e) — CON-1612 Make the openai worker consume its per-worker config from env, each defaulting to the previous hardcoded value, so base-image can bake per-backend values: - MODEL_LOG (was hardcoded /var/log/portal/vllm.log) — the original latent bug: the worker tailed a hardcoded path and ignored MODEL_LOG. Now os.environ.get(MODEL_LOG,...). - MODEL_HEALTH_ENDPOINT (the established env var; the framework's Backend.healthcheck_url default reads it and the vLLM serverless templates set it) — default /health. A path resolves against MODEL_SERVER_URL; a full URL is used as-is (worker venv is fresh, so aiohttp is current). - MODEL_LOAD_LOG_MSG / MODEL_ERROR_LOG_MSGS / MODEL_INFO_LOG_MSGS — newline-delimited env (each line stripped), vLLM defaults; lets an image supply per-backend log grammar (fixes on_error detection being dead for non-vLLM backends under the shared worker). - READINESS + READINESS_TIMEOUT + HEALTHCHECK_PROBE_TIMEOUT (vast-cli WS2) — feature- detected via inspect.signature(WorkerConfig): only passed when the installed vastai supports them, so a new worker on an OLD framework degrades to log mode, not TypeError. Timeout floats safe-parse AND require positive-finite (0/neg/inf/nan -> default, since they feed asyncio timeouts). Default READINESS=logs (current behaviour). Backwards-compat / merge note: absent-env reproduces today's behaviour, and READINESS stays 'logs' until an image/template opts in. NOT a pure no-op merge, though: reusing MODEL_HEALTH_ENDPOINT *activates* an env the vLLM templates already set (previously the worker hardcoded /health and ignored it). Audited: all deployed openai serverless templates (010/011/012) set http://127.0.0.1:18000/health == the prior hardcoded value, so it is a no-op for the CURRENT fleet — but the health URL gates readiness even in log mode, so any future openai template MUST set MODEL_HEALTH_ENDPOINT to :18000/health (keep this audit as a merge gate). Scoped to openai only.
…enchmark A single template must serve both on-demand and serverless. The on-demand recommended templates set only the engine-specific model var (VLLM_MODEL / SGLANG_MODEL / LLAMA_MODEL), never MODEL_NAME, so the serverless benchmark had no model id and raised. Resolve it with precedence MODEL_NAME -> VLLM_MODEL -> SGLANG_MODEL -> LLAMA_MODEL: an explicit MODEL_NAME still wins (templates can override), else the served-model id is recovered from the engine var. vLLM/SGLang serve under exactly that value (no --served-model-name rewrite) so it matches /v1/models; llama.cpp ignores the request model field. Done in the worker, not a boot script, because the engine var is present in the worker env on BOTH serverless launch paths (onstart-curl and base supervisor), whereas a boot-computed value never reaches the onstart-curl worker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Health-gated readiness should backstop a genuinely-wedged worker, not gate a slow-but- valid cold start. 300s trips on large-model cold starts (weight download + load + graph capture) on slower hosts, forcing operators to tune it just to get serving working. 1800s (30 min) covers realistic big-model starts so it works out of the box; lower it only to get faster failure detection. Still env-overridable via READINESS_TIMEOUT. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…framework Matches the vast-cli default bump: SGLang /health can do real work (~20s internally), so a 10s per-probe timeout risked a false regression error on a stall once loaded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
I think a better approach would be to split the "openai" dir into multiple dirs, |
| # `MODEL_HEALTH_ENDPOINT` is the established env var (the framework reads it too, and | ||
| # the vLLM serverless templates set it). A path resolves against MODEL_SERVER_URL; a | ||
| # full URL is used as-is (the worker venv is built fresh, so aiohttp is current). | ||
| MODEL_HEALTHCHECK_ENDPOINT = os.environ.get("MODEL_HEALTH_ENDPOINT", "/health") |
Contributor
There was a problem hiding this comment.
I think all these should be os.environ.get defaulted in the vast-cli backend.py file, and should fallback to the specific value in the WorkerConfig, and finally fall back to the global default defined in backend.py. Only configuration for a specific implementation of a backend template (i.e. different models) should be controlled by pyworker-level os.environ.get flags, all global pyworker stuff should be os.environ.get defaulted by backend.py itself.
Contributor
Author
|
Fine to split it, no strong feelings there. Main thing for me: the engine/version-specific config (health endpoint, log path, load/error log strings) should stay in the image and be read by the worker, not defaulted in backend.py. Those log strings can change between engine releases, and only the image is pinned to a version. Global stuff like timeouts is fine in backend.py. |
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
2 participants
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
Make the openai PyWorker config env-driven, so the base image can bake per-backend values (BACKEND / MODEL_LOG / MODEL_HEALTH_ENDPOINT / READINESS / timeouts) while absent-env reproduces today's behaviour exactly. This is what lets a single template serve vLLM / SGLang / llama.cpp instead of the worker hardcoding vLLM's values.
Changes (
workers/openai/worker.py)MODEL_LOG(was hardcoded/var/log/portal/vllm.log),MODEL_HEALTH_ENDPOINT, the log-action grammar (MODEL_LOAD_LOG_MSG/MODEL_ERROR_LOG_MSGS/MODEL_INFO_LOG_MSGS), and — feature-detected —READINESS/READINESS_TIMEOUT/HEALTHCHECK_PROBE_TIMEOUT. Absent env = byte-for-byte today's behaviour.MODEL_NAME → VLLM_MODEL → SGLANG_MODEL → LLAMA_MODEL(first non-empty), so a single on-demand template that sets only the engine var works serverless without a duplicateMODEL_NAME.inspect.signature(WorkerConfig), so a new worker on an oldervastaidegrades to log mode instead of aTypeError._env_float(rejects malformed/non-finite/non-positive → default; these feed asyncio timeouts) and_env_lines(newline-delimited → stripped list) helpers.READINESS_TIMEOUTdefault 1800 andHEALTHCHECK_PROBE_TIMEOUTdefault 30, matching the framework (feat(serverless): health-gated readiness mode (readiness='healthcheck') vast-cli#468).Pairs with the health-gated readiness framework change in vast-ai/vast-cli#468. No behaviour change for absent-env; verified backwards-compat against the previous hardcoded values.