Skip to content

docs: explain and surface the prometheus-budget closing point - #374

Merged
shimib merged 2 commits into
mainfrom
fix/budget-gate-max-concurrency
Jul 29, 2026
Merged

docs: explain and surface the prometheus-budget closing point#374
shimib merged 2 commits into
mainfrom
fix/budget-gate-max-concurrency

Conversation

@shimib

@shimib shimib commented Jul 29, 2026

Copy link
Copy Markdown
Member

Fixes #363

max_concurrency on the prometheus-budget gate is a per-ready-pod divisor. The gate closes only once load reaches:

ready_pods × max_concurrency × (1 − baseline)

Nothing said that, and nothing reported the resolved number. A max_concurrency the pool can never reach leaves the gate permanently open — every batch request dispatches regardless of live traffic, which is the exact failure the gate exists to prevent — and it fails silently.

What changed

Surface the closing point. CreateGate now logs it when it builds a prometheus-budget gate:

"prometheus-budget gate configured" pool=optimized-baseline maxConcurrency=100 baseline=0.05 closesAtLoadPerReadyPod=95

That is the "startup log line reporting the resolved closing point" the issue asks for. It uses the per-pod form because ready_pods is a live Prometheus reading, not known at construction time — but per-pod is the number you compare against vllm:num_requests_running / ready_pods, so it is the actionable half.

Document it as per-pod capacity, in three places that all previously described it as an opaque constant:

  • the CreateGate doc comment,
  • a new Sizing max_concurrency subsection in the README's prometheus-budget reference,
  • a new Size max_concurrency for your pool section in the e2e guide, with a kubectl logs | grep for the line above and a PromQL query for deriving the value from your own peak.

Two ways to pick a value are documented: match the EPP saturation detector's MaxConcurrency (so the async gate and the EPP agree on when the pool is full), or measure the per-pod peak of vllm:num_requests_running under the load you consider saturated.

On the shipped value

The issue offers two fixes — drop the value, or "better, document it … and tell the reader to derive it". I took the second, and deliberately left max_concurrency: "100" in docs/guides/e2e-deploy/llm-d-async-values.yaml, because on the setup the guide actually deploys the value is reachable:

  • The guide deploys a single Qwen3-0.6B replica on 1× A100, not the 8B/2-pod pool the finding was measured on. Closing point is 95 concurrent on one pod.
  • The guide's own saturation test drives it there. hey with 200 workers, and the guide already records Expected: vllm:num_requests_running = 200 (saturated) and Expected: value = -1 (200 running / 100 max). guidellm reaches ~110. Both close the gate today.
  • 100 matches the EPP saturation detector default, and the llm-d optimized-baseline router recipe does not configure the detector — so the two components currently agree. Changing one side to 8 would desynchronise them.

Dropping it to 8 would therefore falsify the guide's own stated expectations and the * 100 divisors in its verification queries, to fix a mis-sizing that only appears once the reader swaps in a different model. The real defect is that the guide presents the number with no indication that it is per-pod or that it must be re-derived — which is what this PR fixes, in the values file comment and the new section. Happy to lower it as well if you'd rather the shipped default err small; that is a one-line change plus the expected values in the load-test sections.

Note on the "no metric" half of the finding

Once #372 lands, async_gate_metric_value / async_gate_metric_threshold expose the live D and the baseline as a joinable pair, so an unreachable threshold is visible as a series that never approaches its threshold. This PR covers the log-line half.

Verification

The Go change is one log.Log.WithName(...).Info(...) call using the package-level controller-runtime logger already used elsewhere in this package (metric_dispatch_gate.go, cascade_metric_source.go) and in pkg/redis/quota_gate.go. No API change, no behavior change to gate evaluation. The values file was re-parsed to confirm the comment-only edit leaves gate_params byte-identical: {"pool"=>"optimized-baseline", "max_concurrency"=>"100", "baseline"=>"0.05"}.

shimib added a commit that referenced this pull request Jul 29, 2026
Signed-off-by: Shimi Bandiel <shimib@google.com>
shimib added 2 commits July 29, 2026 14:09
max_concurrency is a per-ready-pod divisor: the prometheus-budget gate
closes only once load reaches ready_pods * max_concurrency * (1 -
baseline). Nothing said so, and nothing reported the resolved number,
so a max_concurrency the pool can never reach left the gate
permanently open with every batch request dispatching regardless of
live traffic -- silently, which is the exact failure the gate exists
to prevent.

Log the resolved closing point when the gate is built:

  "prometheus-budget gate configured" pool=... maxConcurrency=100
  baseline=0.05 closesAtLoadPerReadyPod=95

Document max_concurrency as per-pod capacity in the README, the
CreateGate doc comment, and the e2e guide, with two ways to pick a
value: match the EPP saturation detector's MaxConcurrency so the async
gate and EPP agree on when the pool is full, or measure the per-pod
peak of vllm:num_requests_running under the load you consider
saturated.

The guide keeps max_concurrency: "100". It is reachable in the setup
the guide actually deploys -- a single Qwen3-0.6B replica driven by
the guide's own 200-worker load test, which the guide already records
as reaching 200 concurrent -- and it matches the EPP default that the
llm-d optimized-baseline recipe leaves unset. Lowering it would
contradict the expected values the guide states. What was missing was
any indication that the number is per-pod and must be re-derived for a
different model, pool size, or workload, so that is what this adds, in
the values file and in a new "Size max_concurrency for your pool"
section.

Fixes #363

Signed-off-by: Shimi Bandiel <shimib@google.com>
Signed-off-by: Shimi Bandiel <shimib@google.com>
@shimib
shimib force-pushed the fix/budget-gate-max-concurrency branch from 20f8375 to 9cc87da Compare July 29, 2026 21:14
@shimib
shimib merged commit 6f93465 into main Jul 29, 2026
8 checks passed
@shimib
shimib deleted the fix/budget-gate-max-concurrency branch July 29, 2026 21:50
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.

docs/guides/e2e-deploy.md ships a max_concurrency the gate can never reach

2 participants