From b0cb308106c91eb7fb8a028be1d34f54c7729281 Mon Sep 17 00:00:00 2001
From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com>
Date: Tue, 11 Jul 2023 01:18:58 -0400
Subject: [PATCH] refactor: flatten IoCounter return value (#1253)
* refactor: rewrite io stats collection function result
* refactor: flatten IoCounters vector results
---
src/app/data_harvester/disks.rs | 3 ++-
src/app/data_harvester/disks/freebsd.rs | 3 ++-
.../data_harvester/disks/unix/linux/counters.rs | 6 ++++--
.../data_harvester/disks/unix/macos/counters.rs | 4 ++--
src/app/data_harvester/disks/windows.rs | 3 ++-
src/app/data_harvester/disks/zfs_io_counters.rs | 14 +++++++-------
6 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/src/app/data_harvester/disks.rs b/src/app/data_harvester/disks.rs
index 5075f335..e27f7f75 100644
--- a/src/app/data_harvester/disks.rs
+++ b/src/app/data_harvester/disks.rs
@@ -64,7 +64,8 @@ cfg_if! {
pub fn get_io_usage() -> anyhow::Result<IoHarvest> {
let mut io_hash: HashMap<String, Option<IoData>> = HashMap::new();
- for io in io_stats()?.into_iter().flatten() {
+ // TODO: Maybe rewrite this to not do a result of vec of result...
+ for io in io_stats()?.into_iter() {
let mount_point = io.device_name().to_string_lossy();
io_hash.insert(
diff --git a/src/app/data_harvester/disks/freebsd.rs b/src/app/data_harvester/disks/freebsd.rs
index a10fa642..4c392ebc 100644
--- a/src/app/data_harvester/disks/freebsd.rs
+++ b/src/app/data_harvester/disks/freebsd.rs
@@ -29,6 +29,7 @@ struct FileSystem {
}
pub fn get_io_usage() -> error::Result<IoHarvest> {
+ // TODO: Should this (and other I/O collectors) fail fast? In general, should collection ever fail fast?
#[allow(unused_mut)]
let mut io_harvest: HashMap<String, Option<IoData>> =
get_disk_info().map(|storage_system_information| {
@@ -43,7 +44,7 @@ pub fn get_io_usage() -> error::Result<IoHarvest> {
{
use crate::app::data_harvester::disks::zfs_io_counters;
if let Ok(zfs_io) = zfs_io_counters::zfs_io_stats() {
- for io in zfs_io.into_iter().flatten() {
+ for io in zfs_io.into_iter() {
let mount_point = io.device_name().to_string_lossy();
io_harvest.insert(
mount_point.to_string(),
diff --git a/src/app/data_harvester/disks/unix/linux/counters.rs b/src/app/data_harvester/disks/unix/linux/counters.rs
index d65863ba..c51aecf7 100644
--- a/src/app/data_harvester/disks/unix/linux/counters.rs
+++ b/src/app/data_harvester/disks/unix/linux/counters.rs
@@ -64,7 +64,7 @@ impl FromStr for IoCounters {
}
/// Returns an iterator of disk I/O stats. Pulls data from `/proc/diskstats`.
-pub fn io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
+pub fn io_stats() -> anyhow::Result<Vec<IoCounters>> {
const PROC_DISKSTATS: &str = "/proc/diskstats";
let mut results = vec![];
@@ -74,7 +74,9 @@ pub fn io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
// This saves us from doing a string allocation on each iteration compared to `lines()`.
while let Ok(bytes) = reader.read_line(&mut line) {
if bytes > 0 {
- results.push(IoCounters::from_str(&line));
+ if let Ok(counters) = IoCounters::from_str(&line) {
+ results.push(counters);
+ }
line.clear();
} else {
break;
diff --git a/src/app/data_harvester/disks/unix/macos/counters.rs b/src/app/data_harvester/disks/unix/macos/counters.rs
index 9f31da38..731e0092 100644
--- a/src/app/data_harvester/disks/unix/macos/counters.rs
+++ b/src/app/data_harvester/disks/unix/macos/counters.rs
@@ -44,6 +44,6 @@ fn get_device_io(device: io_kit::IoObject) -> anyhow::Result<IoCounters> {
}
/// Returns an iterator of disk I/O stats. Pulls data through IOKit.
-pub fn io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
- Ok(get_disks()?.map(get_device_io).collect())
+pub fn io_stats() -> anyhow::Result<Vec<IoCounters>> {
+ Ok(get_disks()?.filter_map(|d| get_device_io(d).ok()).collect())
}
diff --git a/src/app/data_harvester/disks/windows.rs b/src/app/data_harvester/disks/windows.rs
index 2dfd59dc..3bf872de 100644
--- a/src/app/data_harvester/disks/windows.rs
+++ b/src/app/data_harvester/disks/windows.rs
@@ -11,7 +11,7 @@ mod bindings;
use bindings::*;
/// Returns I/O stats.
-pub(crate) fn io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
+pub(crate) fn io_stats() -> anyhow::Result<Vec<IoCounters>> {
let volume_io = all_volume_io()?;
Ok(volume_io
@@ -23,6 +23,7 @@ pub(crate) fn io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
IoCounters::new(name, read_bytes, write_bytes)
})
+ .flatten()
.collect::<Vec<_>>())
}
diff --git a/src/app/data_harvester/disks/zfs_io_counters.rs b/src/app/data_harvester/disks/zfs_io_counters.rs
index a53e8af3..a4f30397 100644
--- a/src/app/data_harvester/disks/zfs_io_counters.rs
+++ b/src/app/data_harvester/disks/zfs_io_counters.rs
@@ -2,7 +2,7 @@ use crate::app::data_harvester::disks::IoCounters;
/// Returns zpool I/O stats. Pulls data from `sysctl kstat.zfs.{POOL}.dataset.{objset-*}`
#[cfg(target_os = "freebsd")]
-pub fn zfs_io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
+pub fn zfs_io_stats() -> anyhow::Result<Vec<IoCounters>> {
use sysctl::Sysctl;
let zfs_ctls: Vec<_> = sysctl::Ctl::new("kstat.zfs.")?
.into_iter()
@@ -27,7 +27,7 @@ pub fn zfs_io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
.collect();
use itertools::Itertools;
- let results: Vec<anyhow::Result<IoCounters>> = zfs_ctls
+ let results: Vec<IoCounters> = zfs_ctls
.iter()
.chunks(3)
.into_iter()
@@ -50,7 +50,7 @@ pub fn zfs_io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
}
}
}
- Some(Ok(IoCounters::new(ds_name, nread, nwrite)))
+ Some(IoCounters::new(ds_name, nread, nwrite))
})
.collect();
Ok(results)
@@ -58,7 +58,7 @@ pub fn zfs_io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
/// Returns zpool I/O stats. Pulls data from `/proc/spl/kstat/zfs/*/objset-*`.
#[cfg(target_os = "linux")]
-pub fn zfs_io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
+pub fn zfs_io_stats() -> anyhow::Result<Vec<IoCounters>> {
if let Ok(zpools) = std::fs::read_dir("/proc/spl/kstat/zfs") {
let zpools_vec: Vec<std::path::PathBuf> = zpools
.filter_map(|e| {
@@ -90,7 +90,7 @@ pub fn zfs_io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
})
})
.collect();
- let io_counters: Vec<anyhow::Result<IoCounters>> = datasets_vec
+ let io_counters: Vec<IoCounters> = datasets_vec
.iter()
.filter_map(|ds| {
// get io-counter from each dataset
@@ -130,9 +130,9 @@ pub fn zfs_io_stats() -> anyhow::Result<Vec<anyhow::Result<IoCounters>>> {
}
}
});
+
let counter = IoCounters::new(name.to_owned(), read, write);
- //log::debug!("adding io counter for zfs {:?}", counter);
- Some(Ok(counter))
+ Some(counter)
} else {
None
}