From 8f8064df80f223ef3e2282f3345294504edffb08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Sat, 13 Jun 2026 10:02:16 +0800 Subject: [PATCH] feat(runtime): add memory observability status snapshot (#3397) Co-authored-by: houseme --- .../background-controller-contract.md | 11 +++ .../background-services-inventory.md | 2 +- docs/architecture/migration-progress.md | 87 ++++++++-------- rustfs/src/memory_observability.rs | 98 ++++++++++++++++++- 4 files changed, 150 insertions(+), 48 deletions(-) diff --git a/docs/architecture/background-controller-contract.md b/docs/architecture/background-controller-contract.md index bd5062720..38434c7c5 100644 --- a/docs/architecture/background-controller-contract.md +++ b/docs/architecture/background-controller-contract.md @@ -103,6 +103,17 @@ Any future `BGC-003` status implementation must satisfy all of these: - scanner, heal, lifecycle, and replication status must not hide their queue and admission coupling. +## BGC-003 Snapshot Pilot + +The first read-only snapshot is memory observability status. It reports the +service name, whether observability metrics currently enable the sampler, the +configured sampler interval, runtime-token cancellation state, and the absence +of a dedicated shutdown handle. + +This snapshot intentionally does not define an admin route, scheduler, service +registry, worker start/stop path, readiness signal, peer signal, storage write, +or metrics emission change. + ## Future Reconcile Rules Future reconcile work is allowed only after a read-only status snapshot exists. diff --git a/docs/architecture/background-services-inventory.md b/docs/architecture/background-services-inventory.md index 26e5432c9..f8ed76f4f 100644 --- a/docs/architecture/background-services-inventory.md +++ b/docs/architecture/background-services-inventory.md @@ -48,7 +48,7 @@ not define a new scheduler, controller framework, or shutdown contract. | Audit runtime | `rustfs/src/server/audit.rs::start_audit_system` starts audit only when module switches and configured targets allow it. `AuditSystem::reload_config` replaces runtime targets. | Dispatches audit events to configured targets and manages replay workers. | Audit observability records config reloads and target delivery metrics. | Do not couple audit lifecycle to notification lifecycle even though the runtime patterns are similar. | | Dynamic config reload | Admin config handlers call `apply_dynamic_config_for_subsystem`, then `signal_dynamic_config_reload` or `signal_config_snapshot_reload` through the global notification system. | Applies scanner/heal runtime config, audit reloads, notification reloads, and peer reload signals. | Logs local and peer reload failures. Audit reload increments audit config reload metrics. | This is admin-triggered fanout, not a background scheduler. Controller work should preserve per-subsystem validation and error boundaries. | | Metrics runtime | `crates/obs/src/metrics/scheduler.rs::init_metrics_runtime` spawns multiple interval loops for cluster, bucket, node, resource, audit, notification, and replication bandwidth metrics. | Periodically collects and reports metrics. | Reports through the metrics runtime and logs cancellation warnings. | Keep intervals and collector grouping stable while adding status snapshots. | -| Memory observability | `rustfs/src/memory_observability.rs::init_memory_observability` spawns a token-cancelled sampler. | Periodically records memory snapshots. | Emits memory observability metrics. | This can be a low-risk pilot for read-only controller status because it already has a simple token loop. | +| Memory observability | `rustfs/src/memory_observability.rs::init_memory_observability` spawns a token-cancelled sampler. | Periodically records memory snapshots. | Emits memory observability metrics and exposes a read-only status snapshot for metrics enablement, interval, cancellation source, and shutdown handle shape. | This is the first low-risk pilot for read-only controller status because it already has a simple token loop. | | Allocator reclaim | `rustfs/src/allocator_reclaim.rs::init_allocator_reclaim` spawns a token-cancelled reclaim loop when enabled. | Observes reclaimable work and may run allocator reclaim after idle intervals. | Emits reclaim enabled/backend counters, active-request gauges, scanner/heal activity gauges, and reclaim result counters. | A controller must preserve idle-streak logic and backend-specific force behavior. | | Auto-tuner | `rustfs/src/init.rs::init_auto_tuner` optionally spawns a 60-second loop when `RUSTFS_AUTOTUNER_ENABLED` is true. | Tunes concurrency manager settings from performance metrics. | Logs iteration success/failure. | Treat as behavior-sensitive; a future controller needs explicit rollback because it can change runtime concurrency. | | Update check | `rustfs/src/init.rs::init_update_check` spawns one async task with a 30-second timeout when update checks are enabled. | Performs version check network I/O and logs available updates. | Logs result only. | This is a one-shot task, not a controller candidate for the first BGC PRs. | diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 065b64e06..7921cd1e2 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,18 +5,16 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Current Context - Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660) -- Branch: `overtrue/arch-kms-dev-defaults` -- Baseline: `origin/main` at `a85cc0354c02fc55e2dd8eb64cfc6155c37921c7` -- PR type for this branch: `security-change` -- Runtime behavior changes: KMS development-only defaults now fail closed unless - `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true` or an admin configure request - sets `allow_insecure_dev_defaults=true`. -- Rust code changes: harden Local KMS missing-master-key/temp-dir defaults, - Vault HTTP/default-token/skip-TLS defaults, KMS service-manager validation, - admin configure request conversion, and server CLI KMS configuration. +- Branch: `overtrue/arch-background-status-snapshot` +- Baseline: `origin/main` at `f7bf4fd3e4b304c75c880f28d8c45329366571f6` +- PR type for this branch: `contract` +- Runtime behavior changes: none; memory observability startup, sampler loop, + cancellation, and metrics emission behavior are unchanged. +- Rust code changes: add a typed read-only memory observability status snapshot + and reuse the existing sampler interval parsing helper. - CI/script changes: none. -- Docs changes: record KMS compatibility notes and mark `KMSD-002` through - `KMSD-005` complete. +- Docs changes: record `BGC-003` status snapshot scope and update background + controller migration progress. ## Phase 0 Tasks @@ -381,56 +379,57 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block start/stop path, storage write, readiness change, peer signal, or runtime behavior change. - Verification: docs-only architecture checks and diff hygiene. +- [x] `BGC-003` Add read-only status snapshot. + - Acceptance: memory observability exposes a typed status snapshot that reports + service state, metrics enablement, configured interval, cancellation source, + and shutdown handle shape. + - Must preserve: no controller framework, admin route, worker lifecycle + change, storage write, readiness change, peer signal, or metrics emission + behavior change. + - Verification: focused memory observability tests, compile checks, migration + guards, formatting, and pre-commit quality gate. ## Next PRs -1. `security-change`: apply IAM and plugin secret redaction in `S-014`. -2. `security-change`: classify and add strict serde ingress tests in `S-015`. +1. `behavior-change`: pilot one low-risk controller behind status-first tests + in `BGC-004`. +2. `test-only`: add controller harness coverage for cancellation and + duplicate-worker prevention in `TEST-BGC-001`. ## Pre-Push Review Log | Expert | Status | Notes | |---|---|---| -| Quality/architecture | pass | Confirmed KMS unsafe-development defaults are enforced in `KmsConfig::validate()` and reused by env loading, admin configure conversion, service-manager lifecycle, and direct backend construction. | -| Migration preservation | pass | Confirmed KMS runtime key operations, cache behavior, redaction behavior, storage SSE call sites, and admin route contracts keep their existing shapes except for the explicit development opt-in field. | -| Testing/verification | pass | Confirmed KMS crate tests, rustfs config/SSE focused tests, rustfs lib/test compile, migration guards, format check, and diff check cover this larger security-change slice; full pre-commit is skipped under the current larger-granularity instruction. | +| Quality/architecture | pass | Confirmed the snapshot is a typed read-only contract in `memory_observability` with no generic controller framework, service registry, or admin route. | +| Migration preservation | pass | Confirmed memory observability startup, sampler loop, runtime token cancellation, interval defaulting, and metrics emission behavior remain unchanged. | +| Testing/verification | pass | Confirmed focused tests, rustfs library check, formatting, diff hygiene, migration guards, Rust risk scan, and `make pre-commit` all passed. | ## Verification Notes -Passed on `a85cc0354c02fc55e2dd8eb64cfc6155c37921c7`: -- `cargo check -p rustfs --lib --tests`. -- `cargo check -p rustfs-kms --tests`. -- `cargo test -p rustfs-kms --no-fail-fast`; 57 passed, 1 ignored, doc-test - passed. -- `cargo clippy -p rustfs-kms --all-targets -- -D warnings`. -- `cargo test -p rustfs --lib config::config_test --no-fail-fast`; 20 passed. -- `cargo test -p rustfs --lib storage::sse::tests::test_kms --no-fail-fast`; - 1 passed. -- `cargo test -p rustfs --lib -- --list | rg "sse.*kms|kms.*sse|config::config_test::tests::test_config_new_defaults"`. +Passed on `f7bf4fd3e1b3b48b3bec0702a6d8a840b45a5160`: +- `cargo test -p rustfs memory_observability --lib`; 5 passed. +- `cargo check -p rustfs --lib`. +- `cargo fmt --all`. - `cargo fmt --all --check`. +- `git diff --check`. - `./scripts/check_architecture_migration_rules.sh`. - `./scripts/check_layer_dependencies.sh`. - `./scripts/check_metrics_migration_refs.sh`. -- `git diff --check`. +- Rust risk scan for unwrap/expect/casts/string errors/debug output in + `rustfs/src/memory_observability.rs`; only pre-existing test `expect` calls + were matched. +- `make pre-commit`; all checks passed, including nextest 5857 passed and 111 + skipped. Notes: -- Full pre-commit may be skipped if focused tests, compile checks, and guards - pass, per the current instruction to increase PR granularity. -- `cargo test -p rustfs --lib storage::sse::tests::test_sse_kms --no-fail-fast` - and `cargo test -p rustfs --lib test_sse_kms_roundtrip --no-fail-fast` - matched 0 tests because the `rio-v2` roundtrip test is feature-gated out of - the default `rustfs --lib` test binary. -- This slice includes a minimal compile unblock after PR #3365: table catalog's - ECStore object backend now declares `NamespaceLocking` where it calls - `new_ns_lock`; the lock path itself is unchanged. -- This slice does not change KMS authorization actions, key operation behavior, - storage encryption metadata formats, or cache semantics. +- This slice only exposes in-memory status from existing inputs. It does not + start, stop, resize, or wake memory observability workers. +- There is no admin/API exposure in this PR; future `BGC-004` work must remain + status-first before adding reconcile behavior. ## Handoff Notes -- Keep this KMS slice as a `security-change` PR covering KMSD-002 through - KMSD-005 plus the minimal table catalog compile unblock from PR #3365. -- Do not change KMS key operation behavior, storage SSE metadata formats, IAM - policy actions, admin route wiring, or cache semantics in this PR. -- If more time is available before the next slice, start `S-014` with IAM/plugin - secret redaction; otherwise start `S-015` strict serde ingress tests. +- Next migration slice can start `BGC-004` by choosing one low-risk controller + and proving idempotent reconcile without duplicate workers. +- Keep scanner, heal, lifecycle, replication, disk health, and config reload out + of broad controller movement until dedicated preservation tests exist. diff --git a/rustfs/src/memory_observability.rs b/rustfs/src/memory_observability.rs index ffec18a77..6b8980b51 100644 --- a/rustfs/src/memory_observability.rs +++ b/rustfs/src/memory_observability.rs @@ -16,6 +16,7 @@ use rustfs_io_metrics::{ record_cgroup_memory_split, record_cpu_usage, record_memory_usage, record_process_memory_split, snapshot_process_resource_and_system, }; +use serde::Serialize; use std::collections::HashMap; use std::path::Path; use std::sync::{Mutex, OnceLock}; @@ -28,6 +29,37 @@ static MEMORY_SYSTEM: OnceLock> = OnceLock::new(); const ENV_MEMORY_OBSERVABILITY_INTERVAL_SECS: &str = "RUSTFS_MEMORY_OBSERVABILITY_INTERVAL_SECS"; const DEFAULT_MEMORY_OBSERVABILITY_INTERVAL_SECS: u64 = 15; +const MEMORY_OBSERVABILITY_SERVICE_NAME: &str = "memory_observability"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum MemoryObservabilityServiceState { + Disabled, + Running, + Stopping, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum MemoryObservabilityCancellationSource { + RuntimeToken, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum MemoryObservabilityShutdownHandle { + RuntimeTokenOnly, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub struct MemoryObservabilityStatusSnapshot { + pub service: &'static str, + pub state: MemoryObservabilityServiceState, + pub metrics_enabled: bool, + pub interval_secs: u64, + pub cancellation_source: MemoryObservabilityCancellationSource, + pub shutdown_handle: MemoryObservabilityShutdownHandle, +} #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] struct CgroupMemorySnapshot { @@ -116,6 +148,41 @@ fn read_cgroup_memory_snapshot() -> Option { read_cgroup_v2().or_else(read_cgroup_v1) } +fn configured_memory_observability_interval_secs() -> u64 { + rustfs_utils::get_env_u64(ENV_MEMORY_OBSERVABILITY_INTERVAL_SECS, DEFAULT_MEMORY_OBSERVABILITY_INTERVAL_SECS).max(1) +} + +fn build_memory_observability_status_snapshot( + metrics_enabled: bool, + interval_secs: u64, + cancellation_requested: bool, +) -> MemoryObservabilityStatusSnapshot { + let state = if !metrics_enabled { + MemoryObservabilityServiceState::Disabled + } else if cancellation_requested { + MemoryObservabilityServiceState::Stopping + } else { + MemoryObservabilityServiceState::Running + }; + + MemoryObservabilityStatusSnapshot { + service: MEMORY_OBSERVABILITY_SERVICE_NAME, + state, + metrics_enabled, + interval_secs: interval_secs.max(1), + cancellation_source: MemoryObservabilityCancellationSource::RuntimeToken, + shutdown_handle: MemoryObservabilityShutdownHandle::RuntimeTokenOnly, + } +} + +pub fn memory_observability_status_snapshot(ctx: &CancellationToken) -> MemoryObservabilityStatusSnapshot { + build_memory_observability_status_snapshot( + rustfs_obs::observability_metric_enabled(), + configured_memory_observability_interval_secs(), + ctx.is_cancelled(), + ) +} + async fn record_memory_snapshot() { match tokio::task::spawn_blocking(|| { let (resource, process) = snapshot_process_resource_and_system(); @@ -148,8 +215,7 @@ async fn record_memory_snapshot() { } pub fn init_memory_observability(ctx: CancellationToken) { - let interval_secs = - rustfs_utils::get_env_u64(ENV_MEMORY_OBSERVABILITY_INTERVAL_SECS, DEFAULT_MEMORY_OBSERVABILITY_INTERVAL_SECS); + let interval_secs = configured_memory_observability_interval_secs(); let interval = Duration::from_secs(interval_secs.max(1)); tokio::spawn(async move { @@ -172,7 +238,10 @@ pub fn init_memory_observability(ctx: CancellationToken) { #[cfg(test)] mod tests { - use super::{CgroupMemorySnapshot, parse_kv_stats, read_optional_u64}; + use super::{ + CgroupMemorySnapshot, MemoryObservabilityCancellationSource, MemoryObservabilityServiceState, + MemoryObservabilityShutdownHandle, build_memory_observability_status_snapshot, parse_kv_stats, read_optional_u64, + }; use std::fs; use std::path::PathBuf; @@ -206,4 +275,27 @@ mod tests { assert_eq!(snapshot.active_file_bytes, None); assert_eq!(snapshot.inactive_file_bytes, None); } + + #[test] + fn memory_observability_snapshot_reports_disabled_when_metrics_are_disabled() { + let snapshot = build_memory_observability_status_snapshot(false, 15, false); + + assert_eq!(snapshot.service, "memory_observability"); + assert_eq!(snapshot.state, MemoryObservabilityServiceState::Disabled); + assert!(!snapshot.metrics_enabled); + assert_eq!(snapshot.interval_secs, 15); + assert_eq!(snapshot.cancellation_source, MemoryObservabilityCancellationSource::RuntimeToken); + assert_eq!(snapshot.shutdown_handle, MemoryObservabilityShutdownHandle::RuntimeTokenOnly); + } + + #[test] + fn memory_observability_snapshot_reports_running_and_stopping_states() { + let running = build_memory_observability_status_snapshot(true, 0, false); + let stopping = build_memory_observability_status_snapshot(true, 30, true); + + assert_eq!(running.state, MemoryObservabilityServiceState::Running); + assert_eq!(running.interval_secs, 1); + assert_eq!(stopping.state, MemoryObservabilityServiceState::Stopping); + assert_eq!(stopping.interval_secs, 30); + } }