Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements the get_request_metrics method in MultiKVConnectorWorkerMetadata within the vllm-pd-request-metrics.patch file, allowing per-request transfer metrics from child connectors to be merged and forwarded to the scheduler. It also adds a comprehensive set of regression tests in tests/test_multiconnector_request_metrics.py to validate this behavior and registers the new test suite in the Buildkite pipeline. There are no review comments, so I have no feedback to provide.
0z5a
force-pushed
the
fix/multiconnector-request-metrics
branch
from
September 18, 2026 20:58
d635de8 to
5ac0a84
Compare
0z5a
force-pushed
the
fix/multiconnector-request-metrics
branch
from
September 24, 2026 05:53
5ac0a84 to
e1638f4
Compare
…Connector
The pd-request-metrics patch adds KVConnectorWorkerMetadata.get_request_metrics()
and makes the scheduler accumulate request.kv_transfer_metrics from it. It
implements that method on NixlRequestMetrics and consumes it in
vllm/v1/core/sched/scheduler.py, but not on MultiKVConnectorWorkerMetadata:
the wrapper only overrides aggregate(), so it inherited the no-op default from
KVConnectorWorkerMetadata and returned {} for every request.
With a MultiConnector configured the worker metadata that reaches the scheduler
is always a MultiKVConnectorWorkerMetadata, so every per-request transfer metric
a child connector reported was silently dropped. Verified on the real classes:
before the fix a child reporting kv_transfer_bytes for req-a yielded {} from the
wrapper.
Add get_request_metrics() to the wrapper. It forwards each child's metrics;
None children are skipped, request ids and field names are unioned, and a metric
name claimed by more than one child is not summed: the children are distinct
connector types whose transfers are not known to be disjoint, so the first owner
is kept and the collision is logged rather than silently double counted.
tests/test_multiconnector_request_metrics.py drives the real production classes
(NixlRequestMetrics as the child, MultiKVConnectorWorkerMetadata as the wrapper);
it goes from 8 failed/2 passed to 10 passed. The patch is validated by reverse
applying the previous revision and re-applying this one over the built vLLM tree.
Signed-off-by: 0z5a <0z5a@users.noreply.github.com>
Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
0z5a
force-pushed
the
fix/multiconnector-request-metrics
branch
from
September 24, 2026 07:07
e1638f4 to
b03e42b
Compare
0z5a
marked this pull request as draft
October 1, 2026 14:31
Author
|
Superseded by #460, now merged into |
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.
Incremental review: Files changed.
Base:
0z5a-branchat791f101e20db377b28e15f12a00863baaea2605f.Preserve per-request transfer metrics when
MultiConnectorwrapsNixlConnector. Its worker metadata previously dropped the child's request statistics before scheduler aggregation.Add the merge to the existing Docker vLLM patch: combine distinct requests and fields, sum matching counters, skip absent child metrics and rebuild each read without mutating child metadata. No new test files are submitted.
vllm/vime:latestSingle-L20 prefill/decode experiment:
The recovered byte count establishes propagation. These single runs do not establish latency improvement. CI uses a prebuilt image and does not rebuild this Docker patch; the patched-runtime checks above were run separately.