switch to using upstream vllm with new metric #54
Conversation
Signed-off-by: Kunjan Patel <[email protected]>
…en test function, delete old metrics Signed-off-by: Kunjan Patel <[email protected]> Signed-off-by: Kunjan <[email protected]>
k8s-ci-robot
added
the
do-not-merge/invalid-commit-message
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: coolkp The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
coolkp
changed the title
k8s-ci-robot
removed
the
do-not-merge/invalid-commit-message
|
/assign conliu |
|
@coolkp: GitHub didn't allow me to assign the following users: conliu. Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| @@ -15,7 +15,7 @@ spec: | |||
| spec: | |||
| containers: | |||
| - name: lora | |||
| image: "ghcr.io/tomatillo-and-multiverse/vllm:demo" | |||
| image: "vllm/vllm-openai:latest" | |||
There was a problem hiding this comment.
let's pin to a digest instead of latest.
| klog "k8s.io/klog/v2" | ||
| ) | ||
|
|
||
| const ( | ||
| ActiveLoRAAdaptersMetricName = "vllm:info_active_adapters_info" | ||
| LoraRequestInfoMetricName = "vllm:lora_requests_info" | ||
| LoraRequestInfoMetricNameRunningAdapters = "running_adapters" |
There was a problem hiding this comment.
LoraRequestInfoMetricNameRunningAdapters -> LoraRequestInfoRunningAdaptersMetricName
| ActiveLoRAAdaptersMetricName = "vllm:info_active_adapters_info" | ||
| LoraRequestInfoMetricName = "vllm:lora_requests_info" | ||
| LoraRequestInfoMetricNameRunningAdapters = "running_adapters" | ||
| LoraRequestInfoMetricNameMaxAdapters = "max_lora" |
There was a problem hiding this comment.
LoraRequestInfoMaxAdaptersMetricName
| } | ||
|
|
||
| return updated, errs | ||
| } | ||
|
|
||
| // getLatestLoraMetric gets latest lora metric series in gauge metric family `vllm:lora_requests_info` | ||
| // reason its specially fetched is because each label key value pair permutation generates new series | ||
| // and only most recent is useful. This is timestamp metric so value of each series is the creation timestamp. |
There was a problem hiding this comment.
This is timestamp metric so value of each series is the creation timestamp. -> The value of each series is the creation timestamp so we can retrieve the latest by sorting the value.
| } | ||
| return latest, time.Unix(0, int64(latestTs*1000)), nil | ||
| } | ||
|
|
||
| // getLatestMetric gets the latest metric of a family. This should be used to get the latest Gauge metric. | ||
| func getLatestMetric(metricFamilies map[string]*dto.MetricFamily, metricName string) (*dto.Metric, time.Time, error) { |
There was a problem hiding this comment.
Can you add a comment there that vllm doesn't set the metric timestamp, so this function essentially selects the first metric. This helps clarify why we need the getLatestLoraMetric function.
Fixes Switch to upstream vLLM #22