From 9e57de3959ecaf1049174c8ab9660bf0882cab57 Mon Sep 17 00:00:00 2001 From: Jeff Larson Date: Tue, 28 Jul 2026 19:24:28 -0700 Subject: [PATCH] feat(server): visibility-map health canary for metric_series_rollups (JEF-594) Adds a JEF-594 self-monitoring canary for the JEF-591 covering index's core assumption: index-only scans on metric_series_rollups only stay cheap while the visibility map is current, and autovacuum falling behind on this high-churn table would silently regress it back to the JEF-548-class latency with no signal until dashboards get slow. On the existing self-telemetry interval (ADR 0014), collect_metrics now emits, tagged table=metric_series_rollups: - watcher.db.dead_tuple_ratio -- n_dead_tup / (n_live_tup + n_dead_tup) from pg_stat_user_tables, always available (no extension needed). - watcher.db.last_autovacuum_age_seconds -- age of the last real autovacuum run; skipped (not zeroed) before the first one ever completes. - watcher.db.vm_all_visible_fraction -- all-visible pages / relpages via pg_visibility_map_summary (reads only the VM fork, not pg_visibility(), which would also touch every heap page). pg_visibility is a contrib extension and not "trusted", so the app's non-superuser role can't (and doesn't try to) CREATE EXTENSION it itself -- that's a cluster-ops action, not read-only introspection. When it's absent, the query fails with SQLSTATE 42883 (undefined_function); that's treated as "unavailable" (logged once, not per tick) rather than failing the whole self-telemetry snapshot, so dead-tuple ratio and autovacuum age still cover the canary on their own. Tests (against a real Postgres): pure unit tests for the ratio math, plus an integration test that seeds a rollup row, forces deterministic pg_stat_user_tables/pg_class state (ANALYZE, not VACUUM -- manual VACUUM doesn't set last_autovacuum, so that gauge's absence is asserted too), and exercises the pg_visibility path end to end when the test role can install it. DECISION NEEDED (cross-repo): a declarative alert-rule threshold for these metrics (ADR 0012) lives in the ../cluster chart's server.alerts values, not this repo -- this PR only makes the metrics alertable. Suggested starting thresholds, given the 2% autovacuum scale factor tuned in migration 0013: dead_tuple_ratio > 0.10 (5x the tuned scale factor) or vm_all_visible_fraction < 0.5, agg=avg, window_secs=300. Closes JEF-594 Co-authored-by: Claude Sonnet 5 --- server/src/selfmon.rs | 141 ++++++++++++++++++++++++++++++++++++++++++ server/tests/smoke.rs | 78 +++++++++++++++++++++++ 2 files changed, 219 insertions(+) diff --git a/server/src/selfmon.rs b/server/src/selfmon.rs index 12a1bc2..fd3c091 100644 --- a/server/src/selfmon.rs +++ b/server/src/selfmon.rs @@ -217,6 +217,13 @@ const TRACKED_TABLES: [&str; 6] = [ "alert_rules", ]; +/// The table the index-only-scan / visibility-map health canary (JEF-594) +/// watches. The JEF-591 covering index only produces index-only scans while +/// this high-churn table's visibility map stays current; if autovacuum falls +/// behind, scans silently degrade to heap fetches and the JEF-548-class +/// latency returns with no signal until dashboards get slow. +const ROLLUP_TABLE: &str = "metric_series_rollups"; + /// Build the `watcher_*` metric points from cheap catalog/aggregate queries plus /// the in-process counters. async fn collect_metrics(pool: &PgPool) -> anyhow::Result> { @@ -353,9 +360,115 @@ async fn collect_metrics(pool: &PgPool) -> anyhow::Result> { vec![point(v, &[], nanos)], )); } + + // Index-only-scan / visibility-map health canary (JEF-594) — see + // `ROLLUP_TABLE` doc comment. `pg_stat_user_tables` is always available (no + // extension needed); the row is absent only before the catalog's stats have + // ever been populated for the table, which shouldn't happen post-migration + // but is handled the same way as the other optional gauges above. + let vacuum_stats: Option<(i64, i64, Option)> = sqlx::query_as( + "SELECT n_live_tup, n_dead_tup, extract(epoch FROM now() - last_autovacuum)::float8 + FROM pg_stat_user_tables + WHERE schemaname = 'public' AND relname = $1", + ) + .bind(ROLLUP_TABLE) + .fetch_optional(pool) + .await?; + if let Some((live, dead, autovacuum_age)) = vacuum_stats { + if let Some(ratio) = dead_tuple_ratio(live, dead) { + metrics.push(gauge( + "watcher.db.dead_tuple_ratio", + "1", + vec![point(ratio, &[("table", ROLLUP_TABLE)], nanos)], + )); + } + // NULL until the table's first autovacuum ever runs — skip rather than + // reporting a misleading zero (that would read as "just vacuumed"). + if let Some(age) = autovacuum_age { + metrics.push(gauge( + "watcher.db.last_autovacuum_age_seconds", + "s", + vec![point(age, &[("table", ROLLUP_TABLE)], nanos)], + )); + } + } + + // The all-visible fraction needs the `pg_visibility` contrib extension, + // which isn't guaranteed to be installed (it's not a "trusted" extension, + // so the app's non-superuser role can't `CREATE EXTENSION` it itself even if + // it wanted to — installing it is a cluster-ops action, not read-only + // introspection). Degrade gracefully: an undefined-function error just means + // the extension isn't there, so skip the gauge rather than failing the whole + // snapshot; the dead-tuple ratio and autovacuum age above still cover the + // canary on their own. + match rollup_vm_all_visible_fraction(pool).await { + Ok(Some(fraction)) => metrics.push(gauge( + "watcher.db.vm_all_visible_fraction", + "1", + vec![point(fraction, &[("table", ROLLUP_TABLE)], nanos)], + )), + Ok(None) => {} + Err(e) if is_undefined_function(&e) => log_pg_visibility_missing_once(), + Err(e) => tracing::warn!("self-telemetry: pg_visibility query failed: {e}"), + } + Ok(metrics) } +/// `numerator / denominator`, or `None` when `denominator` is zero (an empty +/// or not-yet-analyzed table) rather than reporting a misleading 0. +fn safe_ratio(numerator: i64, denominator: i64) -> Option { + (denominator > 0).then(|| numerator as f64 / denominator as f64) +} + +/// Fraction of `ROLLUP_TABLE`'s tuples that are dead — the same quantity +/// autovacuum's own dead-tuple threshold check watches. +fn dead_tuple_ratio(n_live_tup: i64, n_dead_tup: i64) -> Option { + safe_ratio(n_dead_tup, n_live_tup + n_dead_tup) +} + +/// Fraction of `ROLLUP_TABLE`'s pages the visibility map marks all-visible — +/// index-only scans avoid a heap fetch only for these. +fn vm_all_visible_fraction(all_visible: i64, relpages: i64) -> Option { + safe_ratio(all_visible, relpages) +} + +/// `pg_visibility_map_summary` reads only the visibility-map fork (no heap +/// I/O), so this is cheap enough for the self-telemetry interval — +/// deliberately not `pg_visibility()`, which would read every heap page to +/// cross-check its flag against the VM bit. Errors with SQLSTATE `42883` +/// (undefined function) when the `pg_visibility` extension isn't installed; +/// the caller treats that as "unavailable", not a failure. +async fn rollup_vm_all_visible_fraction(pool: &PgPool) -> Result, sqlx::Error> { + let (all_visible, relpages): (i64, i64) = sqlx::query_as( + "SELECT s.all_visible, c.relpages::int8 + FROM pg_visibility_map_summary($1::regclass) s, pg_class c + WHERE c.oid = $1::regclass", + ) + .bind(ROLLUP_TABLE) + .fetch_one(pool) + .await?; + Ok(vm_all_visible_fraction(all_visible, relpages)) +} + +fn is_undefined_function(err: &sqlx::Error) -> bool { + matches!(err, sqlx::Error::Database(db_err) if db_err.code().as_deref() == Some("42883")) +} + +/// Logs the missing-extension notice once per process rather than every +/// self-telemetry tick. +fn log_pg_visibility_missing_once() { + static LOGGED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + if !LOGGED.swap(true, Ordering::Relaxed) { + tracing::info!( + "self-telemetry: pg_visibility extension not installed -- \ + watcher.db.vm_all_visible_fraction will not be emitted; \ + watcher.db.dead_tuple_ratio and watcher.db.last_autovacuum_age_seconds \ + still cover the JEF-594 canary" + ); + } +} + // --------------------------------------------------------------------------- // OTLP construction helpers. // --------------------------------------------------------------------------- @@ -468,4 +581,32 @@ mod tests { }; assert!(!stalled.healthy()); } + + #[test] + fn dead_tuple_ratio_divides_dead_by_total() { + // 20 dead out of 100 total tuples → 20%. + assert_eq!(dead_tuple_ratio(80, 20), Some(0.2)); + } + + #[test] + fn dead_tuple_ratio_none_when_table_empty() { + assert_eq!(dead_tuple_ratio(0, 0), None); + } + + #[test] + fn vm_all_visible_fraction_divides_by_relpages() { + // 45 of 50 pages all-visible → 90%. + assert_eq!(vm_all_visible_fraction(45, 50), Some(0.9)); + } + + #[test] + fn vm_all_visible_fraction_none_when_no_pages() { + assert_eq!(vm_all_visible_fraction(0, 0), None); + } + + // `is_undefined_function` (the SQLSTATE-42883 check) isn't unit-tested with a + // mock `DatabaseError` — matching `otlp::is_failover_error`, its peer + // SQLSTATE-classifying helper, which is likewise only exercised against a + // real Postgres error in an integration test rather than a hand-rolled + // trait impl. See `rollup_vacuum_health_canary_*` in tests/smoke.rs. } diff --git a/server/tests/smoke.rs b/server/tests/smoke.rs index e9ca31a..08fe0ac 100644 --- a/server/tests/smoke.rs +++ b/server/tests/smoke.rs @@ -1113,6 +1113,84 @@ async fn self_telemetry_emits_watcher_ops_metrics() { assert!(tbl["series_count"].as_i64().unwrap() >= 2); } +#[tokio::test] +#[serial] +async fn rollup_vacuum_health_canary_emits_dead_tuple_ratio_and_optional_vm_fraction() { + let Some(pool) = pool_or_skip().await else { + return; + }; + // Seed a row in metric_series_rollups (on-ingest per-series rollups, ADR + // 0020). pg_stat_user_tables' n_live_tup/n_dead_tup update as soon as the + // stats collector sees the DML, no VACUUM/ANALYZE needed -- but + // pg_visibility_map_summary's page-count denominator (pg_class.relpages) + // only updates on ANALYZE/VACUUM, so run one to make the VM fraction + // deterministic without waiting on autovacuum's naptime. + ingest( + &app(pool.clone()), + gauge_request("checkout.latency", 12.0, now_nanos()), + ) + .await; + sqlx::query("ANALYZE metric_series_rollups") + .execute(&pool) + .await + .expect("analyze"); + + selfmon::emit_once(&pool).await.expect("emit"); + + let router = app(pool.clone()); + let (status, metrics) = get_json(&router, "/api/metrics?service=watcher").await; + assert_eq!(status, StatusCode::OK); + let list = metrics.as_array().unwrap(); + + let dead_ratio = list + .iter() + .find(|m| m["name"] == "watcher.db.dead_tuple_ratio") + .expect("dead_tuple_ratio present once the table has tuples"); + let ratio = dead_ratio["last_value"].as_f64().unwrap(); + assert!((0.0..=1.0).contains(&ratio), "ratio {ratio} out of range"); + + // last_autovacuum stays NULL until the autovacuum daemon has actually run + // (a manual ANALYZE/VACUUM does not set it) -- the gauge must not fabricate + // a value for it rather than skip. + assert!( + !list + .iter() + .any(|m| m["name"] == "watcher.db.last_autovacuum_age_seconds"), + "must not report an autovacuum age before autovacuum has ever run" + ); + + // pg_visibility is an optional contrib extension (JEF-594): the app never + // creates it itself (it's not "trusted", so it needs superuser -- that's a + // cluster-ops concern, not read-only introspection), but exercise the + // happy path when the test DB's role can install it, so the extraction + // query itself is covered end to end. + let vm_extension_available = sqlx::query("CREATE EXTENSION IF NOT EXISTS pg_visibility") + .execute(&pool) + .await + .is_ok(); + if !vm_extension_available { + eprintln!("skipping vm_all_visible_fraction assertion: pg_visibility unavailable"); + return; + } + selfmon::emit_once(&pool) + .await + .expect("emit with pg_visibility"); + let (status, metrics) = get_json(&router, "/api/metrics?service=watcher").await; + assert_eq!(status, StatusCode::OK); + let fraction = metrics + .as_array() + .unwrap() + .iter() + .find(|m| m["name"] == "watcher.db.vm_all_visible_fraction") + .expect("vm_all_visible_fraction present once pg_visibility is installed")["last_value"] + .as_f64() + .unwrap(); + assert!( + (0.0..=1.0).contains(&fraction), + "fraction {fraction} out of range" + ); +} + // --- Self-log instrumentation (JEF-452) ------------------------------------ #[tokio::test]