Skip to content

[Benchmark] Fix client seed synchronization in multi-turn benchmark#28512

Merged
DarkLight1337 merged 1 commit intovllm-project:mainfrom
ai-jz:fix/benchmark-client-seed-upstream
Nov 16, 2025
Merged

[Benchmark] Fix client seed synchronization in multi-turn benchmark#28512
DarkLight1337 merged 1 commit intovllm-project:mainfrom
ai-jz:fix/benchmark-client-seed-upstream

Conversation

@ai-jz
Copy link
Contributor

@ai-jz ai-jz commented Nov 12, 2025

Problem

All clients in multi-turn benchmark use the same seed value, causing them to generate identical random sequences. This leads to synchronized behavior where all clients sleep for the same duration and make requests at the same time, defeating the purpose of Poisson-distributed request timing.

Evidence

Real logs showing the synchronization issue:

10-11-2025 23:43:34 [INFO] - Client 60 will use conversation ID CONV_ID_60 (active conversations 1)
10-11-2025 23:43:34 [INFO] - Client 61 will use conversation ID CONV_ID_61 (active conversations 1)
10-11-2025 23:43:34 [INFO] - Client 62 will use conversation ID CONV_ID_62 (active conversations 1)
10-11-2025 23:43:34 [INFO] - Client 63 will use conversation ID CONV_ID_63 (active conversations 1)
10-11-2025 23:43:44 [INFO] - Sleeping for 0.796 seconds...
10-11-2025 23:43:44 [INFO] - Sleeping for 0.796 seconds...
10-11-2025 23:43:44 [INFO] - Sleeping for 0.796 seconds...
10-11-2025 23:43:54 [INFO] - Sleeping for 0.796 seconds...

All clients sleep for exactly 0.796 seconds because they use the same random seed.

Solution

Add client_id to the seed value to ensure each client has a unique but reproducible random sequence:

# Before
random.seed(args.seed)
np.random.seed(args.seed)

# After
client_seed = args.seed + client_id + 1
random.seed(client_seed)
np.random.seed(client_seed)

Impact

  • Each client now generates different random intervals
  • Request timing is properly distributed according to Poisson process
  • Benchmark results will be more realistic
  • Results remain reproducible with the same seed and client configuration

@mergify mergify bot added the performance Performance-related issues label Nov 12, 2025
@ai-jz ai-jz marked this pull request as draft November 12, 2025 04:08
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly fixes a critical flaw in the multi-turn benchmark where clients were synchronized due to using the same random seed. The implemented solution of creating a unique seed for each client based on its ID is a good step forward. My review includes one suggestion to further improve the statistical robustness of the benchmark by using a more advanced seeding strategy (numpy.random.SeedSequence) designed for parallel applications. This would ensure the random behavior of clients is not just unique but also statistically independent, which is crucial for the validity of the benchmark results.

@ai-jz ai-jz marked this pull request as ready for review November 12, 2025 04:22
@ai-jz
Copy link
Contributor Author

ai-jz commented Nov 13, 2025

@DarkLight1337 could you have a look?

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) November 13, 2025 03:54
@DarkLight1337
Copy link
Member

Nice catch

@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 13, 2025
All clients were using the same seed, causing them to generate identical
random intervals for request timing. This led to synchronized behavior
where all clients would sleep for the same duration.

Fix by setting unique seed per client: args.seed + client_id + 1
The +1 ensures no client uses the same seed as the main process.

Since each client runs in its own process (via multiprocessing.Process),
each has its own RNG state, so we just need to seed them differently.

Simple 2-line change:
- random.seed(args.seed + client_id + 1)
- np.random.seed(args.seed + client_id + 1)

Signed-off-by: ai-jz <aijz.xplr@gmail.com>
auto-merge was automatically disabled November 16, 2025 05:09

Head branch was pushed to by a user without write access

@ai-jz ai-jz force-pushed the fix/benchmark-client-seed-upstream branch from fd3bc12 to faf52f6 Compare November 16, 2025 05:09
@ai-jz
Copy link
Contributor Author

ai-jz commented Nov 16, 2025

@DarkLight1337 I updated the commit to include the DCO sign-off, which seems a new requirement of vLLM.

This update has no code change - only the commit message was amended with "Signed-off-by".

@DarkLight1337 DarkLight1337 merged commit d231876 into vllm-project:main Nov 16, 2025
15 checks passed
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
kitaekatt pushed a commit to kitaekatt/vllm that referenced this pull request Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance-related issues ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants