[CI/Build][MiniCPM-o] Derive duplex admission-probe limit from the deploy config - #6678
Conversation
…ploy config test_duplex_admission_and_expiry_reaper hardcoded verify_admission_limit=2, so the admission probe opened 2 sessions and waited for the 3rd to be rejected. PR vllm-project#6619 repointed this test from minicpmo_4_5_duplex.yaml, which sets duplex_session.max_sessions to 2, to the shipping minicpmo_4_5.yaml, which sets it to 4. With room for 4 the 3rd session is admitted, no error event is emitted, and _receive_until blocks until the timeout: FAILED tests/e2e/online_serving/test_minicpmo_4_5_duplex_expansion.py::test_duplex_admission_and_expiry_reaper[three-stage-single-gpu] - TimeoutError Read the capacity from the deploy config the test starts the server with, so the limit lives in one place and a later capacity change cannot turn into an unrelated admission-probe timeout. Fixes vllm-project#6671 Signed-off-by: Deep Shah <deep@socratic.co>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to be related to model: minicpm. Model owners: @y-null @dshah1333, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
|
Approve with a couple of should-fixes. The timeout in #6671 is the probe, not the server. Please load the same object the server uses. Also add a CPU unit test that does not boot a server: |
Export the new accessor and raise on a declared but invalid duplex_session.max_sessions instead of silently falling back to the runtime default, which would test a limit the server never applies. Signed-off-by: Deep Shah <deep@socratic.co>
686a3a8 to
43894b8
Compare
| def get_deploy_duplex_max_sessions(rel_path: str, default: int = 1) -> int: | ||
| """Return ``duplex_session.max_sessions`` from a deploy yaml. | ||
|
|
||
| ``default`` mirrors ``DuplexSessionConfig.max_sessions`` so a deploy config |
There was a problem hiding this comment.
maybe DuplexSessionRuntimeConfig rather than DuplexSessionConfig?
…loader Review follow-up. get_deploy_duplex_max_sessions re-implemented defaulting on top of a raw safe_load, which matches minicpmo_4_5.yaml but not a config that inherits capacity from its base_config. Load through load_deploy_config so base_config merging and DuplexSessionRuntimeConfig defaults stay in lockstep with the server. Add a CPU test over the shipped deploy configs covering a declared capacity, a config that declares none, and an overlay inheriting one from its base, so a capacity edit surfaces there instead of as another nightly admission timeout. Signed-off-by: Deep Shah <deep@socratic.co>
43894b8 to
ce23fca
Compare
|
LGTM |
|
Self-review:
|
Purpose
Fixes #6671.
test_duplex_admission_and_expiry_reapertimes out on both CUDA and NPU:My read is the server is fine here and the test just carries a stale number.
The test hardcodes
verify_admission_limit = 2, so the probe opens 2 sessions and waits for the 3rd to be rejected. #6619 repointed this test fromminicpmo_4_5_duplex.yaml, which setsduplex_session.max_sessionsto 2, to the shippingminicpmo_4_5.yaml, which sets it to 4. With room for 4 the 3rd session is admitted, no error event ever arrives, and_receive_untilblocks until the timeout.The capacity flows deploy yaml ->
omni_config.py:1424->serving.py:916->duplex_session.py:404, and that last one only rejects oncelen(_sessions) >= max_sessions.So this reads the limit from the deploy config the test starts the server with instead of hardcoding it. The number then lives in one place and a later capacity change can't turn into an unrelated probe timeout.
Test Plan
vLLM Version: 0.27.0
vLLM-Omni Commit: a57246e (see the note under Test Result)
Served MiniCPM-o 4.5 on 1x H100 80GB from the unmodified shipping yaml, then ran the test's own
run_lifecycle_probesadmission probe at both limits:Test Result
TimeoutError, same as #6671ok=True,overflow_error_code=resource_exhaustedServer log at the configured capacity:
ruff checkandruff format --checkpass on the 3 changed files.Two notes on how I validated this:
duplex_session.pyis identical between that commit and main, and the shipping yaml hasmax_sessions: 4in both, so I think the result carries over. Happy to rerun on main if you'd rather see that.