diff --git a/CHANGELOG.md b/CHANGELOG.md index 85475d3..bfe6ba2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,31 @@ EXPERIMENTAL, so on-disk formats and the CLI may change without notice. ## [Unreleased] +### Added + +- **Fabric deletes its own old logs.** The daemon wrote one validation log per + day and never removed any of them, so the directory grew without limit for as + long as the daemon ran. One machine had accumulated **2.4 GB across 20 daily + files**, with a single noisy day reaching 587 MB. Nothing in the tree + implemented retention, pruning, or a maximum age. + + The most recent **45** daily logs are now kept and older ones are deleted. + Forty-five is derived from the job rather than rounded: retention has to + outlast a month away from the machine so a fault in the first week is still + readable on return, plus margin for the gap before anyone looks. At the + observed 8.8–10.3 MB per day that is roughly 420 MB. + + `FABRIC_LOG_RETENTION_DAYS` overrides the count; `0` disables deletion and + restores the old unbounded behaviour. An unparseable value falls back to the + default rather than to unbounded, because failing open on an unattended + machine is the worst outcome. The resolved value is recorded in the + `diagnostic_logging_init` line. + + This bounds the file count, which is what stops indefinite growth. It does not + bound bytes — a single day has reached 587 MB — and capping that is a question + about log volume, not retention. Logs written before this bound are not + reclaimed; that is an operator's call. + ### Fixed documentation - **The detached replay buffer is capped, and the retention docs said it was diff --git a/README.md b/README.md index 0e4d691..22d042a 100644 --- a/README.md +++ b/README.md @@ -512,6 +512,33 @@ still reports reachable. A peer with no recorded loss is omitted, and `sessions no losses recorded` means nothing has dropped since the counters were last reset. +### Fabric deletes its own old logs + +The daemon writes one validation log per day to `/logs/` and **keeps the +most recent 45, deleting older ones**. Say this out loud rather than let someone +find it out: a log from more than 45 days ago is gone, and an empty directory is +a bad way to learn that. + +Forty-five days is chosen from the job the logs do, not rounded for looks. It has +to outlast a month away from the machine, so a fault in the first week is still +readable on return, with margin for the gap before anyone looks. At the observed +rate of 8.8–10.3 MB per day that is roughly 420 MB. + +`FABRIC_LOG_RETENTION_DAYS` overrides the count. **`0` disables deletion +entirely** and restores unbounded growth, which is the right trade only if you +would rather spend disk than lose history. An unparseable value falls back to the +default rather than to unbounded, because failing open on an unattended machine +is the worst outcome. The daemon records the value it resolved in the +`diagnostic_logging_init` line, so the running config is checkable. + +This bounds the **number of files**, which is what stops indefinite growth. It +does not bound bytes: one noisy day has reached 587 MB, so a bad run still costs +far more than the daily average suggests. Capping that is a question about log +volume, not retention. + +Fabric does not reclaim logs written before this bound existed. Deleting those is +an operator's call. + ## Developing Fabric (dev vs prod) A production fabric daemon is often load-bearing (it may be your only path to a @@ -548,6 +575,11 @@ The same pattern applies to any per-instance daemon: per-instance home/socket/identity, prod is the one service, dev is a manual run on a distinct home. +Three branches on `origin` are neither merged into `main` nor known to be +obsolete, so `git branch --merged` will never retire them. Before you tidy them +away, read [docs/unresolved-branches.md](docs/unresolved-branches.md) — it says +what is known, what is not, and why deleting them on a guess is the wrong trade. + ## Commands ```sh diff --git a/docs/unresolved-branches.md b/docs/unresolved-branches.md new file mode 100644 index 0000000..1db3bd0 --- /dev/null +++ b/docs/unresolved-branches.md @@ -0,0 +1,51 @@ +# Unresolved branches on `origin` + +Three remote branches are neither merged into `main` nor safe to assume are +obsolete. This note exists so the next person meets a recorded fact instead of a +puzzle. + +| Branch | Commit | Size against `main` | +| --- | --- | --- | +| `agent/production-sync-scan-counters` | `24a0fd8` Expose production sync scan counters | 7 files, +234 / −32 | +| `agent/reliable-shell-sync-noop` | `a5671e9` Restore shell compatibility and skip converged scans | 8 files, +1078 / −91 | +| `agent/suppress-sync-self-events` | `558f094` Suppress delayed sync self-events | 1 file, +551 / −38 | + +Measured on 2026-08-04 against `main` at `5999384`. + +## What is known + +Each branch is a single commit. None is an ancestor of `main`, so +`git branch --merged` will never list them and ancestry alone will never retire +them. + +They belong to the `af8fb02` line — the four pull requests (#25, #26, #28, #29) +that were **not** merged into `main` directly. That line was instead absorbed by +a separate reconciliation, which preserved the behaviour while arriving at it by +a different route. `main` is therefore *believed* to contain the substance of +all three. + +## What is not known + +**Nobody has proven that.** "Absorbed by a different route" is a claim about +behaviour, not about commits, and no one has gone branch by branch to confirm +that every change in each one is represented in `main` today. + +The diffs are large and `main` has moved a long way since, so a three-dot diff +answers nothing on its own: it shows divergence from a shared ancestor, not +missing behaviour. + +## Why they are still here + +Deleting a remote branch that is not an ancestor of `main` is **not reversible +for anyone but the person who still has it locally**. Keeping a branch costs +nothing. Tidiness is not worth an irreversible action on evidence that is only +probably right. + +Verifying equivalence is real archaeology and needs judgement per branch. Nobody +needs these branches today, so that cost has not been paid. + +## What to do if you care + +Do not delete them on the strength of this note. Either prove per branch that +`main` covers the behaviour — and record the proof here — or leave them alone. +If you prove it, say which commit in `main` covers each one, then delete. diff --git a/src/daemon.rs b/src/daemon.rs index 7c630d8..923e5af 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -637,10 +637,54 @@ async fn build_daemon_endpoint( Ok(endpoint) } +/// How many daily validation logs to keep before the oldest is deleted. +/// +/// Derived from the job the logs have to do, not rounded to look tidy. The +/// principal travels for about a month and is the only person who investigates +/// an incident, so retention has to outlast a trip: a fault in week one must +/// still be readable when he gets home. That sets a floor of about 31 days. +/// Two more weeks of margin covers the gap between returning and looking. +/// +/// Measured cost at the observed rate of 8.8 to 10.3 MB per day: roughly 420 MB. +/// Before this bound existed nothing was ever deleted, and one machine had +/// accumulated 2.4 GB across 20 days. +/// +/// This bounds the FILE COUNT, which is what stops indefinite growth. It does +/// not bound bytes: a single noisy day has reached 587 MB, so a bad run can +/// still cost far more than the daily average suggests. Capping that is a +/// question about log volume, not retention, and it is not what this solves. +pub const DEFAULT_LOG_RETENTION_DAYS: usize = 45; + +/// Resolve the retention window, honouring `FABRIC_LOG_RETENTION_DAYS`. +/// +/// `0` disables deletion and restores the old unbounded behaviour, for an +/// operator who would rather spend disk than lose history. +fn resolve_log_retention_days(raw: Option<&str>) -> Option { + match raw.map(str::trim) { + None | Some("") => Some(DEFAULT_LOG_RETENTION_DAYS), + Some(value) => match value.parse::() { + Ok(0) => None, + Ok(days) => Some(days), + // An unparseable override must not silently disable the bound; the + // whole point is that nobody is watching this machine. + Err(_) => Some(DEFAULT_LOG_RETENTION_DAYS), + }, + } +} + pub fn init_daemon_tracing(home: &FabricHome) -> Result<()> { home.prepare()?; - let appender = - tracing_appender::rolling::daily(home.validation_log_dir(), home.validation_log_prefix()); + let retention = + resolve_log_retention_days(env::var("FABRIC_LOG_RETENTION_DAYS").ok().as_deref()); + let mut builder = tracing_appender::rolling::Builder::new() + .rotation(tracing_appender::rolling::Rotation::DAILY) + .filename_prefix(home.validation_log_prefix()); + if let Some(days) = retention { + builder = builder.max_log_files(days); + } + let appender = builder + .build(home.validation_log_dir()) + .context("failed to build the validation log appender")?; let subscriber = tracing_subscriber::fmt() .with_ansi(false) .with_env_filter(validation_log_filter()) @@ -653,6 +697,7 @@ pub fn init_daemon_tracing(home: &FabricHome) -> Result<()> { target: VALIDATION_LOG_TARGET, event = "diagnostic_logging_init", iroh_path_trace = env::var_os("FABRIC_IROH_PATH_TRACE").is_some(), + log_retention_days = retention.unwrap_or(0), "fabric validation logging initialized" ); } @@ -5172,6 +5217,51 @@ mod tests { Ok(()) } + /// The retention default is a decided value, so pin it. + /// + /// It is derived from the job: retention must outlast a month-long trip so a + /// fault in week one is still readable on return, plus margin for the gap + /// before anyone looks. Changing it should require changing this assertion + /// and saying why. + #[test] + fn log_retention_default_outlasts_a_month_long_trip() { + assert_eq!(DEFAULT_LOG_RETENTION_DAYS, 45); + assert!( + DEFAULT_LOG_RETENTION_DAYS > 31, + "retention must exceed a month-long trip or an early fault is deleted \ + before the only person who investigates it gets home" + ); + assert_eq!(resolve_log_retention_days(None), Some(45)); + } + + /// An explicit 0 restores the old unbounded behaviour, on purpose. + #[test] + fn zero_days_disables_deletion() { + assert_eq!(resolve_log_retention_days(Some("0")), None); + } + + #[test] + fn an_override_is_honoured() { + assert_eq!(resolve_log_retention_days(Some("7")), Some(7)); + assert_eq!(resolve_log_retention_days(Some(" 7 ")), Some(7)); + } + + /// A typo must not silently restore unbounded growth. + /// + /// Failing open here would be the worst outcome: nobody is at this machine's + /// keyboard, so the defect would return unnoticed and look like the fix + /// never worked. + #[test] + fn an_unparseable_override_falls_back_to_the_bound_not_to_unbounded() { + for bad in ["", " ", "lots", "-1", "9999999999999999999999", "7d"] { + assert_eq!( + resolve_log_retention_days(Some(bad)), + Some(DEFAULT_LOG_RETENTION_DAYS), + "{bad:?} must fall back to the bound, never to unbounded" + ); + } + } + /// The counters must move because the real notice path ran. /// /// The store has its own unit tests, and they prove only that the store