From c1382654405c26dd3236f4c983ad72f4e62dc051 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Wed, 8 Jul 2026 17:31:46 +0800 Subject: [PATCH] refactor(ecstore): migrate the S3 region into InstanceContext (Phase 5 Slice 4) (#4462) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5 Slice 4 (backlog#939): move the S3 region — a write-once identity scalar — out of the GLOBAL_REGION process static into the per-instance InstanceContext, so two instances can serve different regions. - InstanceContext gains `region: OnceLock` with `set_region()` / `region()`. `set_region` keeps the write-once fail-fast contract: a second write panics, exactly as the process global did (not downgraded to a warn). - global.rs `set_global_region` / `get_global_region` keep their signatures and forward to the current instance's context; the GLOBAL_REGION static is removed. Single-instance: startup writes the bootstrap context (which the ECStore adopts), so reads are unchanged. Tests: set/get round-trip, two contexts hold distinct regions, and a second set_region panics (fail-fast preserved). Verification: cargo test -p rustfs-ecstore (9 instance-context tests green), cargo clippy -p rustfs-ecstore --all-targets (clean), make pre-commit (pass). Refs: backlog#939 (Phase 5, Slice 4). Stacked on Slice 3 (#4417). --- crates/ecstore/src/runtime/global.rs | 15 +++---- crates/ecstore/src/runtime/instance.rs | 56 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/crates/ecstore/src/runtime/global.rs b/crates/ecstore/src/runtime/global.rs index 9e62e39e5..f8a6da4a6 100644 --- a/crates/ecstore/src/runtime/global.rs +++ b/crates/ecstore/src/runtime/global.rs @@ -45,11 +45,11 @@ pub const DISK_RESERVE_FRACTION: f64 = 0.15; // // Tier A (needs migration): GLOBAL_OBJECT_API, GLOBAL_LOCAL_DISK_*, // GLOBAL_ROOT_DISK_THRESHOLD, GLOBAL_LIFECYCLE_SYS, GLOBAL_EVENT_NOTIFIER, etc. -// Tier B (keep as static): GLOBAL_RUSTFS_PORT, GLOBAL_REGION, env var caches, etc. +// Tier B (keep as static): GLOBAL_RUSTFS_PORT, env var caches, etc. // -// Phase 5 (backlog#939): the erasure setup type moved into the per-instance -// `InstanceContext` (see `super::instance`); the erasure predicates below now -// forward to the current instance's context. +// Phase 5 (backlog#939): the erasure setup type and the S3 region moved into +// the per-instance `InstanceContext` (see `super::instance`); their facades +// below now forward to the current instance's context. lazy_static! { static ref GLOBAL_RUSTFS_PORT: OnceLock = OnceLock::new(); static ref GLOBAL_DEPLOYMENT_ID: OnceLock = OnceLock::new(); @@ -66,7 +66,6 @@ lazy_static! { pub static ref GLOBAL_LOCAL_NODE_NAME_FALLBACK: String = "127.0.0.1:9000".to_string(); pub static ref GLOBAL_LOCAL_NODE_NAME_HEX_FALLBACK: String = rustfs_utils::crypto::hex(GLOBAL_LOCAL_NODE_NAME_FALLBACK.as_bytes()); - pub static ref GLOBAL_REGION: OnceLock = OnceLock::new(); pub static ref GLOBAL_LOCAL_LOCK_CLIENT: OnceLock> = OnceLock::new(); pub static ref GLOBAL_LOCK_CLIENTS: OnceLock>> = OnceLock::new(); pub static ref GLOBAL_BUCKET_MONITOR: OnceLock> = OnceLock::new(); @@ -291,9 +290,7 @@ pub(crate) type TypeLocalDiskSetDrives = Vec>>>; /// # Returns /// * None pub fn set_global_region(region: s3s::region::Region) { - GLOBAL_REGION - .set(region) - .expect("GLOBAL_REGION should be initialized once during startup"); + current_ctx().set_region(region); } /// Get the global region @@ -302,7 +299,7 @@ pub fn set_global_region(region: s3s::region::Region) { /// * `Option` - The global region, if set /// pub fn get_global_region() -> Option { - GLOBAL_REGION.get().cloned() + current_ctx().region() } /// Initialize the global background services cancellation token diff --git a/crates/ecstore/src/runtime/instance.rs b/crates/ecstore/src/runtime/instance.rs index 1bf8b7eb0..ef3611005 100644 --- a/crates/ecstore/src/runtime/instance.rs +++ b/crates/ecstore/src/runtime/instance.rs @@ -41,6 +41,7 @@ use crate::layout::endpoints::SetupType; use rustfs_lock::{GlobalLockManager, get_global_lock_manager}; +use s3s::region::Region; use std::sync::{Arc, OnceLock}; use tokio::sync::RwLock; @@ -66,6 +67,11 @@ pub struct InstanceContext { /// contend). Single-instance aliases the process singleton, so behavior is /// unchanged; see [`bootstrap_ctx`]. lock_manager: Arc, + /// This instance's S3 region (Phase 5 Slice 4, backlog#939). + /// + /// Write-once like the process global it replaces: startup sets it exactly + /// once and a duplicate write fails fast (see [`InstanceContext::set_region`]). + region: OnceLock, } impl InstanceContext { @@ -85,6 +91,7 @@ impl InstanceContext { Self { erasure_kind: RwLock::new(SetupType::Unknown), lock_manager, + region: OnceLock::new(), } } @@ -93,6 +100,21 @@ impl InstanceContext { self.lock_manager.clone() } + /// Set this instance's S3 region. + /// + /// Write-once: panics on a second write, preserving the startup fail-fast + /// contract of the process global it replaces. + pub fn set_region(&self, region: Region) { + self.region + .set(region) + .expect("instance region should be initialized once during startup"); + } + + /// This instance's S3 region, if it has been set. + pub fn region(&self) -> Option { + self.region.get().cloned() + } + /// Update this instance's erasure setup type. pub async fn update_erasure_type(&self, setup_type: SetupType) { *self.erasure_kind.write().await = setup_type; @@ -229,4 +251,38 @@ mod tests { "fresh contexts must not share a lock manager" ); } + + fn region(name: &str) -> Region { + name.parse().expect("valid test region") + } + + // A fresh context has no region until set; set-then-get round-trips. + #[tokio::test] + async fn region_set_and_get() { + let ctx = InstanceContext::new(); + assert_eq!(ctx.region(), None); + ctx.set_region(region("us-east-1")); + assert_eq!(ctx.region(), Some(region("us-east-1"))); + } + + // Two contexts hold independent regions. + #[tokio::test] + async fn distinct_contexts_have_distinct_region() { + let ctx_a = InstanceContext::new(); + let ctx_b = InstanceContext::new(); + ctx_a.set_region(region("us-east-1")); + ctx_b.set_region(region("eu-west-1")); + assert_eq!(ctx_a.region(), Some(region("us-east-1"))); + assert_eq!(ctx_b.region(), Some(region("eu-west-1"))); + } + + // The region is write-once: a second set fails fast, preserving the + // startup contract of the process global it replaced. + #[tokio::test] + #[should_panic(expected = "initialized once")] + async fn set_region_twice_panics() { + let ctx = InstanceContext::new(); + ctx.set_region(region("us-east-1")); + ctx.set_region(region("eu-west-1")); + } }