Skip to content

Commit

Permalink
Fix long-standing bug in reported 'host' total and used memory
Browse files Browse the repository at this point in the history
  • Loading branch information
nlcamp committed Mar 10, 2023
1 parent bb70ec6 commit 7f74994
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 21 deletions.
8 changes: 4 additions & 4 deletions edgelet/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion edgelet/edgelet-docker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ nix = "0.26"
serde = "1"
serde_json = "1"
serial_test = "1"
sysinfo = "0.25"
sysinfo = "0.28"
thiserror = "1"
tokio = { version = "1", features = ["parking_lot", "sync"] }
url = "2"
Expand Down
20 changes: 6 additions & 14 deletions edgelet/edgelet-docker/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -853,11 +853,11 @@ where
}

fn total_memory_bytes(system_resources: &System) -> u64 {
system_resources.total_memory() * 1024
system_resources.total_memory()
}

fn used_memory_bytes(system_resources: &System) -> u64 {
system_resources.used_memory() * 1024
system_resources.used_memory()
}

fn parse_top_response<'de, D>(resp: &InlineResponse2001) -> Result<Vec<i32>, D::Error>
Expand Down Expand Up @@ -1095,20 +1095,12 @@ mod tests {

// Compare the total memory returned by the 'total_memory_bytes()' helper method
// to the value in /proc/meminfo
// TODO: Adjust this test when we upgrade to sysinfo >= v0.26
#[test]
fn test_total_memory_bytes() {
// Use 'total_memory_bytes()' helper method to get total memory
let system_resources = System::new_all();
let total_memory_bytes = total_memory_bytes(&system_resources);

// Convert to KiB, which are the units returned by v0.25 of the sysinfo crate.
// We perform this conversion to workaround a bug in our code causing us to report
// 1.024 times the actual number of bytes for host-level total memory.
// TODO: If we decide to fix this bug, remove this conversion and compare the values
// in units of bytes.
let total_memory_kibibyte = total_memory_bytes / 1024;

// Get expected total memory directly from /proc/meminfo
let cat_proc_meminfo = Command::new("cat")
.arg("/proc/meminfo")
Expand All @@ -1130,11 +1122,11 @@ mod tests {
.spawn()
.expect("Failed to execute 'grep -o [0-9]*'");
let output = grep_value.wait_with_output().unwrap();
let expected_total_memory_kilobyte_str = str::from_utf8(&output.stdout).unwrap().trim();
let expected_total_memory_kibibyte =
expected_total_memory_kilobyte_str.parse::<u64>().unwrap() * 1024 / 1000;
let expected_total_memory_kilobytes_str = str::from_utf8(&output.stdout).unwrap().trim();
let expected_total_memory_bytes =
expected_total_memory_kilobytes_str.parse::<u64>().unwrap() * 1024;

// Compare
assert_eq!(total_memory_kibibyte, expected_total_memory_kibibyte);
assert_eq!(total_memory_bytes, expected_total_memory_bytes);
}
}
2 changes: 1 addition & 1 deletion edgelet/iotedge/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ nix = "0.26"
regex = "1"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
sysinfo = "0.25"
sysinfo = "0.28"
tabwriter = "1"
termcolor = "1"
thiserror = "1"
Expand Down
2 changes: 1 addition & 1 deletion edgelet/iotedge/src/check/additional_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl DiskInfo {
#[cfg(unix)]
fn pretty_kbyte(bytes: u64) -> String {
#[allow(clippy::cast_precision_loss)]
match Byte::from_unit(bytes as f64, ByteUnit::KiB) {
match Byte::from_unit(bytes as f64, ByteUnit::B) {
Ok(b) => b.get_appropriate_unit(true).format(2),
Err(err) => format!("could not parse bytes value: {:?}", err),
}
Expand Down

0 comments on commit 7f74994

Please sign in to comment.