Skip to content

Remove --disable-log-requests from vLLM benchmark scripts#1371

Merged
Oseltamivir merged 1 commit into
mainfrom
fix/vllm-disable-log-requests
May 13, 2026
Merged

Remove --disable-log-requests from vLLM benchmark scripts#1371
Oseltamivir merged 1 commit into
mainfrom
fix/vllm-disable-log-requests

Conversation

@Oseltamivir
Copy link
Copy Markdown
Collaborator

Summary

  • Newer vLLM releases removed --disable-log-requests (request logging is now off by default; opt in with --enable-log-requests). Passing the old flag fails immediately with vllm: error: unrecognized arguments: --disable-log-requests (example CI failure).
  • Drops the flag from the four remaining single-node benchmark scripts: gptoss_fp4_b200.sh, kimik2.5_int4_b200.sh, kimik2.5_int4_h200.sh, minimaxm2.5_fp8_mi300x.sh.
  • Behavior matches the prior intent — logging stays off, no opt-in added.

Test plan

  • Re-run the failing benchmark job(s) and confirm vllm serve starts without the argparse error.
  • Spot-check server logs to confirm per-request logging is not emitted.

Out of scope

  • perf-changelog.yaml mentions of the flag are historical PR descriptions — left untouched.
  • utils/bench_serving/benchmark_serving.py docstring still references the flag; it is vendored upstream vLLM code, left to avoid future merge conflicts.

The flag was removed in newer vLLM releases (logging is off by default;
opt in with --enable-log-requests). Leaving it triggers
"unrecognized arguments: --disable-log-requests" at server start.
@github-actions
Copy link
Copy Markdown
Contributor

Thanks for the contribution! For vLLM & SGLang, please ensure that your recipes is similar to the official vLLM recipes and/or the SGLang cookbook

If it is not, please create a PR first before we can merge your single node PR into the master branch. Let's ensure that the documentation is first class such that the entire ML community can benefit from your hard work! Thank you

PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. If re-running failed jobs is attempted, PR authors are responsible for ensuring it passes. See GitHub's docs on re-running failed jobs: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs#re-running-failed-jobs-in-a-workflow

As a rule of thumb, generally, PR authors should request a review & get a PR approval from the respective companies' CODEOWNERS before requesting a review from core maintainers.

If additional help is needed, PR authors can reach out to core maintainers over Slack.

@Oseltamivir Oseltamivir merged commit 0552ead into main May 13, 2026
5 checks passed
@Oseltamivir Oseltamivir deleted the fix/vllm-disable-log-requests branch May 13, 2026 18:21
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Mechanical removal of deprecated vLLM flag from four benchmark scripts to fix CI argparse errors.

Extended reasoning...

Overview

This PR removes the --disable-log-requests flag from four single-node vLLM benchmark scripts (gptoss_fp4_b200.sh, kimik2.5_int4_b200.sh, kimik2.5_int4_h200.sh, minimaxm2.5_fp8_mi300x.sh). Newer vLLM releases removed this flag (request logging is now off by default, with opt-in via --enable-log-requests), so passing it causes vllm serve to fail at argparse. The behavior intent (logging stays off) is preserved.

Security risks

None. These are benchmark shell scripts that only modify vLLM server CLI arguments. No auth, crypto, network exposure, or data-handling logic is touched.

Level of scrutiny

Very low. This is a purely mechanical fix that follows the same pattern as the recent commit e03a0e5 ("Remove --disable-log-requests from vLLM single-node benchmark scripts"), which addressed other scripts. The diff is four identical line removals.

Other factors

No bugs were flagged by the bug hunting system. The PR description explains the out-of-scope items (vendored upstream code and historical changelog entries are intentionally left alone), which is the correct call. CI re-running the failing benchmark jobs will validate the fix.

functionstackx pushed a commit that referenced this pull request May 17, 2026
…or --disable-log-requests removal

The original commit set the image to v0.20.2 instead of v0.21.0.
Merging main picks up commit 0552ead which removed the deprecated
--disable-log-requests flag from the benchmark script (PR #1371).
Both issues caused the CI sweep to fail with 'unrecognized arguments'.
functionstackx added a commit that referenced this pull request May 18, 2026
* $Update minimaxm2.5-fp8-mi300x-vllm vLLM image to v0.20.2\n\nRef #1154\n\nCo-authored-by: Klaud Cold <Klaud-Cold@users.noreply.github.com>

* fix(minimaxm2.5-fp8-mi300x-vllm): bump image to v0.21.0, merge main for --disable-log-requests removal

The original commit set the image to v0.20.2 instead of v0.21.0.
Merging main picks up commit 0552ead which removed the deprecated
--disable-log-requests flag from the benchmark script (PR #1371).
Both issues caused the CI sweep to fail with 'unrecognized arguments'.

---------

Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: claude-fix-bot <claude-fix-bot@local>
Co-authored-by: functionstackx <47992694+functionstackx@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant