Incorporate PollSleepPolicy into CompletionQueueActor (#4736) - #4736
Open
samlurye wants to merge 4 commits into
Open
Incorporate PollSleepPolicy into CompletionQueueActor (#4736)#4736samlurye wants to merge 4 commits into
samlurye wants to merge 4 commits into
Conversation
Contributor
|
@samlurye has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117269425. |
samlurye
force-pushed
the
export-D117269425
branch
from
August 25, 2026 00:40
6c50252 to
e476586
Compare
samlurye
force-pushed
the
export-D117269425
branch
from
August 25, 2026 00:45
e476586 to
1455480
Compare
samlurye
force-pushed
the
export-D117269425
branch
from
August 26, 2026 20:46
1455480 to
c3018e5
Compare
samlurye
force-pushed
the
export-D117269425
branch
from
August 26, 2026 20:46
c3018e5 to
144886b
Compare
samlurye
added a commit
to samlurye/monarch-1
that referenced
this pull request
Aug 26, 2026
) Summary: Pull Request resolved: meta-pytorch#4736 Move `PollSleepPolicy` into `CompletionQueueActor`. This is the same logic that `QueuePairActor` uses to determine how long to wait before the next poll. Reviewed By: zdevito Differential Revision: D117269425
samlurye
force-pushed
the
export-D117269425
branch
from
August 26, 2026 20:50
144886b to
4b131d9
Compare
Summary: Introduce the `IbvCompletionQueue` trait, which will wrap real hardware-backed completion queue. The upcoming `CompletionQueueActor` will be generic over `IbvCompletionQueue`, allowing the batched polling logic to be tested without real hardware. Reviewed By: zdevito Differential Revision: D116689368
Summary: Add `CompletionQueueActor`: one poller for many CQs, holding a route from each queue pair's completions back to that queue pair. Each CQ actor starts empty. When a new QP starts up, the manager actor will (eventually, not yet implemented) assign a CQ and, independently, a CQ actor to that QP according to some user configured policy. This design allows us to try different concurrency models (e.g., one CQ actor polls every CQ in the process vs. one CQ actor for every NIC polls every CQ for that specific NIC). When the QP is ready to start posting, it sends an `Attach` message to the assigned CQ actor with: the CQ to poll; an atomic counter indicating how many WRs that QP has posted so far; and a port for sending batches of completions back to the QP. The CQ actor does not poll unless there is work to be done. QPs wake it by sending `Posted` notifications when they have posted new WRs. Between yields, the CQ actor loop polls every CQ that it tracks, requesting 64 CQEs at a time from each CQ. When it consumes CQEs, it routes them back to the relevant QP so that it can update its credit tracking and send replies to the original caller. QP detaching logic and other failure handling is left for future commits. Nothing uses this code yet. Reviewed By: zdevito Differential Revision: D116806594
…-pytorch#4719) Summary: When a QP actor stops, the safest approach is to wait for all pending WRs to drain before actually destroying it. In order to not introduce an async dependency into a QP's teardown, we instead hand the underlying resources and CQ lease to the CQ actor via `Detach` when the QP actor drops. The CQ actor keeps the QP's resources and lease alive until all pending WRs have drained. This is important for correctness: we do not want the dead QP's qp_num to be reassigned until we are sure it will produce no more CQEs; similarly, we cannot safely drop its CQ lease until we are sure it will produce no more CQEs. It is also useful for reasoning about behavior: different RDMA providers treat pending WRs and their CQEs differently when the associated QP is destroyed. Keeping the QP alive until all CQEs have drained means we can have one uniform implementation. Reviewed By: zdevito Differential Revision: D116853973
samlurye
force-pushed
the
export-D117269425
branch
2 times, most recently
from
August 26, 2026 22:49
09f4ddc to
c9480ae
Compare
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:
Move
PollSleepPolicyintoCompletionQueueActor. This is the same logic thatQueuePairActoruses to determine how long to wait before the next poll.Reviewed By: zdevito
Differential Revision: D117269425