From 90f9c24503f1a695521bff9f5202703674516d9a Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Tue, 30 Jun 2026 04:52:23 +0800 Subject: [PATCH] refactor(runtime): clean scalar globals (#4094) --- crates/ecstore/src/runtime/global.rs | 24 +++++++++++++------ docs/architecture/global-state-inventory.md | 4 ++-- scripts/check_architecture_migration_rules.sh | 2 +- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/crates/ecstore/src/runtime/global.rs b/crates/ecstore/src/runtime/global.rs index fd92a5e19..d8f5861b4 100644 --- a/crates/ecstore/src/runtime/global.rs +++ b/crates/ecstore/src/runtime/global.rs @@ -47,7 +47,7 @@ pub const DISK_RESERVE_FRACTION: f64 = 0.15; // Tier B (keep as static): GLOBAL_RUSTFS_PORT, GLOBAL_REGION, env var caches, etc. lazy_static! { static ref GLOBAL_RUSTFS_PORT: OnceLock = OnceLock::new(); - static ref globalDeploymentIDPtr: OnceLock = OnceLock::new(); + static ref GLOBAL_DEPLOYMENT_ID: OnceLock = OnceLock::new(); pub static ref GLOBAL_OBJECT_API: OnceLock> = OnceLock::new(); pub static ref GLOBAL_IsErasure: RwLock = RwLock::new(false); pub static ref GLOBAL_IsDistErasure: RwLock = RwLock::new(false); @@ -82,6 +82,9 @@ pub fn get_global_bucket_monitor() -> Option> { GLOBAL_BUCKET_MONITOR.get().cloned() } +// Startup-owned process globals intentionally fail fast on duplicate writes. +// A second write means startup published conflicting runtime scalar state. + /// Global cancellation token for background services (data scanner and auto heal) static GLOBAL_BACKGROUND_SERVICES_CANCEL_TOKEN: OnceLock = OnceLock::new(); @@ -106,7 +109,9 @@ pub fn global_rustfs_port() -> u16 { /// # Returns /// * None pub fn set_global_rustfs_port(value: u16) { - GLOBAL_RUSTFS_PORT.set(value).expect("set_global_rustfs_port fail"); + GLOBAL_RUSTFS_PORT + .set(value) + .expect("GLOBAL_RUSTFS_PORT should be initialized once during startup"); } /// Set the global deployment id @@ -118,7 +123,9 @@ pub fn set_global_rustfs_port(value: u16) { /// * None /// pub fn set_global_deployment_id(id: Uuid) { - globalDeploymentIDPtr.set(id).expect("operation should succeed"); + GLOBAL_DEPLOYMENT_ID + .set(id) + .expect("GLOBAL_DEPLOYMENT_ID should be initialized once during startup"); } /// Get the global deployment id @@ -127,7 +134,7 @@ pub fn set_global_deployment_id(id: Uuid) { /// * `Option` - The global deployment id as a string, if set /// pub fn get_global_deployment_id() -> Option { - globalDeploymentIDPtr.get().map(|v| v.to_string()) + GLOBAL_DEPLOYMENT_ID.get().map(|v| v.to_string()) } /// Set the global endpoints /// @@ -140,7 +147,7 @@ pub fn get_global_deployment_id() -> Option { pub fn set_global_endpoints(eps: Vec) { GLOBAL_Endpoints .set(EndpointServerPools::from(eps)) - .expect("GLOBAL_Endpoints set failed") + .expect("GLOBAL_Endpoints should be initialized once during storage startup") } /// Get the global endpoints @@ -282,7 +289,9 @@ pub(crate) type TypeLocalDiskSetDrives = Vec>>>; /// # Returns /// * None pub fn set_global_region(region: s3s::region::Region) { - GLOBAL_REGION.set(region).expect("operation should succeed"); + GLOBAL_REGION + .set(region) + .expect("GLOBAL_REGION should be initialized once during startup"); } /// Get the global region @@ -323,7 +332,8 @@ pub fn get_background_services_cancel_token() -> Option<&'static CancellationTok /// pub fn create_background_services_cancel_token() -> CancellationToken { let cancel_token = CancellationToken::new(); - init_background_services_cancel_token(cancel_token.clone()).expect("Background services cancel token already initialized"); + init_background_services_cancel_token(cancel_token.clone()) + .expect("background services cancel token should be initialized once during startup"); cancel_token } diff --git a/docs/architecture/global-state-inventory.md b/docs/architecture/global-state-inventory.md index dc2e7e096..34f6271cb 100644 --- a/docs/architecture/global-state-inventory.md +++ b/docs/architecture/global-state-inventory.md @@ -29,7 +29,7 @@ caches separate from runtime migration targets. | Owner-local compatibility | Existing compatibility adapters that are allowed to read globals while callers migrate to AppContext-first or owner-local runtime-source APIs. | `rustfs/src/*/runtime_sources.rs`, `rustfs/src/*/storage_api.rs`, `crates/*/storage_api.rs` | | Test or fixture state | Static setup used by tests to amortize expensive ECStore setup or isolate compatibility harness state. | `rustfs/src/app/*_test.rs`, `crates/scanner/tests/*`, `crates/ecstore/src/**/tests` | | Cache or constant | Regexes, metrics descriptors, defaults, KVS registrations, headers, path constants, and small process caches that are not runtime ownership handles. | `crates/config`, `crates/obs/src/metrics`, `crates/utils`, `rustfs/src/server/readiness.rs` | -| Legacy naming or review-needed | Mixed-case `GLOBAL_*`, old MinIO-port naming, stale comments, or names that need owner confirmation before code movement. | `GLOBAL_IsErasure`, `GLOBAL_Endpoints`, `GLOBAL_LocalNodeName`, `globalDeploymentIDPtr` | +| Legacy naming or review-needed | Mixed-case `GLOBAL_*`, old MinIO-port naming, stale comments, or names that need owner confirmation before code movement. | `GLOBAL_IsErasure`, `GLOBAL_Endpoints`, `GLOBAL_LocalNodeName` | ## Runtime Migration Inventory @@ -46,7 +46,7 @@ migration PR removes or replaces each item. | `GLOBAL_REPLICATION_POOL`, `GLOBAL_REPLICATION_STATS`, `GLOBAL_BUCKET_MONITOR` | `crates/ecstore/src/bucket/replication/*`, `crates/ecstore/src/runtime/global.rs` | Runtime migration target | Replication pool/stat access now stays behind replication owner and ECStore runtime-source helpers; bucket-monitor direct access now stays behind ECStore runtime helpers while AppContext/runtime-source resolvers remain the caller boundary. | | `GLOBAL_TierConfigMgr`, `GLOBAL_STORAGE_CLASS`, `GLOBAL_CONFIG_SYS`, `GLOBAL_SERVER_CONFIG` | `crates/ecstore/src/config`, `crates/config`, `rustfs/src/app/context/runtime_sources.rs` | Runtime migration target | Tier config manager reads and reloads now use the ECStore runtime-source helper; move remaining config state through config/runtime-source owners only, without combining storage-class behavior or persistence changes. | | `GLOBAL_EventNotifier`, `GLOBAL_NotificationSys` | `crates/ecstore/src/runtime/global.rs`, `crates/ecstore/src/runtime/sources.rs`, and `crates/ecstore/src/services/*` | Runtime migration target | `GLOBAL_EventNotifier` access now stays behind ECStore runtime-source helpers; move remaining notification ownership only through notify/runtime-source boundaries. | -| `GLOBAL_BOOT_TIME`, `GLOBAL_BACKGROUND_SERVICES_CANCEL_TOKEN`, `globalDeploymentIDPtr`, `GLOBAL_REGION`, `GLOBAL_RUSTFS_PORT`, `GLOBAL_LocalNodeName`, `GLOBAL_LocalNodeNameHex` | `crates/ecstore/src/runtime/global.rs`, `crates/ecstore/src/runtime/sources.rs` | Runtime migration target | Boot time, background service cancellation token reads, and ECStore local-node-name fallback reads now stay behind the ECStore runtime-source API; deployment ID, region, and port direct access now stays behind owner helpers until the remaining scalar handles migrate. | +| `GLOBAL_BOOT_TIME`, `GLOBAL_BACKGROUND_SERVICES_CANCEL_TOKEN`, `GLOBAL_DEPLOYMENT_ID`, `GLOBAL_REGION`, `GLOBAL_RUSTFS_PORT`, `GLOBAL_LocalNodeName`, `GLOBAL_LocalNodeNameHex` | `crates/ecstore/src/runtime/global.rs`, `crates/ecstore/src/runtime/sources.rs` | Runtime migration target | Boot time, background service cancellation token reads, and ECStore local-node-name fallback reads now stay behind the ECStore runtime-source API; deployment ID, region, and port direct access now stays behind owner helpers until the remaining scalar handles migrate. | | `GLOBAL_LOCAL_LOCK_CLIENT`, `GLOBAL_LOCK_CLIENTS`, `GLOBAL_LOCK_MANAGER` | `crates/ecstore/src/runtime/global.rs`, `crates/lock` | Runtime migration target / process-global split | ECStore lock client direct access now stays behind ECStore runtime helpers; preserve lock quorum and lock client selection while keeping the process-level lock manager separate from endpoint-specific clients. | | `GLOBAL_CONN_MAP`, `GLOBAL_LOCAL_NODE_NAME`, `GLOBAL_RUSTFS_HOST`, `GLOBAL_RUSTFS_ADDR`, `GLOBAL_ROOT_CERT`, `GLOBAL_MTLS_IDENTITY`, `GLOBAL_OUTBOUND_TLS_GENERATION` | `crates/common`, `crates/tls-runtime`, `crates/ecstore/src/runtime/sources.rs` | Runtime migration target / process-global split | Internode connection cache, common local node name, RustFS host/address reads, and outbound TLS material reads are now owned behind `rustfs_common` helpers; migrate the remaining transport and TLS state only after internode transport and outbound TLS ownership are explicit, without changing cached channel reuse or TLS reload semantics. | | `GLOBAL_RUSTFS_RPC_SECRET` | `crates/credentials`, `crates/ecstore/src/runtime/sources.rs` | Runtime migration target / process-global split | RPC auth token writes now stay behind the `rustfs_credentials` helper boundary; migrate only if runtime secret ownership changes, preserving lazy environment and credential-derived token semantics. | diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh index 7c739901f..5cf9b4576 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -2685,7 +2685,7 @@ fi ( cd "$ROOT_DIR" - rg -n --with-filename '\bGLOBAL_(REGION|RUSTFS_PORT)\b|\bglobalDeploymentIDPtr\b|\bruntime::global::global_rustfs_port\b' \ + rg -n --with-filename '\bGLOBAL_(DEPLOYMENT_ID|REGION|RUSTFS_PORT)\b|\bglobalDeploymentIDPtr\b|\bruntime::global::global_rustfs_port\b' \ crates rustfs fuzz \ --glob '*.rs' | rg -v '^(crates/common/src/globals|crates/ecstore/src/runtime/(global|sources))\.rs:' || true