Repository navigation
benchmarking: add node disk throughput to server telemetry - #2807
Haowei Cai (Roy) (roycaihw) wants to merge 2 commits into
Conversation
Server telemetry now reports `node_disk.read_mb_s` and `write_mb_s`: disk
throughput in MiB/s per device on the nodes hosting a worker pod, from
cAdvisor's `container_fs_{reads,writes}_bytes_total` on the root cgroup.
Like node PSI, it is a one-minute rate read every 10s, with every node's
steady-state samples pooled (per device) and `nodes` counting the nodes
seen. The `server_summary` row in stats.jsonl gains
`node_disk_<device>_<read|write>_<p>_mb_s` for p50 through p99.
The kubelet's cAdvisor already exports these counters and monitoring.yaml
already keeps them, so only queries are added.
|
cc Max Smythe (@maxsmythe) Nishanth Kotla (@Nishanth29) PTAL. This puts disk throughput next to the node PSI in server telemetry, for the interference scenario proposed in #2806. |
Nishanth Kotla (Nishanth29)
left a comment
There was a problem hiding this comment.
LGTM. Left a couple of small nits, none blocking, fine as a follow-up.
| measurements[f"pod_psi_{res}_stall_{p}"] = ( | ||
| pod_psi.get(f"{res}_stall_pct", {}).get(p) | ||
| ) | ||
| # Keyed by device name, so /dev/nvme0n1 gives node_disk_nvme0n1_*. |
There was a problem hiding this comment.
Could we also add a key that doesn't depend on the device, like node_disk_max_write_p99_mb_s for the busiest disk? Device names change across machine types (nvme0n1, the extra local SSDs on lssd, root), so it's hard to compare one series across runs. Fine as a follow-up.
There was a problem hiding this comment.
We should be able to do things like this via post-processing (e.g. data lake methods).
While hardcoding analysis is convenient (and things like SQL can be fiddly), it is also brittle.
There was a problem hiding this comment.
Agreed Max Smythe (@maxsmythe). Keeping server_telemetry to raw per-device series avoids baking machine-topology assumptions into the collector, and we can aggregate across devices (e.g. GREATEST/max) in the data lake query for cross-machine dashboarding.
Nishanth Kotla (@Nishanth29) already LGTM'd this. Since CI is all green, Max Smythe (@maxsmythe) could you grant the maintainer stamp / queue the PR? Thanks!
There was a problem hiding this comment.
Do we want to merge something that we know we want to change? Or has change already been made?
| including `/dev/root`, which can read 0 while the disk holding it carries the | ||
| traffic. `node_disk.nodes` is how many nodes were seen. In `stats.jsonl` the | ||
| keys name the device without `/dev/`, as in | ||
| `node_disk_nvme0n1_write_p99_mb_s`, and a device that was never seen has none. |
There was a problem hiding this comment.
nit: a device seen only outside the steady window still gets keys, just null (test_node_disk_null_not_zero). could this say so too?
A device seen only outside the steady-state window still gets its node_disk_* keys in stats.jsonl, all null, while a device never seen gets none. The README only described the second case.
Server telemetry reports node and pod PSI but not how much the node's disks are writing, which is what slows suspend and resume when co-located actors write heavily (#2806).
This adds
node_disk.read_mb_sandwrite_mb_s: MiB/s per device on the nodes hosting a worker pod, from cAdvisor'scontainer_fs_{reads,writes}_bytes_totalon the root cgroup (container="node"after monitoring.yaml's relabel). It follows node PSI: a one-minute rate read every 10s, steady-state samples pooled across nodes per device, andnodescounting the nodes seen. Theserver_summaryrow instats.jsonlgainsnode_disk_<device>_<read|write>_<p50|p90|p95|p99>_mb_s.The kubelet already exports these counters and monitoring.yaml already keeps them, so only queries are added. cAdvisor has no busy time or IOPS for the root cgroup, so this is throughput only.
How I tested it
./hack/verify/benchmarking-python.sh, with new tests for pooling across nodes, the steady window, reads vs writes, and missing data.Against a live cluster's Prometheus, for a run where 8 actors kept a 290 MiB/s boot disk busy:
write_mb_sfor/dev/nvme0n1read p50 288 / p99 289 MiB/s, in line with node-exporter on the same disk, and a second disk read p50 32 MiB/s.Tests pass
Appropriate changes to documentation are included in the PR