Repository navigation
Conversation
| "color": "green" | ||
| }, | ||
| { | ||
| "color": "red", |
There was a problem hiding this comment.
No need for read color. Please verify the grafana dashboard.
| }, | ||
| "editorMode": "code", | ||
| "expr": "metrics_DiskStatus_Value{instance=~\"${instance}\"}", | ||
| "legendFormat": "${baseLegend} {{mountpoint}}", |
There was a problem hiding this comment.
The legendFormat here uses "${baseLegend} {{mountpoint}}", but all existing panels with labels (e.g. id=270, 271, 272) put the label variables before ${baseLegend} — e.g. "{{statusCode}} ${baseLegend}", "{{dbBackend}} {{operation}}/{{status}} ${baseLegend}".
For consistency, consider changing to "{{mountpoint}} ${baseLegend}".
| "mode": "off" | ||
| } | ||
| }, | ||
| "mappings": [], |
There was a problem hiding this comment.
The mappings field is empty for the DiskStatus panel. Consider adding value mappings to map the numeric status values (0-4) to readable status names (HEALTHY, READ_OR_WRITE_FAILURE, IO_HANG, HIGH_DISK_USAGE, CRITICAL_ERROR).
|
@SteNicholas @zaynt4606 Addressed all the commetns. Could you take another look? Thanks! |
|
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
36b6805 to
a31dbe4
Compare
|
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
a31dbe4 to
7f84cfc
Compare
|
@yew1eb, I rechecked the latest revision (
The red thresholds have been removed as requested. Could you please apply these two remaining dashboard adjustments as well? |
…tatus value mappings
|
@zaynt4606 Done. Both legendFormat order and value mappings are fixed in 1f10baf. |
|
Merge Summary:
Posted by |
|
Thanks @yew1eb for the contribution! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3782 +/- ##
============================================
+ Coverage 58.74% 60.52% +1.79%
- Complexity 231 324 +93
============================================
Files 399 401 +2
Lines 28058 28400 +342
Branches 2740 2788 +48
============================================
+ Hits 16479 17187 +708
+ Misses 10380 9895 -485
- Partials 1199 1318 +119 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What changes were proposed in this pull request?
Add two gauges on the worker to expose disk health status, registered in
LocalDeviceMonitor.init()and evaluated lazily from the liveDiskInfo.statusat scrape time:UnhealthyDiskCount— the number of local disks whose status is notHEALTHY, intended for alerting (e.g.> 0).DiskStatuswith amountpointlabel — the currentDiskStatusvalue (0: HEALTHY, 1: READ_OR_WRITE_FAILURE, 2: IO_HANG, 3: HIGH_DISK_USAGE, 4: CRITICAL_ERROR) of each disk, for locating the exact unhealthy disk.Also included: two corresponding panels in the Grafana dashboard (
assets/grafana/celeborn-dashboard.json) and the new metrics indocs/monitoring.md.Why are the changes needed?
DeviceMonitoralready maintainsDiskInfo.statusfor every local disk, but none of the existing metrics expose it —DeviceOSFreeBytes/DeviceCelebornFreeBytesreport capacity, andDevice_<name>_<Status>_Countonly counts non-critical error events. To notice a broken disk, operators have to grep worker logs, which makes alerting on disk failures impossible.Does this PR resolve a correctness bug?
Does this PR introduce any user-facing change?
Two new worker metrics are exposed:
UnhealthyDiskCountandDiskStatus(with amountpointlabel).How was this patch tested?
monitor disk status metricsinDeviceMonitorSuitecovering the multi-disk scenario: all disks healthy initially, then one disk markedREAD_OR_WRITE_FAILURE(aggregate count becomes 1 and only that disk's gauge changes), then recovered toHEALTHY../build/mvn test -pl workerpasses.python3 dev/lint_grafana.py assets/grafana/celeborn-dashboard.jsonpasses../build/mvn spotless:check -pl workerpasses.