Skip to content

[Bugfix][Rust Frontend] Rust vllm-bench accepts --max-concurrency 0 and hangs forever (Python rejects it since #54887) - #64390

Merged
esmeetu merged 3 commits into
vllm-project:mainfrom
SIDDARTHAREDDY8:oss-bhai-2026-10-28-burst-2
Oct 29, 2026
Merged

esmeetu merged 3 commits into
vllm-project:mainfrom
SIDDARTHAREDDY8:oss-bhai-2026-10-28-burst-2

Conversation

@SIDDARTHAREDDY8

@SIDDARTHAREDDY8 SIDDARTHAREDDY8 commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #64319.

Rust vllm-bench accepted --max-concurrency 0 and then hung forever on Semaphore::new(0), while Python vllm bench serve rejects 0 since #54887. This change rejects 0 fail-fast on every Rust bench path that builds a concurrency semaphore:

  • Serve path (config.rs): --max-concurrency 0 and --multi-turn-concurrency 0 are rejected with a clear error instead of hanging.
  • Sweep path (sweep.rs): any 0 in --sweep-max-concurrency is rejected before the first sweep point runs.
  • mm-processor path (mm_processor.rs): --max-concurrency 0 is rejected at the entry point, before the managed engine is spawned, since this subcommand does not go through BenchConfig::from_args.

Why this is not duplicating an existing PR: a search for open PRs mentioning #64319 returns only this PR, and #64319 itself has no linked PR.

Tests run:

  • cargo test -p vllm-bench --lib: 191 passed, 0 failed, 12 ignored.
  • cargo test -p vllm-bench --lib concurrency: 8 passed, 0 failed (the zero/positive max-concurrency, multi-turn, and sweep cases).
  • rustfmt clean under the workspace rustfmt.toml (style edition 2024, chain width 80).

No output, accuracy, or serving behavior changes, so no model evaluation results apply. No new CLI args or defaults, so no documentation update is needed.

AI-assistance disclosure: AI assistance was used in preparing this change.

Signed-off-by: SIDDARTHA REDDY <75976672+SIDDARTHAREDDY8@users.noreply.github.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added rust bug Something isn't working labels Oct 29, 2026
@esmeetu

esmeetu commented Oct 29, 2026

Copy link
Copy Markdown
Member

Fix looks correct. Two requests before merge:
(1) mm-processor --max-concurrency 0 still hits Semaphore::new(0) at mm_processor.rs:243. Please cover it too, or use a clap value_parser!(usize).range(1..) on the concurrency args.
(2) Please trim the explanatory comments to match the surrounding validation code, and rewrite the PR description per AGENTS.md: a human-written summary, the tests run, and an explicit AI-assistance disclosure.
Please also use the [Bugfix][Rust Frontend] title prefix.

…natory comments

Signed-off-by: SIDDARTHA REDDY <75976672+SIDDARTHAREDDY8@users.noreply.github.com>
@SIDDARTHAREDDY8 SIDDARTHAREDDY8 changed the title Fix: [Bug]: Rust vllm-bench accepts --max-concurrency 0 and hangs forever (Python rejects it since #54887) [Bugfix][Rust Frontend] Rust vllm-bench accepts --max-concurrency 0 and hangs forever (Python rejects it since #54887) Oct 29, 2026
@SIDDARTHAREDDY8

Copy link
Copy Markdown
Contributor Author

Addressed both requests in commit edc704d:

(1) mm-processor is now covered: run_mm_processor rejects --max-concurrency 0 with --max-concurrency must be greater than 0 before the managed engine is spawned. I used a fail-fast check rather than a clap range(1..) parser because this subcommand does not go through BenchConfig::from_args, and it matches the validation style already used in this PR.

(2) Trimmed the explanatory comments in config.rs and sweep.rs to match the surrounding validation code, and rewrote the PR description with a summary, the tests run, and an explicit AI-assistance disclosure. The title now uses the [Bugfix][Rust Frontend] prefix.

Verification: cargo test -p vllm-bench --lib passes (191 passed, 0 failed, 12 ignored) and rustfmt is clean.

@esmeetu

esmeetu commented Oct 29, 2026

Copy link
Copy Markdown
Member

/ci run

@esmeetu esmeetu added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 29, 2026
@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #98247 for commit 6980dba2fb81.

ℹ️ This PR is 1239 commits behind upstream main. Merged upstream main into this branch as 6980dba2fb81 before starting CI. Pull the branch before pushing more changes.

@github-actions

Copy link
Copy Markdown

✅ @SIDDARTHAREDDY8, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Run commands test your branch with every commit currently on its upstream target branch. If it is behind, the bot merges the target branch into it first (pull before pushing again). Append --allow-stale to a run command to test an outdated branch as is, at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@esmeetu
esmeetu merged commit a1e2127 into vllm-project:main Oct 29, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed rust

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Rust vllm-bench accepts --max-concurrency 0 and hangs forever (Python rejects it since #54887)

3 participants