feat(registry,api): harden warm-pool recovery, admission, and runtime shedding#525
feat(registry,api): harden warm-pool recovery, admission, and runtime shedding#525Gajesh2007 wants to merge 11 commits into
Conversation
…it for busy boxes Replace the fully-idle cold-candidate requirement (any pending request or busy backend slot disqualified with not_idle) with a bounded-busy gate: eligible when pendingCount + sum over all slots of running+waiting <= EIGENINFERENCE_WARM_POOL_ALLOW_BUSY_LOAD_MAX (default 0 = exact historical behavior). A non-idle candidate must additionally fit the model weights WITHOUT evicting co-resident models (new busy_no_evict_headroom reason), mirroring the dispatch path's no-eviction rule in freeMemoryAdmits. Fixes postmortem 2026-07-06 layer 7: during recovery every box carrying a single request read as ineligible, eligible_cold hit 0, and MIN_WARM floors were clamped into a no-op.
…ightly-busy leg) Pins the postmortem §3.3 recovery path: after a provider idle-unloads, the floor alone (zero demand pressure) re-warms the box on the next tick, and with ALLOW_BUSY_LOAD_MAX=2 the floor also acts on a box still serving one request of a co-resident model — the case the historical fully-idle gate clamped to eligible_cold=0.
…_COMBINED_ADMISSION_CAP Per-model quality caps are checked independently, so a box saturated on model A still admits model B (the cross-model hard edge blocking the open-pool flip). When the flag is on (default OFF — byte-identical behavior while dormant), hasConcurrencyHeadroomForModelCapLocked also requires the normalized box budget: sum over slots(load_s/qc_s) + 1/qc_candidate <= overcommit(candidate) qc uses the same qualityConcurrency the per-model cap uses (static rates only, never observed-under-load EWMAs); unresolvable slot qc fails open per slot; an unloaded box always admits; the no-benchmark non-dedicated trust rule mirrors effectiveMaxConcurrencyForModelLocked.
…embership The uptime-neutral 429 on the no-eligible-provider preflight branch was triggered by IsDedicatedModel && HasProviderForModel, so a NON-dedicated model whose providers were all gate-excluded (cooldowns, thermal, floors) hard-503'd even though the fleet serves it — and clearing EIGENINFERENCE_DEDICATED_MODELS would silently flip sheds to 503s. Re-key the trigger on HasProviderForModel alone: provider exists => 429 + Retry-After; model truly absent => 503. DEDICATED_MODELS becomes a pure routing-policy env with instant rollback. Retry-After computation and the historical outcome tag (outcome:dedicated_capacity_429) are preserved for existing dashboards; a new reason tag (dedicated | provider_exists) splits the old trigger from the re-keyed one. IsDedicatedModel is consulted for that tag only.
The controller already tallied per-reason cold-candidate disqualifications into its snapshots, but they were only visible in warm_pool_tick logs — the Jul 6 recovery stalled for hours on eligible_cold=0 before anyone could see WHY (postmortem mistake #4: knobs raised blind, restart to grep). Emit warm_pool.eligible_cold + warm_pool.cold_ineligible per model and warm_pool.cold_disqualified per (model, reason) from the existing 15s gauge loop (body extracted to pushDDGauges so tests can drive one emission against a live UDP collector).
GET/PUT /v1/admin/reject-models reads/replaces the per-model shed set at
runtime; EIGENINFERENCE_REJECT_MODELS still seeds the initial set at
startup. Every shed flip previously required a coordinator restart, and
each restart wipes the in-memory calibrator/TPS/breaker/warm-pool state
(1,589 provider sessions died to coordinator restarts in one 48h window).
- Auth mirrors /v1/admin/drain: requireAuth wrapper (admin key accepted as
a pseudo-account, no credentials = 401) + isAdminAuthorized in-handler
(admin key OR Privy admin; otherwise 403).
- PUT is a FULL replacement ({"models": [...]}; empty list = shed
nothing) and logs old -> new loudly.
- rejectModels is now guarded by an RWMutex (it was startup-write-only
before; the endpoint makes it runtime-mutable on the admission hot path).
- scripts/admin.sh grows reject-models get|set|clear following the
existing subcommand pattern.
…SY_LOAD_MAX, and the runtime reject-models endpoint
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This PR introduces a runtime-mutable model-shed facility and minor scheduling bookkeeping; it is neutral-to-positive on security posture but adds one new admin-only attack surface that warrants a brief note. Trust boundaries touched
Per-threat assessmentT-002 — Unbounded request body causes coordinator OOMℹ️ Neutral. The changes in T-004 — Consumer API key used for admin-scoped operationsℹ️ Neutral. The two new routes ( T-005 — Consumer denies inference requests (billing repudiation)ℹ️ Neutral. New T-006 — Unauthenticated WebSocket floods provider registryℹ️ Neutral. T-007 / T-012 / T-027 — Weight hash / model tamperingℹ️ Neutral. Registry changes are limited to the shed list and T-009 / T-015 / T-034 / T-036 — Attestation chain / trust escalationℹ️ Neutral. The T-024 — Admin API key compromise / release manipulation
T-025 — Stale release cache (SEC-021)ℹ️ Neutral. Cache invalidation logic not touched. T-038 — Slow-header / HTTP server configℹ️ Neutral. T-040 — Host header injection in URL generationℹ️ Neutral. No URL generation touched. T-041 — Profile signing identity exfiltrationℹ️ Neutral. T-010 — Cancellation not propagated to inference engineℹ️ Neutral. T-008 — Provider sends plaintext SSE on encryption failureℹ️ Neutral. No changes to the encryption/SSE path in New attack surface not covered by existing threatsRuntime shed list as a denial-of-service lever (new, low severity)
Lock ordering note (
|
|
Found 1 test failure on Blacksmith runners: Failure
|
ethenotethan
left a comment
ethenotethan
left a comment
There was a problem hiding this comment.
Automated Code Review — Layr-Labs/d-inference#
Verdict: REQUEST_CHANGES
Security — 1 finding(s) (1 blocking)
- 🟡 [MEDIUM]
coordinator/api/admin_reject_models.go:52-56— Model names not validated for injection or path traversal- Suggestion: Add validation to reject model names containing special characters, path separators, or control characters that could be used in injection attacks
Performance — 1 finding(s) (1 blocking)
- 🟡 [MEDIUM]
coordinator/api/admin_reject_models_test.go:176-195— Concurrent test spawns 8 goroutines performing 200 operations each without context cancellation or timeout- Suggestion: Add context with timeout and use errgroup to properly manage goroutine lifecycle and error handling
Type_diligence — 2 finding(s)
- 🔵 [INFO]
coordinator/api/admin_reject_models.go:33— map[string]any used for JSON response where typed struct would be clearer- Suggestion: Define a typed response struct like
type RejectModelsResponse struct { Models []string \json:"models"` }` instead of map[string]any
- Suggestion: Define a typed response struct like
- 🔵 [INFO]
coordinator/api/admin_reject_models.go:62-65— map[string]any used for JSON response where typed struct would be clearer- Suggestion: Define a typed response struct like
type PutRejectModelsResponse struct { Models []string \json:"models"`; Previous []string `json:"previous"` }` instead of map[string]any
- Suggestion: Define a typed response struct like
Additive_complexity — ✅ No issues found
4 finding(s) total, 2 blocking. Verdict: REQUEST_CHANGES.
🤖 Automated review by Centaur · DAR-186
Review round for PR #525 (findings F1, F3, F4): - PUT /v1/admin/reject-models now bounds the payload (max 128 entries, max 256 bytes per name) and rejects invalid UTF-8 and control characters with a 400 that names the offending index. '/' stays legal since real model IDs contain it (mlx-community/...). Validation runs before ReplaceRejectModels so a rejected payload never half-applies. The strings only ever flow into map lookups, slog, and JSON — never filesystem paths or commands — so this is defense-in-depth for the admin surface, not an injection fix. - Both handlers now return typed response structs (rejectModelsResponse, rejectModelsReplaceResponse) instead of map[string]any; wire shape unchanged and pinned by TestAdminRejectModelsWireShape. - Table-driven validation tests through the real HTTP mux, plus a direct test for the invalid-UTF-8 leg (unreachable via JSON decode, guards the env-seeding path).
ethenotethan
left a comment
ethenotethan
left a comment
There was a problem hiding this comment.
Automated Code Review — Layr-Labs/d-inference#
Verdict: REQUEST_CHANGES
Security — 1 finding(s)
- 🔵 [INFO]
coordinator/api/admin_reject_models.go:58-70— validateRejectModelName allows forward slashes which could enable path traversal if model names are used in file operations- Suggestion: Consider restricting forward slashes or ensure model names are never used in file path operations without proper sanitization
Performance — 1 finding(s)
- 🔵 [INFO]
coordinator/api/admin_reject_models.go:102-115— Map allocation without pre-sizing in validation loop- Suggestion: Pre-allocate the map with capacity len(req.Models): set := make(map[string]bool, len(req.Models))
Type_diligence — 1 finding(s) (1 blocking)
- 🟡 [MEDIUM]
coordinator/api/admin_reject_models.go:103-115— map[string]bool creation without validating duplicate model names could silently overwrite entries- Suggestion: Check for duplicate model names in the request slice before creating the map, or document that later entries overwrite earlier ones
Additive_complexity — ✅ No issues found
3 finding(s) total, 1 blocking. Verdict: REQUEST_CHANGES.
🤖 Automated review by Centaur · DAR-186
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b906978e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Review round addressed @ethenotethan — ready for re-review.
|
Finding 1 (503 for structural no-provider): the no-eligible-provider shed
preflight keyed the 429-vs-503 split on the bare HasProviderForModel, which
ignores the public routing gates — so a non-dedicated model advertised only by
private-only, runtime-unverified, below-trust, or render-broken providers (which
can NEVER serve a public request) was 429'd as transient capacity instead of
503'd. Add registry.HasCapableProviderForModel, the capability-aware sibling
that applies the structural liveness/trust/privacy + trait gates while ignoring
transient exclusions (dedicated rule, cooldowns, breaker, thermal). Status stays
independent of IsDedicatedModel; the dedicated-on-non-dedicated-box case still
reads as a transient 429.
Finding 2 (busy no-evict units): the bounded-busy warm-pool no-evict fit
compared the RAW catalog decimal-GB size against freeGB (reported in GiB),
under-counting the model's in-memory footprint. Normalize with
coldLoadCatalogGBToMemGiB (×1.2 overhead + decimal-GB→GiB) so it mirrors the
provider's ModelLoadAdmission gate — a near-threshold model no longer passes
here and then fails load_model.
Finding 3 (require models key): PUT /v1/admin/reject-models with {}, a typoed
key, or {"models":null} yielded nil and silently CLEARED the shed list. Decode
into *[]string so an absent/null key is a 400; only an explicit {"models":[]}
clears.
Finding 4 (reject queued on shed): adding a model to the runtime shed set only
gated future admission — already-queued requests kept dispatching to the pulled
model. The PUT handler now fails the queued public waiters for newly-shed models
(self-route preserved, mirroring the shed's admission exemption) via
registry.RejectQueuedRequestsForShedModels.
Each fix ships a fails-without-fix regression test (httptest for the admin
endpoints).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aba4931bb2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ethenotethan
left a comment
ethenotethan
left a comment
There was a problem hiding this comment.
Automated Code Review — Layr-Labs/d-inference#
Verdict: COMMENT
Security — ⚠️ SKIPPED
Error: OpenRouter HTTP 402: {"error":{"message":"Insufficient credits. Add more using https://openrouter.ai/settings/credits","code":402}}
Performance — ⚠️ SKIPPED
Error: OpenRouter HTTP 402: {"error":{"message":"Insufficient credits. Add more using https://openrouter.ai/settings/credits","code":402}}
Type_diligence — ⚠️ SKIPPED
Error: OpenRouter HTTP 402: {"error":{"message":"Insufficient credits. Add more using https://openrouter.ai/settings/credits","code":402}}
Additive_complexity — ⚠️ SKIPPED
Error: OpenRouter HTTP 402: {"error":{"message":"Insufficient credits. Add more using https://openrouter.ai/settings/credits","code":402}}
✅ All four passes clean. No issues found.
🤖 Automated review by Centaur · DAR-186
Gajesh2007
changed the title
ethenotethan
left a comment
ethenotethan
left a comment
There was a problem hiding this comment.
Automated Code Review — Layr-Labs/d-inference#
Verdict: REQUEST_CHANGES
Security — 1 finding(s)
- 🔵 [INFO]
coordinator/api/admin_reject_models.go:58-72— validateRejectModelName allows forward slashes which could enable path traversal if model names are used in file operations- Suggestion: Consider restricting forward slashes in model names or ensure model names are never used directly in file paths without proper sanitization
Performance — 2 finding(s) (2 blocking)
- 🟡 [MEDIUM]
coordinator/registry/concurrency_cap_combined_test.go:95— Unbounded slice allocation in test loop without pre-allocation- Suggestion: Pre-allocate the slice with known capacity:
slots := make([]combinedSlotLoad, 0, len(p.BackendCapacity.Slots)+len(pendingByModel))
- Suggestion: Pre-allocate the slice with known capacity:
- 🟡 [MEDIUM]
coordinator/registry/concurrency_cap.go:105— Unbounded slice allocation in hot path without pre-allocation- Suggestion: Pre-allocate the slice with known capacity:
slots := make([]combinedSlotLoad, 0, len(p.BackendCapacity.Slots)+len(pendingByModel))
- Suggestion: Pre-allocate the slice with known capacity:
Type_diligence — 1 finding(s) (1 blocking)
- 🟡 [MEDIUM]
coordinator/api/admin_reject_models.go:99— Missing nil check for req.Models pointer before dereferencing- Suggestion: The code checks
if req.Models == nilbut then immediately dereferences*req.Modelswithout verifying the pointer is valid. Add explicit validation:if req.Models == nil { return 400 }; models := *req.Models
- Suggestion: The code checks
Additive_complexity — ✅ No issues found
4 finding(s) total, 3 blocking. Verdict: REQUEST_CHANGES.
🤖 Automated review by Centaur · DAR-186
Superseded by head 5176e66. The actionable input-hardening feedback was implemented, all associated threads are resolved, and full Go/Swift gates plus independent rereviews pass.
|
CI triage on |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5176e66a6f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !warmPoolModelSlotHeadroomLocked(p, model) { | ||
| return warmPoolCandidate{}, warmColdBusyNoEvict | ||
| } | ||
| if size := r.catalogSizeGBLocked(model); size > 0 && size*coldLoadCatalogGBToMemGiB > freeGB { |
There was a problem hiding this comment.
When EIGENINFERENCE_WARM_POOL_ALLOW_BUSY_LOAD_MAX admits a busy cold candidate, this check only compares padded weights against totalMemoryGB - gpuActiveGB. The provider load path requires requiredToLoadGb(weightsGb: ..., headroomGb: Self.loadHeadroomGb) and compares it to availableMemoryGb(), which also subtracts the unified-memory reserve and outstanding KV (provider-swift/Sources/ProviderCore/ProviderLoop+ModelLoading.swift:225-229 and :543-556). On a near-threshold busy box, especially when free_for_load_gb passed by assuming resident MLX memory is reclaimable, this can still issue load_model that the provider rejects, causing failed warms and pending-load cooldowns instead of selecting a truly loadable candidate.
Useful? React with 👍 / 👎.
2 participants
Summary
This PR makes the coordinator-B/open-pool layer safe to merge after PRs #523 and #524. It restores warm-pool recovery on lightly busy providers, adds an optional box-wide admission cap, makes runtime model shedding atomic across admission and queued work, keeps permanently undersized pools on the 503 path, and adds the operational telemetry and runbook needed for the v0.7.5 release train.
All new routing behavior is either default-preserving or explicitly operator-gated:
EIGENINFERENCE_WARM_POOL_ALLOW_BUSY_LOAD_MAXdefaults to0(the historical fully-idle rule).EIGENINFERENCE_COMBINED_ADMISSION_CAPdefaults tofalse.PUT /v1/admin/reject-modelsor the startup reject set is non-empty.Before
flowchart TB subgraph Behavior[Behavior] B1[Warm-pool tick] --> B2[Add coordinator pending and backend running/waiting] B2 --> B3[Mirrored work can exceed the busy limit] B3 --> B4[eligible_cold stays zero and MIN_WARM cannot recover] B5[Runtime shed PUT] --> B6[Admission set changes] B6 --> B7[Queued alias request can still dispatch] B8[Model A fills a box] --> B9[Independent per-model cap can still admit model B] end subgraph Code[Code] C1[warmPoolCandidateReasonLocked] --> C2[pendingCount plus backend slot load] C3[Server rejectModels map] --> C4[Queue sweep by concrete queue key] C5[hasConcurrencyHeadroomForModelCapResolvedLocked] --> C6[Per-model cap only] C7[HasCapableProviderForModel] --> C8[No cold hardware-fit check] endAfter
flowchart TB subgraph Behavior[Behavior] B1[Warm-pool tick] --> B2[Use max of coordinator and backend occupancy] B2 --> B3{No-eviction memory and resident-slot headroom?} B3 -- yes --> B4[MIN_WARM can load a lightly busy provider] B3 -- no --> B5[Fail closed without sending a doomed load] B6[Runtime shed PUT] --> B7[Atomically replace admission and queue snapshots] B7 --> B8[Matching alias/build waiters return model_shed 429] B9[Model A fills a box] --> B10[Optional combined load over quality-concurrency cap rejects B] end subgraph Code[Code] C1[Server.replaceRejectModels] --> C2[Registry.RejectQueuedRequestsForShedModels] C2 --> C3[RequestQueue.ReplaceShedModels] C3 --> C4[Enqueue / requeue / assignment share one locked snapshot] C4 --> C5[Queue state linearizes assignment vs timeout/cancel] C6[ProviderLoop.updateAggregateCapacity] --> C7[max_model_slots plus occupied_model_slots] C7 --> C8[warmPoolModelSlotHeadroomLocked] C9[combinedAdmissionHeadroomLocked] --> C10[Per-model solo rates plus pending-only cold models] C11[HasCapableProviderForModel] --> C12[Nonresident modelFitsHardware gate] endWhat Changed
max(coordinator pending, sum backend running+waiting), avoiding mirrored-request double counting. Busy candidates additionally require no-eviction memory fit and a v0.7.5 provider heartbeat proving a free model slot.max_model_slotsandoccupied_model_slots. Occupancy includes resident, unloading, actively loading, and distinct globally queued model loads. Legacy/unknown busy providers fail closed; fully idle warming remains backward compatible.ErrModelShedproduces a prompt 429 instead of a false queue timeout.environment=proddispatch as immediate active rollout.Validation
Candidate head:
5176e66acd coordinator && go test ./... -race -count=1- all 21 packages passed.cd coordinator && go vet ./...- passed.cd coordinator && go build ./cmd/coordinator- passed.cd provider-swift && swift test- 1,343 tests passed in 115 suites.git diff --check- passed.Fresh GitHub checks are required on this pushed head before merge.
Release And Deploy Sequence
master, close its review/CI gates, and merge it.master. Do not deploy an intermediate coordinator after either PR.master, close all one-engine v0.7.5 release blockers, and run the complete provider release gates.environment=devto validate the signed/notarized bundle, installer, update path, and owned-provider canary in pre-production.environment=prod). Either path registers an active release and begins fleet rollout immediately; monitor and stop/roll back on failed gates.No coordinator, provider release, tag, or production rollout is performed by this PR.