Author SHA1 Message Date
osobh bc2996e536 Merge pull request 'Fix dashboard disk usage overstating used space fleet-wide' (#114) from fix-filesystem-usage-overstates-used into main
Build with clawstor cache / Cargo build (clawstor-cached) (push) Failing after 2s
2026-08-02 23:37:20 +00:00
osobhandClaude Sonnet 5 ef02b22b72 Fix dashboard disk usage overstating used space fleet-wide
Build with clawstor cache / Cargo build (clawstor-cached) (pull_request) Failing after 3s
Reported by the operator: dashboard's "used" figure for morpheus
didn't match `df`. Root cause: filesystem_usage() computed
`used = total - f_bavail`. f_bavail is space available to an
*unprivileged* user, which excludes ext4's reserved-blocks-for-root
margin (~5% of the filesystem by default) -- so that formula folded
the entire reserved margin into "used" on every node, not just
morpheus. Worse on bigger disks: this overstated tank's usage by
~1GB less noticeably relative to its 1.9TB size, but the same
absolute-percentage bug applies everywhere.

`df`'s Used column is `total - f_bfree` (raw free blocks, reserved
or not) -- matching that formula is what makes the dashboard agree
with `df` instead of silently running high. `available_bytes` still
reports f_bavail (what's actually writable), unchanged.

Verified against `df -h /` on all three nodes post-fix: tank
93.6GiB vs df's 93G, architect 93.6GiB vs 94G, morpheus 135.5GiB vs
136G -- all now agree within rounding.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
2026-08-02 16:37:07 -07:00
osobh cc26052cfe Merge pull request 'Recover replication baseline from remote when locally pruned' (#113) from fix-replicate-broken-incremental-base into main
Build with clawstor cache / Cargo build (clawstor-cached) (push) Failing after 3s
2026-08-02 20:59:37 +00:00
osobhandClaude Sonnet 5 11a259b762 Recover replication baseline from remote when locally pruned
Build with clawstor cache / Cargo build (clawstor-cached) (pull_request) Failing after 2s
Found during a routine fleet health sweep: tank's daily replication
to architect had silently broken. replicate_to_cold() records the
last-replicated snapshot name in a local state file and reuses it as
the incremental send base next time -- but never handles the case
where local snapshot retention (snapshot_retain_hours = 24) prunes
that exact snapshot before the next replication run. When that
happens it silently falls back to a FULL send, which then hard-fails
against a non-empty destination ("must destroy them to overwrite
it").

Root cause of the timing gap: the daemon's replication tick is a
24-hour interval, same order of magnitude as local retention. Every
claw-store.service restart resets that tick's countdown without
resetting the hourly-snapshot pruning tick, so a day of frequent
restarts (routine during active deployment work) is enough for the
two to drift out of sync -- the recorded baseline ages out locally
before replication ever gets to reuse it.

Fix: when the recorded baseline is gone, query the remote's actual
snapshot list over SSH (new ZfsOps::list_remote_snapshots) and find
the newest snapshot both sides still share by tag, rather than
giving up and attempting a full send. Only truly falls back to full
when no common snapshot exists anywhere. Verified live against tank
-> architect: correctly recovered daily-2026-08-01-0000 as the base
and completed an incremental send.

Added test coverage for all three paths (recorded baseline present,
recovered from remote, no common snapshot found) -- replicate_to_cold
had none before this, since LAST_REPLICATED_PATH was a hardcoded
absolute path with no way to inject a test double. Split into a
public wrapper plus replicate_to_cold_with_state_path() so tests can
use a temp file instead of touching real /var/lib/claw-store state.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
2026-08-02 13:59:24 -07:00
osobh ec24f37d90 Merge pull request 'Fix stdout/stderr ordering in ShutdownPrepCheck' (#112) from fix-shutdown-prep-check-stderr-ordering into main
Build with clawstor cache / Cargo build (clawstor-cached) (push) Failing after 2s
2026-08-01 02:45:42 +00:00
osobhandClaude Sonnet 5 e5fcb8b3f4 Fix stdout/stderr ordering in ShutdownPrepCheck
Build with clawstor cache / Cargo build (clawstor-cached) (pull_request) Failing after 3s
Command::output() captures stdout and stderr as two separate
buffers. check() was concatenating stdout-then-stderr, which throws
away chronological order entirely -- every stderr line (e.g.
"Error: send_to_host not set" from `claw-store replicate` on a node
with no downstream replication target, which is normal and expected
on architect) landed at the very end of the report regardless of
when it actually printed, making a mid-script, already-handled
condition look like a failure that happened after "DRY RUN
COMPLETE".

Fix: invoke via `bash -c "script --dry-run 2>&1"` so stderr merges
into stdout inside the shell, before either stream reaches us --
true chronological order preserved, single buffer to read.

Verified on architect: the send_to_host message now appears exactly
where it happens, inside the "taking final snapshot + replicating"
step, with DRY RUN COMPLETE correctly last.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
2026-07-31 19:45:29 -07:00
osobh 9debd84e95 Merge pull request 'Honor zfs_dataset = "none"; surface shutdown-prep panel from fleet view' (#111) from fix-zfs-none-and-shutdown-panel-discoverability into main
Build with clawstor cache / Cargo build (clawstor-cached) (push) Failing after 3s
2026-08-01 00:03:14 +00:00
4 changed files with 249 additions and 10 deletions
+41 -1
View File
@@ -655,6 +655,17 @@ fn decode_nibble(b: u8) -> Option<u8> {
/// statvfs on the given path. Uses libc directly — cheap enough /// statvfs on the given path. Uses libc directly — cheap enough
/// that we don't need to cache. Silent on error (returns None). /// that we don't need to cache. Silent on error (returns None).
///
/// Bug fix 2026-08-02: `used` was computed as `total - f_bavail`.
/// `f_bavail` is space available to an *unprivileged* user, which
/// excludes ext4's reserved-blocks-for-root margin (~5% of the
/// filesystem by default) — so that formula silently folded the
/// entire reserved margin into "used", overstating usage by exactly
/// that amount on every node (worse on bigger disks: ~21GB on a
/// 466GB root, proportionally more on multi-TB ones). `df`'s Used
/// column is `total - f_bfree` (raw free blocks, root-reserved or
/// not) — matching that here is what makes the dashboard agree with
/// `df` instead of silently running high.
fn filesystem_usage(path: &std::path::Path) -> Option<FilesystemUsage> { fn filesystem_usage(path: &std::path::Path) -> Option<FilesystemUsage> {
let cpath = std::ffi::CString::new(path.as_os_str().to_str()?).ok()?; let cpath = std::ffi::CString::new(path.as_os_str().to_str()?).ok()?;
// SAFETY: statvfs writes to a zero-initialised struct; we // SAFETY: statvfs writes to a zero-initialised struct; we
@@ -667,7 +678,8 @@ fn filesystem_usage(path: &std::path::Path) -> Option<FilesystemUsage> {
let bsize = stat.f_frsize as u64; let bsize = stat.f_frsize as u64;
let total = stat.f_blocks as u64 * bsize; let total = stat.f_blocks as u64 * bsize;
let avail = stat.f_bavail as u64 * bsize; let avail = stat.f_bavail as u64 * bsize;
let used = total.saturating_sub(avail); let free = stat.f_bfree as u64 * bsize;
let used = total.saturating_sub(free);
Some(FilesystemUsage { Some(FilesystemUsage {
mount_point: path.display().to_string(), mount_point: path.display().to_string(),
total_bytes: total, total_bytes: total,
@@ -676,6 +688,34 @@ fn filesystem_usage(path: &std::path::Path) -> Option<FilesystemUsage> {
}) })
} }
#[cfg(test)]
mod filesystem_usage_tests {
use super::filesystem_usage;
#[test]
fn used_plus_available_never_exceeds_total() {
// `available` is a subset of the space `used` now excludes
// (used = total - free, and available = f_bavail <= f_bfree),
// so this invariant holds regardless of any root-reserved
// margin -- the regression this guards against is `used`
// being computed as `total - available`, which collapses
// that margin into `used` and can make `used + available`
// overshoot `total` in the other direction (it wouldn't here,
// but the two numbers would silently disagree with `df`).
let fs = filesystem_usage(std::path::Path::new(".")).expect("statvfs on cwd");
assert!(fs.total_bytes > 0);
assert!(fs.used_bytes <= fs.total_bytes);
assert!(fs.available_bytes <= fs.total_bytes.saturating_sub(fs.used_bytes) + 4096,
"available ({}) should fit within total-used ({}) modulo one block; got used={}, total={}",
fs.available_bytes, fs.total_bytes.saturating_sub(fs.used_bytes), fs.used_bytes, fs.total_bytes);
}
#[test]
fn missing_path_returns_none() {
assert!(filesystem_usage(std::path::Path::new("/this/path/does/not/exist/at/all")).is_none());
}
}
/// Query systemd for a user-scope timer's next-fire + last result. /// Query systemd for a user-scope timer's next-fire + last result.
/// Shells out to systemctl. Silent on any failure — dashboards /// Shells out to systemctl. Silent on any failure — dashboards
/// should degrade to "unknown" rather than 500. /// should degrade to "unknown" rather than 500.
+12 -4
View File
@@ -87,16 +87,24 @@ pub async fn check() -> Result<ShutdownPrepCheckReply> {
if !script.exists() { if !script.exists() {
bail!("shutdown-prep script not found at {}", script.display()); bail!("shutdown-prep script not found at {}", script.display());
} }
// `2>&1` inside the shell merges stderr into stdout *before*
// either stream is piped back to us, preserving true
// chronological order. Capturing stdout/stderr separately (as
// `Command::output()` does by default) and concatenating them
// after the fact loses interleaving entirely — every stderr line
// lands at the very end regardless of when it was actually
// printed, which makes a mid-script warning (e.g. "replicate not
// configured on this node") look like a failure that happened
// after "DRY RUN COMPLETE".
let run = Command::new("bash") let run = Command::new("bash")
.arg(&script) .arg("-c")
.arg("--dry-run") .arg(format!("{} --dry-run 2>&1", script.display()))
.output(); .output();
let output = timeout(CHECK_TIMEOUT, run) let output = timeout(CHECK_TIMEOUT, run)
.await .await
.context("shutdown-prep --dry-run timed out")? .context("shutdown-prep --dry-run timed out")?
.context("spawning shutdown-prep --dry-run")?; .context("spawning shutdown-prep --dry-run")?;
let mut combined = String::from_utf8_lossy(&output.stdout).into_owned(); let combined = String::from_utf8_lossy(&output.stdout).into_owned();
combined.push_str(&String::from_utf8_lossy(&output.stderr));
Ok(ShutdownPrepCheckReply { Ok(ShutdownPrepCheckReply {
ready: output.status.success(), ready: output.status.success(),
output: combined, output: combined,
+146 -4
View File
@@ -66,6 +66,23 @@ pub fn replicate_to_cold(
remote_user: &str, remote_user: &str,
remote_host: &str, remote_host: &str,
remote_dataset: &str, remote_dataset: &str,
) -> Result<()> {
replicate_to_cold_with_state_path(
zfs, dataset, remote_user, remote_host, remote_dataset,
std::path::Path::new(LAST_REPLICATED_PATH),
)
}
/// Same as [`replicate_to_cold`] with the state-file path injectable
/// -- lets tests exercise the baseline-recovery logic against a temp
/// file instead of the real `/var/lib/claw-store/...` path.
pub fn replicate_to_cold_with_state_path(
zfs: &dyn ZfsOps,
dataset: &str,
remote_user: &str,
remote_host: &str,
remote_dataset: &str,
state_path: &std::path::Path,
) -> Result<()> { ) -> Result<()> {
let snaps = zfs.list_snapshots(dataset)?; let snaps = zfs.list_snapshots(dataset)?;
let latest = match snaps.last().cloned() { let latest = match snaps.last().cloned() {
@@ -77,12 +94,64 @@ pub fn replicate_to_cold(
}; };
// Use the last successfully replicated snapshot as the incremental base. // Use the last successfully replicated snapshot as the incremental base.
// Only valid if it still exists in the current snapshot list. // Only valid if it still exists in the current snapshot list -- local
let prev = std::fs::read_to_string(LAST_REPLICATED_PATH) // retention (snapshot_retain_hours) can prune it out from under us
// between replication runs (e.g. a daemon restart resets the
// replication tick's 24h timer without resetting the hourly-snapshot
// pruning tick, so a slow/interrupted replication cadence can let the
// recorded snapshot age out locally before it's ever used again).
let recorded = std::fs::read_to_string(state_path)
.ok() .ok()
.map(|s| s.trim().to_string()) .map(|s| s.trim().to_string())
.filter(|s| !s.is_empty() && snaps.contains(s)); .filter(|s| !s.is_empty() && snaps.contains(s));
// Recorded snapshot is gone -- don't give up and fall back to a full
// send (which fails outright against a non-empty destination, as
// opposed to just being wasteful). Ask the remote what it actually
// has and find the newest snapshot both sides still share, by tag
// (the `@kind-timestamp` suffix -- dataset paths differ between
// source and destination, e.g. slab/projects vs
// data/archive/tank-projects, but tags are written identically).
let prev = match recorded {
Some(p) => Some(p),
None => {
match zfs.list_remote_snapshots(remote_user, remote_host, remote_dataset) {
Ok(remote_snaps) => {
let remote_tags: std::collections::HashSet<&str> = remote_snaps
.iter()
.filter_map(|s| s.split_once('@').map(|(_, tag)| tag))
.collect();
let fallback = snaps
.iter()
.rev()
.skip(1) // exclude `latest` itself
.find(|s| {
s.split_once('@')
.map(|(_, tag)| remote_tags.contains(tag))
.unwrap_or(false)
})
.cloned();
if let Some(f) = &fallback {
tracing::warn!(
"recorded replication baseline was pruned locally; \
recovered a common snapshot from the remote instead: {}",
f
);
}
fallback
}
Err(e) => {
tracing::warn!(
error = %e,
"could not query remote snapshots to recover a replication baseline; \
falling back to full send"
);
None
}
}
}
};
if prev.as_deref() == Some(latest.as_str()) { if prev.as_deref() == Some(latest.as_str()) {
tracing::info!("replication already up to date ({})", latest); tracing::info!("replication already up to date ({})", latest);
return Ok(()); return Ok(());
@@ -99,10 +168,10 @@ pub fn replicate_to_cold(
zfs.send_to_remote(&latest, prev.as_deref(), remote_user, remote_host, remote_dataset)?; zfs.send_to_remote(&latest, prev.as_deref(), remote_user, remote_host, remote_dataset)?;
// Record this snapshot as the new baseline for the next incremental send. // Record this snapshot as the new baseline for the next incremental send.
if let Some(parent) = std::path::Path::new(LAST_REPLICATED_PATH).parent() { if let Some(parent) = state_path.parent() {
let _ = std::fs::create_dir_all(parent); let _ = std::fs::create_dir_all(parent);
} }
std::fs::write(LAST_REPLICATED_PATH, &latest)?; std::fs::write(state_path, &latest)?;
Ok(()) Ok(())
} }
@@ -177,4 +246,77 @@ mod tests {
assert!(!is_sunday_midnight("2026-06-29-0000")); // Monday assert!(!is_sunday_midnight("2026-06-29-0000")); // Monday
assert!(!is_sunday_midnight("2026-06-28-0100")); // Sunday but not midnight assert!(!is_sunday_midnight("2026-06-28-0100")); // Sunday but not midnight
} }
/// Unique temp path per test so parallel test runs don't clobber
/// each other's replication-baseline state file.
fn temp_state_path(name: &str) -> std::path::PathBuf {
std::env::temp_dir().join(format!("clawstor-test-last-replicated-{name}-{}", std::process::id()))
}
#[test]
fn replicate_uses_recorded_baseline_when_still_present() {
let zfs = MockZfs::default();
zfs.snapshot("slab/projects", "hourly-1").unwrap();
zfs.snapshot("slab/projects", "hourly-2").unwrap();
let state = temp_state_path("recorded-present");
std::fs::write(&state, "slab/projects@hourly-1").unwrap();
replicate_to_cold_with_state_path(
&zfs, "slab/projects", "user", "host", "remote/ds", &state,
).unwrap();
let sends = zfs.sends();
assert_eq!(sends.len(), 1);
assert_eq!(sends[0].0, "slab/projects@hourly-2");
assert_eq!(sends[0].1.as_deref(), Some("slab/projects@hourly-1"));
let _ = std::fs::remove_file(&state);
}
#[test]
fn replicate_recovers_baseline_from_remote_when_recorded_one_was_pruned() {
let zfs = MockZfs::default();
// Local retention already pruned hourly-1 -- only hourly-2 and
// hourly-3 remain locally. The remote, however, still has
// hourly-2 (it just hasn't received hourly-3 yet).
zfs.snapshot("slab/projects", "hourly-2").unwrap();
zfs.snapshot("slab/projects", "hourly-3").unwrap();
zfs.set_remote_snapshots(vec![
"remote/ds@hourly-1".to_string(),
"remote/ds@hourly-2".to_string(),
]);
let state = temp_state_path("recovers-from-remote");
// Recorded baseline (hourly-1) no longer exists locally.
std::fs::write(&state, "slab/projects@hourly-1").unwrap();
replicate_to_cold_with_state_path(
&zfs, "slab/projects", "user", "host", "remote/ds", &state,
).unwrap();
let sends = zfs.sends();
assert_eq!(sends.len(), 1);
assert_eq!(sends[0].0, "slab/projects@hourly-3");
// Recovered hourly-2 as the base by matching tags against the
// remote's actual snapshot list, NOT a full send.
assert_eq!(sends[0].1.as_deref(), Some("slab/projects@hourly-2"));
let _ = std::fs::remove_file(&state);
}
#[test]
fn replicate_falls_back_to_full_send_when_truly_no_common_snapshot() {
let zfs = MockZfs::default();
zfs.snapshot("slab/projects", "hourly-9").unwrap();
zfs.set_remote_snapshots(vec!["remote/ds@hourly-1".to_string()]);
let state = temp_state_path("no-common-snapshot");
let _ = std::fs::remove_file(&state); // no recorded baseline at all
replicate_to_cold_with_state_path(
&zfs, "slab/projects", "user", "host", "remote/ds", &state,
).unwrap();
let sends = zfs.sends();
assert_eq!(sends.len(), 1);
assert_eq!(sends[0].0, "slab/projects@hourly-9");
assert_eq!(sends[0].1, None, "no common tag exists -- must fall back to full send");
let _ = std::fs::remove_file(&state);
}
} }
+50 -1
View File
@@ -13,6 +13,12 @@ pub trait ZfsOps: Send + Sync {
remote_user: &str, remote_host: &str, remote_user: &str, remote_host: &str,
remote_dataset: &str) -> Result<()>; remote_dataset: &str) -> Result<()>;
fn clone_snapshot(&self, snapshot: &str, dest_dataset: &str) -> Result<()>; fn clone_snapshot(&self, snapshot: &str, dest_dataset: &str) -> Result<()>;
/// List snapshot names (full `dataset@tag` form, remote-side
/// naming) currently on a remote dataset over SSH. Used to find a
/// real incremental base when the locally-recorded one has been
/// pruned -- see `snapshot::replicate_to_cold`.
fn list_remote_snapshots(&self, remote_user: &str, remote_host: &str,
remote_dataset: &str) -> Result<Vec<String>>;
} }
pub struct SystemZfs; pub struct SystemZfs;
@@ -89,11 +95,48 @@ impl ZfsOps for SystemZfs {
} }
Ok(()) Ok(())
} }
fn list_remote_snapshots(&self, remote_user: &str, remote_host: &str,
remote_dataset: &str) -> Result<Vec<String>> {
let out = std::process::Command::new("ssh")
.args([
&format!("{remote_user}@{remote_host}"),
"zfs", "list", "-H", "-t", "snapshot", "-o", "name", "-r", remote_dataset,
])
.output()
.context("running ssh zfs list on remote")?;
if !out.status.success() {
bail!("remote zfs list failed: {}", String::from_utf8_lossy(&out.stderr));
}
Ok(String::from_utf8_lossy(&out.stdout)
.lines()
.filter(|l| !l.is_empty())
.map(String::from)
.collect())
}
} }
#[derive(Default, Clone)] #[derive(Default, Clone)]
pub struct MockZfs { pub struct MockZfs {
snapshots: Arc<Mutex<Vec<String>>>, snapshots: Arc<Mutex<Vec<String>>>,
/// Configurable via `set_remote_snapshots` -- what
/// `list_remote_snapshots` returns, for exercising the
/// baseline-recovery path in `snapshot::replicate_to_cold`.
remote_snapshots: Arc<Mutex<Vec<String>>>,
/// Every `send_to_remote` call, recorded as `(snapshot,
/// incremental_from)`, so tests can assert which base was
/// actually used.
sends: Arc<Mutex<Vec<(String, Option<String>)>>>,
}
impl MockZfs {
pub fn set_remote_snapshots(&self, snaps: Vec<String>) {
*self.remote_snapshots.lock().unwrap() = snaps;
}
pub fn sends(&self) -> Vec<(String, Option<String>)> {
self.sends.lock().unwrap().clone()
}
} }
impl ZfsOps for MockZfs { impl ZfsOps for MockZfs {
@@ -115,8 +158,9 @@ impl ZfsOps for MockZfs {
Ok(()) Ok(())
} }
fn send_to_remote(&self, _snap: &str, _incr: Option<&str>, fn send_to_remote(&self, snap: &str, incr: Option<&str>,
_user: &str, _host: &str, _dest: &str) -> Result<()> { _user: &str, _host: &str, _dest: &str) -> Result<()> {
self.sends.lock().unwrap().push((snap.to_string(), incr.map(String::from)));
Ok(()) Ok(())
} }
@@ -125,6 +169,11 @@ impl ZfsOps for MockZfs {
.push(format!("{} -> {}", snapshot, dest_dataset)); .push(format!("{} -> {}", snapshot, dest_dataset));
Ok(()) Ok(())
} }
fn list_remote_snapshots(&self, _remote_user: &str, _remote_host: &str,
_remote_dataset: &str) -> Result<Vec<String>> {
Ok(self.remote_snapshots.lock().unwrap().clone())
}
} }
#[cfg(test)] #[cfg(test)]