[BugFix]Fix the issue of DCP and DP services getting stuck - #12036
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a service-stalling bug occurring during DCP (Data Parallel) prefill operations when using MTP. The issue stemmed from inconsistent execution counts between DP workers during the prefill phase. By enabling DCP prefill to execute the MTP step multiple times, the PR ensures synchronization across workers and resolves the communication deadlock. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. Tip 💡 Consider Linking a Related Issue or RFCYour PR title contains the [BugFix] tag, indicating a bug fix or new feature. Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:
🙏 Thanks for helping us keep the project well-organized! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Ops][BugFix] Fix MTP speculative decoding for DCP prefill and update accuracy testsSuggested PR Summary:
### What this PR does / why we need it?
This pull request introduces support for DCP (Decode Context Parallel) prefill batches in MTP (Multi-Token Prediction) speculative decoding. Specifically, it:
- Updates `pcp_utils.py` to prepare CP MTP metadata for both decode and DCP-prefill batches, and pre-allocates MTP slot mappings when DCP prefill slots are needed.
- Adjusts the early-exit condition in `llm_base_proposer.py` during prefill.
- Updates the end-to-end accuracy tests to use `DPVllmRunner` when data parallel is enabled.
Feedback and issues identified:
1. In `test_accuracy.py`, the condition `data_parallel_size > 2` will fail to use `DPVllmRunner` when `data_parallel_size` is exactly 2, causing the test to crash. This should be changed to `> 1`.
2. In `llm_base_proposer.py`, changing the early-exit condition to `self.pcp_size > 1 and is_prefill` introduces a hang risk in hybrid PCP/DCP configurations due to desynchronization between active prefill ranks and decode ranks.
3. In `pcp_utils.py`, restricting `needs_dcp_prefill_slots` to `self.pcp_world_size == 1` will cause failures in hybrid configurations.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Tested with end-to-end accuracy tests in `test_accuracy.py`.
|
/nightly multi-node-deepseek-r1-w8a8-longseq
|
|
/e2e tests/e2e/pull_request/four_card/context_parallel/test_accuracy.py::test_models_pcp_dcp_full_feature_accuracy |
|
/nightly multi-node-deepseek-r1-w8a8-longseq
|
|
/rerun Rerun:
|
|
/nightly multi-node-deepseek-r1-w8a8-longseq
|
Signed-off-by: weiguihua2 <weiguihua2@huawei.com>
Signed-off-by: weiguihua2 <weiguihua2@huawei.com>
Signed-off-by: weiguihua2 <weiguihua2@huawei.com>
…ect#12036) ### What this PR does / why we need it? When fixing the issue of dcp overlaying dp, a curl request causes the service to get stuck. When mtp=3 and there is only one request, for prefill, during the execution of mtp by dp0, only step0 is executed. The other two steps are copies of step0. dp1 performs a dummy run, and is_prefill is considered false, so it does not handle this special logic. As a result, all three steps are executed, leading to dp0 and dp1 executing a different number of times, ultimately causing the communication to get stuck. The current modification method is that dcp prefill will also execute the mtp step multiple times. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - vLLM version: v0.24.0 - vLLM main: vllm-project/vllm@85c09e9 --------- Signed-off-by: weiguihua2 <weiguihua2@huawei.com>
…ect#12036) ### What this PR does / why we need it? When fixing the issue of dcp overlaying dp, a curl request causes the service to get stuck. When mtp=3 and there is only one request, for prefill, during the execution of mtp by dp0, only step0 is executed. The other two steps are copies of step0. dp1 performs a dummy run, and is_prefill is considered false, so it does not handle this special logic. As a result, all three steps are executed, leading to dp0 and dp1 executing a different number of times, ultimately causing the communication to get stuck. The current modification method is that dcp prefill will also execute the mtp step multiple times. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - vLLM version: v0.24.0 - vLLM main: vllm-project/vllm@85c09e9 --------- Signed-off-by: weiguihua2 <weiguihua2@huawei.com>
…ect#12036) ### What this PR does / why we need it? When fixing the issue of dcp overlaying dp, a curl request causes the service to get stuck. When mtp=3 and there is only one request, for prefill, during the execution of mtp by dp0, only step0 is executed. The other two steps are copies of step0. dp1 performs a dummy run, and is_prefill is considered false, so it does not handle this special logic. As a result, all three steps are executed, leading to dp0 and dp1 executing a different number of times, ultimately causing the communication to get stuck. The current modification method is that dcp prefill will also execute the mtp step multiple times. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - vLLM version: v0.24.0 - vLLM main: vllm-project/vllm@85c09e9 --------- Signed-off-by: weiguihua2 <weiguihua2@huawei.com>
…ect#12036) ### What this PR does / why we need it? When fixing the issue of dcp overlaying dp, a curl request causes the service to get stuck. When mtp=3 and there is only one request, for prefill, during the execution of mtp by dp0, only step0 is executed. The other two steps are copies of step0. dp1 performs a dummy run, and is_prefill is considered false, so it does not handle this special logic. As a result, all three steps are executed, leading to dp0 and dp1 executing a different number of times, ultimately causing the communication to get stuck. The current modification method is that dcp prefill will also execute the mtp step multiple times. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - vLLM version: v0.24.0 - vLLM main: vllm-project/vllm@85c09e9 --------- Signed-off-by: weiguihua2 <weiguihua2@huawei.com>
…ect#12036) ### What this PR does / why we need it? When fixing the issue of dcp overlaying dp, a curl request causes the service to get stuck. When mtp=3 and there is only one request, for prefill, during the execution of mtp by dp0, only step0 is executed. The other two steps are copies of step0. dp1 performs a dummy run, and is_prefill is considered false, so it does not handle this special logic. As a result, all three steps are executed, leading to dp0 and dp1 executing a different number of times, ultimately causing the communication to get stuck. The current modification method is that dcp prefill will also execute the mtp step multiple times. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - vLLM version: v0.24.0 - vLLM main: vllm-project/vllm@85c09e9 --------- Signed-off-by: weiguihua2 <weiguihua2@huawei.com>
4 participants
What this PR does / why we need it?
When fixing the issue of dcp overlaying dp, a curl request causes the service to get stuck.
When mtp=3 and there is only one request, for prefill, during the execution of mtp by dp0, only step0 is executed. The other two steps are copies of step0. dp1 performs a dummy run, and is_prefill is considered false, so it does not handle this special logic. As a result, all three steps are executed, leading to dp0 and dp1 executing a different number of times, ultimately causing the communication to get stuck.
The current modification method is that dcp prefill will also execute the mtp step multiple times.
Does this PR introduce any user-facing change?
No
How was this patch tested?