[feat] introduce payload transfer abstraction and NIXL-UCX backend - #163
[feat] introduce payload transfer abstraction and NIXL-UCX backend#163Ginray wants to merge 1 commit into
Conversation
CLA Signature Guide@Ginray , thanks for your pull request. The following commit(s) are not associated with a signed Contributor License Agreement (CLA).
To sign CLA, click here. To check if your email is configured correctly, refer to the FAQs. Once you've signed the CLA or updating your email, please comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d18010f305
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raise RuntimeError(f"duplicate PUT transfer_id: {descriptor.transfer_id}") | ||
| token = self.prepare_receive(descriptor) | ||
| prepared = True | ||
| self._pending_puts[descriptor.transfer_id] = _PendingPut( |
There was a problem hiding this comment.
Expire abandoned NIXL preparations
When a client process is killed after receiving PUT_DATA_READY, no commit or cancel request arrives, so this entry and its registered receive buffer remain retained for the storage unit's lifetime; the analogous GET preparation also retains its encoded frames. A single abandoned large transfer can therefore permanently consume substantial memory, and repeated client failures can OOM the storage unit, so pending transfers need deadline-based cleanup independent of client cancellation.
Useful? React with 👍 / 👎.
| if operation in (ZMQRequestType.PUT_DATA, ZMQRequestType.GET_DATA): | ||
| with monitor.measure(op_type=operation.name): |
There was a problem hiding this comment.
Measure NIXL operations as logical PUTs and GETs
When payload_transfer.backend is nixl-ucx, requests use the new *_PREPARE and *_COMMIT types, so they bypass this only monitor.measure branch. Consequently the existing PUT_DATA/GET_DATA operation statistics stay at zero even while NIXL traffic is active; aggregate the strategy requests into the corresponding logical PUT/GET measurements so this supported backend remains traceable through observability.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
d18010f to
a674b43
Compare
CLA Signature PassGinray, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
a674b43 to
1326cdb
Compare
CLA Signature PassGinray, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
Signed-off-by: Ginray <ginray0215@gmail.com>
1326cdb to
7bc6cf3
Compare
CLA Signature PassGinray, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
Motivation
SimpleStoragecurrently couples payload transfer with its ZMQ request path, which makes it difficult to add alternative high-throughput transports without changing storage routing and public APIs.This PR introduces a small payload-transfer abstraction so that
SimpleStoragecan keep ZMQ as its control plane while allowing Host-to-Host payloads to use alternative transports such as NIXL-UCX.The goals are to:
SimpleStorageAPIs and default ZMQ behavior compatible;Summary
PayloadTransferas the payload backend interface forSimpleStorage.Design
AsyncSimpleStorageManagerselects the configured payload backend and delegates logical payload PUT/GET operations toPayloadTransfer.On the StorageUnit side, the selected
PayloadTransferhandles backend-specific PUT/GET requests, whileSimpleStorageUnitcontinues to own storage semantics, data access, CLEAR, metrics, and checkpoint operations.For NIXL-UCX:
Usage
The default remains ZMQ.
To enable NIXL-UCX:
See
docs/nixl_ucx_payload.mdfor installation, UCX configuration, and RDMA verification.Compatibility
SimpleStoragePUT/GET/CLEAR callers and public APIs remain unchanged.Validation
Two-node A2 SimpleStorage tests covered 33 payload sizes from 128 KiB to 1 GiB. All PUT/GET/CLEAR operations and data checks passed.
Native UCX is shown only as a benchmark reference and is not part of this PR.
Median MiB/s, PUT / GET.
The fixed real DAPO payload produced identical digests and training inputs through ZMQ and NIXL-UCX. Both paths completed 20/20 VIME steps on 80 real DAPO problems without OOM or malformed output.
Follow-up
mainonce it is considered stable.