[benchmarks]allow skip ready check for bench serve

[luccafong]

Summary: Allow skip ready check for bench serve through --ready-check-timeout-sec 0

Test Plan: vllm bench serve --ready-check-timeout-sec 0

Differential Revision: D82995002

[facebook-github-bot]

@luccafong has exported this pull request. If you are a Meta employee, you can view the originating diff in D82995002.

[gemini-code-assist]

Code Review

This pull request introduces a --skip-ready-check flag for the bench serve command, allowing users to bypass the initial endpoint readiness check. The implementation is clear and correctly adds the new command-line argument and conditional logic. I've added one comment regarding user-facing log messages to improve clarity when the check is skipped. Additionally, while the change is functionally correct, it would benefit from corresponding test cases to verify the new flag's behavior and prevent future regressions.

[yeqcharlotte]

we can achieve same purpose by setting --ready-check-timeout-sec=0?

[luccafong]

we can achieve same purpose by setting --ready-check-timeout-sec=0?

I think it will raise the error directly

[ywang96]

What's the use case for this? I actually found the wait_for_endpooint util pretty handy (now I don't need to fire vllm serve and vllm bench serve command sequentially)

[luccafong]

What's the use case for this? I actually found the wait_for_endpooint util pretty handy (now I don't need to fire vllm serve and vllm bench serve command sequentially)

when the workload is huge, and we don't want to wait for 1 request to completed.

[ywang96]

when the workload is huge, and we don't want to wait for 1 request to completed.

Ah ok - then I think it's probably better to modify wait_for_endpooint so that it pings /health instead of sending one dummy request. (and maybe we make another flag --do-validate-benchmark-dataset) Do you think that makes more sense?

I also don't have a strong opinion on this current PR so going to approve it.

[minosfuture]

when the workload is huge, and we don't want to wait for 1 request to completed.

Ah ok - then I think it's probably better to modify wait_for_endpooint so that it pings /health instead of sending one dummy request. (and maybe we make another flag --do-validate-benchmark-dataset) Do you think that makes more sense?

I also don't have a strong opinion on this current PR so going to approve it.

agree that pinging /health would be a better solution so we don't completely skip the readiness check (but just skip using single request test).

[ywang96]

when the workload is huge, and we don't want to wait for 1 request to completed.

Ah ok - then I think it's probably better to modify wait_for_endpooint so that it pings /health instead of sending one dummy request. (and maybe we make another flag --do-validate-benchmark-dataset) Do you think that makes more sense?
I also don't have a strong opinion on this current PR so going to approve it.

agree that pinging /health would be a better solution so we don't completely skip the readiness check (but just skip using single request test).

@minosfuture Feel free to make a follow-up PR!