feat(proxy): congestion-aware per-API-key fair-share stream admission - #1536
Open
Soju06 wants to merge 2 commits into
Open
feat(proxy): congestion-aware per-API-key fair-share stream admission#1536Soju06 wants to merge 2 commits into
Soju06 wants to merge 2 commits into
Conversation
One bursty API key could exhaust the pool-wide per-account stream slots and starve every other key (issue #1535). Add a work-conserving max-min fair-share gate on stream-lease admission: below the configured pool congestion threshold admission is unchanged; at or above it, a key may take a new stream only while it holds fewer than max(2, pool_capacity // active_keys) streams, so light keys are starvation-proof and over-share keys cannot reacquire freed slots until they drop back under their share. Denials surface the new stable local-overload reason api_key_stream_fair_share and inherit the existing capacity-wait park-and-retry plus 429 + Retry-After semantics. Leases are attributed to API keys on RuntimeState under the existing runtime lock, scoped to the selection's candidate accounts. Default off via the new proxy_api_key_fair_share_congestion_threshold_pct setting (env + nullable dashboard override, 0-100, 0 disables). Fixes #1535 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cess mode Inflight uses live-sum aggregation across sibling workers, and each worker's independent lease counters can admit its own pool capacity, so exporting capacity with livemax made the utilization ratio incomparable (two workers at 6/8 read as 12/8). Use the same live-sum semantics for the capacity gauge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5853e543f
ℹ️ 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".
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
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.
Summary
Adds a congestion-aware per-API-key fair-share gate on stream-lease admission, so one bursty key can no longer exhaust the pool-wide per-account stream slots and starve every other key.
Fixes #1535
pool_inflight * 100 >= pool_capacity * threshold_pct, integer math over the selection's candidate account set): a key admits a new stream only whilekey_inflight + 1 <= max(2, pool_capacity // active_keys). Keys holding fewer than two streams can never be denied (light interactive keys are starvation-proof); over-share keys cannot reacquire freed slots until they drop back under, so freed capacity flows to the keys the congestion was starving.api_key_stream_fair_shareregistered in_LOCAL_ACCOUNT_CAP_ERROR_CODESandLOCAL_OVERLOAD_CODES: transport parks withwaiting_for_account_capacitykeepalives and re-runs selection (each retry re-evaluates the gate with fresh counters), then 429rate_limit_error+Retry-Afteron budget exhaustion. Zero new transport code. Deliberately absent from the bridge soft-affinity reroute set — the gate is per-key and pool-wide, so rerouting to another account cannot help.AccountLeasecarriesapi_key_id;RuntimeState.stream_key_inflightmaintains per-account per-key counts under the existing runtime lock across acquire/release/stale-reclaim (entries deleted at zero, pruned with account runtime state). Candidate-set scoping means account-scoped keys are measured against the pool they can actually use.proxy_api_key_fair_share_congestion_threshold_pct(env + nullable dashboard override with null-inherits-env, 0-100, default 0 = off), surfaced next to the per-account capacity limits in routing settings (en/ko/zh-CN).codex_lb_stream_pool_capacity/codex_lb_stream_pool_inflight(the previously missing pool numerator/denominator), countercodex_lb_api_key_fair_share_rejections_total, and a warning log with the denial numbers. No per-key metric labels by design (cardinality); the log carriesapi_key_id.Production motivation
Sanitized from the deployment in #1535: one agentic fan-out key held ~30-36 concurrent streams (91 live upstream streams, ~87% of a 13-account × 8-slot pool), one account additionally hit a genuine upstream 429, and every remaining candidate then filtered at
account_stream_cap— selection degraded tono_accountsand all keys, including interactive ones holding 0-2 streams, received sustained 503 bursts (20-60% of requests per minute) while client retries amplified the load.Simplicity gates (P1/P2)
0disables; the gate short-circuits before any counting work).MAX_SETTINGS_FIELDS115 → 116): the congestion threshold cannot be a hardcoded default — the right value depends on pool size and workload mix, and0is the P1 off switch. The companion minimum-guarantee constant (2 streams) stayed hardcoded rather than becoming a second setting.OpenSpec
openspec/changes/add-api-key-stream-fair-share/— proposal, design (D1-D9), tasks, and deltas forproxy-admission-control,proxy-runtime-observability,frontend-architecture.openspec validate add-api-key-stream-fair-share --strictpasses.Testing
tests/unit/test_fair_share.py(pure math: thresholds incl. the>=boundary, capacity floors, min-guarantee dominance, divisor counting, message content).tests/unit/test_load_balancer_concurrency.py: per-key lease lifecycle (acquire/release/stale-reclaim/delete-at-zero), default-off characterization, congested deny-heavy/admit-light, re-admission after release, keyless bypass with pool counting, response-create non-interference, sticky-path denial.tests/unit/test_load_balancer_contract.py: unchanged defaults + denial shape.Screenshots
Local verification
make lint/make typecheck/ architecture ratchets: green (select_accountandload_balancer.pystay inside their line budgets; new math lives infair_share.py).codex review --base origin/main: one P2 (multiprocess gauge aggregation) — fixed in the second commit; re-review clean.🤖 Generated with Claude Code