Conversation
This commit adds a variety of real disk metrics for the block devices that back Elasticsearch data paths. A collection of statistics are read from /proc/diskstats and are used to report the raw metrics for operations and read/write bytes. Relates elastic#15915
There was a problem hiding this comment.
Pull request overview
This PR adds I/O statistics collection functionality for Linux systems by reading from /proc/diskstats. The implementation tracks device-level read/write operations and sectors transferred, exposing these metrics through the Elasticsearch nodes stats API.
Changes:
- Added
DeviceStatsandIoStatsclasses to track I/O metrics per device and aggregated totals - Modified
FsInfoconstructor to accept an optionalIoStatsparameter - Implemented
/proc/diskstatsparsing inFsProbeto collect device metrics on Linux - Added device number tracking to
NodePathandESFileStorefor mapping paths to devices
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java | Added DeviceStats and IoStats classes; updated FsInfo constructor to include I/O statistics |
| core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java | Implemented I/O statistics collection by parsing /proc/diskstats |
| core/src/main/java/org/elasticsearch/monitor/fs/FsService.java | Refactored to pass previous FsInfo for delta calculations |
| core/src/main/java/org/elasticsearch/env/NodeEnvironment.java | Added major/minor device number fields to NodePath |
| core/src/main/java/org/elasticsearch/env/ESFileStore.java | Added device number extraction from /proc/self/mountinfo |
| core/src/main/resources/org/elasticsearch/bootstrap/security.policy | Added read permission for /proc/diskstats |
| docs/reference/cluster/nodes-stats.asciidoc | Documented new I/O statistics fields in the API |
| core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java | Added tests for I/O statistics parsing and aggregation |
| core/src/test/java/org/elasticsearch/monitor/fs/DeviceStatsTests.java | Added unit tests for DeviceStats delta calculations |
| test/framework/src/main/java/org/elasticsearch/cluster/MockInternalClusterInfoService.java | Updated mock to include null IoStats parameter |
| core/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java | Updated test mocks to include null IoStats parameter |
| core/src/main/java/org/elasticsearch/monitor/MonitorService.java | Removed empty Javadoc comments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| currentSectorsWritten, | ||
| previousDeviceStats != null ? previousDeviceStats.currentSectorsWritten : -1, | ||
| currentSectorsRead, | ||
| previousDeviceStats != null ? previousDeviceStats.currentSectorsRead : -1, |
There was a problem hiding this comment.
The parameter order is incorrect in the constructor call. currentSectorsWritten and currentSectorsRead are swapped. Line 220 should pass currentSectorsRead and line 222 should pass currentSectorsWritten to match the private constructor's parameter order (lines 234-236).
| builder.endObject(); | ||
| } | ||
| builder.endArray(); | ||
| builder.startObject("total"); |
There was a problem hiding this comment.
Missing builder.endObject() call to close the "total" object started on this line. This will produce malformed JSON output.
| used by Elasticsearch completed since starting Elasticsearch. | ||
|
|
||
| `fs.io_stats.read_operations` (Linux only):: | ||
| The total number of read operations for across all devices used by |
There was a problem hiding this comment.
Corrected grammar by removing extra 'for' - should be 'The total number of read operations across all devices'.
| The total number of read operations for across all devices used by | |
| The total number of read operations across all devices used by |
2 participants
PR_015