[liranschour]

Overview

Part of #63602 (gap 3). When a P2P session is torn down, the source no longer releases a NIXL WRITE that may still be running. It keeps that transfer's handle, its source pins and the peer's old registration until the transport reports the transfer stopped, bounded at 120 s.

Claims

• ServerRole.close() no longer releases PROC/PEND transfers or fails (unpins) the store jobs they read.
• remove_remote_peer() no longer frees the descriptor list and remote agent that a running transfer was prepped against.
• A reconnect under the same peer_id never sees the old session's retained transfers.
• Retention is bounded. Past _RETAINED_HOLD_CAP_S (120 s) the transfer is force-released with an error log, so a wedged handle can't permanently shrink the cache.

Validation

The issue's CPU reproducer test_gap3_unsafe_close.py:

main:     cancel_calls=[([0], 'immediate')]; remaining_handles=[]; failed_stores_returned_for_unpin=[1]   -> FAILED
this PR:  cancel_calls=[([0], 'wait')];      remaining_handles=[0]; failed_stores_returned_for_unpin=[]  -> PASSED

New tests, run with this PR's test files on main's code (7 of 8 fail for the reported reason):

$ pytest tests/v1/kv_offload/tiering/p2p/test_sessions.py tests/v1/kv_offload/tiering/p2p/test_data_transport.py -k "retain or retire or releases_stopped or without_inflight"
E   AssertionError: assert [([0], 'immediate')] == [([0], 'wait')]
E   AttributeError: 'SessionCloseResult' object has no attribute 'retained'
E   AssertionError: Expected 'release_dlist_handle' to not have been called. Called 1 times.
E   AttributeError: 'NixlTransport' object has no attribute 'poll_retired'
...
7 failed, 1 passed

(The one that passes, test_remove_remote_peer_without_inflight_releases_at_once, guards the unchanged path.)

With this PR:

$ LD_LIBRARY_PATH=.../nvidia/cu13/lib pytest tests/v1/kv_offload/tiering/p2p
338 passed
$ pre-commit run --files <changed>      # mypy included
Passed

Not validated on GPU. CI runs these tests under misc.yaml (pytest -v -s v1/kv_offload), and they are CPU-only, using the fake transport and a mocked NIXL agent. An e2e run that actually wedges a WRITE (for example rc_gda re-enabled, as in #63499) has not been done yet.

Details

Root cause. On reap, _reap_dead_sessions / _reap_idle_sessions call session.close() and then ServerRole.close(). That ran transport.cancel(all inflight) in the default "immediate" mode and returned every store job as failed. The manager then called remove_remote_peer(). Releasing a PROC handle does not stop the RDMA WRITE (NixlTransport.cancel docstring; #59790 saw a WRITE land after remove_remote_agent). So this unpinned CPU blocks the NIC may still read, and freed NIXL request and descriptor state while it was still in use. _drain_abort and _timeout_pending_store_jobs already refuse to do this, and teardown was the one path that didn't.

Change.

• ServerRole.close() cancels in "wait" mode. Transfers that are still running come back as SessionCloseResult.retained (tid → store job_ids), and their jobs are left out of failed_stores.
• NixlTransport.remove_remote_peer()
  • Marks the peer's in-flight entries retired. poll() skips retired entries, so a reconnected session under the same id can't receive a stale completion.
  • Defers releasing the old dlist and remote agent until no retired transfer references them.
  • poll_retired() releases transfers that have stopped. It uses the same terminal rule as wait-mode cancel: done, a non-PROC state, or NIXL_ERR_REMOTE_DISCONNECT. It does not charge the peer, since a reconnected peer under that id isn't at fault.
• DataTransport gets default poll_retired() / force_release_retired(), so other transports need no change.
• Manager
  • Keeps _retained and calls _poll_retained() once per poll round. A store job fails (unpins) only once no retained transfer reads it.
  • Past 120 s it calls force_release_retired() and logs an error.
  • Shutdown keeps its existing bounded drain, now including retained transfers.

Tradeoffs and limits.

• The 120 s cap is a judgment call: after it, the old unsafe release happens anyway, but rarely and loudly instead of on every teardown. Whether NIXL/UCX teardown can fence a stalled WRITE, which would let the cap shrink or go away, is the open fencing question in [Bug]: P2P KV offload tier does not fail fast on peer faults #63602.
• This is the source side only. Consumer slot reuse after disconnect is [Bugfix][KV Offload][P2P] Quarantine consumer slots until writes settle #61667.
• This PR should land before [Bugfix][KV Offload][P2P] Disconnect a peer whose AbortAck times out #63715 (consumer disconnect on a missed AbortAck), because that change makes teardowns more frequent. The gap 2 source-side stall deadline is stacked on this PR and follows separately.

Complexity: about 190 non-test lines (medium), all within vllm/v1/kv_offload/tiering/p2p/. No core components are touched.

---

Pull Request Checklist

• I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

• AI assistance was used during the creation of this PR.

• Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

• Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

• Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

• Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

🤖 Generated with Claude Code

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[orozery]

/ci run

[github-actions]

✅ @liranschour, CI is now available for this PR.

• /ci run starts upstream CI; /amd-ci run starts AMD CI only.
• Run commands test your branch with every commit currently on its upstream target branch. If it is behind, the bot merges the target branch into it first (pull before pushing again). Append --allow-stale to a run command to test an outdated branch as is, at your own risk.
• /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
• /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
• /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

[github-actions]

✅ Triggered Buildkite CI #97963 for commit bade76b2cb4f.

ℹ️ This PR is 11 commits behind upstream main. Merged upstream main into this branch as bade76b2cb4f before starting CI. Pull the branch before pushing more changes.