[ilmarkov]

Overview

Rust's strict NixlStats decoder rejected two fields emitted by current Python NIXL stats. In MultiConnector payloads, that caused the NIXL child to be treated as opaque data, so the Rust metrics recorder dropped its transfer histograms. The recorder also omitted handshake/notification failures from the combined failure counter and did not export late notifications.

Claims

• Decode current Python NIXL MultiConnector stats while retaining compatibility with payloads that omit newer fields.
• Record NIXL transfer histograms and align the Rust failed-transfer counter with Python by including transfer, handshake, and notification failures.
• Export vllm:nixl_num_notifications_after_expiry from the Rust frontend.
• This does not duplicate #58563: that PR adds descriptor-driven metrics for other connectors and explicitly leaves NIXL on its existing typed path. This change fixes that typed path and completes the Rust metric coverage.

Validation

• cargo test --manifest-path rust/Cargo.toml -p vllm-engine-core-client kv_connector_stats_are_recorded_into_mooncake_and_nixl_metrics — passed (1 test); verifies the combined failure count and late-notification counter.
• cargo test --manifest-path rust/Cargo.toml -p vllm-engine-core-client python_msgpack_fixtures_match_rust_encoding_and_record_nixl_metrics — passed (1 test); decodes the Python-generated fixture and checks rendered failure, expiry, and histogram metrics.
• cargo fmt --manifest-path rust/Cargo.toml --all --check, git diff --check, and commit pre-commit hooks — passed.

Details

EngineCore sends MultiConnector stats as a map from connector class name to its stats payload. Current Python NIXL stats include num_failed_handshakes and num_notifications_after_expiry; the Rust NixlStats used deny_unknown_fields but did not declare these fields. Serde therefore rejected the typed NIXL child, the flattened other map retained it as opaque data, and the recorder ignored it.

The relevant flow is: (1) the P/D connector accumulates transfer and notification outcomes, (2) EngineCore serializes connector stats into the MultiConnector MessagePack payload, and (3) the Rust frontend decodes NIXL stats and records the matching Prometheus metrics. Adding the missing fields with defaults keeps older payloads compatible. The recorder now groups transfer, handshake, and notification failures under the existing failed-transfer counter, keeps the dedicated notification-failure counter for compatibility, and records notifications received after expiry separately.

The regression extends the Python-generated MessagePack fixture and asserts the decoded fields and rendered counters, including a failed handshake, a failed notification, and two notifications after expiry.

---

Pull Request Checklist

• I used vLLM's /pr-checklist skill.

• 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: The focused Rust tests pass; the live Rust/Python NIXL smoke comparison remains required before this PR is ready for upstream review.

• 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.

AI assistance was used. The human submitter must review every changed line and confirm the live P/D smoke comparison before marking this PR ready for review.

BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)

[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.

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @ilmarkov.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[BugenZhao]

Thanks @ilmarkov!

Shall we remove #[serde(deny_unknown_fields)] on pub struct XxxStats { so that a new field doesn't break the decoding of the entire stats?

I've done this but failed to push to your PR :) BugenZhao@8bb27bd

[ilmarkov]

@BugenZhao Done, thanks!

[BugenZhao]

/ci run

[github-actions]

✅ @ilmarkov, 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 #97523 for commit 812bd7d8439d.

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