feat: add hami_gpu_device_health metric to scheduler - #2617
Conversation
DeviceUsage.Health is used internally to skip unhealthy devices in Fit()
but was never emitted as a Prometheus metric. Operators have no way to
alert on an unhealthy GPU without inspecting node annotations manually.
Add hami_gpu_device_health gauge (1=healthy, 0=unhealthy) in
collectNodeMetrics, with label set {node, device_uuid, device_index,
device_type} consistent with hami_gpu_memory_limit_bytes.
Signed-off-by: G. Manoj Kumar <manojkumar148700@gmail.com>
Signed-off-by: manoj-1407 <manojkumar148700@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: manoj-1407 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe scheduler now exposes ChangesGPU device health metrics
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| "Realized MIG instance identity and scheduler placement", | ||
| []string{"node", "device_uuid", "device_index", "mig_uuid", "profile", "gpu_instance_id", "compute_instance_id", "placement_start", "placement_size"}, nil, | ||
| ) | ||
| nodeGPUDeviceHealthDesc := prometheus.NewDesc( |
There was a problem hiding this comment.
name says gpu but this loop runs for every backend. on purpose?
There was a problem hiding this comment.
yeah it's intentional — Health is set on all device types, the name reflects what the metric measures rather than which backend it's coming from
| } | ||
| } | ||
|
|
||
| healthVal := float64(0) |
There was a problem hiding this comment.
is health live at scrape time or a cached value?
There was a problem hiding this comment.
it's a cached value from the scheduler's in-memory device list, same as the other metrics in collectNodeMetrics
| } | ||
| } | ||
|
|
||
| healthVal := float64(0) |
There was a problem hiding this comment.
if a node has zero devices in devicelists, does this metric emit anything for that node at all?
There was a problem hiding this comment.
nothing gets emitted in that case, the loop just doesn't run — same behavior as the existing MIG metric
|
@mesutoezdil hey, any specific reason for closing? happy to address any concerns |
|
we have no time and energy for llm answers. read again the contribution rules. |
2 participants
What type of PR is this?
/kind feature
What this PR does / why we need it:
scheduler was silently skipping unhealthy GPUs during scoring using
DeviceUsage.Health but nothing ever made it to Prometheus. only way
to know a device was unhealthy was manually checking node annotations.
added hami_gpu_device_health gauge (1=healthy, 0=unhealthy) so you
can actually alert on it.
Which issue(s) this PR fixes:
Fixes #2612
Special notes for your reviewer:
same 0/1 pattern as hami_mig_device_info. descriptor kept local to
the function like everything else in there.
Does this PR introduce a user-facing change?:
yes — hami_gpu_device_health gauge
I used Claude to help identify this gap. I reviewed the code and
understand the implementation.
Summary by CodeRabbit
1; unhealthy devices report0.