blockdevice: add SysBlockDeviceRotational for targeted rotational reads#824
Merged
SuperQ merged 1 commit intoJun 18, 2026
Merged
Conversation
ruthwikkakumani
force-pushed
the
blockdevice-add-rotational-method
branch
from
13c41b6 to
a9b3d58
Compare
ruthwikkakumani
added a commit
to ruthwikkakumani/node_exporter
that referenced
this pull request
Jun 17, 2026
Replace fs.SysBlockDeviceRotational() (which required a replace directive pointing to a personal procfs fork) with an inline sysFilePath()-based read of /sys/block/<dev>/queue/rotational. This makes the PR self-contained against the official prometheus/procfs v0.20.1 and unblocks CI. The 1-file-per-device I/O reduction (vs the previous ~30-file SysBlockDeviceQueueStats call) is preserved, fixing the 10× scrape regression on systems with many block devices (prometheus#3282). Once prometheus/procfs#824 lands, a follow-up can switch to blockdevice.FS.SysBlockDeviceRotational() per SuperQ's suggestion. Signed-off-by: Ruthwik Kakumani <ruthwikkakumani08@gmail.com>
ruthwikkakumani
added a commit
to ruthwikkakumani/node_exporter
that referenced
this pull request
Jun 17, 2026
…ometheus#3282) Fix a 10× scrape-latency regression introduced in prometheus#3022 on systems with many block devices. Previously, Update() called SysBlockDeviceQueueStats(dev) for every block device per scrape, reading ~30 sysfs files per device. On systems with 1,000+ devices this resulted in 30,000+ unnecessary file reads per scrape, causing timeouts. Replace the expensive struct call with a targeted rotationalLabel() helper that delegates to blockdevice.FS.SysBlockDeviceRotational(dev) (added in prometheus/procfs#824). This reads exactly 1 sysfs file per device: /sys/block/<device>/queue/rotational Returns "1" for rotational (HDD) and "0" for non-rotational (SSD/NVMe), failing closed to "0" on any error — matching the zero-initialised-struct behaviour of the old code. Also add: - TestRotationalLabel: dynamic t.TempDir()-based tests via blockdevice.FS, covering HDD, SSD, and missing-file cases. - BenchmarkDiskstatsUpdate: measures the full Update() call to catch future per-device sysfs I/O regressions before merge. Fixes prometheus#3282 Signed-off-by: Ruthwik Kakumani <ruthwikkakumani08@gmail.com>
ruthwikkakumani
added a commit
to ruthwikkakumani/node_exporter
that referenced
this pull request
Jun 17, 2026
…ometheus#3282) Fix a 10× scrape-latency regression introduced in prometheus#3022 on systems with many block devices. Previously, Update() called SysBlockDeviceQueueStats(dev) for every block device per scrape, reading ~30 sysfs files per device. On systems with 1,000+ devices this resulted in 30,000+ unnecessary file reads per scrape, causing timeouts. Replace the expensive struct call with a targeted rotationalLabel() helper that delegates to blockdevice.FS.SysBlockDeviceRotational(dev) (added in prometheus/procfs#824). This reads exactly 1 sysfs file per device: /sys/block/<device>/queue/rotational Returns "1" for rotational (HDD) and "0" for non-rotational (SSD/NVMe), failing closed to "0" on any error — matching the zero-initialised-struct behaviour of the old code. Also add: - TestRotationalLabel: dynamic t.TempDir()-based tests via blockdevice.FS, covering HDD, SSD, and missing-file cases. - BenchmarkDiskstatsUpdate: measures the full Update() call to catch future per-device sysfs I/O regressions before merge. Fixes prometheus#3282 Signed-off-by: Ruthwik Kakumani <ruthwikkakumani08@gmail.com>
SuperQ
approved these changes
Jun 18, 2026
Add FS.SysBlockDeviceRotational(device string) (uint64, error), a lightweight counterpart to SysBlockDeviceQueueStats that reads only /sys/block/<device>/queue/rotational. This is needed by node_exporter's diskstats collector to fix a 10× scrape-latency regression on systems with many block devices (prometheus/node_exporter#3282). The previous implementation called SysBlockDeviceQueueStats which reads ~30 sysfs files per device. SysBlockDeviceRotational reduces that to exactly 1 read per device. Returns 1 for rotational (HDD), 0 for non-rotational (SSD/NVMe). Follows the same single-file read pattern as SysBlockDeviceSize. Signed-off-by: ruthwikkakumani <ruthwikkakumani@users.noreply.github.com>
ruthwikkakumani
force-pushed
the
blockdevice-add-rotational-method
branch
from
a9b3d58 to
6f28a71
Compare
ruthwikkakumani
added a commit
to ruthwikkakumani/node_exporter
that referenced
this pull request
Jun 18, 2026
…ometheus#3282) Fix a 10× scrape-latency regression introduced in prometheus#3022 on systems with many block devices. Previously, Update() called SysBlockDeviceQueueStats(dev) for every block device per scrape, reading ~30 sysfs files per device. On systems with 1,000+ devices this resulted in 30,000+ unnecessary file reads per scrape, causing timeouts. Replace the expensive struct call with a targeted rotationalLabel() helper that delegates to blockdevice.FS.SysBlockDeviceRotational(dev) (added in prometheus/procfs#824). This reads exactly 1 sysfs file per device: /sys/block/<device>/queue/rotational Returns "1" for rotational (HDD) and "0" for non-rotational (SSD/NVMe), failing closed to "0" on any error — matching the zero-initialised-struct behaviour of the old code. Also add: - TestRotationalLabel: dynamic t.TempDir()-based tests via blockdevice.FS, covering HDD, SSD, and missing-file cases. - BenchmarkDiskstatsUpdate: measures the full Update() call to catch future per-device sysfs I/O regressions before merge. Fixes prometheus#3282 Signed-off-by: Ruthwik Kakumani <ruthwikkakumani08@gmail.com>
Contributor
Author
|
@SuperQ thanks for the approval! Could you also approve the CI workflow runs so the checks can complete? They're currently blocked on "2 workflows awaiting approval". Thank you! |
ruthwikkakumani
added a commit
to ruthwikkakumani/node_exporter
that referenced
this pull request
Jun 18, 2026
…ometheus#3282) Fix a 10× scrape-latency regression introduced in prometheus#3022 on systems with many block devices. Previously, Update() called SysBlockDeviceQueueStats(dev) for every block device per scrape, reading ~30 sysfs files per device. On systems with 1,000+ devices this resulted in 30,000+ unnecessary file reads per scrape, causing timeouts. Replace the expensive struct call with a targeted rotationalLabel() helper that delegates to blockdevice.FS.SysBlockDeviceRotational(dev) (added in prometheus/procfs#824). This reads exactly 1 sysfs file per device: /sys/block/<device>/queue/rotational Returns "1" for rotational (HDD) and "0" for non-rotational (SSD/NVMe), failing closed to "0" on any error — matching the zero-initialised-struct behaviour of the old code. Also add: - TestRotationalLabel: dynamic t.TempDir()-based tests via blockdevice.FS, covering HDD, SSD, and missing-file cases. - BenchmarkDiskstatsUpdate: measures the full Update() call to catch future per-device sysfs I/O regressions before merge. Fixes prometheus#3282 Signed-off-by: Ruthwik Kakumani <ruthwikkakumani08@gmail.com>
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
2 participants
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.
What
Add
FS.SysBlockDeviceRotational(device string) (uint64, error)— a targetedsingle-file reader for
/sys/block/<device>/queue/rotational.Why
Required by
node_exporter's diskstats collector to fix a 10× scrape-latencyregression on systems with many block devices
(prometheus/node_exporter#3282).
The previous implementation called
SysBlockDeviceQueueStats()which reads~30 sysfs files per device to populate a full
BlockQueueStatsstruct,consuming only the
Rotationalfield. On systems with 1,000+ devices thiscaused 30,000+ unnecessary file reads per scrape.
SysBlockDeviceRotationalreduces this to exactly 1 read per device.How
Follows the same single-file read pattern as
SysBlockDeviceSize()usingutil.ReadUintFromFile. Returns1for rotational (HDD),0fornon-rotational (SSD/NVMe).
A unit test (
TestSysBlockDeviceRotational) is included, using the existingsdafixture (rotational=1) and verifying error behaviour for devices withouta
queue/rotationalfile.Related