-
-
Notifications
You must be signed in to change notification settings - Fork 11.2k
Revert "[V1] Exception Handling when Loading KV Cache from Remote Store" #21778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
There was a problem hiding this 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 reverts the "Exception Handling when Loading KV Cache from Remote Store" feature due to compatibility issues. The changes correctly remove the RandomDropConnector test utility and all associated logic for fault-tolerant KV cache loading from the scheduler, workers, and various data structures.
My review of the changes confirms that the revert appears to be clean and complete. I did not find any remaining artifacts or new issues introduced by this change. This should effectively address the compatibility concerns mentioned in the pull request description.
0a8633c to
db93e20
Compare
njhill
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @KuntaiDu.
I agree this is a very high priority feature but we should take into account the overlapping existing work and make sure the right people have had a chance to comment.
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu>
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu> Signed-off-by: x22x22 <wadeking@qq.com>
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu>
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu>
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu> Signed-off-by: Jinzhen Lin <linjinzhen@hotmail.com>
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu> Signed-off-by: Noam Gat <noamgat@gmail.com>
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu> Signed-off-by: Paul Pak <paulpak58@gmail.com>
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu> Signed-off-by: Diego-Castan <diego.castan@ibm.com>
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu>
…re" (vllm-project#21778) Signed-off-by: KuntaiDu <kuntai@uchicago.edu>
Reverts #21534
This PR introduces fault tolerance to KV cache connector, which is very important feature to have. That said, I think this PR should be reverted because this PR, though has less core change, still contains some compatibility issue and need more feedbacks from community.
This PR is a consensus decision from #21534 .