fix(npu): unblock async FFN teardown - #260
Open
ShwStone wants to merge 1 commit into
Open
Conversation
Idle CAM async FFN ranks wait in async_dispatch_recv during teardown. Closing HCCL while that receive is in flight can hang the worker and turn an otherwise successful E2E run into a cleanup failure. Use a full one-token CAM dispatch/combine shutdown handshake. Attention sends an out-of-range-layer marker and waits for the FFN dummy combine acknowledgement. FFN keeps its connector-driven loop alive despite a local shutdown event, acknowledges the marker without model execution, and exits only after the CAM transaction completes. Apply join-before-close only to CAMAsyncAFDConnector while preserving close-before-join for control-plane connectors. If the handshake or bounded daemon join fails, defer unsafe connector and model cleanup, and keep async shutdown errors observable. Add regression coverage for the wire marker, acknowledgement, shutdown races, connector-specific close ordering, timeout retry, and failure propagation. Document the teardown-only cost and removal path once CAM supports cancelling a pending receive. Fixes vllm-project#251 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: ShwStone <HaowenShi@outlook.com>
ShwStone
force-pushed
the
fix/npu-async-ffn-shutdown-sentinel
branch
from
August 20, 2026 06:30
e78c52e to
03b57b3
Compare
ShwStone
marked this pull request as ready for review
August 20, 2026 08:36
ShwStone
requested review from
hsliuustc0106 and
jiangkuaixue123
as code owners
August 20, 2026 08:36
jiangkuaixue123
left a comment
jiangkuaixue123
left a comment
Collaborator
There was a problem hiding this comment.
Overall, this PR introduces a significant amount of shutdown-specific logic into the runtime, including a custom sentinel protocol, additional state handling, and different cleanup paths for different connector types. The resulting lifecycle is fairly complex and may be difficult for future maintainers to understand and reason about.
Could we consider addressing this in the test runner or E2E test cleanup instead—for example, by tolerating or handling forced termination during teardown—rather than adding a new shutdown protocol to the production runtime? That may provide a simpler and more maintainable solution to the original test failure.
39 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
afd-eager-async-camafd-async-ubatchCloses #251