[None][fix] Prevent hang in disagg gen-only benchmark when KV cache is insufficient - #12157
[None][fix] Prevent hang in disagg gen-only benchmark when KV cache is insufficient#12157qiaoxj07 wants to merge 3 commits into
Conversation
…s insufficient In gen-only benchmark mode, the GEN server waits for benchmark_req_queues_size generation requests to be scheduled before starting the forward pass. If the KV cache cannot hold that many requests, the scheduled count plateaus and the loop hangs forever. Detect KV cache saturation by tracking the scheduled gen count across iterations. After 10 consecutive iterations with no increase, lower benchmark_req_queues_size to the actual scheduled capacity and proceed. This applies to both the attention-DP (allgathered total) and non-attention-DP paths. Signed-off-by: Xianjie <5410381+qiaoxj07@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds KV-cache saturation detection to the executor loop with tracking variables (prev_gen_count, stall_count, max_stall_iterations) that identify consecutive iterations where scheduled generation count stagnates. Upon saturation detection, the benchmark_req_queues_size is dynamically lowered to the observed total_gen_count and forwarding is allowed. This logic is applied consistently across two codepaths in the _executor_loop_overlap method. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run |
|
PR_Github #38713 [ run ] triggered by Bot. Commit: |
There was a problem hiding this comment.
Can we have a test to verify gen-only benchmarking? It seems like we've had a few issues not caught in pre-merge lately. Thanks.
@Tabrizian
There was a problem hiding this comment.
I only have a test case for R1, but I think It's too large to putinto pre-merge.
Is there any other solution?
There was a problem hiding this comment.
Had a chat with @chienchunhung and he kindly agreed to add a test for this.
|
PR_Github #38713 [ run ] completed with state
|
…stall helper Deduplicate the KV cache saturation detection code that was repeated in both the attention-DP and non-attention-DP branches of _executor_loop_overlap. No behavior change. Signed-off-by: Xianjie <5410381+qiaoxj07@users.noreply.github.com>
|
/bot run |
|
PR_Github #38828 [ run ] triggered by Bot. Commit: |
|
PR_Github #38827 Bot args parsing error: usage: /bot [-h] |
|
PR_Github #38828 [ run ] completed with state
|
|
/bot run |
|
PR_Github #38878 [ run ] triggered by Bot. Commit: |
|
PR_Github #38878 [ run ] completed with state
|
|
/bot run |
|
PR_Github #38929 [ run ] triggered by Bot. Commit: |
|
PR_Github #38929 [ run ] completed with state |
|
Closed, another PR will fix this issue. |
4 participants
Summary
benchmark_req_queues_sizegeneration requests to be scheduled before starting the forward pass. If the KV cache cannot hold that many requests, the scheduled count plateaus and the wait loop hangs forever (sleeping 10s indefinitely).benchmark_req_queues_sizeto the actual scheduled capacity and proceed with the forward pass.total_gen_count) and the non-attention-DP path (scheduled_batch.num_generation_requests).Test plan
TLLM_BENCHMARK_REQ_QUEUES_SIZEset higher than available KV cache capacity and verify the server proceeds instead of hanging🤖 Generated with Claude Code
Summary by CodeRabbit