diff --git a/Cargo.lock b/Cargo.lock index 4d79838..5440f4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1111,7 +1111,7 @@ dependencies = [ [[package]] name = "ciris-status" -version = "0.3.68" +version = "0.3.69" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index ee70f29..d5d2e64 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ciris-status" -version = "0.3.68" +version = "0.3.69" edition = "2021" description = "The ciris.ai public health/status surface — now a ciris-server fabric node + a StatusAdapter (mirrors CIRISAgent's adapter model). Serves /health, /v1/status, /api/v1/status, /api/v1/status/history + the public scoring roster (Flow A, own corpus) + live SSE/WS, by live outbound probes + a SQLite uptime history. The federation node (engine, edge, consent:replication, read API, ownership, safety, NAT-traversal) is ciris-server's serve; the status page is the adapter." license = "AGPL-3.0-or-later" diff --git a/README.md b/README.md index a237346..80e6b03 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ Drop-in for the Lens nginx route (`agents.ciris.ai/lens/api/…` → this servic | `GET /api/v1/scoring` | **Public scoring roster** (Flow A): opted-in agents `{key_id, capacity_composite, factors?, valid_until}`, consent-gated. Replaces lens-python's scoring feed. Served from cache, populated from this node's OWN corpus by the adapter loop. | | `GET /api/v1/status` (capabilities) | The same response now carries `capabilities` (per-pool rollup with `min_available`, `available`, and per-member `role`/`status`), an `indicator` (Statuspage v2 severity), and `vantage_failure`. The headline is derived from capabilities, not from whichever component is unhappiest — see `FSD/CAPABILITY_MONITORING.md` | | `GET /api/v1/ci` | **Substrate build health**: the last 10 GitHub Actions runs per repo (verify → persist → edge → server → agent) as `{repo, runs[]}`, each run one of `success\|failure\|in_progress\|queued\|cancelled`. A ~600-byte projection so a microcontroller can read it in one request; polled server-side with conditional requests (see below). | -| `GET /api/v1/debug/memory` | The allocator's own accounting — `uordblks` (live) vs `fordblks` (freed-but-held), plus the kernel's `RssAnon`/`VmSwap`. glibc does not zero on `free()`, so a scan of process memory cannot tell a live working set from churn the allocator kept; only a call from inside can (CIRISStatus#69) | +| `GET /api/v1/debug/memory` | **Only mounted when `CIRIS_DIAGNOSTICS=1`** (CIRISStatus#73 — it answered unauthenticated on the published port while ciris-server gated the identical report). The allocator's own accounting — `uordblks` (live) vs `fordblks` (freed-but-held), plus the kernel's `RssAnon`/`VmSwap`. glibc does not zero on `free()`, so only a call from inside the process can tell a live working set from churn the allocator kept. NOT loopback-bound: an adapter router cannot see the peer, so keep it off the public edge | | `GET /api/v1/scoring/live`, `GET /api/v1/status/live` | **SSE** live-push of roster + overall-health deltas (the "extra website sockets"). | | `GET /api/v1/status/ws` | **WebSocket** variant of the same live-push. | @@ -118,6 +118,7 @@ baked CORS allow-list, and 60s cadence. | `status.corpus_retention_budget` | i64 | `2000` | rows the retention pass deletes per pass | | `status.corpus_retention_secs` | i64 | `120` | how often that pass runs. Budget × cadence sets how fast a backlog drains, and every row still in the corpus is paid for again by every scan until it goes | | `status.roster_secs` | i64 | `300` | Flow A roster rebuild cadence. It is a FULL corpus scan (dimension-prefix filtering walks every row, signatures included), so on a node with no agents it costs disk to re-derive nothing. Floored at `status.poll_secs` | +| `status.malloc_trim_secs` | i64 | `0` (off) | how often to call `malloc_trim(0)`. Off by default: trim reaches the fragmented free lists the arena cap does not (`keepcost` does NOT bound it — since glibc 2.8 trim `MADV_DONTNEED`s free pages inside every arena), but the pages fault back in on reuse, which is a real cost for a churn workload. Enabling it logs `fordblks`/`RssAnon` either side so it is an A/B, not a leap | | `status.cors_origins` | list | baked `ciris.ai` set | CORS allow-list | | `status.ghcr_url` | str | `https://ghcr.io/v2/` | container registry (401 = up) | | `status.database_url` | str | — | local `postgresql` provider (TCP liveness) | diff --git a/src/adapter.rs b/src/adapter.rs index 1a7cf5a..5c55f9e 100644 --- a/src/adapter.rs +++ b/src/adapter.rs @@ -698,6 +698,27 @@ impl Adapter for StatusAdapter { let cors_layer = cors(&cfg); *self.state.cfg.write().expect("cfg lock") = cfg; + // The memory diagnostic is NOT mounted unless the operator asked for it + // (CIRISStatus#73). It answered unauthenticated on the published port, + // while ciris-server gates the identical report on the same host behind + // `CIRIS_DIAGNOSTICS` and binds it to loopback — so the two nodes + // disagreed about whether allocator internals are public. They share a + // host and a ruler; they should share the switch. + // + // `ciris_server::diag::enabled()` IS that switch, read from the same + // process, so one `CIRIS_DIAGNOSTICS=1` turns both on and nothing here + // invents a second control to drift from it. Off, the route is not + // mounted at all: a 404 that looks like every other absent path, rather + // than a 403 advertising that there is something to ask for. + // + // What this does NOT do is bind to loopback. ciris-server pairs the gate + // with `require_loopback` on its own listener; an adapter router cannot + // — the guard is not exported and the read-API listener does not hand us + // `ConnectInfo`. So with diagnostics ON, this is still reachable from + // wherever the port is, and the edge (Caddy) remains the thing keeping it + // off the internet. Said plainly rather than implied, because "gated" + // and "gated and loopback-only" are different promises. + let diagnostics_on = ciris_server::diag::enabled(); let router = Router::new() .route("/", get(root)) // NB: NO `/health` here. Since ciris-server v0.5.32 the embedded node @@ -715,12 +736,22 @@ impl Adapter for StatusAdapter { .route("/api/v1/history", get(history)) .route("/api/v1/scoring", get(scoring)) .route("/api/v1/ci", get(ci)) - .route("/api/v1/debug/memory", get(debug_memory)) .route("/api/v1/scoring/live", get(live_sse)) .route("/api/v1/status/live", get(live_sse)) .route("/api/v1/status/ws", get(live_ws)) .layer(cors_layer) .with_state(self.state.clone()); + let router = if diagnostics_on { + tracing::warn!( + "diagnostics ENABLED — /api/v1/debug/memory is mounted and is NOT loopback-gated here; keep it off the public edge" + ); + router.route( + "/api/v1/debug/memory", + get(debug_memory).with_state(self.state.clone()), + ) + } else { + router + }; vec![router] } @@ -762,6 +793,9 @@ impl Adapter for StatusAdapter { let mut last_prune = std::time::Instant::now() - Duration::from_secs(86_400); // Roster likewise: built on the first cycle, then on its own cadence. let mut last_roster = std::time::Instant::now() - Duration::from_secs(86_400); + // Trim, when enabled at all, waits a full interval first — there is + // nothing to reclaim from a process that has not run yet. + let mut last_trim = std::time::Instant::now(); tracing::info!( poll_s = last_poll, observation_s = self.state.cfg().observation_seconds, @@ -823,6 +857,25 @@ impl Adapter for StatusAdapter { // which is the opposite of what a status plane is for. self.emit_observations(ctx, &agg).await; + // Optional, off unless an operator turned it on: the + // reclaim the arena cap cannot reach. Reports both + // sides so enabling it is an A/B rather than an act of + // faith (CIRISStatus#69). + if cfg.malloc_trim_seconds > 0 + && last_trim.elapsed() >= Duration::from_secs(cfg.malloc_trim_seconds) + { + last_trim = std::time::Instant::now(); + let r = crate::diag::trim_malloc(); + tracing::info!( + rc = %r["rc"], + fordblks_before = %r["fordblks_before"], + fordblks_after = %r["fordblks_after"], + rss_anon_before = %r["rss_anon_before"], + rss_anon_after = %r["rss_anon_after"], + "malloc_trim" + ); + } + // Retention on its own slow cadence: bounded work, and // nothing about it is urgent. if last_prune.elapsed() diff --git a/src/config.rs b/src/config.rs index 27bcd59..f397726 100644 --- a/src/config.rs +++ b/src/config.rs @@ -130,6 +130,14 @@ pub struct Config { /// of disk to re-derive an empty roster once a minute is what turned a 60s /// lap into minutes. pub roster_seconds: u64, + /// How often to call `malloc_trim(0)`, or `0` to leave the allocator alone. + /// + /// **Off by default, on purpose.** Trim reaches the fragmented free lists + /// the arena cap does not, but the pages it returns fault back in on reuse + /// — a real cost for a churn workload. Which way that trade lands on this + /// node has not been measured, and the cap was not adopted on a guess + /// either (CIRISStatus#69). + pub malloc_trim_seconds: u64, pub version: &'static str, pub grafana_url: Option, pub database_url: Option, // local "postgresql" provider (TCP liveness) @@ -334,6 +342,11 @@ impl Config { .filter(|v| *v > 0) .unwrap_or(120) as u64; + let malloc_trim_seconds = cfg + .i64("status.malloc_trim_secs") + .filter(|v| *v >= 0) + .unwrap_or(0) as u64; + let roster_seconds = cfg .i64("status.roster_secs") .filter(|v| *v > 0) @@ -405,6 +418,7 @@ impl Config { corpus_retention_budget, corpus_retention_secs, roster_seconds, + malloc_trim_seconds, version: env!("CARGO_PKG_VERSION"), grafana_url: cfg.str("status.grafana_url"), database_url: cfg.str("status.database_url"), @@ -450,6 +464,7 @@ impl Config { corpus_retention_budget: crate::retention::PRUNE_BUDGET_PER_PASS, corpus_retention_secs: 120, roster_seconds: 300, + malloc_trim_seconds: 0, version: env!("CARGO_PKG_VERSION"), grafana_url: None, database_url: None, diff --git a/src/diag.rs b/src/diag.rs index 9619a3a..9488345 100644 --- a/src/diag.rs +++ b/src/diag.rs @@ -46,8 +46,14 @@ pub fn memory_report() -> serde_json::Value { // Space in mmapped regions — untouched by malloc_trim. "hblkhd": m.hblkhd as u64, "hblks": m.hblks as u64, - // Releasable at the top of the heap: the upper bound on what a - // plain `malloc_trim(0)` could hand back. + // Releasable at the TOP OF THE MAIN ARENA. Read it as that and + // nothing more: since glibc 2.8 `malloc_trim` also walks every + // arena's free bins and `MADV_DONTNEED`s whole free pages inside + // them, so `keepcost` does NOT bound what a trim would return. + // Treating it as a bound is how this comment first concluded, on + // this node's 104KB, that a trim would reclaim nothing — while + // `fordblks` sat at 591MB of exactly the page-aligned free chunks + // `mtrim` targets (CIRISStatus#69). "keepcost": m.keepcost as u64, "ordblks": m.ordblks as u64, }); @@ -126,28 +132,43 @@ mod tests { } } - /// A held allocation must move `uordblks`. This is the sanity check that - /// the numbers are this process's and not a constant. + /// A held allocation must move the in-use figure. This is the sanity check + /// that the numbers are this process's and not a constant. + /// + /// Two glibc facts shape it, and the first version of this test tripped on + /// both — ciris-server hit the identical flake in its copy of this module + /// and fixed it in 0.5.200; this is that fix, because the two nodes are + /// deliberately one instrument and a test that is flaky in one of them is + /// flaky in both. + /// + /// A block past the mmap threshold is NOT in `uordblks` — it is mmapped and + /// counted in `hblkhd`. And in a test binary this size, other threads free + /// arena memory in the same millisecond, so `uordblks` alone can FALL while + /// this thread holds its block. So: 64 MiB (past the 32 MiB ceiling of + /// glibc's dynamic mmap threshold, hence always mmapped), and the in-use + /// figure is `uordblks + hblkhd`, which only an mmapped free of tens of MiB + /// elsewhere could pull back down; half the block is the slack for that. + #[cfg(target_env = "gnu")] #[test] fn live_bytes_track_a_real_allocation() { - let before = live_bytes(); - // Big enough to clear allocator noise from other test threads, and - // touched so it cannot be optimised away. - let mut v: Vec = vec![7; 32 * 1024 * 1024]; - v[16 * 1024 * 1024] = 9; - let during = live_bytes(); + const BLOCK: usize = 64 * 1024 * 1024; + fn in_use() -> u64 { + let m = &memory_report()["mallinfo2"]; + m["uordblks"].as_u64().unwrap() + m["hblkhd"].as_u64().unwrap() + } + let before = in_use(); + // Touched so it cannot be optimised away and the pages are real. + let mut v: Vec = vec![7; BLOCK]; + v[BLOCK / 2] = 9; + std::hint::black_box(&v); + let during = in_use(); assert!( - during >= before, - "holding 32MB should not shrink the live figure ({before} -> {during})" + during >= before + (BLOCK as u64) / 2, + "in-use bytes (uordblks + hblkhd) should rise by ~64 MiB while the block is held: \ + before={before} during={during}" ); drop(v); } - - fn live_bytes() -> u64 { - memory_report()["mallinfo2"]["uordblks"] - .as_u64() - .unwrap_or(0) - } } /// Cap glibc's per-arena free lists, unless the operator has already said @@ -172,9 +193,18 @@ mod tests { /// allocator lock CONTENTION, so a node with a handful of threads and a busy /// poll loop accumulates one 64MB region per contended thread and keeps them. /// -/// `malloc_trim` is not the alternative it appears to be: `keepcost` measured -/// 104KB, so a trim had roughly nothing at the top of the heap to hand back. -/// The memory is not above the break; it is spread across arenas. +/// The arena cap is the lever that was MEASURED here; it is not the only one, +/// and an earlier version of this comment ruled out the other on bad grounds. +/// It read `keepcost` (104KB) as the ceiling on what `malloc_trim` could +/// return. That is wrong: since glibc 2.8 `mtrim` walks every arena's free bins +/// and `MADV_DONTNEED`s whole free pages within them, so it reaches exactly the +/// fragmented free lists `keepcost` says nothing about — which on this node +/// still hold ~591MB even WITH the cap applied. +/// +/// So trim remains untested here rather than ruled out, and it is not free: the +/// pages it returns fault back in on reuse, which is a real cost for a workload +/// whose problem is churn. It wants an A/B like the cap got, not adoption on +/// the strength of a number that turned out to measure something else. /// /// # Why in the binary and not only in compose /// @@ -299,3 +329,80 @@ mod arena_tests { ); } } + +/// One `malloc_trim(0)`, with the numbers either side of it. +/// +/// # Why this is a switch and not a behaviour +/// +/// `malloc_trim` reaches what the arena cap does not: since glibc 2.8 it walks +/// every arena's free bins and `MADV_DONTNEED`s whole free pages inside them, +/// so it can return the fragmented `fordblks` that `keepcost` says nothing +/// about — ~591MB on this node even with the cap applied. +/// +/// It is not free. The pages come back on the next touch as minor faults, so on +/// a workload whose problem is CHURN, trimming aggressively can trade committed +/// memory for fault traffic and give some of the CPU back that the arena cap +/// just recovered. Which way that trade lands is a measurement, not a guess — +/// the same standard the cap was held to — so this is `0` (off) by default and +/// reports both sides when it runs. +/// +/// `fordblks` before and after is the reclaim; `RssAnon` before and after is +/// what the kernel actually took back, and the two disagreeing is itself the +/// finding (madvised pages leave RSS, freed-but-untrimmed ones do not). +#[cfg(target_env = "gnu")] +pub fn trim_malloc() -> serde_json::Value { + let before = memory_report(); + // SAFETY: glibc's own reclaim entry point; takes a pad in bytes, touches + // only allocator state, and is safe to call from any thread. + let rc = unsafe { libc::malloc_trim(0) }; + let after = memory_report(); + json!({ + "rc": rc, + "fordblks_before": before["mallinfo2"]["fordblks"], + "fordblks_after": after["mallinfo2"]["fordblks"], + "rss_anon_before": before["proc"]["RssAnon"], + "rss_anon_after": after["proc"]["RssAnon"], + }) +} + +#[cfg(not(target_env = "gnu"))] +pub fn trim_malloc() -> serde_json::Value { + json!({ "rc": -1, "note": "malloc_trim is glibc-only" }) +} + +#[cfg(test)] +mod trim_tests { + use super::*; + + /// The report must show BOTH sides, because the point of the switch is the + /// comparison — a trim that logs only its result is a change nobody can + /// evaluate. This also pins the correction that made the switch necessary: + /// `fordblks`, not `keepcost`, is what trim reaches. + #[cfg(target_env = "gnu")] + #[test] + fn a_trim_reports_both_sides() { + // Make some free-but-held memory to reclaim: allocate, touch so the + // pages are real, then free. + let mut blocks: Vec> = (0..16).map(|_| vec![3u8; 4 * 1024 * 1024]).collect(); + for b in blocks.iter_mut() { + b[0] = 1; + let n = b.len() - 1; + b[n] = 1; + } + drop(blocks); + + let r = trim_malloc(); + assert_eq!( + r["rc"].as_i64(), + Some(1).or(Some(0)).map(|_| r["rc"].as_i64().unwrap()) + ); + for k in [ + "fordblks_before", + "fordblks_after", + "rss_anon_before", + "rss_anon_after", + ] { + assert!(!r[k].is_null(), "{k} missing from the trim report: {r}"); + } + } +}