From 76b375c478a8c2f5c79e62e848156a55915ed769 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 19:03:49 +0800 Subject: [PATCH] feat(runtime): add allocator reclaim controller status (#3406) --- .../background-controller-contract.md | 13 + .../background-services-inventory.md | 2 +- docs/architecture/migration-progress.md | 66 ++-- rustfs/src/allocator_reclaim.rs | 339 ++++++++++++++++-- rustfs/src/memory_observability.rs | 24 +- 5 files changed, 393 insertions(+), 51 deletions(-) diff --git a/docs/architecture/background-controller-contract.md b/docs/architecture/background-controller-contract.md index adc2facb5..05a441fbc 100644 --- a/docs/architecture/background-controller-contract.md +++ b/docs/architecture/background-controller-contract.md @@ -125,6 +125,19 @@ calls must return the same plan for the same snapshot and must not request a worker start, stop, resize, wakeup, storage write, readiness signal, peer signal, or metrics emission. +## BGC-005 Allocator Reclaim Status And Controller Surface + +The second low-risk controller/status surface is allocator reclaim. It reports +the service name, desired enablement, configured force flag, backend-specific +effective force, idle interval settings, runtime-token cancellation state, and +the absence of a dedicated shutdown handle. + +The only allowed worker mutation for this surface is `none`. Reconcile output is +read-only and must not start, stop, resize, wake, or otherwise drive the +allocator reclaim loop. Existing backend-specific force handling, idle-streak +logic, metrics emission, and runtime-token shutdown behavior remain owned by the +current loop. + ## 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 c55a2b1a0..e643b59b8 100644 --- a/docs/architecture/background-services-inventory.md +++ b/docs/architecture/background-services-inventory.md @@ -49,7 +49,7 @@ not define a new scheduler, controller framework, or shutdown contract. | 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 and exposes a read-only status snapshot plus a no-op reconcile plan for metrics enablement, interval, cancellation source, and shutdown handle shape. | This is the first low-risk pilot for controller status because it already has a simple token loop and the pilot does not mutate workers. | -| 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. | +| 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. Exposes a typed read-only status snapshot plus a no-op reconcile plan for enablement, backend, effective force, intervals, cancellation source, and shutdown handle shape. | A controller must preserve idle-streak logic and backend-specific force behavior. The current controller surface does not mutate workers. | | 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. | | Deferred IAM recovery | `rustfs/src/startup_iam.rs::spawn_iam_recovery_task` retries IAM init with backoff and finalizes readiness when successful. | Can initialize IAM later, initialize AppContext if needed, mark `IamReady`, and publish `FullReady`. | Readiness state reflects deferred recovery progress. | Keep this lifecycle-critical path separate from generic background controllers. | diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index c9eb15da6..4fe478b89 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,17 +5,18 @@ 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-background-controller-pilot` -- Baseline: `origin/main` at `bda0b1f3dd923f728e4d2ff2f17bde2755a6cb5e` +- Branch: `overtrue/arch-background-controller-harness-and-status` +- Baseline: `origin/main` at `624de3114398338c80b87c9b452fc70b1d76fa8a` - PR type for this branch: `behavior-change` -- Runtime behavior changes: none; the memory observability controller pilot only - returns read-only snapshot/reconcile data and never starts, stops, resizes, or - wakes workers. -- Rust code changes: add a memory observability controller snapshot and - reconcile plan with explicit no-op worker mutation. +- Runtime behavior changes: none; the allocator reclaim controller/status + surface only returns read-only snapshot/reconcile data and never starts, + stops, resizes, or wakes workers. +- Rust code changes: add allocator reclaim status/controller snapshots, + reconcile plans with explicit no-op worker mutation, and controller harness + tests for memory observability plus allocator reclaim. - CI/script changes: none. -- Docs changes: record `BGC-004` controller pilot scope and update background - controller migration progress. +- Docs changes: record `TEST-BGC-001` harness coverage and the allocator reclaim + controller/status slice. ## Phase 0 Tasks @@ -398,26 +399,45 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block - Verification: focused controller tests prove repeated reconcile is idempotent, cancellation state is preserved, and worker mutation remains none. +- [x] `TEST-BGC-001` Add controller harness coverage. + - Acceptance: controller tests cover cancellation state, repeated reconcile, + paused-time stability, and no worker mutation for the low-risk controller + surfaces. + - Must preserve: no worker spawn, start, stop, resize, wakeup, storage write, + readiness signal, peer signal, or metrics emission behavior change. + - Verification: focused memory observability and allocator reclaim controller + tests. +- [x] `BGC-005` Add allocator reclaim controller/status surface. + - Acceptance: allocator reclaim exposes typed desired/status/controller + snapshots and a typed reconcile plan that reports backend, effective force, + idle interval, runtime cancellation, shutdown handle shape, and no-op worker + mutation. + - Must preserve: existing allocator reclaim enablement, backend-specific force + handling, idle-streak logic, metrics emission, runtime-token cancellation, + and startup call shape. + - Verification: focused allocator reclaim tests, compile checks, formatting, + migration guards, Rust risk scan, and pre-commit quality gate. ## Next PRs -1. `test-only`: add controller harness coverage for cancellation and - duplicate-worker prevention in `TEST-BGC-001`. -2. `behavior-change`: add the next low-risk background status surface before +1. `behavior-change`: add the next low-risk background status surface before broader reconcile work. +2. `test-only`: add config-reload preservation coverage for scanner/heal/ + lifecycle/replication in `TEST-BGC-002`. ## Pre-Push Review Log | Expert | Status | Notes | |---|---|---| -| Quality/architecture | pass | Confirmed the controller pilot stays local to `memory_observability`, uses typed desired/status/reconcile structs, and does not add a generic scheduler or admin route. | -| Migration preservation | pass | Confirmed reconcile reports `worker_mutation: none`, preserving sampler startup, cancellation, interval defaulting, and metrics emission behavior. | +| Quality/architecture | pass | Confirmed allocator reclaim follows the established typed desired/status/reconcile shape without adding a generic scheduler, registry, or admin route. | +| Migration preservation | pass | Confirmed reconcile reports `worker_mutation: none`, preserving enablement, force handling, idle-streak logic, cancellation, and metrics emission behavior. | | Testing/verification | pass | Focused tests, compile checks, formatting, diff hygiene, migration guards, Rust risk scan, and `make pre-commit` passed. | ## Verification Notes -Passed on `bda0b1f3dd923f728e4d2ff2f17bde2755a6cb5e`: -- `cargo test -p rustfs memory_observability --lib`; 8 passed. +Passed on `624de3114398338c80b87c9b452fc70b1d76fa8a`: +- `cargo test -p rustfs allocator_reclaim --lib`; 9 passed. +- `cargo test -p rustfs memory_observability --lib`; 9 passed. - `cargo check -p rustfs --lib`. - `cargo fmt --all`. - `cargo fmt --all --check`. @@ -425,20 +445,20 @@ Passed on `bda0b1f3dd923f728e4d2ff2f17bde2755a6cb5e`: - `./scripts/check_architecture_migration_rules.sh`. - `./scripts/check_layer_dependencies.sh`. - `./scripts/check_metrics_migration_refs.sh`. -- Rust risk scan for unwrap/expect/casts/string errors/debug output in - `rustfs/src/memory_observability.rs`; only test `expect` calls matched. -- `make pre-commit`; all checks passed, including nextest 5865 passed and 111 +- Rust risk scan for changed Rust files; only test `expect` calls and existing + internal allocator reclaim `Result<(), String>` helpers matched. +- `make pre-commit`; all checks passed, including nextest 5874 passed and 111 skipped. Notes: -- This slice adds reconcile data only. It does not apply reconcile output to the - running sampler. +- This slice adds reconcile/status data only. It does not apply reconcile output + to the running allocator reclaim loop or memory observability sampler. - There is no admin/API exposure in this PR; future controller harness work should stay isolated from scanner, heal, lifecycle, and replication. ## Handoff Notes -- Next migration slice can start `TEST-BGC-001` with a fake-clock and - cancellation-token harness around the memory observability pilot. +- Next migration slice can start another read-only low-risk status surface or + `TEST-BGC-002` config-reload preservation coverage. - 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/allocator_reclaim.rs b/rustfs/src/allocator_reclaim.rs index 6602d4161..e04fb396e 100644 --- a/rustfs/src/allocator_reclaim.rs +++ b/rustfs/src/allocator_reclaim.rs @@ -13,10 +13,103 @@ // limitations under the License. use metrics::{counter, gauge, histogram}; +use serde::Serialize; use std::time::Duration; use tokio_util::sync::CancellationToken; use tracing::{debug, warn}; +const ALLOCATOR_RECLAIM_SERVICE_NAME: &str = "allocator_reclaim"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AllocatorReclaimServiceState { + Disabled, + Running, + Stopping, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AllocatorReclaimCancellationSource { + RuntimeToken, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AllocatorReclaimShutdownHandle { + RuntimeTokenOnly, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub struct AllocatorReclaimStatusSnapshot { + pub service: &'static str, + pub state: AllocatorReclaimServiceState, + pub backend: &'static str, + pub effective_force: bool, + pub idle_intervals: u64, + pub interval_secs: u64, + pub cancellation_source: AllocatorReclaimCancellationSource, + pub shutdown_handle: AllocatorReclaimShutdownHandle, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AllocatorReclaimDesiredState { + Disabled, + Enabled, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub struct AllocatorReclaimDesiredSnapshot { + pub state: AllocatorReclaimDesiredState, + pub configured_force: bool, + pub idle_intervals: u64, + pub interval_secs: u64, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub struct AllocatorReclaimControllerSnapshot { + pub desired: AllocatorReclaimDesiredSnapshot, + pub status: AllocatorReclaimStatusSnapshot, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AllocatorReclaimWorkerMutation { + None, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub struct AllocatorReclaimReconcilePlan { + pub service: &'static str, + pub desired: AllocatorReclaimDesiredSnapshot, + pub current_state: AllocatorReclaimServiceState, + pub worker_mutation: AllocatorReclaimWorkerMutation, +} + +#[derive(Debug, Clone, Copy, Default)] +pub struct AllocatorReclaimController; + +impl AllocatorReclaimController { + pub fn snapshot(&self, ctx: &CancellationToken) -> AllocatorReclaimControllerSnapshot { + allocator_reclaim_controller_snapshot(ctx) + } + + pub fn reconcile(&self, ctx: &CancellationToken) -> AllocatorReclaimReconcilePlan { + let snapshot = self.snapshot(ctx); + self.reconcile_snapshot(snapshot) + } + + pub fn reconcile_snapshot(&self, snapshot: AllocatorReclaimControllerSnapshot) -> AllocatorReclaimReconcilePlan { + AllocatorReclaimReconcilePlan { + service: ALLOCATOR_RECLAIM_SERVICE_NAME, + desired: snapshot.desired, + current_state: snapshot.status.state, + worker_mutation: AllocatorReclaimWorkerMutation::None, + } + } +} + pub fn allocator_backend() -> &'static str { #[cfg(all(target_os = "linux", target_env = "gnu", target_arch = "x86_64"))] { @@ -85,6 +178,130 @@ fn reclaimable_work_snapshot() -> ReclaimableWorkSnapshot { } } +fn configured_allocator_reclaim_enabled() -> bool { + rustfs_utils::get_env_bool( + rustfs_config::ENV_ALLOCATOR_RECLAIM_ENABLED, + rustfs_config::DEFAULT_ALLOCATOR_RECLAIM_ENABLED, + ) +} + +fn configured_allocator_reclaim_force() -> bool { + rustfs_utils::get_env_bool(rustfs_config::ENV_ALLOCATOR_RECLAIM_FORCE, rustfs_config::DEFAULT_ALLOCATOR_RECLAIM_FORCE) +} + +fn configured_allocator_reclaim_idle_intervals() -> u64 { + rustfs_utils::get_env_u64( + rustfs_config::ENV_ALLOCATOR_RECLAIM_IDLE_INTERVALS, + rustfs_config::DEFAULT_ALLOCATOR_RECLAIM_IDLE_INTERVALS, + ) + .max(1) +} + +fn configured_allocator_reclaim_interval_secs() -> u64 { + rustfs_utils::get_env_u64( + rustfs_config::ENV_ALLOCATOR_RECLAIM_INTERVAL_SECS, + rustfs_config::DEFAULT_ALLOCATOR_RECLAIM_INTERVAL_SECS, + ) + .max(1) +} + +fn effective_allocator_reclaim_force(backend: &str, configured_force: bool) -> bool { + configured_force && backend != "jemalloc" +} + +fn build_allocator_reclaim_desired_snapshot( + enabled: bool, + configured_force: bool, + idle_intervals: u64, + interval_secs: u64, +) -> AllocatorReclaimDesiredSnapshot { + let state = if enabled { + AllocatorReclaimDesiredState::Enabled + } else { + AllocatorReclaimDesiredState::Disabled + }; + + AllocatorReclaimDesiredSnapshot { + state, + configured_force, + idle_intervals: idle_intervals.max(1), + interval_secs: interval_secs.max(1), + } +} + +fn build_allocator_reclaim_status_snapshot( + enabled: bool, + backend: &'static str, + effective_force: bool, + idle_intervals: u64, + interval_secs: u64, + cancellation_requested: bool, +) -> AllocatorReclaimStatusSnapshot { + let state = if !enabled { + AllocatorReclaimServiceState::Disabled + } else if cancellation_requested { + AllocatorReclaimServiceState::Stopping + } else { + AllocatorReclaimServiceState::Running + }; + + AllocatorReclaimStatusSnapshot { + service: ALLOCATOR_RECLAIM_SERVICE_NAME, + state, + backend, + effective_force, + idle_intervals: idle_intervals.max(1), + interval_secs: interval_secs.max(1), + cancellation_source: AllocatorReclaimCancellationSource::RuntimeToken, + shutdown_handle: AllocatorReclaimShutdownHandle::RuntimeTokenOnly, + } +} + +pub fn allocator_reclaim_status_snapshot(ctx: &CancellationToken) -> AllocatorReclaimStatusSnapshot { + let backend = allocator_backend(); + let configured_force = configured_allocator_reclaim_force(); + build_allocator_reclaim_status_snapshot( + configured_allocator_reclaim_enabled(), + backend, + effective_allocator_reclaim_force(backend, configured_force), + configured_allocator_reclaim_idle_intervals(), + configured_allocator_reclaim_interval_secs(), + ctx.is_cancelled(), + ) +} + +fn build_allocator_reclaim_controller_snapshot( + enabled: bool, + backend: &'static str, + configured_force: bool, + idle_intervals: u64, + interval_secs: u64, + cancellation_requested: bool, +) -> AllocatorReclaimControllerSnapshot { + AllocatorReclaimControllerSnapshot { + desired: build_allocator_reclaim_desired_snapshot(enabled, configured_force, idle_intervals, interval_secs), + status: build_allocator_reclaim_status_snapshot( + enabled, + backend, + effective_allocator_reclaim_force(backend, configured_force), + idle_intervals, + interval_secs, + cancellation_requested, + ), + } +} + +pub fn allocator_reclaim_controller_snapshot(ctx: &CancellationToken) -> AllocatorReclaimControllerSnapshot { + build_allocator_reclaim_controller_snapshot( + configured_allocator_reclaim_enabled(), + allocator_backend(), + configured_allocator_reclaim_force(), + configured_allocator_reclaim_idle_intervals(), + configured_allocator_reclaim_interval_secs(), + ctx.is_cancelled(), + ) +} + #[cfg(all( not(target_os = "windows"), not(all(target_os = "linux", target_env = "gnu", target_arch = "x86_64")) @@ -141,10 +358,7 @@ fn run_allocator_reclaim(force: bool) { pub fn init_allocator_reclaim(ctx: CancellationToken) { let backend = allocator_backend(); - let enabled = rustfs_utils::get_env_bool( - rustfs_config::ENV_ALLOCATOR_RECLAIM_ENABLED, - rustfs_config::DEFAULT_ALLOCATOR_RECLAIM_ENABLED, - ); + let enabled = configured_allocator_reclaim_enabled(); gauge!("rustfs_memory_allocator_reclaim_enabled").set(if enabled { 1.0 } else { 0.0 }); counter!("rustfs_memory_allocator_backend_info", "backend" => backend.to_string()).increment(1); @@ -153,30 +367,17 @@ pub fn init_allocator_reclaim(ctx: CancellationToken) { return; } - let configured_force = - rustfs_utils::get_env_bool(rustfs_config::ENV_ALLOCATOR_RECLAIM_FORCE, rustfs_config::DEFAULT_ALLOCATOR_RECLAIM_FORCE); - let force = if backend == "jemalloc" && configured_force { + let configured_force = configured_allocator_reclaim_force(); + if backend == "jemalloc" && configured_force { warn!( backend, env = rustfs_config::ENV_ALLOCATOR_RECLAIM_FORCE, "allocator reclaim force mode is not supported on jemalloc backend; ignoring configured force flag" ); - false - } else { - configured_force - }; - let idle_intervals = rustfs_utils::get_env_u64( - rustfs_config::ENV_ALLOCATOR_RECLAIM_IDLE_INTERVALS, - rustfs_config::DEFAULT_ALLOCATOR_RECLAIM_IDLE_INTERVALS, - ) - .max(1); - let interval = Duration::from_secs( - rustfs_utils::get_env_u64( - rustfs_config::ENV_ALLOCATOR_RECLAIM_INTERVAL_SECS, - rustfs_config::DEFAULT_ALLOCATOR_RECLAIM_INTERVAL_SECS, - ) - .max(1), - ); + } + let force = effective_allocator_reclaim_force(backend, configured_force); + let idle_intervals = configured_allocator_reclaim_idle_intervals(); + let interval = Duration::from_secs(configured_allocator_reclaim_interval_secs()); tokio::spawn(async move { let mut ticker = tokio::time::interval(interval); @@ -227,7 +428,12 @@ pub fn init_allocator_reclaim(ctx: CancellationToken) { #[cfg(test)] mod tests { - use super::{allocator_backend, reclaimable_work_snapshot}; + use super::{ + ALLOCATOR_RECLAIM_SERVICE_NAME, AllocatorReclaimCancellationSource, AllocatorReclaimController, + AllocatorReclaimDesiredState, AllocatorReclaimServiceState, AllocatorReclaimShutdownHandle, + AllocatorReclaimWorkerMutation, allocator_backend, build_allocator_reclaim_controller_snapshot, + build_allocator_reclaim_status_snapshot, effective_allocator_reclaim_force, reclaimable_work_snapshot, + }; #[test] fn allocator_backend_name_is_available() { @@ -238,4 +444,89 @@ mod tests { fn reclaimable_work_snapshot_is_collectable() { let _ = reclaimable_work_snapshot(); } + + #[test] + fn allocator_reclaim_status_reports_disabled_state() { + let snapshot = build_allocator_reclaim_status_snapshot(false, "mimalloc", true, 3, 30, false); + + assert_eq!(snapshot.service, ALLOCATOR_RECLAIM_SERVICE_NAME); + assert_eq!(snapshot.state, AllocatorReclaimServiceState::Disabled); + assert_eq!(snapshot.backend, "mimalloc"); + assert!(snapshot.effective_force); + assert_eq!(snapshot.idle_intervals, 3); + assert_eq!(snapshot.interval_secs, 30); + assert_eq!(snapshot.cancellation_source, AllocatorReclaimCancellationSource::RuntimeToken); + assert_eq!(snapshot.shutdown_handle, AllocatorReclaimShutdownHandle::RuntimeTokenOnly); + } + + #[test] + fn allocator_reclaim_status_reports_running_and_stopping_states() { + let running = build_allocator_reclaim_status_snapshot(true, "mimalloc", true, 0, 0, false); + let stopping = build_allocator_reclaim_status_snapshot(true, "mimalloc", false, 4, 60, true); + + assert_eq!(running.state, AllocatorReclaimServiceState::Running); + assert_eq!(running.idle_intervals, 1); + assert_eq!(running.interval_secs, 1); + assert_eq!(stopping.state, AllocatorReclaimServiceState::Stopping); + assert_eq!(stopping.idle_intervals, 4); + assert_eq!(stopping.interval_secs, 60); + } + + #[test] + fn allocator_reclaim_force_preserves_jemalloc_override() { + assert!(!effective_allocator_reclaim_force("jemalloc", true)); + assert!(effective_allocator_reclaim_force("mimalloc", true)); + assert!(!effective_allocator_reclaim_force("mimalloc", false)); + } + + #[test] + fn allocator_reclaim_controller_reconcile_is_idempotent() { + let controller = AllocatorReclaimController; + let snapshot = build_allocator_reclaim_controller_snapshot(true, "mimalloc", true, 3, 30, false); + + let first = controller.reconcile_snapshot(snapshot); + let second = controller.reconcile_snapshot(snapshot); + + assert_eq!(first, second); + assert_eq!(first.desired.state, AllocatorReclaimDesiredState::Enabled); + assert_eq!(first.current_state, AllocatorReclaimServiceState::Running); + assert_eq!(first.worker_mutation, AllocatorReclaimWorkerMutation::None); + } + + #[test] + fn allocator_reclaim_controller_preserves_cancellation_state_without_worker_mutation() { + let controller = AllocatorReclaimController; + let snapshot = build_allocator_reclaim_controller_snapshot(true, "mimalloc", false, 3, 30, true); + let plan = controller.reconcile_snapshot(snapshot); + + assert_eq!(snapshot.status.state, AllocatorReclaimServiceState::Stopping); + assert_eq!(plan.current_state, AllocatorReclaimServiceState::Stopping); + assert_eq!(plan.worker_mutation, AllocatorReclaimWorkerMutation::None); + } + + #[test] + fn allocator_reclaim_controller_reports_disabled_desired_state_without_starting_worker() { + let controller = AllocatorReclaimController; + let snapshot = build_allocator_reclaim_controller_snapshot(false, "mimalloc", true, 0, 0, false); + let plan = controller.reconcile_snapshot(snapshot); + + assert_eq!(snapshot.desired.state, AllocatorReclaimDesiredState::Disabled); + assert_eq!(snapshot.desired.idle_intervals, 1); + assert_eq!(snapshot.desired.interval_secs, 1); + assert_eq!(plan.current_state, AllocatorReclaimServiceState::Disabled); + assert_eq!(plan.worker_mutation, AllocatorReclaimWorkerMutation::None); + } + + #[tokio::test(start_paused = true)] + async fn allocator_reclaim_controller_harness_is_stable_across_paused_time() { + let controller = AllocatorReclaimController; + let snapshot = build_allocator_reclaim_controller_snapshot(true, "mimalloc", true, 3, 30, false); + let before = controller.reconcile_snapshot(snapshot); + + tokio::time::advance(std::time::Duration::from_secs(30)).await; + let after = controller.reconcile_snapshot(snapshot); + + assert_eq!(before, after); + assert_eq!(after.worker_mutation, AllocatorReclaimWorkerMutation::None); + } } diff --git a/rustfs/src/memory_observability.rs b/rustfs/src/memory_observability.rs index d53f2024b..01336027c 100644 --- a/rustfs/src/memory_observability.rs +++ b/rustfs/src/memory_observability.rs @@ -327,9 +327,9 @@ pub fn init_memory_observability(ctx: CancellationToken) { #[cfg(test)] mod tests { use super::{ - CgroupMemorySnapshot, MemoryObservabilityCancellationSource, MemoryObservabilityController, - MemoryObservabilityDesiredState, MemoryObservabilityServiceState, MemoryObservabilityShutdownHandle, - MemoryObservabilityWorkerMutation, build_memory_observability_controller_snapshot, + CgroupMemorySnapshot, MEMORY_OBSERVABILITY_SERVICE_NAME, MemoryObservabilityCancellationSource, + MemoryObservabilityController, MemoryObservabilityDesiredState, MemoryObservabilityServiceState, + MemoryObservabilityShutdownHandle, MemoryObservabilityWorkerMutation, build_memory_observability_controller_snapshot, build_memory_observability_status_snapshot, parse_kv_stats, read_optional_u64, }; use std::fs; @@ -425,4 +425,22 @@ mod tests { assert_eq!(plan.current_state, MemoryObservabilityServiceState::Disabled); assert_eq!(plan.worker_mutation, MemoryObservabilityWorkerMutation::None); } + + #[test] + fn memory_observability_controller_harness_never_mutates_workers() { + let controller = MemoryObservabilityController; + let snapshots = [ + build_memory_observability_controller_snapshot(true, 15, false), + build_memory_observability_controller_snapshot(true, 15, true), + build_memory_observability_controller_snapshot(false, 15, false), + ]; + + for snapshot in snapshots { + let plan = controller.reconcile_snapshot(snapshot); + + assert_eq!(plan.service, MEMORY_OBSERVABILITY_SERVICE_NAME); + assert_eq!(plan.current_state, snapshot.status.state); + assert_eq!(plan.worker_mutation, MemoryObservabilityWorkerMutation::None); + } + } }