From f7003dfdddc4165006f467a854479383b911503c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Sun, 23 Aug 2026 04:24:04 +0800 Subject: [PATCH 1/7] fix(admin): four site-replication interop correctness fixes (B5-rc T2) (#6399) * fix(admin): send versioningEnabled on site replication make-bucket ops The outbound make-with-versioning bucket-op query only carried operation/createdAt/lockEnabled. MinIO's own create-bucket hook sends versioningEnabled=true on this op, so align the outbound query with MinIO's site-replication make-bucket wire contract. Route both outbound builders (bootstrap plan and create-bucket hook) through one shared builder that always appends versioningEnabled=true. RustFS's own inbound handler force-enables versioning either way, so RustFS-to-RustFS behavior is unchanged; the MinIO release verified against (RELEASE.2025-09-07) also force-enables versioning regardless of the flag, so this aligns the wire contract rather than changing observable behavior there. * fix(admin): propagate purge-deleted-bucket errors in site replication The purge-deleted-bucket branch of the peer bucket-ops handler dropped the delete_bucket error and answered 200, so a peer-driven purge that failed (disk full, quorum loss) was reported as success while the bucket survived on this site. Tolerate only bucket-not-found (the purge raced an earlier replay or a local delete) and propagate every other error through ApiError like the sibling delete branches do. * fix(admin): derive fallback site deployment ID with UUIDv5 deployment_id_for_endpoint used DefaultHasher, whose algorithm is not guaranteed stable across Rust releases. The fallback fires when a peer response carries an empty deploymentID; the result is persisted in site-replication state, used for collision disambiguation, and broadcast to peers, so a toolchain bump could re-derive a different ID for the same endpoint. Note that the add preflight currently rejects that case upstream of this fallback. Derive UUIDv5 (NAMESPACE_URL) over the canonical endpoint instead, and log a structured warn when a peer metainfo response arrives without a deploymentID. Already persisted fallback IDs are non-empty and therefore never re-derived, so existing state is unaffected. * fix(admin): stream site replication devnull body without 1MB cap The site-replication devnull endpoint buffered the request body through read_plain_admin_body, which enforces the 1MB admin body cap. MinIO peers stream multi-megabyte probe bodies to this endpoint during site netperf link checks and expect an unbounded discard, so any larger probe got a 400 and was misreported as a broken link. Stream and discard the body chunk by chunk with no size cap instead, mirroring MinIO's io.Discard drain. The response stays 204 with an empty body. --- Cargo.lock | 1 + rustfs/Cargo.toml | 4 +- rustfs/src/admin/handlers/site_replication.rs | 153 ++++++++++++++---- rustfs/src/admin/site_replication_identity.rs | 29 +++- rustfs/src/admin/storage_api.rs | 3 +- 5 files changed, 150 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 637202fd5..b7b1ec220 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12688,6 +12688,7 @@ dependencies = [ "js-sys", "rand 0.10.2", "serde_core", + "sha1_smol", "wasm-bindgen", ] diff --git a/rustfs/Cargo.toml b/rustfs/Cargo.toml index 28f4ad259..af5514870 100644 --- a/rustfs/Cargo.toml +++ b/rustfs/Cargo.toml @@ -322,7 +322,7 @@ thiserror = { workspace = true } tracing.workspace = true url = { workspace = true } urlencoding = { workspace = true } -uuid = { workspace = true, features = ["v4", "fast-rng", "macro-diagnostics"] } +uuid = { workspace = true, features = ["v4", "v5", "fast-rng", "macro-diagnostics"] } zip = { workspace = true } libc = { workspace = true } rand = { workspace = true, features = ["serde"] } @@ -345,7 +345,7 @@ libsystemd.workspace = true libmimalloc-sys.workspace = true [dev-dependencies] -uuid = { workspace = true, features = ["v4", "fast-rng", "macro-diagnostics"] } +uuid = { workspace = true, features = ["v4", "v5", "fast-rng", "macro-diagnostics"] } serial_test = { workspace = true } tempfile = { workspace = true } aws-config = { workspace = true } diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index 1bb0379df..23c8b680b 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -41,7 +41,7 @@ use crate::admin::storage_api::config::save_admin_config; use crate::admin::storage_api::contract::bucket::{ BucketOperations, BucketOptions, DeleteBucketOptions, MakeBucketOptions, SRBucketDeleteOp, }; -use crate::admin::storage_api::error::Error as StorageError; +use crate::admin::storage_api::error::{Error as StorageError, is_err_bucket_not_found}; use crate::admin::storage_api::runtime::ECStore; use crate::admin::utils::{encode_compatible_admin_payload, read_compatible_admin_body}; use crate::auth::constant_time_eq; @@ -55,6 +55,7 @@ use crate::storage::storage_api::{ use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use futures::StreamExt; use hmac::{Hmac, Mac}; use http::header::{CONTENT_TYPE, HOST}; use http::{HeaderMap, HeaderValue, Uri}; @@ -2096,6 +2097,18 @@ async fn remote_add_preflight_info(site: &PeerSite) -> S3Result Option { query_pairs(uri).get("bootstrapToken").cloned() } -fn bootstrap_bucket_make_op_path(bucket: &SRBucketInfo) -> String { +/// Query for a peer `make-with-versioning` bucket op. `versioningEnabled` +/// always travels so the outbound query matches MinIO's site-replication +/// make-bucket wire contract: MinIO's own create-bucket hook sends +/// `versioningEnabled=true` on this op. RustFS's inbound handler +/// force-enables versioning either way. +fn make_with_versioning_bucket_op_path(bucket: &str, created_at: Option<&str>, lock_enabled: bool) -> String { let mut query = form_urlencoded::Serializer::new(String::new()); - query.append_pair("bucket", &bucket.bucket); - query.append_pair("operation", "make-with-versioning"); - if let Some(created_at) = bucket - .created_at - .and_then(|value| value.format(&time::format_description::well_known::Rfc3339).ok()) - { - query.append_pair("createdAt", &created_at); + query.append_pair("bucket", bucket); + query.append_pair("operation", SITE_REPLICATION_BUCKET_OP_MAKE_WITH_VERSIONING); + query.append_pair("versioningEnabled", "true"); + if let Some(created_at) = created_at { + query.append_pair("createdAt", created_at); } - if bucket.object_lock_config.is_some() { + if lock_enabled { query.append_pair("lockEnabled", "true"); } - format!("/rustfs/admin/v3/site-replication/peer/bucket-ops?{}", query.finish()) + format!("{SITE_REPLICATION_PEER_BUCKET_OPS_PATH}?{}", query.finish()) +} + +fn bootstrap_bucket_make_op_path(bucket: &SRBucketInfo) -> String { + let created_at = bucket + .created_at + .and_then(|value| value.format(&time::format_description::well_known::Rfc3339).ok()); + make_with_versioning_bucket_op_path(&bucket.bucket, created_at.as_deref(), bucket.object_lock_config.is_some()) } fn bootstrap_bucket_meta_item(bucket: &SRBucketInfo, item_type: &str, updated_at: Option) -> SRBucketMeta { @@ -4246,16 +4269,7 @@ async fn broadcast_site_replication_make_bucket( .format(&time::format_description::well_known::Rfc3339) .unwrap_or_default(); - let path = { - let mut query = form_urlencoded::Serializer::new(String::new()); - query.append_pair("bucket", bucket); - query.append_pair("operation", "make-with-versioning"); - query.append_pair("createdAt", &created_at); - if lock_enabled { - query.append_pair("lockEnabled", "true"); - } - format!("/rustfs/admin/v3/site-replication/peer/bucket-ops?{}", query.finish()) - }; + let path = make_with_versioning_bucket_op_path(bucket, Some(&created_at), lock_enabled); let path = if let Some(token) = bootstrap_token { with_site_replication_bootstrap_token(&path, token) } else { @@ -10206,13 +10220,25 @@ impl Operation for SiteReplicationStatusHandler { } } +/// `POST /v3/site-replication/devnull` — peer link-check upload drain. +/// MinIO streams multi-megabyte probe bodies here during site netperf link +/// checks and expects an unbounded discard (its handler copies to io.Discard); +/// buffering through the 1MB admin body cap turned any larger probe into a +/// 400 and a false link failure. Stream and discard instead — no size cap. +async fn drain_site_replication_devnull(mut input: Body) -> S3Result<()> { + while let Some(chunk) = input.next().await { + chunk.map_err(|e| s3_error!(InvalidRequest, "failed to read devnull stream: {}", e))?; + } + Ok(()) +} + pub struct SiteReplicationDevNullHandler {} #[async_trait::async_trait] impl Operation for SiteReplicationDevNullHandler { async fn call(&self, req: S3Request, _params: Params<'_, '_>) -> S3Result> { validate_site_replication_admin_request(&req, AdminAction::SiteReplicationOperationAction).await?; - let _ = read_plain_admin_body(req.input).await?; + drain_site_replication_devnull(req.input).await?; Ok(empty_response(StatusCode::NO_CONTENT)) } } @@ -10471,6 +10497,19 @@ impl Operation for SRPeerJoinHandler { } } +/// Outcome of a peer-driven `purge-deleted-bucket` replay. A bucket that is +/// already gone means the purge raced an earlier replay or a local delete — +/// that is success — but any other failure must reach the sender like the +/// sibling delete branches do: swallowing it answered 200 while the bucket +/// survived on this site. +fn purge_deleted_bucket_result(result: Result<(), StorageError>) -> S3Result<()> { + match result { + Ok(()) => Ok(()), + Err(err) if is_err_bucket_not_found(&err) => Ok(()), + Err(err) => Err(ApiError::from(err).into()), + } +} + pub struct SRPeerBucketOpsHandler {} #[async_trait::async_trait] @@ -10570,16 +10609,18 @@ impl Operation for SRPeerBucketOpsHandler { .map_err(ApiError::from)?; } "purge-deleted-bucket" => { - let _ = store - .delete_bucket( - &bucket, - &DeleteBucketOptions { - force: true, - srdelete_op: SRBucketDeleteOp::Purge, - ..Default::default() - }, - ) - .await; + purge_deleted_bucket_result( + store + .delete_bucket( + &bucket, + &DeleteBucketOptions { + force: true, + srdelete_op: SRBucketDeleteOp::Purge, + ..Default::default() + }, + ) + .await, + )?; } _ => return Err(s3_error!(InvalidRequest, "unsupported site replication bucket operation")), } @@ -13925,6 +13966,54 @@ mod tests { assert!(!query_flag(&uri, "missing")); } + /// A5 red-light: a `purge-deleted-bucket` replay must report success when + /// the bucket is already gone, and must propagate every other failure — + /// the swallowed error answered 200 while the bucket survived. + #[test] + fn test_purge_deleted_bucket_result_tolerates_only_missing_bucket() { + assert!(purge_deleted_bucket_result(Ok(())).is_ok()); + assert!(purge_deleted_bucket_result(Err(StorageError::BucketNotFound("photos".to_string()))).is_ok()); + assert!(purge_deleted_bucket_result(Err(StorageError::VolumeNotFound)).is_ok()); + let err = purge_deleted_bucket_result(Err(StorageError::StorageFull)) + .expect_err("non-not-found delete failures must propagate"); + assert_ne!(*err.code(), S3ErrorCode::NoSuchBucket); + } + + /// C5 red-light: the site-replication devnull drain must accept bodies + /// beyond the 1MB admin body cap — MinIO's link check streams large + /// probe bodies and treats a 400 as a broken link. + #[tokio::test] + async fn test_site_replication_devnull_drains_body_beyond_admin_cap() { + let body = Body::from(vec![0u8; MAX_ADMIN_REQUEST_BODY_SIZE + 1]); + drain_site_replication_devnull(body) + .await + .expect("devnull must drain bodies larger than the admin body cap"); + } + + /// A3 red-light: `versioningEnabled` must travel on every outbound + /// make-with-versioning bucket op so the query matches MinIO's + /// site-replication make-bucket wire contract (MinIO's own hook sends + /// `versioningEnabled=true` on this op). + #[test] + fn test_make_with_versioning_op_paths_send_versioning_enabled() { + let bucket = SRBucketInfo { + bucket: "photos".to_string(), + created_at: Some(OffsetDateTime::UNIX_EPOCH), + object_lock_config: Some(BASE64_STANDARD.encode("")), + ..Default::default() + }; + let bootstrap = bootstrap_bucket_make_op_path(&bucket); + assert!(bootstrap.contains("operation=make-with-versioning"), "{bootstrap}"); + assert!(bootstrap.contains("versioningEnabled=true"), "{bootstrap}"); + assert!(bootstrap.contains("createdAt="), "{bootstrap}"); + assert!(bootstrap.contains("lockEnabled=true"), "{bootstrap}"); + + // The broadcast path (create-bucket hook) shares the same builder. + let broadcast = make_with_versioning_bucket_op_path("photos", Some("1970-01-01T00:00:00Z"), false); + assert!(broadcast.contains("versioningEnabled=true"), "{broadcast}"); + assert!(!broadcast.contains("lockEnabled"), "{broadcast}"); + } + #[tokio::test] #[serial] async fn test_add_bootstrap_scope_only_allows_expected_bucket_setup_until_guard_drops() { diff --git a/rustfs/src/admin/site_replication_identity.rs b/rustfs/src/admin/site_replication_identity.rs index dc6440b4d..24784160b 100644 --- a/rustfs/src/admin/site_replication_identity.rs +++ b/rustfs/src/admin/site_replication_identity.rs @@ -13,9 +13,9 @@ // limitations under the License. use rustfs_madmin::{PeerInfo, SyncStatus}; -use std::collections::{BTreeMap, hash_map::DefaultHasher}; -use std::hash::{Hash, Hasher}; +use std::collections::BTreeMap; use url::Url; +use uuid::Uuid; fn has_http_scheme(endpoint: &str) -> bool { endpoint.get(..7).is_some_and(|prefix| prefix.eq_ignore_ascii_case("http://")) @@ -66,10 +66,12 @@ pub fn site_identity_key(endpoint: &str) -> String { .unwrap_or_else(|| trimmed.to_ascii_lowercase()) } +/// Fallback deployment ID for a peer that reported none. UUIDv5 over the +/// canonical endpoint: the ID is persisted in site-replication state and +/// broadcast to peers, so it must be identical across Rust toolchains +/// (`DefaultHasher` is not) and across spellings of the same endpoint. pub fn deployment_id_for_endpoint(endpoint: &str) -> String { - let mut hasher = DefaultHasher::new(); - endpoint.hash(&mut hasher); - format!("{:016x}", hasher.finish()) + Uuid::new_v5(&Uuid::NAMESPACE_URL, canonical_endpoint(endpoint).as_bytes()).to_string() } pub fn same_identity_endpoint(left: &str, right: &str) -> bool { @@ -174,6 +176,23 @@ mod tests { } } + /// B8 red-light: the fallback deployment ID must be a toolchain-stable + /// UUIDv5 over the canonical endpoint — `DefaultHasher` output is not + /// guaranteed stable across Rust releases, yet the ID is persisted in + /// site-replication state and broadcast to peers. + #[test] + fn deployment_id_for_endpoint_is_stable_uuid_v5_over_canonical_endpoint() { + let endpoint = "https://node-a.example.com:9000"; + let id = deployment_id_for_endpoint(endpoint); + let parsed = uuid::Uuid::parse_str(&id).expect("fallback deployment ID must be a UUID"); + assert_eq!(parsed.get_version_num(), 5, "fallback deployment ID must be UUIDv5"); + // Deterministic for the same endpoint and for spelling variants that + // share a canonical form; distinct endpoints stay distinct. + assert_eq!(id, deployment_id_for_endpoint(endpoint)); + assert_eq!(id, deployment_id_for_endpoint(" HTTPS://Node-A.Example.Com:9000/ ")); + assert_ne!(id, deployment_id_for_endpoint("https://node-b.example.com:9000")); + } + #[test] fn canonical_endpoint_accepts_case_insensitive_scheme() { assert_eq!( diff --git a/rustfs/src/admin/storage_api.rs b/rustfs/src/admin/storage_api.rs index 318718b49..bdf28f15e 100644 --- a/rustfs/src/admin/storage_api.rs +++ b/rustfs/src/admin/storage_api.rs @@ -51,7 +51,7 @@ mod ecstore_disk { } mod ecstore_error { - pub(crate) use crate::storage::storage_api::ecstore_error::StorageError; + pub(crate) use crate::storage::storage_api::ecstore_error::{StorageError, is_err_bucket_not_found}; } #[allow(unused_imports)] @@ -919,6 +919,7 @@ pub(crate) mod contract { } pub(crate) mod error { + pub(crate) use super::ecstore_error::is_err_bucket_not_found; pub(crate) use super::{Error, StorageError}; } From 20d1266496ba50dbec09b2263c504416840c61f7 Mon Sep 17 00:00:00 2001 From: cxymds Date: Sun, 23 Aug 2026 04:24:48 +0800 Subject: [PATCH 2/7] fix(heal): fence format repair during pool transitions (#6342) * fix(heal): fence format repair during pool transitions * fix(heal): fence format writes during transitions --- crates/ecstore/src/core/pools.rs | 2 +- crates/ecstore/src/core/sets.rs | 28 +- crates/ecstore/src/store/heal.rs | 335 +++++++++++++++++- crates/heal/src/heal/task/heal_erasure_set.rs | 11 +- crates/heal/src/heal/task/tests.rs | 28 ++ crates/test-utils/src/lib.rs | 12 + 6 files changed, 402 insertions(+), 14 deletions(-) diff --git a/crates/ecstore/src/core/pools.rs b/crates/ecstore/src/core/pools.rs index 9d8e0d01a..4ca460c20 100644 --- a/crates/ecstore/src/core/pools.rs +++ b/crates/ecstore/src/core/pools.rs @@ -2026,7 +2026,7 @@ impl PoolMeta { self.load_no_lock(pool).await } - async fn load_no_lock(&mut self, pool: Arc) -> Result<()> + pub(crate) async fn load_no_lock(&mut self, pool: Arc) -> Result<()> where S: EcstoreObjectIO, { diff --git a/crates/ecstore/src/core/sets.rs b/crates/ecstore/src/core/sets.rs index d9b354a08..0c79c080f 100644 --- a/crates/ecstore/src/core/sets.rs +++ b/crates/ecstore/src/core/sets.rs @@ -988,14 +988,11 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for Sets { } } -#[async_trait::async_trait] -impl crate::storage_api_contracts::heal::HealOperations for Sets { - type Error = Error; - type HealResultItem = HealResultItem; - type HealOptions = HealOpts; - - #[tracing::instrument(skip(self))] - async fn heal_format(&self, dry_run: bool) -> Result<(HealResultItem, Option)> { +impl Sets { + pub(crate) async fn heal_format_with_fence(&self, dry_run: bool, fence_lost: F) -> Result<(HealResultItem, Option)> + where + F: Fn() -> bool + Send + Sync, + { let (disks, init_errs) = init_storage_disks_with_errors( &self.endpoints.endpoints, &DiskOption { @@ -1068,6 +1065,9 @@ impl crate::storage_api_contracts::heal::HealOperations for Sets { // Save new formats `format.json` on unformatted disks. for (index, (fm, disk)) in tmp_new_formats.iter_mut().zip(disks.iter()).enumerate() { if fm.is_some() && disk.is_some() { + if fence_lost() { + return Ok((res, Some(StorageError::SlowDown))); + } if let Err(err) = save_format_file(disk, fm).await { if let Some(disk) = disk.as_ref() { let _ = disk.close().await; @@ -1101,6 +1101,18 @@ impl crate::storage_api_contracts::heal::HealOperations for Sets { } Ok((res, None)) } +} + +#[async_trait::async_trait] +impl crate::storage_api_contracts::heal::HealOperations for Sets { + type Error = Error; + type HealResultItem = HealResultItem; + type HealOptions = HealOpts; + + #[tracing::instrument(skip(self))] + async fn heal_format(&self, dry_run: bool) -> Result<(HealResultItem, Option)> { + self.heal_format_with_fence(dry_run, || false).await + } #[tracing::instrument(skip(self))] async fn heal_bucket(&self, bucket: &str, opts: &HealOpts) -> Result { let mut result = HealResultItem { diff --git a/crates/ecstore/src/store/heal.rs b/crates/ecstore/src/store/heal.rs index ffac77751..a7aed758d 100644 --- a/crates/ecstore/src/store/heal.rs +++ b/crates/ecstore/src/store/heal.rs @@ -13,7 +13,12 @@ // limitations under the License. use super::*; +use crate::core::pools::POOL_META_NAME; +use crate::services::rebalance::{REBAL_META_NAME, RebalStatus}; +use crate::set_disk::get_lock_acquire_timeout; use crate::storage_api_contracts::heal::HealOperations as _; +use crate::storage_api_contracts::namespace::NamespaceLocking as _; +use rustfs_lock::NamespaceLockGuard; use tracing::trace; const LOG_COMPONENT_ECSTORE: &str = "ecstore"; @@ -30,7 +35,119 @@ fn invalid_heal_pool_index(pool_idx: usize, pool_count: usize) -> Error { ) } +#[derive(Debug, Clone, Copy)] +enum HealFormatPoolSkip { + Completed, + Retryable, +} + +fn classify_heal_format_pool( + pool_idx: usize, + pool_cmd_line: &str, + pool_meta: &PoolMeta, + rebalance_meta: Option<&RebalanceMeta>, +) -> Option { + let Some(pool) = pool_meta.pools.get(pool_idx) else { + return Some(HealFormatPoolSkip::Retryable); + }; + + if pool.id != pool_idx || pool_cmd_line.is_empty() || pool.cmd_line.is_empty() || pool.cmd_line != pool_cmd_line { + return Some(HealFormatPoolSkip::Retryable); + } + + if let Some(decommission) = pool.decommission.as_ref() { + if decommission.complete { + return Some(HealFormatPoolSkip::Completed); + } + if decommission.failed || decommission.canceled || decommission.queued || pool_meta.is_suspended(pool_idx) { + return Some(HealFormatPoolSkip::Retryable); + } + } + + if let Some(meta) = rebalance_meta { + let Some(pool_stats) = meta.pool_stats.get(pool_idx) else { + return Some(HealFormatPoolSkip::Retryable); + }; + if pool_stats.info.stopping || (pool_stats.participating && pool_stats.info.status == RebalStatus::Started) { + return Some(HealFormatPoolSkip::Retryable); + } + } + + None +} + +fn heal_format_pool_skip_error(skip: HealFormatPoolSkip) -> Error { + match skip { + HealFormatPoolSkip::Completed => StorageError::NoHealRequired, + HealFormatPoolSkip::Retryable => StorageError::SlowDown, + } +} + +fn heal_format_fence_lost_error() -> Error { + StorageError::SlowDown +} + impl ECStore { + async fn acquire_heal_format_fence( + &self, + ) -> Result<(NamespaceLockGuard, NamespaceLockGuard, PoolMeta, Option)> { + let metadata_pool = self + .pools + .first() + .cloned() + .ok_or_else(|| Error::other("heal format requires at least one storage pool"))?; + + // Metadata fence order is part of the decommission/rebalance protocol: + // pool.bin must always be acquired before rebalance.bin. + let pool_lock = metadata_pool.new_ns_lock(RUSTFS_META_BUCKET, POOL_META_NAME).await?; + let pool_guard = pool_lock.get_write_lock(get_lock_acquire_timeout()).await?; + let rebalance_lock = metadata_pool.new_ns_lock(RUSTFS_META_BUCKET, REBAL_META_NAME).await?; + let rebalance_guard = rebalance_lock.get_write_lock(get_lock_acquire_timeout()).await?; + + if pool_guard.is_lock_lost() || rebalance_guard.is_lock_lost() { + return Err(heal_format_fence_lost_error()); + } + + let mut pool_meta = PoolMeta::default(); + pool_meta.load_no_lock(metadata_pool.clone()).await?; + if pool_meta.pools.len() != self.pools.len() + || pool_meta.pools.iter().enumerate().any(|(pool_idx, pool)| { + pool.id != pool_idx || pool.cmd_line.is_empty() || pool.cmd_line != self.pools[pool_idx].endpoints.cmd_line + }) + { + return Err(heal_format_fence_lost_error()); + } + + let mut rebalance_meta = RebalanceMeta::new(); + let rebalance_meta = match rebalance_meta + .load_with_opts( + metadata_pool, + ObjectOptions { + no_lock: true, + ..Default::default() + }, + ) + .await + { + Ok(()) => Some(rebalance_meta), + Err(Error::ConfigNotFound) => None, + Err(err) => return Err(err), + }; + + if rebalance_meta + .as_ref() + .is_some_and(|meta| meta.pool_stats.len() != self.pools.len()) + { + return Err(heal_format_fence_lost_error()); + } + + if pool_guard.is_lock_lost() || rebalance_guard.is_lock_lost() { + return Err(heal_format_fence_lost_error()); + } + + Ok((pool_guard, rebalance_guard, pool_meta, rebalance_meta)) + } + fn get_pools_for_heal_object(&self, opts: &HealOpts) -> Result>> { match opts.pool { Some(pool_idx) => Ok(vec![ @@ -52,9 +169,26 @@ impl ECStore { }; let mut count_no_heal = 0; + let mut count_completed = 0; let mut first_error = None; - for pool in self.pools.iter() { - let (mut result, err) = pool.heal_format(dry_run).await?; + for (pool_idx, pool) in self.pools.iter().enumerate() { + let (pool_guard, rebalance_guard, pool_meta, rebalance_meta) = self.acquire_heal_format_fence().await?; + if pool_guard.is_lock_lost() || rebalance_guard.is_lock_lost() { + first_error.get_or_insert(heal_format_fence_lost_error()); + break; + } + if let Some(skip) = classify_heal_format_pool(pool_idx, &pool.endpoints.cmd_line, &pool_meta, rebalance_meta.as_ref()) + { + if matches!(skip, HealFormatPoolSkip::Completed) { + count_completed += 1; + } else { + first_error.get_or_insert(heal_format_pool_skip_error(skip)); + } + continue; + } + + let fence_lost = || pool_guard.is_lock_lost() || rebalance_guard.is_lock_lost(); + let (mut result, err) = pool.heal_format_with_fence(dry_run, fence_lost).await?; if let Some(err) = err { match err { StorageError::NoHealRequired => { @@ -69,11 +203,18 @@ impl ECStore { r.set_count += result.set_count; r.before.drives.append(&mut result.before.drives); r.after.drives.append(&mut result.after.drives); + + // A lease can be lost after the final write; fail closed before + // reporting the pool as successfully healed. + if pool_guard.is_lock_lost() || rebalance_guard.is_lock_lost() { + first_error.get_or_insert(heal_format_fence_lost_error()); + break; + } } if let Some(err) = first_error { return Ok((r, Some(err))); } - if count_no_heal == self.pools.len() { + if count_no_heal + count_completed == self.pools.len() { info!( event = EVENT_HEAL_FORMAT_COMPLETED, component = LOG_COMPONENT_ECSTORE, @@ -302,6 +443,7 @@ mod tests { use crate::disk::{DeleteOptions, DiskOption, format::FormatV3, new_disk}; use crate::layout::endpoints::{EndpointServerPools, Endpoints, PoolEndpoints}; use crate::runtime::instance::InstanceContext; + use crate::services::rebalance::{RebalanceInfo, RebalanceStats}; use crate::storage_api_contracts::bucket::{BucketOperations, MakeBucketOptions}; use crate::storage_api_contracts::object::{ObjectIO as _, ObjectOperations}; use crate::store::init_format::{load_format_erasure, save_format_file}; @@ -353,6 +495,164 @@ mod tests { } } + fn pool_meta_with_decommission(info: PoolDecommissionInfo) -> PoolMeta { + PoolMeta { + pools: vec![PoolStatus { + id: 0, + cmd_line: "pool-0".to_string(), + last_update: OffsetDateTime::UNIX_EPOCH, + decommission: Some(info), + }], + ..Default::default() + } + } + + #[test] + fn heal_format_pool_state_barriers_are_classified() { + let active = pool_meta_with_decommission(PoolDecommissionInfo { + start_time: Some(OffsetDateTime::UNIX_EPOCH), + ..Default::default() + }); + assert!(matches!( + classify_heal_format_pool(0, "pool-0", &active, None), + Some(HealFormatPoolSkip::Retryable) + )); + + for info in [ + PoolDecommissionInfo { + failed: true, + ..Default::default() + }, + PoolDecommissionInfo { + canceled: true, + ..Default::default() + }, + ] { + assert!(matches!( + classify_heal_format_pool(0, "pool-0", &pool_meta_with_decommission(info), None), + Some(HealFormatPoolSkip::Retryable) + )); + } + + let completed = pool_meta_with_decommission(PoolDecommissionInfo { + complete: true, + ..Default::default() + }); + assert!(matches!( + classify_heal_format_pool(0, "pool-0", &completed, None), + Some(HealFormatPoolSkip::Completed) + )); + } + + #[test] + fn heal_format_pool_rebalance_barriers_and_identity_are_fail_closed() { + let identity_meta = pool_meta_with_decommission(PoolDecommissionInfo::default()); + let rebalance = RebalanceMeta { + pool_stats: vec![RebalanceStats { + participating: true, + info: RebalanceInfo { + status: RebalStatus::Started, + ..Default::default() + }, + ..Default::default() + }], + ..Default::default() + }; + assert!(matches!( + classify_heal_format_pool(0, "pool-0", &identity_meta, Some(&rebalance)), + Some(HealFormatPoolSkip::Retryable) + )); + + let stopping = RebalanceMeta { + pool_stats: vec![RebalanceStats { + info: RebalanceInfo { + stopping: true, + ..Default::default() + }, + ..Default::default() + }], + ..Default::default() + }; + assert!(matches!( + classify_heal_format_pool(0, "pool-0", &identity_meta, Some(&stopping)), + Some(HealFormatPoolSkip::Retryable) + )); + + let identity = pool_meta_with_decommission(PoolDecommissionInfo::default()); + assert!(matches!( + classify_heal_format_pool(0, "pool-new", &identity, None), + Some(HealFormatPoolSkip::Retryable) + )); + + let identity_without_decommission = PoolMeta { + pools: vec![PoolStatus { + id: 0, + cmd_line: "pool-0".to_string(), + last_update: OffsetDateTime::UNIX_EPOCH, + decommission: None, + }], + ..Default::default() + }; + assert!(matches!( + classify_heal_format_pool(0, "pool-new", &identity_without_decommission, None), + Some(HealFormatPoolSkip::Retryable) + )); + + assert!(matches!( + classify_heal_format_pool(0, "", &identity_meta, None), + Some(HealFormatPoolSkip::Retryable) + )); + + assert!(matches!( + classify_heal_format_pool(0, "pool-0", &PoolMeta::default(), None), + Some(HealFormatPoolSkip::Retryable) + )); + + let stopped = RebalanceMeta { + stopped_at: Some(OffsetDateTime::UNIX_EPOCH), + pool_stats: vec![RebalanceStats { + participating: true, + info: RebalanceInfo { + status: RebalStatus::Stopped, + ..Default::default() + }, + ..Default::default() + }], + ..Default::default() + }; + assert!(classify_heal_format_pool(0, "pool-0", &identity_meta, Some(&stopped)).is_none()); + + let stopping_after_stop = RebalanceMeta { + stopped_at: Some(OffsetDateTime::UNIX_EPOCH), + pool_stats: vec![RebalanceStats { + participating: true, + info: RebalanceInfo { + status: RebalStatus::Started, + stopping: true, + ..Default::default() + }, + ..Default::default() + }], + ..Default::default() + }; + assert!(matches!( + classify_heal_format_pool(0, "pool-0", &identity_meta, Some(&stopping_after_stop)), + Some(HealFormatPoolSkip::Retryable) + )); + } + + #[test] + fn skipped_heal_format_pool_is_never_reported_as_success() { + assert!(matches!( + heal_format_pool_skip_error(HealFormatPoolSkip::Retryable), + StorageError::SlowDown + )); + assert!(matches!( + heal_format_pool_skip_error(HealFormatPoolSkip::Completed), + StorageError::NoHealRequired + )); + } + async fn multi_pool_heal_store() -> (tempfile::TempDir, Arc, CancellationToken) { let temp_dir = tempfile::tempdir().expect("multi-pool heal test directory should be created"); let mut pool_endpoints = Vec::new(); @@ -889,6 +1189,18 @@ mod tests { bucket_fence_registry: std::sync::Arc::default(), }; + let err = store + .handle_heal_format(false) + .await + .expect_err("missing pool metadata must fail closed before format writes"); + assert!(matches!(err, StorageError::SlowDown)); + + let pool_meta = PoolMeta::new(&store.pools, &PoolMeta::default()); + pool_meta + .save(store.pools.clone()) + .await + .expect("pool metadata should be persisted before format heal"); + let (result, err) = store .handle_heal_format(false) .await @@ -902,5 +1214,22 @@ mod tests { .await .expect("the later pool should be healed despite the first pool error"); assert_eq!(healed.erasure.this, recoverable_format.erasure.sets[0][2]); + + let mut completed_meta = PoolMeta::new(&store.pools, &PoolMeta::default()); + for status in &mut completed_meta.pools { + status.decommission = Some(PoolDecommissionInfo { + complete: true, + ..Default::default() + }); + } + completed_meta + .save(store.pools.clone()) + .await + .expect("completed pool metadata should be persisted"); + let (_, err) = store + .handle_heal_format(false) + .await + .expect("completed pools should be reported as a no-op"); + assert!(matches!(err, Some(StorageError::NoHealRequired))); } } diff --git a/crates/heal/src/heal/task/heal_erasure_set.rs b/crates/heal/src/heal/task/heal_erasure_set.rs index e0cfe5a90..7b25bcba2 100644 --- a/crates/heal/src/heal/task/heal_erasure_set.rs +++ b/crates/heal/src/heal/task/heal_erasure_set.rs @@ -231,6 +231,10 @@ impl HealTask { "Heal erasure set format repair skipped because no format heal was required" ); } else { + let error = e; + if error.is_recoverable_heal() { + return Err(error); + } error!( target: "rustfs::heal::task", event = EVENT_HEAL_ERASURE_SET_RESULT, @@ -239,7 +243,7 @@ impl HealTask { task_id = %self.id, set_disk_id, result = "format_failed", - error = %e, + error = %error, "Heal erasure set failed" ); { @@ -247,7 +251,7 @@ impl HealTask { progress.update_progress(4, 4, 0, 0); } return Err(Error::TaskExecutionFailed { - message: format!("Failed to heal disk format for {set_disk_id}: {e}"), + message: format!("Failed to heal disk format for {set_disk_id}: {error}"), }); } } else { @@ -284,6 +288,9 @@ impl HealTask { Err(Error::TaskCancelled) => return Err(Error::TaskCancelled), Err(Error::TaskTimeout) => return Err(Error::TaskTimeout), Err(e) => { + if e.is_recoverable_heal() { + return Err(e); + } error!( target: "rustfs::heal::task", event = EVENT_HEAL_ERASURE_SET_RESULT, diff --git a/crates/heal/src/heal/task/tests.rs b/crates/heal/src/heal/task/tests.rs index 464ff3606..f2b442205 100644 --- a/crates/heal/src/heal/task/tests.rs +++ b/crates/heal/src/heal/task/tests.rs @@ -547,6 +547,7 @@ struct MockStorage { heal_object_outcome: Mutex>, heal_object_outcomes: Mutex>>, format_no_heal_required: Mutex, + format_error: Mutex>, global_format_calls: Mutex, replacement_format_calls: Mutex)>>, replacement_targets_ready: Mutex, @@ -867,6 +868,9 @@ impl HealStorageAPI for MockStorage { async fn heal_format(&self, _dry_run: bool) -> Result<(HealResultItem, Option)> { *self.global_format_calls.lock().unwrap() += 1; + if let Some(error) = self.format_error.lock().unwrap().take() { + return Err(error); + } let no_heal_required = *self.format_no_heal_required.lock().unwrap(); if no_heal_required { Ok((HealResultItem::default(), Some(Error::Storage(EcstoreError::NoHealRequired)))) @@ -2052,6 +2056,30 @@ async fn test_erasure_set_heal_continues_after_format_no_heal_required() { ); } +#[tokio::test] +async fn erasure_set_format_slowdown_is_propagated() { + let storage = Arc::new(MockStorage { + format_error: Mutex::new(Some(Error::Storage(EcstoreError::SlowDown))), + ..Default::default() + }); + let request = HealRequest::new( + HealType::ErasureSet { + buckets: Vec::new(), + set_disk_id: "pool_0_set_0".to_string(), + }, + HealOptions::default(), + HealPriority::Normal, + ); + let task = HealTask::from_request(request, storage); + + let error = task + .execute() + .await + .expect_err("format SlowDown must remain recoverable for the task manager"); + + assert!(matches!(error, Error::Storage(EcstoreError::SlowDown))); +} + #[tokio::test] async fn erasure_set_bucket_prepass_failure_stops_before_object_heal() { let temp = TempDir::new().expect("temporary directory should be created"); diff --git a/crates/test-utils/src/lib.rs b/crates/test-utils/src/lib.rs index 59b4f3e0c..0188c7064 100644 --- a/crates/test-utils/src/lib.rs +++ b/crates/test-utils/src/lib.rs @@ -245,6 +245,18 @@ impl TestECStoreEnvBuilder { .await .expect("build test ECStore"); + // The production bootstrap only persists pool.bin from the elected + // first cluster node. Test stores intentionally have no cluster + // election, but heal-format still requires that durable fence before + // it can write any disk format. Materialize the validated topology + // here so the shared fixture models a ready single-node store. + let mut pool_meta = ecstore.pool_meta.read().await.clone(); + pool_meta.dont_save = false; + pool_meta + .save(ecstore.pools.clone()) + .await + .expect("persist test pool metadata"); + if self.init_bucket_metadata { let buckets_list = ecstore .list_bucket(&BucketOptions { From 84eb5aebefcd5295e1408bc58db9bee357dc631a Mon Sep 17 00:00:00 2001 From: houseme Date: Sun, 23 Aug 2026 12:07:20 +0800 Subject: [PATCH 3/7] fix(ecstore): remove inline write debug noise (#6408) * fix(ecstore): remove inline write debug noise Co-Authored-By: heihutu * fix(ecstore): satisfy warning-as-error lints Co-Authored-By: heihutu --------- Co-authored-by: heihutu --- crates/ecstore/src/set_disk/ops/object.rs | 15 +-------------- crates/ecstore/src/store/object.rs | 2 +- crates/ecstore/src/store/rebalance/support.rs | 2 +- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/crates/ecstore/src/set_disk/ops/object.rs b/crates/ecstore/src/set_disk/ops/object.rs index 8f07a7ff7..bb3c9fbf9 100644 --- a/crates/ecstore/src/set_disk/ops/object.rs +++ b/crates/ecstore/src/set_disk/ops/object.rs @@ -2124,26 +2124,13 @@ impl SetDisks { let put_object_size = known_put_object_storage_size(data.size()); let shard_file_size_raw = erasure.shard_file_size(put_object_size); - let is_inline_buffer = - storage_class_config.should_inline(shard_file_size_raw, erasure.data_shards, opts.versioned); + let is_inline_buffer = storage_class_config.should_inline(shard_file_size_raw, erasure.data_shards, opts.versioned); let collect_stage_timing = rustfs_io_metrics::put_stage_metrics_enabled() || issue3031_diag_enabled(); let shard_file_size = shard_file_size_raw; let shard_size = erasure.shard_size(); let write_path = classify_put_write_path(is_inline_buffer, put_object_size, fi.erasure.block_size); let direct_inline_commit = matches!(write_path, SmallWritePath::Inline); - { - use std::io::Write; - let msg = format!( - "INLINE_DEBUG: bucket={} obj={} size={} shard_fs={} ds={} bs={} inline={} direct={} path={} iblock={} ver={}\n", - bucket, object, put_object_size, shard_file_size_raw, erasure.data_shards, fi.erasure.block_size, - is_inline_buffer, direct_inline_commit, write_path.metric_label(), storage_class_config.inline_block(), opts.versioned - ); - if let Ok(mut f) = std::fs::OpenOptions::new().create(true).append(true).open("/tmp/rustfs_inline_debug.log") { - let _ = f.write_all(msg.as_bytes()); - } - let _ = std::io::stderr().write_all(msg.as_bytes()); - } rustfs_io_metrics::record_put_object_path(write_path.metric_label()); let writer_setup_stage_start = collect_stage_timing.then(Instant::now); let (mut writers, errors) = if direct_inline_commit { diff --git a/crates/ecstore/src/store/object.rs b/crates/ecstore/src/store/object.rs index 79a8232f7..7a8653415 100644 --- a/crates/ecstore/src/store/object.rs +++ b/crates/ecstore/src/store/object.rs @@ -3194,7 +3194,7 @@ impl ECStore { // Default return value let mut del_objects = vec![DeletedObject::default(); objects.len()]; - let mut accounting = vec![None; objects.len()]; + let accounting = vec![None; objects.len()]; let mut del_errs = Vec::with_capacity(objects.len()); for _ in 0..objects.len() { diff --git a/crates/ecstore/src/store/rebalance/support.rs b/crates/ecstore/src/store/rebalance/support.rs index 6e4db41b8..434f9c29d 100644 --- a/crates/ecstore/src/store/rebalance/support.rs +++ b/crates/ecstore/src/store/rebalance/support.rs @@ -271,7 +271,7 @@ pub(super) fn resolve_latest_object_info_candidates( .filter(|candidate| latest_candidate_mod_time(candidate) == Some(latest_mod_time)) .collect::>(); - latest_candidates.sort_by(|left, right| right.idx.cmp(&left.idx)); + latest_candidates.sort_by_key(|candidate| std::cmp::Reverse(candidate.idx)); let Some(winner) = latest_candidates.first() else { return Err(Error::ErasureReadQuorum); From 648d5166e24b7afac72f3dc6e36b836744941d72 Mon Sep 17 00:00:00 2001 From: houseme Date: Sun, 23 Aug 2026 12:07:25 +0800 Subject: [PATCH 4/7] feat(allocator): replace mimalloc/libmimalloc-sys with rustfs-mimalloc/rustfs-mimalloc-sys (#6404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the upstream xonatius/mimalloc_rust.git fork (mimalloc + libmimalloc-sys) with the published rustfs-mimalloc (v0.5.0) and rustfs-mimalloc-sys (v0.5.0) crates from crates.io. The new crates are based on mimalloc V3 (v3.5.0) and provide: - MiMalloc global allocator with safe API (collect, stats_json, process_info) - Heap management and arena operations (heap module) - Full FFI bindings to mimalloc V3 Changes: - Workspace deps: mimalloc + libmimalloc-sys (git) → rustfs-mimalloc + rustfs-mimalloc-sys (crates.io) - allocator_reclaim.rs: libmimalloc_sys::mi_collect → rustfs_mimalloc::MiMalloc::collect - memory_observability.rs: raw FFI mi_stats_get_json → MiMalloc::stats_json() - main.rs: heap ownership tests use Heap::contains() (V3 API) - deny.toml: remove xonatius/mimalloc_rust.git from allow-git Co-authored-by: heihutu --- Cargo.lock | 49 ++++++++++++-------------- Cargo.toml | 4 +-- deny.toml | 3 -- rustfs/Cargo.toml | 4 +-- rustfs/src/allocator_reclaim.rs | 8 +---- rustfs/src/main.rs | 18 +++++----- rustfs/src/memory_observability.rs | 55 +++++++++++------------------- 7 files changed, 56 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b7b1ec220..517983884 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1858,9 +1858,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.4.3" +version = "1.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "509591b7bcd67f4ef775afad7662703b4935daaa6ec0e5605cfb1090b32a2b6d" +checksum = "0ad534f4357a5264cce5019c989cf66a4f0dc4e0d1b1d15f8aacec0ff7360273" dependencies = [ "find-msvc-tools", "jobserver", @@ -2522,12 +2522,6 @@ dependencies = [ "subtle", ] -[[package]] -name = "cty" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b365fabc795046672053e29c954733ec3b05e4be654ab130fe8f1f94d7051f35" - [[package]] name = "curve25519-dalek" version = "4.1.3" @@ -5988,15 +5982,6 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6d2cec3eae94f9f509c767b45932f1ada8350c4bdb85af2fcab4a3c14807981" -[[package]] -name = "libmimalloc-sys" -version = "0.1.49" -source = "git+https://github.com/xonatius/mimalloc_rust.git?rev=6d4c41bb10c6d9da1d1b6f07b38c4cc051667f11#6d4c41bb10c6d9da1d1b6f07b38c4cc051667f11" -dependencies = [ - "cc", - "cty", -] - [[package]] name = "libredox" version = "0.1.20" @@ -6397,14 +6382,6 @@ dependencies = [ "synstructure 0.13.2", ] -[[package]] -name = "mimalloc" -version = "0.1.52" -source = "git+https://github.com/xonatius/mimalloc_rust.git?rev=6d4c41bb10c6d9da1d1b6f07b38c4cc051667f11#6d4c41bb10c6d9da1d1b6f07b38c4cc051667f11" -dependencies = [ - "libmimalloc-sys", -] - [[package]] name = "mime" version = "0.3.17" @@ -9162,13 +9139,11 @@ dependencies = [ "insta", "jiff", "libc", - "libmimalloc-sys", "libsystemd", "matchit 0.9.2", "md-5 0.11.0", "metrics", "metrics-util", - "mimalloc", "mime_guess", "opentelemetry", "opentelemetry_sdk", @@ -9204,6 +9179,8 @@ dependencies = [ "rustfs-lock", "rustfs-log-analyzer", "rustfs-madmin", + "rustfs-mimalloc", + "rustfs-mimalloc-sys", "rustfs-notify", "rustfs-object-capacity", "rustfs-object-data-cache", @@ -9875,6 +9852,24 @@ dependencies = [ "tokio", ] +[[package]] +name = "rustfs-mimalloc" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a406f4aa07084301d485beec873af6dccc8e3f8762da244743df92038b1db1a6" +dependencies = [ + "rustfs-mimalloc-sys", +] + +[[package]] +name = "rustfs-mimalloc-sys" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3051b819175f58445d4c369a72f0ab88149f3885ba8bea2aff3be01f53fe7cd" +dependencies = [ + "cc", +] + [[package]] name = "rustfs-notify" version = "1.0.0-rc.3" diff --git a/Cargo.toml b/Cargo.toml index 918af3cce..717a0c6e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -350,8 +350,8 @@ russh-sftp = "2.4.0" dav-server = "0.11.0" # Performance Analysis and Memory Profiling -mimalloc = { version = "0.1.52", git = "https://github.com/xonatius/mimalloc_rust.git", rev = "6d4c41bb10c6d9da1d1b6f07b38c4cc051667f11" } -libmimalloc-sys = { version = "0.1.49", git = "https://github.com/xonatius/mimalloc_rust.git", rev = "6d4c41bb10c6d9da1d1b6f07b38c4cc051667f11", features = ["extended"] } +rustfs-mimalloc = { version = "0.5.0" } +rustfs-mimalloc-sys = { version = "0.5.0" } hotpath = { version = "0.23.3", default-features = false } # Snapshot testing for output format regression detection insta = { version = "1.48" } diff --git a/deny.toml b/deny.toml index 296b229e4..c6fb8facc 100644 --- a/deny.toml +++ b/deny.toml @@ -43,9 +43,6 @@ allow-git = [ # RustFS fork carrying presigned expiry and constant-time authentication fixes. # owner: rustfs-maintainers review: 2026-10 "https://github.com/rustfs/s3s.git", - # MiMalloc fork pinned for hotpath allocation counting support. - # owner: houseme review: 2026-10 - "https://github.com/xonatius/mimalloc_rust.git", ] [bans] diff --git a/rustfs/Cargo.toml b/rustfs/Cargo.toml index af5514870..03afb5628 100644 --- a/rustfs/Cargo.toml +++ b/rustfs/Cargo.toml @@ -336,13 +336,13 @@ opentelemetry = { workspace = true } tracing-opentelemetry = { workspace = true } # Data structures hashbrown = { workspace = true, features = ["serde", "rayon"] } -mimalloc = { workspace = true } +rustfs-mimalloc = { workspace = true } [target.'cfg(target_os = "linux")'.dependencies] libsystemd.workspace = true [target.'cfg(not(target_os = "windows"))'.dependencies] -libmimalloc-sys.workspace = true +rustfs-mimalloc-sys.workspace = true [dev-dependencies] uuid = { workspace = true, features = ["v4", "v5", "fast-rng", "macro-diagnostics"] } diff --git a/rustfs/src/allocator_reclaim.rs b/rustfs/src/allocator_reclaim.rs index 0c31390c3..eba1a8948 100644 --- a/rustfs/src/allocator_reclaim.rs +++ b/rustfs/src/allocator_reclaim.rs @@ -369,14 +369,8 @@ pub fn allocator_reclaim_controller_snapshot(ctx: &CancellationToken) -> Allocat } #[cfg(not(target_os = "windows"))] -#[allow(unsafe_code)] fn collect_allocator_memory(force: bool) -> Result<(), String> { - // SAFETY: `mi_collect` is provided by the active global allocator backend - // on this target family. It is explicitly intended to reclaim retained - // pages/segments and does not require additional invariants from the caller. - unsafe { - libmimalloc_sys::mi_collect(force); - } + rustfs_mimalloc::MiMalloc::collect(force); Ok(()) } diff --git a/rustfs/src/main.rs b/rustfs/src/main.rs index 5e9d5075b..7f4bea1d2 100644 --- a/rustfs/src/main.rs +++ b/rustfs/src/main.rs @@ -26,22 +26,22 @@ struct MiMallocAllocator; unsafe impl GlobalAlloc for MiMallocAllocator { unsafe fn alloc(&self, layout: Layout) -> *mut u8 { // SAFETY: the caller upholds GlobalAlloc's contract for layout. - unsafe { mimalloc::MiMalloc.alloc(layout) } + unsafe { rustfs_mimalloc::MiMalloc.alloc(layout) } } unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { // SAFETY: the caller upholds GlobalAlloc's contract for layout. - unsafe { mimalloc::MiMalloc.alloc_zeroed(layout) } + unsafe { rustfs_mimalloc::MiMalloc.alloc_zeroed(layout) } } unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { // SAFETY: ptr and layout came from this allocator and are forwarded unchanged. - unsafe { mimalloc::MiMalloc.dealloc(ptr, layout) } + unsafe { rustfs_mimalloc::MiMalloc.dealloc(ptr, layout) } } unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { // SAFETY: ptr and layout came from this allocator and are forwarded unchanged. - unsafe { mimalloc::MiMalloc.realloc(ptr, layout, new_size) } + unsafe { rustfs_mimalloc::MiMalloc.realloc(ptr, layout, new_size) } } } @@ -51,7 +51,7 @@ static GLOBAL: hotpath::CountingAllocator = hotpath::Counting #[cfg(not(all(feature = "hotpath", feature = "hotpath-alloc")))] #[global_allocator] -static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc; +static GLOBAL: rustfs_mimalloc::MiMalloc = rustfs_mimalloc::MiMalloc; fn main() { let _hotpath_guard = hotpath::HotpathGuardBuilder::new("main").build(); @@ -71,8 +71,9 @@ mod tests { allocation.extend_from_slice(&[7_u8; 64]); assert_eq!(allocation.len(), 64); + let heap = rustfs_mimalloc::heap::Heap::main(); // SAFETY: the live Vec pointer is valid to inspect for heap ownership. - assert!(unsafe { libmimalloc_sys::mi_is_in_heap_region(allocation.as_ptr().cast()) }); + assert!(unsafe { heap.contains(allocation.as_ptr()) }); } #[test] @@ -85,12 +86,13 @@ mod tests { let layout = Layout::from_size_align(32, 8).expect("valid test allocation layout"); let grown_layout = Layout::from_size_align(64, 8).expect("valid grown test allocation layout"); let allocator = super::MiMallocAllocator; + let heap = rustfs_mimalloc::heap::Heap::main(); // SAFETY: The pointer is checked for null before use and later released // through the same allocator with the corresponding layout. let ptr = unsafe { allocator.alloc_zeroed(layout) }; assert!(!ptr.is_null()); - assert!(unsafe { libmimalloc_sys::mi_is_in_heap_region(ptr.cast()) }); + assert!(unsafe { heap.contains(ptr) }); assert!(unsafe { std::slice::from_raw_parts(ptr, 32).iter().all(|byte| *byte == 0) }); // SAFETY: `ptr` was allocated by `allocator` with `layout`; on failure @@ -102,7 +104,7 @@ mod tests { panic!("mimalloc realloc failed in allocator smoke test"); } - assert!(unsafe { libmimalloc_sys::mi_is_in_heap_region(grown_ptr.cast()) }); + assert!(unsafe { heap.contains(grown_ptr) }); // SAFETY: `grown_ptr` was reallocated by `allocator` and is released // with the matching grown layout. unsafe { allocator.dealloc(grown_ptr, grown_layout) }; diff --git a/rustfs/src/memory_observability.rs b/rustfs/src/memory_observability.rs index 3e29d24a0..12bd4b3c5 100644 --- a/rustfs/src/memory_observability.rs +++ b/rustfs/src/memory_observability.rs @@ -17,10 +17,7 @@ use rustfs_io_metrics::{ record_cpu_usage, record_memory_usage, record_process_memory_split, }; use serde::Serialize; -#[cfg(any(test, not(target_os = "windows")))] use serde_json::Value; -#[cfg(not(target_os = "windows"))] -use std::ffi::CStr; use std::path::Path; use std::sync::{Arc, Mutex, OnceLock}; use std::time::Duration; @@ -231,7 +228,18 @@ fn read_cgroup_memory_snapshot() -> Option { read_cgroup_v2().or_else(read_cgroup_v1) } -#[cfg(any(test, not(target_os = "windows")))] +fn read_allocator_memory_snapshot() -> Option { + let json = rustfs_mimalloc::MiMalloc::stats_json(); + if json.is_empty() { + return None; + } + let observation = parse_mimalloc_stats_json(&json)?; + Some(AllocatorMemorySnapshot { + backend: crate::allocator_reclaim::allocator_backend(), + observation, + }) +} + fn numeric_json_value(value: &Value) -> Option { match value { Value::Number(number) => number @@ -242,7 +250,6 @@ fn numeric_json_value(value: &Value) -> Option { } } -#[cfg(any(test, not(target_os = "windows")))] fn numeric_json_field(value: &Value, field: &str) -> Option { match value { Value::Object(fields) => fields @@ -254,7 +261,6 @@ fn numeric_json_field(value: &Value, field: &str) -> Option { } } -#[cfg(any(test, not(target_os = "windows")))] fn mimalloc_stat_field(value: &Value, metric: &str, field: &str) -> Option { match value { Value::Object(fields) => { @@ -271,12 +277,10 @@ fn mimalloc_stat_field(value: &Value, metric: &str, field: &str) -> Option } } -#[cfg(any(test, not(target_os = "windows")))] fn mimalloc_stat_current(value: &Value, metric: &str) -> Option { mimalloc_stat_field(value, metric, "current") } -#[cfg(any(test, not(target_os = "windows")))] fn mimalloc_stat_sum(value: &Value, metrics: &[&str], field: &str) -> Option { metrics .iter() @@ -285,7 +289,6 @@ fn mimalloc_stat_sum(value: &Value, metrics: &[&str], field: &str) -> Option 0) } -#[cfg(any(test, not(target_os = "windows")))] fn parse_mimalloc_stats_json(stats_json: &str) -> Option { let value = serde_json::from_str::(stats_json).ok()?; let malloc_metrics = ["malloc_normal", "malloc_huge"]; @@ -312,33 +315,6 @@ fn parse_mimalloc_stats_json(stats_json: &str) -> Option Option { - // SAFETY: `mi_stats_get_json` returns a null-terminated JSON buffer owned by - // mimalloc when called with a null input buffer. The mimalloc API requires - // freeing that buffer with `mi_free`; parsing finishes before the buffer is freed. - let observation = unsafe { - let stats_ptr = libmimalloc_sys::mi_stats_get_json(0, std::ptr::null_mut()); - if stats_ptr.is_null() { - return None; - } - - let observation = CStr::from_ptr(stats_ptr).to_str().ok().and_then(parse_mimalloc_stats_json); - libmimalloc_sys::mi_free(stats_ptr.cast()); - observation? - }; - Some(AllocatorMemorySnapshot { - backend: crate::allocator_reclaim::allocator_backend(), - observation, - }) -} - -#[cfg(target_os = "windows")] -fn read_allocator_memory_snapshot() -> Option { - None -} - fn configured_memory_observability_interval_secs() -> u64 { rustfs_utils::get_env_u64(ENV_MEMORY_OBSERVABILITY_INTERVAL_SECS, DEFAULT_MEMORY_OBSERVABILITY_INTERVAL_SECS).max(1) } @@ -566,6 +542,13 @@ mod tests { assert_eq!(parse_mimalloc_stats_json(r#"{ "allocator": "unknown" }"#), None); } + #[test] + fn read_allocator_memory_snapshot_uses_mimalloc_stats_json() { + let snapshot = super::read_allocator_memory_snapshot(); + #[cfg(not(target_os = "windows"))] + assert!(snapshot.is_some(), "allocator snapshot should be available on non-Windows"); + } + #[test] fn memory_observability_snapshot_reports_disabled_when_metrics_are_disabled() { let snapshot = build_memory_observability_status_snapshot(false, 15, false); From b6ba89d9e4216deb52fd49a97079623dfaeceddf Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sun, 23 Aug 2026 12:09:06 +0800 Subject: [PATCH 5/7] docs(testing): document CI gate matrix (#6412) --- CLAUDE.md | 3 +- CONTRIBUTING.md | 2 + docs/testing/ci-gates.md | 149 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 docs/testing/ci-gates.md diff --git a/CLAUDE.md b/CLAUDE.md index 5e75fe3cb..59893c48b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,7 +30,8 @@ make build-docker BUILD_OS=ubuntu22.04 - Crate membership: `Cargo.toml` `[workspace].members` - Architecture, layering, crate map: [ARCHITECTURE.md](ARCHITECTURE.md) - Migration guardrails & readiness contracts: [docs/architecture/](docs/architecture/README.md) -- CI gates: `.github/workflows/ci.yml` (source of truth; never copy its steps into docs) +- CI workflow steps: `.github/workflows/`; event, timeout, and required-status + matrix: [docs/testing/ci-gates.md](docs/testing/ci-gates.md) - Test-layer taxonomy, per-layer entry commands, serial/nextest rules, flake policy: [docs/testing/README.md](docs/testing/README.md) - Tier/ILM transition debugging (xl.meta inspection, versionId tracing): diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7882a486e..5842395f4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,6 +70,8 @@ make pre-pr > For the full test-layer taxonomy (unit / ecstore black-box / e2e / s3s-e2e / S3 compatibility / chaos / fuzz / bench), each layer's entry command, the naming conventions the migration gate depends on, and the serial/nextest rules, see [docs/testing/README.md](docs/testing/README.md). +> For the event, timeout, required-status, and local reproduction matrix, see [docs/testing/ci-gates.md](docs/testing/ci-gates.md). + ### 🔒 Automated Pre-commit Hooks #### What `make pre-commit` and `make pre-pr` actually run diff --git a/docs/testing/ci-gates.md b/docs/testing/ci-gates.md new file mode 100644 index 000000000..d87ce1668 --- /dev/null +++ b/docs/testing/ci-gates.md @@ -0,0 +1,149 @@ +# CI gate matrix + +This file is the source of truth for which validation runs on each event, its +configured wall-clock budget, and whether it can block a merge. Test taxonomy, +naming, and nextest serialization rules remain in [README.md](README.md); e2e +membership and counts remain in +[e2e-suite-inventory.md](e2e-suite-inventory.md). + +The distinction between **required** and **report-only** is load-bearing: +a failing job blocks a merge only when its exact check name is present in the +live `main` ruleset. A workflow name, a `merge_group` trigger, or a red PR check +does not make a job required by itself. + +## Required merge checks + +The live `main` ruleset (`6436880`) currently requires exactly these contexts: + +| Required context | Producer | Validation | +|---|---|---| +| `CLA Check` | `.github/workflows/cla.yml` | Contributor agreement | +| `Quick Checks` | `.github/workflows/ci.yml` | Formatting and repository guard scripts | +| `Test and Lint` | `.github/workflows/ci.yml` | Clippy, workspace nextest excluding `e2e_test`, doctests, and migration proofs | + +For pull requests limited to the paths excluded by the main CI workflow, +`.github/workflows/ci-docs-only.yml` reports `Quick Checks` and +`Test and Lint` under the same names. It runs the real quick checks and the +planning-document guard; it does not claim that Rust compilation or runtime +tests ran. Despite the workflow name, these paths also include selected deploy, +workflow, and lock files. + +Verify the live rule rather than trusting this snapshot before changing merge +policy: + +```bash +gh api repos/rustfs/rustfs/rulesets/6436880 \ + --jq '.rules[] | select(.type == "required_status_checks") | .parameters' +``` + +The ruleset currently has `strict_required_status_checks_policy=false`. +`Continuous Integration` accepts `merge_group` events and runs `e2e-full` for +them, but `End-to-End Tests (full merge gate)` is not currently a required +context. Therefore the repository is prepared to test a merge-queue SHA, but +the workflow alone does not prove that every merge passed that lane. + +## Pull request and merge matrix + +Budgets below are job `timeout-minutes`, not typical runtimes. “Report-only” +means the result is visible and actionable but is not in the live required +context list. + +| Event | Validation | Budget | Merge status | Reproduction | +|---|---|---:|---|---| +| PR, non-doc change | `Quick Checks` | 10 min | Required | `make pre-commit` (broader local umbrella) | +| PR, non-doc change | `Test and Lint` | 90 min | Required | `cargo nextest run --profile ci --all --exclude e2e_test` | +| PR, non-doc change | `Typos` | 10 min | Report-only | `typos` | +| PR, non-doc change | `ILM Integration (serial)` | 90 min | Report-only | Use the exact command in `.github/workflows/ci.yml` | +| PR, non-doc change | rio-v2 / swift / sftp test-and-lint variants | 90 min each | Report-only | `cargo nextest run` with the workflow's feature set | +| PR, non-doc change | `Build RustFS Debug Binary` | 30 min | Report-only; prerequisite for black-box lanes | `cargo build -p rustfs --bins` | +| PR, non-doc change | `io_uring Integration (real)` | 30 min | Report-only | `cargo test -p rustfs-ecstore --lib uring_ -- --test-threads=1 --nocapture` | +| PR, non-doc change | `End-to-End Tests` (`e2e-smoke` plus `s3s-e2e`) | 30 min | Report-only | `cargo nextest run --profile e2e-smoke -p e2e_test`; then `./scripts/e2e-run.sh ./target/debug/rustfs ` | +| PR, non-doc change | `S3 Implemented Tests` | 60 min | Report-only | Build `rustfs`, then run `scripts/s3-tests/run.sh` with `DEPLOY_MODE=binary`, `TEST_MODE=single`, and `MAXFAIL=0` | +| PR, non-doc change | `S3 Lifecycle Behavior Tests` | 30 min | Report-only | Use the accelerated scanner environment in `.github/workflows/ci.yml` with `scripts/s3-tests/run.sh` | +| PR touching dependency or workflow inputs | Cargo Deny / Workflow Pin Report / Dependency Review | 20 / 5 / 30 min | Report-only | `cargo deny check`; `scripts/security/check_workflow_pins.sh` | +| PR touching architecture rules or architecture docs | `Architecture Migration Rules` | 10 min | Report-only | `scripts/check_architecture_migration_rules.sh` | +| PR touching Nix or workspace manifests | `Nix Build & Check` | 60 min | Report-only | `nix flake check` | +| PR limited to main-CI-excluded paths | companion `Quick Checks` and `Test and Lint` | 10 min each | Required | `git diff --check`; `make doc-paths-check` when documentation paths changed | +| `merge_group` | Standard CI plus `e2e-full` | 55 min for `e2e-full` | Standard required contexts only; `e2e-full` report-only | `cargo nextest run --profile e2e-full -p e2e_test` | +| Push to `main` | Standard CI plus `e2e-full` | 55 min for `e2e-full` | Post-merge detection | Same as `merge_group` | +| PR touching fuzz inputs or harness paths | Build plus five 60-second fuzz smoke targets | 60 min build; 30 min per target | Report-only | `MAX_TOTAL_TIME=60 ./scripts/fuzz/run.sh` | +| PR touching selected ecstore disk/format paths | `Rename Safety` on Windows | 60 min | Report-only | Run the four `cargo test -p rustfs-ecstore --lib ` commands in `windows-filesystem.yml` on Windows | + +The authoritative e2e filters live in `.config/nextest.toml`; extend a profile +instead of adding a second ad-hoc selector. Before a profile runs, +`scripts/check_test_wiring.py` compares its exact membership to the committed +digest so a silent test drop fails closed. + +## Scheduled and manual validation + +Scheduled lanes are independent fault domains. They do not block a pull +request, but their workflow-local gate can fail the run and scheduled failures +are routed to the shared failure-issue action. The scheduled-validation +watchdog and freshness workflow separately detect incomplete runs and missing +schedules. + +| Cadence (UTC unless noted) | Workflow / validation | Budget | Verdict and artifacts | Reproduction | +|---|---|---:|---|---| +| Daily 02:17 | Fuzz: five nightly corpus targets | 60 min build; 60 min per target | Gate; corpus/crash artifacts, scheduled failure alert | `MAX_TOTAL_TIME= ./scripts/fuzz/run.sh` | +| Daily 03:17 | MinIO interop (EC + SSE read parity) | 40 min | Gate; scheduled failure alert | Dispatch `minio-interop.yml` or follow its pinned Docker fixture steps | +| Daily 04:29 | Replication / cluster-fault / protocol e2e | 45 / 90 / 90 min | Three independent gates; JUnit, membership, and server logs | `cargo nextest run --profile e2e-repl-nightly -p e2e_test`; `--profile e2e-nightly`; `-j 1 --profile e2e-protocols` | +| Daily 06:31 | Warp performance A/B | 180 min | Regression budget gate; A/B summaries and server logs | `bash scripts/run_hotpath_warp_abba.sh --help` | +| Daily 00:07 Asia/Shanghai (16:07 UTC previous day) | Nightly GNU build and Vault lanes | 150 / 90 / 60 min | Build, live Vault, and HA failover gates | Use the commands and pinned Vault images in `nightly-gnu.yml` | +| Daily 03:23 | Security Audit | 20 / 5 min, plus 30 min on PR dependency review | Cargo Deny and workflow-pin gates; scheduled failure alert | `cargo deny check`; `scripts/security/check_workflow_pins.sh` | +| Daily 23:47 | Scheduled Validation Freshness | 10 min | Fails when a critical schedule was never created or is stale | Dispatch `scheduled-validation-freshness.yml` | +| Sunday 00:11 | Full `Continuous Integration` matrix | Per-job budgets above | Weekly variant coverage, including dormant rio-v2 binary/e2e lanes | Dispatch `ci.yml` | +| Sunday 01:13 | Seven-platform build matrix | 150 min per platform | Build/package integrity; scheduled failure alert | Dispatch `build.yml` with an exact platform set | +| Sunday 02:19 | Ceph s3-tests full sweep: single and real four-node, four shards each | 180 min per shard | Compatibility gate; report, JUnit, exact node IDs, and server logs | `scripts/s3-tests/run.sh` against an existing single or distributed target | +| Sunday 06:41 | Mint | 120 min | **Report-only by design**; per-suite PASS/FAIL/NA and raw `log.json` | Reproduce the pinned Docker sequence in `mint.yml` or dispatch it | +| Sunday 07:43 | Workspace line coverage | 120 min | Report-only trend; lcov and JSON retained 90 days | `make coverage` | +| Monthly, day 1 06:37 | Runner Hygiene | 15 min | Validates runner ephemerality; scheduled failure alert | Dispatch `runner-hygiene.yml` | + +Manual `workflow_dispatch` exists for the scheduled workflows above. Manual +runs are debugging evidence and intentionally do not open scheduled-failure +issues. A manual performance run may explicitly allow a known regression; that +override must not be treated as an ordinary passing baseline. + +## Release validation + +Release validation is post-merge and tag-driven; it does not substitute for a +pull-request gate. + +| Event | Validation | Budget | Result | +|---|---|---:|---| +| Push to `main` or weekly schedule | `Build and Release` platform matrix | 150 min per platform | Build artifacts for all selected targets; no release publication on a main push | +| Valid release or preview tag | `Build and Release` plus asset checks | 150 min per platform | Draft release, checksummed assets, and publish step | +| Successful non-preview release-tag build | Docker image build and image scan | 60 min build; 30 min scan | Multi-architecture images plus vulnerability report | +| Successful release-tag build | DEB/RPM packaging | 30 min per architecture | Packages and checksum files uploaded to the release | +| Successful non-preview release-tag build | Helm template test and package | 30 min build; 30 min publish | Versioned chart and repository index | + +Use an exact preview tag for end-to-end release rehearsal. Manual dispatches +are backfill/debug paths and do not prove the automatic `workflow_run` chain. + +## Evidence requirements + +A green check is useful only when it proves the intended behavior ran: + +- Record the exact commit SHA and run URL. +- Separate product failure from runner prerequisites, service readiness, and + cancellation. Repair the precondition, then rerun the exact workload. +- Preserve membership manifests, JUnit, raw compatibility logs, seeds, and + server logs where the workflow provides them. +- For a bug fix or a new fault checker, provide sensitivity evidence: the old + behavior or an intentional mutation must fail the new oracle, and the fixed + behavior must pass it. +- Never promote a report-only lane to required from one green run. Require at + least 14 days and 30 representative pull requests with at least 99% complete + execution, then update the ruleset and this table together. + +## Change checklist + +Update this file in the same pull request when any of these change: + +- workflow triggers, job names, timeouts, or nextest profile ownership; +- required status contexts or strict/merge-queue policy; +- scheduled cadence, alert routing, artifact contract, or local reproduction; +- report-only versus gating semantics. + +Do not copy per-module test counts here. Update +[e2e-suite-inventory.md](e2e-suite-inventory.md) and its enforced membership +digest instead. From 5f7220944622712df4376f301fe4921a0e0e0e70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Sun, 23 Aug 2026 12:29:52 +0800 Subject: [PATCH 6/7] fix(ecstore): keep unknown-size sentinel in create_bitrot_writer (#6380) SSE and compression wrap the payload so its length is unknown and advertise HashReader::SIZE_PRESERVE_LAYER (-1). Every layer preserved that sentinel except create_bitrot_writer, which clamped it to 0 before calling DiskAPI::create_file. RemoteDisk forwards that size verbatim in the put_file_stream query, so remote peers were told the body was empty. Since the authenticated put-file trailer (#5868) the receiver used the declared size to split body from trailer, turning the clamp into a fatal "auth trailer has trailing data" failure for every SSE PUT on multi-node deployments (rc.2). #6320 relaxed the receiver to only trust size > 0; this change fixes the sender so the sentinel survives end to end and the wire no longer conflates empty objects with unknown-length streams. Refs #6331 --- crates/ecstore/src/io_support/bitrot.rs | 44 +++++++++++++++++++++---- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/crates/ecstore/src/io_support/bitrot.rs b/crates/ecstore/src/io_support/bitrot.rs index 05c81c048..88a3da910 100644 --- a/crates/ecstore/src/io_support/bitrot.rs +++ b/crates/ecstore/src/io_support/bitrot.rs @@ -784,6 +784,24 @@ pub(crate) fn create_deferred_bitrot_reader_with_stripe_handle( /// /// # Returns /// A Result containing the BitrotWriterWrapper or an error +/// Size hint handed to `DiskAPI::create_file` for a bitrot-wrapped shard. +/// +/// A known length is grown by one checksum per shard so the on-disk file size +/// matches what the bitrot writer emits. A negative length is the +/// unknown-size sentinel (`HashReader::SIZE_PRESERVE_LAYER`, used by SSE and +/// compression) and must be preserved: `RemoteDisk::create_file` forwards it +/// in the `put_file_stream` query, and the receiver only treats `size > 0` as +/// a fixed body length when locating the authenticated trailer. Clamping it +/// to `0` would claim an empty body and misframe the stream. `0` stays `0` +/// because a genuinely empty object still means an empty body. +fn bitrot_create_file_size(length: i64, shard_size: usize, checksum_algo: &HashAlgorithm) -> i64 { + if length <= 0 { + return length; + } + let length = length as usize; + (length.div_ceil(shard_size) * checksum_algo.size() + length) as i64 +} + pub async fn create_bitrot_writer( is_inline_buffer: bool, disk: Option<&DiskStore>, @@ -796,12 +814,7 @@ pub async fn create_bitrot_writer( let writer = if is_inline_buffer { CustomWriter::new_inline_buffer() } else if let Some(disk) = disk { - let length = if length > 0 { - let length = length as usize; - (length.div_ceil(shard_size) * checksum_algo.size() + length) as i64 - } else { - 0 - }; + let length = bitrot_create_file_size(length, shard_size, &checksum_algo); let file = disk.create_file("", volume, path, length).await?; #[cfg(feature = "hotpath")] @@ -820,6 +833,25 @@ mod tests { use rustfs_rio::ChunkReader; use std::collections::VecDeque; + #[test] + fn bitrot_create_file_size_grows_known_length_by_checksums() { + // 10 bytes over 4-byte shards = 3 shards, each followed by a 32-byte hash. + assert_eq!(bitrot_create_file_size(10, 4, &HashAlgorithm::HighwayHash256), 10 + 3 * 32); + assert_eq!(bitrot_create_file_size(10, 4, &HashAlgorithm::None), 10); + } + + #[test] + fn bitrot_create_file_size_keeps_empty_and_unknown_distinct() { + assert_eq!(bitrot_create_file_size(0, 4, &HashAlgorithm::HighwayHash256), 0); + // SSE/compression streams advertise SIZE_PRESERVE_LAYER (-1); the remote + // put_file_stream receiver relies on a non-positive size to parse the auth + // trailer from the stream tail, so the sentinel must survive untouched. + assert_eq!( + bitrot_create_file_size(rustfs_rio::HashReader::SIZE_PRESERVE_LAYER, 4, &HashAlgorithm::HighwayHash256), + rustfs_rio::HashReader::SIZE_PRESERVE_LAYER + ); + } + struct TestChunkReader { chunks: VecDeque, } From 23a2c7d7766c5162c9cf5a718b8d58313d8d401e Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sun, 23 Aug 2026 12:32:11 +0800 Subject: [PATCH 7/7] test(kms): stabilize Vault failover validation (#6385) * test(kms): bound Vault failover progress wait * ci(nightly): honor manual dispatch ref * test(kms): preserve Vault worker failures * test(kms): validate Vault circuit recovery --- .github/workflows/nightly-gnu.yml | 9 +- crates/kms/tests/vault_ha_failover_live.rs | 111 ++++++++++++++++----- scripts/test/vault_ha_kms_live.sh | 2 +- 3 files changed, 90 insertions(+), 32 deletions(-) diff --git a/.github/workflows/nightly-gnu.yml b/.github/workflows/nightly-gnu.yml index 946761e54..a1f0260e1 100644 --- a/.github/workflows/nightly-gnu.yml +++ b/.github/workflows/nightly-gnu.yml @@ -39,11 +39,10 @@ jobs: env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" steps: - - name: Checkout main branch + - name: Checkout repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 with: persist-credentials: false - ref: main - name: Setup Rust environment uses: ./.github/actions/setup @@ -89,11 +88,10 @@ jobs: # either casing. NO_PROXY: 127.0.0.1,localhost steps: - - name: Checkout main branch + - name: Checkout repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 with: persist-credentials: false - ref: main - name: Setup Rust environment uses: ./.github/actions/setup @@ -178,11 +176,10 @@ jobs: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" NO_PROXY: 127.0.0.1,localhost steps: - - name: Checkout main branch + - name: Checkout repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 with: persist-credentials: false - ref: main - name: Setup Rust environment uses: ./.github/actions/setup diff --git a/crates/kms/tests/vault_ha_failover_live.rs b/crates/kms/tests/vault_ha_failover_live.rs index 197198d5d..13ece1e03 100644 --- a/crates/kms/tests/vault_ha_failover_live.rs +++ b/crates/kms/tests/vault_ha_failover_live.rs @@ -16,14 +16,14 @@ //! //! `scripts/test/vault_ha_kms_live.sh` owns the official Vault containers and //! kills the active node while this test continuously decrypts through a -//! surviving standby. KV2 and Transit requests must remain successful, use a -//! bounded number of attempts, and leave the circuit and in-flight gauges at -//! zero after a new leader is elected. +//! surviving standby. KV2 and Transit must recover after the bounded circuit +//! interval, use a bounded number of attempts, and leave the circuit and +//! in-flight gauges at zero after a new leader is elected. use std::collections::HashMap; use std::path::{Path, PathBuf}; -use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::{Arc, Mutex}; use std::time::Duration; use metrics_util::MetricKind; @@ -43,6 +43,11 @@ const OPERATION_ATTEMPTS: &str = "rustfs_kms_backend_operation_attempts"; const IN_FLIGHT: &str = "rustfs_kms_backend_in_flight"; const CIRCUIT_OPEN: &str = "rustfs_kms_backend_circuit_open"; const MAX_ATTEMPTS: u32 = 10; +const ATTEMPT_TIMEOUT: Duration = Duration::from_secs(2); +const HEALTHY_PROGRESS_TIMEOUT: Duration = Duration::from_secs(20); +// The circuit remains open for 30s after five failed attempts. +const POST_FAILOVER_PROGRESS_TIMEOUT: Duration = Duration::from_secs(35); +const FAILOVER_ERROR_POLL_INTERVAL: Duration = Duration::from_millis(100); type MetricEntry = ( metrics_util::CompositeKey, @@ -64,7 +69,7 @@ fn config(backend: KmsBackend, backend_config: BackendConfig) -> KmsConfig { backend, backend_config, allow_insecure_dev_defaults: true, - timeout: Duration::from_secs(2), + timeout: ATTEMPT_TIMEOUT, retry_attempts: MAX_ATTEMPTS, enable_cache: false, ..KmsConfig::default() @@ -164,14 +169,31 @@ fn retryable_failures(snapshot: &[MetricEntry], operation: &str) -> u64 { .sum() } -async fn wait_for_count(counter: &AtomicU64, minimum: u64, description: &str) { - tokio::time::timeout(Duration::from_secs(20), async { +async fn wait_for_count( + counter: &AtomicU64, + failure: &Mutex>, + minimum: u64, + description: &str, + timeout: Duration, +) { + tokio::time::timeout(timeout, async { while counter.load(Ordering::SeqCst) < minimum { + if let Some(error) = failure.lock().expect("decrypt failure lock poisoned").as_ref() { + panic!( + "{description} worker failed after {} successful decrypts: {error}", + counter.load(Ordering::SeqCst) + ); + } tokio::time::sleep(Duration::from_millis(25)).await; } }) .await - .unwrap_or_else(|_| panic!("timed out waiting for {description}")); + .unwrap_or_else(|_| { + panic!( + "timed out after {timeout:?} waiting for {description}: completed {}, expected {minimum}", + counter.load(Ordering::SeqCst) + ) + }); } async fn wait_for_file(path: &Path, description: &str) { @@ -189,7 +211,8 @@ async fn decrypt_loop( request: DecryptRequest, expected: Vec, completed: Arc, - failed: Arc, + allow_failover_errors: Arc, + failure: Arc>>, stop: CancellationToken, ) { while !stop.is_cancelled() { @@ -197,8 +220,18 @@ async fn decrypt_loop( Ok(response) if response.plaintext == expected => { completed.fetch_add(1, Ordering::SeqCst); } - Ok(_) | Err(_) => { - failed.store(true, Ordering::SeqCst); + Ok(_) => { + *failure.lock().expect("decrypt failure lock poisoned") = + Some("decrypt returned unexpected plaintext".to_string()); + return; + } + Err(rustfs_kms::KmsError::BackendError { .. } | rustfs_kms::KmsError::OperationTimedOut { .. }) + if allow_failover_errors.load(Ordering::SeqCst) => + { + tokio::time::sleep(FAILOVER_ERROR_POLL_INTERVAL).await; + } + Err(error) => { + *failure.lock().expect("decrypt failure lock poisoned") = Some(error.to_string()); return; } } @@ -296,7 +329,9 @@ async fn exercise_failover(snapshotter: &Snapshotter) { ); let stop = CancellationToken::new(); - let failed = Arc::new(AtomicBool::new(false)); + let allow_failover_errors = Arc::new(AtomicBool::new(false)); + let kv2_failure = Arc::new(Mutex::new(None)); + let transit_failure = Arc::new(Mutex::new(None)); let kv2_completed = Arc::new(AtomicU64::new(0)); let transit_completed = Arc::new(AtomicU64::new(0)); let kv2_worker = tokio::spawn(decrypt_loop( @@ -304,7 +339,8 @@ async fn exercise_failover(snapshotter: &Snapshotter) { kv2_request, kv2_data_key.plaintext_key, Arc::clone(&kv2_completed), - Arc::clone(&failed), + Arc::clone(&allow_failover_errors), + Arc::clone(&kv2_failure), stop.clone(), )); let transit_worker = tokio::spawn(decrypt_loop( @@ -312,12 +348,21 @@ async fn exercise_failover(snapshotter: &Snapshotter) { transit_request, transit_data_key.plaintext_key, Arc::clone(&transit_completed), - Arc::clone(&failed), + Arc::clone(&allow_failover_errors), + Arc::clone(&transit_failure), stop.clone(), )); - wait_for_count(&kv2_completed, 2, "two healthy KV2 decrypts").await; - wait_for_count(&transit_completed, 2, "two healthy Transit decrypts").await; + wait_for_count(&kv2_completed, &kv2_failure, 2, "two healthy KV2 decrypts", HEALTHY_PROGRESS_TIMEOUT).await; + wait_for_count( + &transit_completed, + &transit_failure, + 2, + "two healthy Transit decrypts", + HEALTHY_PROGRESS_TIMEOUT, + ) + .await; + allow_failover_errors.store(true, Ordering::SeqCst); std::fs::write(&marker, b"ready").expect("publish failover readiness marker"); wait_for_file(&elected, "the replacement Vault leader").await; @@ -326,18 +371,39 @@ async fn exercise_failover(snapshotter: &Snapshotter) { let kv2_after_election = kv2_completed.load(Ordering::SeqCst) + 2; let transit_after_election = transit_completed.load(Ordering::SeqCst) + 2; - wait_for_count(&kv2_completed, kv2_after_election, "post-failover KV2 decrypts").await; - wait_for_count(&transit_completed, transit_after_election, "post-failover Transit decrypts").await; + wait_for_count( + &kv2_completed, + &kv2_failure, + kv2_after_election, + "post-failover KV2 decrypts", + POST_FAILOVER_PROGRESS_TIMEOUT, + ) + .await; + wait_for_count( + &transit_completed, + &transit_failure, + transit_after_election, + "post-failover Transit decrypts", + POST_FAILOVER_PROGRESS_TIMEOUT, + ) + .await; stop.cancel(); kv2_worker.await.expect("KV2 decrypt worker must join"); transit_worker.await.expect("Transit decrypt worker must join"); - assert!(!failed.load(Ordering::SeqCst), "no decrypt may fail or return different plaintext"); + assert!( + kv2_failure.lock().expect("KV2 failure lock poisoned").is_none(), + "no KV2 decrypt may fail or return different plaintext" + ); + assert!( + transit_failure.lock().expect("Transit failure lock poisoned").is_none(), + "no Transit decrypt may fail or return different plaintext" + ); } #[test] #[ignore = "requires a real three-node Vault Raft cluster; run scripts/test/vault_ha_kms_live.sh"] -fn vault_raft_leader_failure_preserves_kv2_and_transit_decrypts() { +fn vault_raft_leader_failure_recovers_kv2_and_transit_decrypts() { let recorder = DebuggingRecorder::new(); let snapshotter = recorder.snapshotter(); metrics::with_local_recorder(&recorder, || { @@ -349,11 +415,6 @@ fn vault_raft_leader_failure_preserves_kv2_and_transit_decrypts() { }); let snapshot = snapshotter.snapshot().into_vec(); - assert_eq!( - counter_value(&snapshot, OPERATIONS_TOTAL, &[("outcome", "circuit_open")]), - 0, - "a bounded leader election must not open the circuit" - ); assert_eq!( counter_value(&snapshot, OPERATIONS_TOTAL, &[("outcome", "budget_exhausted")]), 0, diff --git a/scripts/test/vault_ha_kms_live.sh b/scripts/test/vault_ha_kms_live.sh index 59b2ff2fe..786874e16 100755 --- a/scripts/test/vault_ha_kms_live.sh +++ b/scripts/test/vault_ha_kms_live.sh @@ -241,7 +241,7 @@ env \ RUSTFS_TEST_VAULT_FAILOVER_MARKER="$MARKER" \ RUSTFS_TEST_VAULT_OLD_LEADER="$OLD_LEADER" \ cargo test -p rustfs-kms --test vault_ha_failover_live \ - vault_raft_leader_failure_preserves_kv2_and_transit_decrypts -- \ + vault_raft_leader_failure_recovers_kv2_and_transit_decrypts -- \ --ignored --nocapture --test-threads=1 & TEST_PID=$!