Repository navigation
Handle NIXL abort notifications - #30352
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
| if ( | ||
| bootstrap_room in self.request_status | ||
| and self.request_status[bootstrap_room] == KVPoll.Failed | ||
| ): |
There was a problem hiding this comment.
Can be shorten to if self.request_status.get(bootstrap_room) == KVPoll.Failed
| self.update_status(room_to_be_aborted, KVPoll.Failed) | ||
| logger.debug( | ||
| f"Received abort notification for room {room_to_be_aborted}, " | ||
| f"marked as Failed" | ||
| ) |
There was a problem hiding this comment.
Maybe record failure here
| if len(msg) != 4: | ||
| raise ValueError(f"expected 4 frames, got {len(msg)}") |
There was a problem hiding this comment.
This could be covered in the following except block
| mgr = self._make_manager({13: KVPoll.WaitingForInput}) | ||
|
|
||
| handled = mgr._handle_abort_notification([b"ABORT", b"13"]) | ||
|
|
||
| self.assertTrue(handled) | ||
| self.assertEqual(mgr.request_status[13], KVPoll.WaitingForInput) |
There was a problem hiding this comment.
b"13" has poor readability, maybe change it to b"unvalid-room"
ShangmingCai
left a comment
ShangmingCai
left a comment
There was a problem hiding this comment.
Others Look good. cc: @ishandhanani
| if len(msg) != 4: | ||
| raise ValueError(f"expected 4 frames, got {len(msg)}") | ||
| room_to_be_aborted = int(msg[1].decode("ascii")) | ||
| _, room_frame, _, _ = msg |
There was a problem hiding this comment.
This is also not needed; just use msg[sub_id] directly, and catch the error in except Exception as e in this block
ShangmingCai
left a comment
ShangmingCai
left a comment
There was a problem hiding this comment.
After fixing the remaining issue, no other comment from me, CC @ishandhanani, please check and help continue the approving and merging procoess of this PR.
|
#30301 is fixing the same issue? |
4 participants
Motivation
NIXL disaggregation can receive a decode-side abort while the prefill transfer worker is still finishing a KV/aux transfer. Without handling the abort prefill bootstrap_thread can fail with assertion error ('ABORT' not recognized). Follow up to #27372 that initially introduced abort msg
Modifications
ABORTmessages on the NIXL prefill bootstrap socket and mark incomplete rooms asFailed.Failedstatus sticky until the sender clears the room.ABORT_ACKon the NIXL path yet; there is no defined NIXL quiescence/deferred-release contract for safely acknowledging decode-side buffer release, so this PR leaves a TODO instead.Accuracy Tests
Not applicable; this only changes disaggregation control-plane state handling.
Speed Tests and Profiling
Not applicable; no inference hot path changes.
Checklist
CI States
Latest PR Test (Base): ❌ Run #28922034696
Latest PR Test (Extra): ❌ Run #28922034519