From e6bf2a464660ac5cd605fda090f6e73f8035efd0 Mon Sep 17 00:00:00 2001 From: houseme Date: Sat, 5 Sep 2026 16:45:45 +0800 Subject: [PATCH 1/6] fix(admin): report partial background heal coverage (#7178) * chore(deps): refresh SDKs and pin clock skew regression coverage Refresh compatible dependencies for Scanner/Heal V2 batch 1 and verify the production S3 retry/signing path with a deterministic clock. Co-Authored-By: heihutu Co-Authored-By: zhi22915 * fix(admin): report partial background heal coverage Refs rustfs/backlog#2035 and rustfs/backlog#2240. Co-Authored-By: heihutu Co-Authored-By: zhi22915 --------- Co-authored-by: heihutu Co-authored-by: zhi22915 --- crates/madmin/src/client.rs | 72 +++++++++++ rustfs/src/admin/handlers/heal.rs | 197 +++++++++++++++++++++++++++--- 2 files changed, 249 insertions(+), 20 deletions(-) diff --git a/crates/madmin/src/client.rs b/crates/madmin/src/client.rs index 83f6c33d7..b49fde280 100644 --- a/crates/madmin/src/client.rs +++ b/crates/madmin/src/client.rs @@ -182,6 +182,9 @@ pub struct BackgroundHealStatus { pub heal_active_tasks: u64, #[serde(default)] pub cluster_status_complete: bool, + /// Missing on older servers; absent coverage or counts mean unknown. + #[serde(default)] + pub coverage: Option, #[serde(default)] pub progress: Option, /// Remaining wire fields (flattened `BackgroundHealInfo` plus the @@ -190,6 +193,22 @@ pub struct BackgroundHealStatus { pub extra: serde_json::Map, } +/// Node coverage of a background heal status snapshot. Counters describe only +/// nodes with usable snapshots; unknown peers may still be running heal work. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct BackgroundHealCoverage { + #[serde(default)] + pub expected: Option, + #[serde(default)] + pub responded: Option, + #[serde(default)] + pub unknown: Option, + /// Stable reason codes; unknown future codes are preserved verbatim. + #[serde(default)] + pub reasons: Vec, +} + /// `GET /v3/scanner/status` response, typed at the fields operators branch /// on; everything else passes through verbatim. #[derive(Debug, Clone, Deserialize)] @@ -630,9 +649,39 @@ mod tests { assert_eq!(status.state, "active"); assert_eq!(status.heal_queue_length, 3); assert!(status.cluster_status_complete); + assert!(status.coverage.is_none(), "legacy payloads have unknown coverage"); assert!(status.extra.contains_key("healOperations"), "unknown nested payloads must pass through"); } + #[test] + fn background_heal_status_missing_coverage_fields_remain_unknown() { + for raw in [json!({"state": "degraded"}), json!({"state": "degraded", "coverage": {}})] { + let status: BackgroundHealStatus = serde_json::from_value(raw).expect("partial legacy payload decodes"); + assert!(!status.cluster_status_complete); + if let Some(coverage) = status.coverage { + assert_eq!(coverage.expected, None); + assert_eq!(coverage.responded, None); + assert_eq!(coverage.unknown, None); + } + } + } + + #[test] + fn background_heal_status_preserves_future_fields_and_reasons() { + let raw = json!({ + "state": "degraded", "clusterStatusComplete": false, + "coverage": {"expected": 3, "responded": 1, "unknown": 2, "reasons": ["future_reason"], "futureCoverage": true}, + "futureStatus": {"value": 7} + }); + let status: BackgroundHealStatus = serde_json::from_value(raw).expect("future additive fields decode"); + assert_eq!(status.extra["futureStatus"]["value"], 7); + let coverage = status.coverage.expect("coverage supplied"); + assert_eq!(coverage.expected, Some(3)); + assert_eq!(coverage.responded, Some(1)); + assert_eq!(coverage.unknown, Some(2)); + assert_eq!(coverage.reasons, ["future_reason"]); + } + #[test] fn scanner_status_defaults_freshness_to_unknown() { let raw = json!({"enabled": true, "freshness": {"state": "stale"}, "metrics": {}}); @@ -721,6 +770,7 @@ mod tests { let status = client.background_heal_status().await.expect("status decodes"); assert_eq!(status.state, "idle"); + assert!(status.coverage.is_none(), "older HTTP responses retain unknown coverage"); let request = server.recorded(); // The server registers this route POST-only; a GET here answers 405. assert_eq!(request.method, "POST"); @@ -728,6 +778,28 @@ mod tests { assert_eq!(request.query, ""); } + #[tokio::test] + async fn background_heal_status_decodes_partial_coverage_over_http() { + let body = r#"{"state":"degraded","healQueueLength":0,"healActiveTasks":0,"clusterStatusComplete":false,"coverage":{"expected":3,"responded":1,"unknown":2,"reasons":["notification_system_unavailable"]},"futureStatus":true}"#; + let server = TestServer::spawn(body, 200).await; + let client = AdminClient::new(&format!("http://{}", server.addr), "ak", "sk").expect("client builds"); + let status = client + .background_heal_status() + .await + .expect("partial status is a successful response"); + assert_eq!(status.state, "degraded"); + assert!(!status.cluster_status_complete); + assert_eq!(status.extra["futureStatus"], true); + let coverage = status.coverage.expect("partial coverage supplied"); + assert_eq!(coverage.expected, Some(3)); + assert_eq!(coverage.responded, Some(1)); + assert_eq!(coverage.unknown, Some(2)); + assert_eq!(coverage.reasons, ["notification_system_unavailable"]); + let request = server.recorded(); + assert_eq!(request.method, "POST"); + assert_eq!(request.query, "", "reading status must not send heal control parameters"); + } + #[tokio::test] async fn http_error_status_maps_to_a_typed_error_with_body() { let server = TestServer::spawn(r#"{"code":"AccessDenied","message":"denied"}"#, 403).await; diff --git a/rustfs/src/admin/handlers/heal.rs b/rustfs/src/admin/handlers/heal.rs index f10ad9123..74a2da980 100644 --- a/rustfs/src/admin/handlers/heal.rs +++ b/rustfs/src/admin/handlers/heal.rs @@ -39,7 +39,7 @@ use rustfs_utils::path::path_join; use s3s::header::{CONTENT_LENGTH, CONTENT_TYPE}; use s3s::{Body, S3Request, S3Response, S3Result, s3_error}; use serde::{Deserialize, Serialize}; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::future::Future; use std::path::PathBuf; use std::sync::Arc; @@ -261,6 +261,7 @@ struct BackgroundHealStatus<'a> { heal_active_tasks: u64, heal_operations: rustfs_heal::HealOperationsSnapshot, cluster_status_complete: bool, + coverage: &'a BackgroundHealCoverage, #[serde(skip_serializing_if = "Option::is_none")] progress: Option, } @@ -300,6 +301,23 @@ fn background_heal_runtime_state( type BackgroundHealProgress = rustfs_heal::HealProgress; +#[derive(Debug, Serialize)] +struct BackgroundHealCoverage { + expected: usize, + responded: usize, + unknown: usize, + reasons: BTreeSet, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize)] +#[serde(rename_all = "snake_case")] +enum BackgroundHealCoverageReason { + NotificationSystemUnavailable, + PeerTopologyIncomplete, + PeerStatusUnsupported, + PeerStatusUnavailable, +} + #[derive(Debug)] struct ClusterHealStatusSnapshot { info: BackgroundHealInfo, @@ -307,6 +325,7 @@ struct ClusterHealStatusSnapshot { operations: rustfs_heal::HealOperationsSnapshot, progress: Option, complete: bool, + coverage: BackgroundHealCoverage, } fn add_priority_counts(total: &mut rustfs_heal::HealPriorityCounts, next: rustfs_heal::HealPriorityCounts) { @@ -338,6 +357,7 @@ fn add_operations(total: &mut rustfs_heal::HealOperationsSnapshot, next: rustfs_ } fn aggregate_cluster_heal_status(snapshots: Vec) -> ClusterHealStatusSnapshot { + let responded = snapshots.len(); let mut info = BackgroundHealInfo::default(); let mut operations = rustfs_heal::HealOperationsSnapshot::default(); let mut progress = Vec::new(); @@ -379,6 +399,12 @@ fn aggregate_cluster_heal_status(snapshots: Vec) -> Clus operations, progress, complete: true, + coverage: BackgroundHealCoverage { + expected: responded, + responded, + unknown: 0, + reasons: BTreeSet::new(), + }, } } @@ -413,12 +439,14 @@ fn merge_peer_heal_statuses( mut snapshots: Vec, peer_statuses: Vec, String>>, expected_nodes: usize, - topology_complete: bool, + coverage_reason: Option, ) -> S3Result { + let mut reasons: BTreeSet<_> = coverage_reason.into_iter().collect(); for peer_status in peer_statuses { match peer_status { Ok(Some(snapshot)) => snapshots.push(snapshot), Ok(None) => { + reasons.insert(BackgroundHealCoverageReason::PeerStatusUnsupported); warn!( event = EVENT_ADMIN_REQUEST_FAILED, component = LOG_COMPONENT_ADMIN_API, @@ -430,6 +458,7 @@ fn merge_peer_heal_statuses( ); } Err(err) => { + reasons.insert(BackgroundHealCoverageReason::PeerStatusUnavailable); warn!( event = EVENT_ADMIN_REQUEST_FAILED, component = LOG_COMPONENT_ADMIN_API, @@ -452,9 +481,12 @@ fn merge_peer_heal_statuses( // so during a reconfiguration the count can equal `expected_nodes` while // the topology is known-incomplete. Counting alone would report a // definitive answer precisely when the membership itself is in doubt. - let complete = topology_complete && snapshots.len() == expected_nodes; + let complete = reasons.is_empty() && snapshots.len() == expected_nodes; let mut status = aggregate_cluster_heal_status(snapshots); status.complete = complete; + status.coverage.expected = expected_nodes; + status.coverage.unknown = expected_nodes.saturating_sub(status.coverage.responded); + status.coverage.reasons = reasons; // A partial answer must never be mistakable for a definitive verdict: an // unreachable peer might be mid-heal, so reporting the reachable nodes' // "idle" (or disabled/uninitialized) as the cluster state would falsely @@ -497,7 +529,12 @@ async fn read_cluster_heal_status( return Ok(aggregate_cluster_heal_status(snapshots)); } let Some(notification_system) = notification_system else { - return Err(cluster_heal_status_unavailable("notification_system_unavailable")); + return merge_peer_heal_statuses( + snapshots, + Vec::new(), + expected_nodes, + Some(BackgroundHealCoverageReason::NotificationSystemUnavailable), + ); }; // An incomplete peer topology (a down member's client slot, a rolling // upgrade) previously failed the whole endpoint here, before any peer was @@ -540,7 +577,12 @@ async fn read_cluster_heal_status( })) .await; - merge_peer_heal_statuses(snapshots, peer_statuses, expected_nodes, topology_complete) + merge_peer_heal_statuses( + snapshots, + peer_statuses, + expected_nodes, + (!topology_complete).then_some(BackgroundHealCoverageReason::PeerTopologyIncomplete), + ) } async fn query_peer_replacement_recovery_status( @@ -1164,6 +1206,7 @@ fn encode_background_heal_status( heal_operations: rustfs_heal::HealOperationsSnapshot, progress: Option, cluster_status_complete: bool, + coverage: &BackgroundHealCoverage, ) -> S3Result> { let status = BackgroundHealStatus { info, @@ -1172,6 +1215,7 @@ fn encode_background_heal_status( heal_active_tasks: heal_operations.active_tasks, heal_operations, cluster_status_complete, + coverage, progress, }; serde_json::to_vec(&status).map_err(|e| { @@ -1461,6 +1505,7 @@ impl Operation for BackgroundHealStatusHandler { cluster_status.operations, cluster_status.progress, cluster_status.complete, + &cluster_status.coverage, )?; info!( event = EVENT_ADMIN_RESPONSE_EMITTED, @@ -1515,13 +1560,14 @@ impl Operation for ReplacementRecoveryStatusHandler { mod tests { use super::extract_heal_init_params; use super::{ - BackgroundHealProgress, HealInitParams, HealResp, HealRuntimeState, aggregate_cluster_heal_status, - aggregate_replacement_recovery_cluster_status, background_heal_runtime_state, build_heal_channel_request, - build_replacement_recovery_status_response, encode_background_heal_status, encode_heal_control_path, - encode_heal_start_success, encode_heal_task_status, execute_after_heal_control_capability, heal_channel_response_items, - heal_channel_response_progress, heal_channel_response_summary, heal_control_response_id, json_response, - map_heal_response, merge_peer_heal_statuses, peer_topology_complete, query_peer_heal_status, - query_peer_replacement_recovery_status, reject_heal_admission, validate_heal_request_mode, validate_heal_target, + BackgroundHealCoverage, BackgroundHealCoverageReason, BackgroundHealProgress, HealInitParams, HealResp, HealRuntimeState, + aggregate_cluster_heal_status, aggregate_replacement_recovery_cluster_status, background_heal_runtime_state, + build_heal_channel_request, build_replacement_recovery_status_response, encode_background_heal_status, + encode_heal_control_path, encode_heal_start_success, encode_heal_task_status, execute_after_heal_control_capability, + heal_channel_response_items, heal_channel_response_progress, heal_channel_response_summary, heal_control_response_id, + json_response, map_heal_response, merge_peer_heal_statuses, peer_topology_complete, query_peer_heal_status, + query_peer_replacement_recovery_status, read_cluster_heal_status, reject_heal_admission, validate_heal_request_mode, + validate_heal_target, }; use crate::storage::rpc::node_service::heal::{ NodeHealProgress, NodeHealStatusSnapshot, NodeReplacementRecoveryStatusSnapshot, encode_node_replacement_recovery_status, @@ -2175,7 +2221,13 @@ mod tests { ..Default::default() }; - let encoded = encode_background_heal_status(&info, HealRuntimeState::Active, operations, None, true) + let coverage = BackgroundHealCoverage { + expected: 1, + responded: 1, + unknown: 0, + reasons: Default::default(), + }; + let encoded = encode_background_heal_status(&info, HealRuntimeState::Active, operations, None, true, &coverage) .expect("background heal info should serialize"); let json: serde_json::Value = serde_json::from_slice(&encoded).expect("json should deserialize"); @@ -2225,6 +2277,12 @@ mod tests { rustfs_heal::HealOperationsSnapshot::default(), Some(progress), true, + &BackgroundHealCoverage { + expected: 1, + responded: 1, + unknown: 0, + reasons: Default::default(), + }, ) .expect("background heal info should serialize"); let json: serde_json::Value = serde_json::from_slice(&encoded).expect("json should deserialize"); @@ -2398,6 +2456,84 @@ mod tests { assert!(peer_topology_complete(1, 0, 0, 1, 0)); } + #[tokio::test] + async fn test_background_heal_status_without_notification_preserves_local_snapshot() { + let initialized = rustfs_heal::heal_runtime_initialized(); + let info = BackgroundHealInfo { + bitrot_start_cycle: 37, + current_scan_mode: HealScanMode::Deep, + ..Default::default() + }; + for expected in [1, 3] { + let status = tokio::time::timeout(Duration::from_secs(1), read_cluster_heal_status(info.clone(), None, expected)) + .await + .expect("local status must not wait for remote peers") + .expect("missing notification must retain the local snapshot"); + assert_eq!(status.info.bitrot_start_cycle, 37); + assert_eq!(status.info.current_scan_mode, HealScanMode::Deep); + assert_eq!(status.complete, expected == 1); + assert_eq!(status.coverage.expected, expected); + assert_eq!(status.coverage.responded, 1); + assert_eq!(status.coverage.unknown, expected - 1); + if expected == 1 { + assert!(status.coverage.reasons.is_empty()); + } else { + assert!(matches!(status.state, HealRuntimeState::Degraded | HealRuntimeState::Active)); + assert_eq!( + status.coverage.reasons, + [BackgroundHealCoverageReason::NotificationSystemUnavailable].into() + ); + } + let encoded = encode_background_heal_status( + &status.info, + status.state, + status.operations, + status.progress, + status.complete, + &status.coverage, + ) + .expect("fallback status must encode"); + let decoded: rustfs_madmin::client::BackgroundHealStatus = + serde_json::from_slice(&encoded).expect("the actual madmin client must decode the server response"); + assert_eq!(decoded.cluster_status_complete, expected == 1); + let coverage = decoded.coverage.expect("new server supplies coverage"); + assert_eq!(coverage.expected, Some(expected)); + assert_eq!(coverage.responded, Some(1)); + assert_eq!(coverage.unknown, Some(expected - 1)); + if expected > 1 { + assert_eq!(coverage.reasons, ["notification_system_unavailable"]); + } + } + assert_eq!( + rustfs_heal::heal_runtime_initialized(), + initialized, + "reading status must not initialize heal" + ); + } + + #[test] + fn test_background_heal_status_coverage_reasons_are_bounded() { + let local = NodeHealStatusSnapshot::for_test(true, true, BackgroundHealInfo::default(), Default::default(), None); + let peers = (0..100) + .map(|index| { + if index % 2 == 0 { + Ok(None) + } else { + Err("peer unavailable".to_owned()) + } + }) + .collect(); + let status = merge_peer_heal_statuses(vec![local], peers, 101, None).expect("local status remains available"); + assert_eq!(status.coverage.responded, 1); + assert_eq!(status.coverage.unknown, 100); + assert_eq!(status.coverage.reasons.len(), 2); + let encoded = serde_json::to_vec(&status.coverage).expect("coverage encodes"); + assert!(encoded.len() < 256, "coverage must not grow with peer failures"); + let decoded: rustfs_madmin::client::BackgroundHealCoverage = + serde_json::from_slice(&encoded).expect("client coverage decodes"); + assert_eq!(decoded.reasons, ["peer_status_unsupported", "peer_status_unavailable"]); + } + #[test] fn test_peer_status_merge_degrades_explicitly_and_never_claims_idle() { let local = || { @@ -2413,15 +2549,20 @@ mod tests { // but the safety property of the previous fail-closed behaviour is // preserved: the partial answer is labelled Degraded, never Idle, so // unknown peer work cannot be mistaken for "nothing is running". - let partial = merge_peer_heal_statuses(vec![local()], vec![Err("peer timeout".to_string())], 2, true) + let partial = merge_peer_heal_statuses(vec![local()], vec![Err("peer timeout".to_string())], 2, None) .expect("an unreachable peer degrades the answer instead of destroying it"); assert!(!partial.complete); assert_eq!(partial.state, HealRuntimeState::Degraded); + assert_eq!(partial.coverage.expected, 2); + assert_eq!(partial.coverage.responded, 1); + assert_eq!(partial.coverage.unknown, 1); + assert_eq!(partial.coverage.reasons, [BackgroundHealCoverageReason::PeerStatusUnavailable].into()); - let older_peer = merge_peer_heal_statuses(vec![local()], vec![Ok(None)], 2, true) + let older_peer = merge_peer_heal_statuses(vec![local()], vec![Ok(None)], 2, None) .expect("an older peer degrades the answer instead of destroying it"); assert!(!older_peer.complete); assert_eq!(older_peer.state, HealRuntimeState::Degraded); + assert_eq!(older_peer.coverage.reasons, [BackgroundHealCoverageReason::PeerStatusUnsupported].into()); let known_active = NodeHealStatusSnapshot::for_test( true, @@ -2433,12 +2574,12 @@ mod tests { }, None, ); - let partial_active = merge_peer_heal_statuses(vec![known_active], vec![Ok(None)], 2, true) + let partial_active = merge_peer_heal_statuses(vec![known_active], vec![Ok(None)], 2, None) .expect("known active work may be reported as an explicit partial status"); assert!(!partial_active.complete); assert_eq!(partial_active.state, HealRuntimeState::Active); - merge_peer_heal_statuses(Vec::new(), vec![Err("peer timeout".to_string())], 2, true) + merge_peer_heal_statuses(Vec::new(), vec![Err("peer timeout".to_string())], 2, None) .expect_err("no snapshot at all still fails closed"); } @@ -2459,12 +2600,22 @@ mod tests { None, ) }; - let full_count_incomplete_topology = merge_peer_heal_statuses(vec![snapshot()], vec![Ok(Some(snapshot()))], 2, false) - .expect("incomplete topology degrades the answer instead of destroying it"); + let full_count_incomplete_topology = merge_peer_heal_statuses( + vec![snapshot()], + vec![Ok(Some(snapshot()))], + 2, + Some(BackgroundHealCoverageReason::PeerTopologyIncomplete), + ) + .expect("incomplete topology degrades the answer instead of destroying it"); assert!(!full_count_incomplete_topology.complete); assert_eq!(full_count_incomplete_topology.state, HealRuntimeState::Degraded); + assert_eq!(full_count_incomplete_topology.coverage.unknown, 0); + assert_eq!( + full_count_incomplete_topology.coverage.reasons, + [BackgroundHealCoverageReason::PeerTopologyIncomplete].into() + ); - let full_count_complete_topology = merge_peer_heal_statuses(vec![snapshot()], vec![Ok(Some(snapshot()))], 2, true) + let full_count_complete_topology = merge_peer_heal_statuses(vec![snapshot()], vec![Ok(Some(snapshot()))], 2, None) .expect("complete topology and full count is a definitive answer"); assert!(full_count_complete_topology.complete); assert_eq!(full_count_complete_topology.state, HealRuntimeState::Idle); @@ -2478,6 +2629,12 @@ mod tests { rustfs_heal::HealOperationsSnapshot::default(), None, false, + &BackgroundHealCoverage { + expected: 2, + responded: 1, + unknown: 1, + reasons: [BackgroundHealCoverageReason::PeerStatusUnavailable].into(), + }, ) .expect("degraded status must serialize"); let json: serde_json::Value = serde_json::from_slice(&encoded).expect("valid json"); From 42c32381b60d09a46e38554a37664352e6c66ec8 Mon Sep 17 00:00:00 2001 From: houseme Date: Sat, 5 Sep 2026 16:45:53 +0800 Subject: [PATCH 2/6] fix(scanner): make reset cleanup safely reentrant (#7180) * chore(deps): refresh SDKs and pin clock skew regression coverage Refresh compatible dependencies for Scanner/Heal V2 batch 1 and verify the production S3 retry/signing path with a deterministic clock. Co-Authored-By: heihutu Co-Authored-By: zhi22915 * fix(scanner): make reset cleanup safely reentrant Refs rustfs/backlog#2264 and rustfs/backlog#2240. Co-Authored-By: heihutu Co-Authored-By: zhi22915 --------- Co-authored-by: heihutu Co-authored-by: zhi22915 --- crates/scanner/src/scanner/cycle_state.rs | 331 ++++++++++++++++--- crates/scanner/src/scanner/leadership.rs | 8 +- crates/scanner/src/scanner/tests.rs | 380 +++++++++++++++++++++- 3 files changed, 664 insertions(+), 55 deletions(-) diff --git a/crates/scanner/src/scanner/cycle_state.rs b/crates/scanner/src/scanner/cycle_state.rs index dc29e3331..6913ed836 100644 --- a/crates/scanner/src/scanner/cycle_state.rs +++ b/crates/scanner/src/scanner/cycle_state.rs @@ -379,6 +379,12 @@ pub(super) fn decode_recovery_marker_for_reset( if !matches!(marker_revision, DataUsageCacheRevision::Etag(_)) { return Err(ScannerError::Other("cycle recovery marker has no object revision".to_string())); } + if let Ok(value) = serde_json::from_slice::(data) + && let Some(state) = value.get("state") + && !matches!(state.as_str(), Some("blocked" | "cleanup-pending")) + { + return Err(ScannerError::Other("cycle recovery marker state is unsupported".to_string())); + } let compat = serde_json::from_slice::(data).ok(); let _schema_version = compat.as_ref().and_then(|marker| marker.schema_version); let primary_revision = compat @@ -406,7 +412,10 @@ pub(super) fn decode_recovery_marker_for_reset( }; let state = match compat.as_ref().and_then(|marker| marker.state.as_deref()) { Some("cleanup-pending") => "cleanup-pending", - _ => "blocked", + Some("blocked") | None => "blocked", + Some(_) => { + return Err(ScannerError::Other("cycle recovery marker state is unsupported".to_string())); + } }; let now = unix_now_secs(); Ok(ScannerCycleRecoveryMarker { @@ -721,17 +730,19 @@ async fn mark_cycle_recovery_cleanup_pending( mut marker: ScannerCycleRecoveryMarker, marker_revision: &DataUsageCacheRevision, expected_epoch: u64, + owns_reset: &(impl Fn() -> bool + Sync), ) -> Result<(ScannerCycleRecoveryMarker, DataUsageCacheRevision), ScannerError> { marker.state = "cleanup-pending".to_string(); marker.last_attempt_at_unix_secs = unix_now_secs(); let bytes = serde_json::to_vec(&marker) .map_err(|err| ScannerError::Other(format!("failed to encode cycle recovery marker: {err}")))?; - let info = save_config_with_publication_admission_for_epoch( + let info = save_reset_config( storeapi.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), bytes, marker_revision.preconditions(), expected_epoch, + owns_reset, ) .await .map_err(|err| ScannerError::Other(format!("failed to mark cycle recovery cleanup pending: {err}")))?; @@ -933,6 +944,7 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< .get_write_lock_quiet(Duration::from_secs(5)) .await .map_err(|err| ScannerError::Other(format!("scanner leader lock is busy: {err}")))?; + let owns_reset = || !guard.is_lock_lost() && !ctx.is_cancelled(); if guard.is_lock_lost() { return Err(ScannerError::Other("scanner leader lock was lost before recovery reset".to_string())); @@ -952,7 +964,27 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< } Err(err) => return Err(ScannerError::Other(format!("failed to read cycle recovery marker: {err}"))), }; - let marker_data = marker_data.ok_or_else(|| ScannerError::Other("scanner cycle recovery marker is absent".to_string()))?; + let Some(marker_data) = marker_data else { + // A delete may commit before its reply is lost. Confirm both durable + // fences before treating a retry without its marker as completed. + let (cycle, epoch, revision) = read_cycle_state_for_usage_reset(storeapi.clone()).await?; + let floor = persisted_usage_floor(storeapi.clone()).await?; + if !matches!(revision, DataUsageCacheRevision::Etag(_)) + || epoch < floor.leader_epoch + || cycle.next < floor.next_cycle + || !owns_reset() + || scanner_publication_admission_for_epoch(storeapi.clone(), reset_epoch) + .await + .is_none() + { + return Err(ScannerError::Other( + "scanner cycle recovery marker is absent without a completed reset fence".to_string(), + )); + } + set_scanner_cycle_recovery_status(recovery_status("healthy", None, false)); + super::notify_scanner_cycle_recovery_wake(); + return Ok(()); + }; let (marker, force_full_rescan) = match serde_json::from_slice::(&marker_data) { Ok(marker) if validate_recovery_marker(&marker).is_ok() => (marker, false), _ => (decode_recovery_marker_for_reset(&marker_data, &marker_revision)?, true), @@ -1026,8 +1058,10 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< } }; if let Some((primary_cycle, primary_epoch)) = primary_state { + verify_cycle_reset_intent(storeapi.clone(), &marker_revision, &owns_reset).await?; let (cleanup_marker, cleanup_marker_revision) = - mark_cycle_recovery_cleanup_pending(storeapi.clone(), marker.clone(), &marker_revision, reset_epoch).await?; + mark_cycle_recovery_cleanup_pending(storeapi.clone(), marker.clone(), &marker_revision, reset_epoch, &owns_reset) + .await?; set_scanner_cycle_recovery_status(recovery_status_from_marker(&cleanup_marker, "cleanup-pending")); let usage_floor = persisted_usage_floor(storeapi.clone()).await?; let fence_epoch = primary_epoch @@ -1047,12 +1081,14 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< "preserved scanner cycle state exceeds the bounded object size".to_string(), )); } - let preserved_info = save_config_with_publication_admission_for_epoch( + verify_cycle_reset_intent(storeapi.clone(), &cleanup_marker_revision, &owns_reset).await?; + let preserved_info = save_reset_config( storeapi.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), preserved_data, primary_revision.preconditions(), reset_epoch, + &owns_reset, ) .await .map_err(|err| { @@ -1072,9 +1108,17 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< "scanner leader lock was lost after fencing newer cycle state".to_string(), )); } - fence_scanner_usage_epoch_with_expected_epoch(&ctx, storeapi.clone(), fence_epoch, Some(reset_epoch), false) - .await - .map_err(|err| ScannerError::Other(format!("failed to fence preserved scanner usage epoch: {err}")))?; + verify_cycle_reset_intent(storeapi.clone(), &cleanup_marker_revision, &owns_reset).await?; + fence_scanner_usage_epoch_with_expected_epoch( + &ctx, + storeapi.clone(), + fence_epoch, + Some(reset_epoch), + false, + &owns_reset, + ) + .await + .map_err(|err| ScannerError::Other(format!("failed to fence preserved scanner usage epoch: {err}")))?; if guard.is_lock_lost() { return Err(ScannerError::Other( "scanner leader lock was lost after fencing newer cycle state".to_string(), @@ -1088,7 +1132,8 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< "scanner cycle state changed before recovery marker cleanup".to_string(), )); } - delete_config_with_publication_admission_for_epoch( + verify_cycle_reset_intent(storeapi.clone(), &cleanup_marker_revision, &owns_reset).await?; + delete_reset_config( storeapi.clone(), RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), @@ -1100,6 +1145,7 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< ..Default::default() }, reset_epoch, + &owns_reset, ) .await .map_err(|err| { @@ -1149,17 +1195,20 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< // Persist the cleanup-pending phase before rewriting the primary. If the // process dies after the rewrite, startup still sees a durable fence and // cannot mistake the partially completed reset for a healthy state. + verify_cycle_reset_intent(storeapi.clone(), &marker_revision, &owns_reset).await?; let (marker, marker_revision) = if marker.state == "cleanup-pending" { (marker, marker_revision) } else { - mark_cycle_recovery_cleanup_pending(storeapi.clone(), marker, &marker_revision, reset_epoch).await? + mark_cycle_recovery_cleanup_pending(storeapi.clone(), marker, &marker_revision, reset_epoch, &owns_reset).await? }; - let rebuilt_info = save_config_with_publication_admission_for_epoch( + verify_cycle_reset_intent(storeapi.clone(), &marker_revision, &owns_reset).await?; + let rebuilt_info = save_reset_config( storeapi.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), data, primary_revision.preconditions(), reset_epoch, + &owns_reset, ) .await .map_err(|err| { @@ -1178,8 +1227,10 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< "scanner leader lock was lost after rebuilding cycle state".to_string(), )); } + verify_cycle_reset_intent(storeapi.clone(), &marker_revision, &owns_reset).await?; if let Err(err) = - fence_scanner_usage_epoch_with_expected_epoch(&ctx, storeapi.clone(), leader_epoch, Some(reset_epoch), false).await + fence_scanner_usage_epoch_with_expected_epoch(&ctx, storeapi.clone(), leader_epoch, Some(reset_epoch), false, &owns_reset) + .await { set_scanner_cycle_recovery_status(ScannerCycleRecoveryStatus { path: DATA_USAGE_BLOOM_NAME_PATH.clone(), @@ -1249,7 +1300,8 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< )); } - if let Err(err) = delete_config_with_publication_admission_for_epoch( + verify_cycle_reset_intent(storeapi.clone(), &marker_revision, &owns_reset).await?; + if let Err(err) = delete_reset_config( storeapi.clone(), RUSTFS_META_BUCKET, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), @@ -1261,6 +1313,7 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< ..Default::default() }, reset_epoch, + &owns_reset, ) .await { @@ -1310,6 +1363,57 @@ pub async fn reset_scanner_cycle_recovery(ctx: CancellationToken, storeapi: Arc< Ok(()) } +async fn verify_cycle_reset_intent( + storeapi: Arc, + expected_revision: &DataUsageCacheRevision, + owns_reset: &(impl Fn() -> bool + Sync), +) -> Result<(), ScannerError> { + let revision = read_config_revision(storeapi, DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()) + .await + .map_err(|err| ScannerError::Other(format!("failed to verify scanner cycle reset intent: {err}")))?; + if &revision != expected_revision { + return Err(ScannerError::Other("scanner cycle reset intent changed".to_string())); + } + if !owns_reset() { + return Err(ScannerError::Other("scanner cycle reset ownership was lost".to_string())); + } + Ok(()) +} + +async fn save_reset_config( + storeapi: Arc, + path: &str, + data: Vec, + preconditions: crate::HTTPPreconditions, + expected_epoch: u64, + owns_reset: &(impl Fn() -> bool + Sync), +) -> Result { + let Some(_admission) = scanner_publication_admission_for_epoch(storeapi.clone(), expected_epoch).await else { + return Err(EcstoreError::other(SCANNER_PUBLICATION_EPOCH_CHANGED)); + }; + if !owns_reset() { + return Err(EcstoreError::other("scanner reset ownership was lost before write")); + } + save_config_with_preconditions(storeapi, path, data, preconditions).await +} + +async fn delete_reset_config( + storeapi: Arc, + bucket: &str, + path: &str, + options: ScannerObjectOptions, + expected_epoch: u64, + owns_reset: &(impl Fn() -> bool + Sync), +) -> Result { + let Some(_admission) = scanner_publication_admission_for_epoch(storeapi.clone(), expected_epoch).await else { + return Err(EcstoreError::other(SCANNER_PUBLICATION_EPOCH_CHANGED)); + }; + if !owns_reset() { + return Err(EcstoreError::other("scanner reset ownership was lost before delete")); + } + storeapi.delete_config_object(bucket, path, options).await +} + fn scanner_usage_state_reset_paths() -> Vec { vec![ DATA_USAGE_OBJ_NAME_PATH.as_str().to_string(), @@ -1333,8 +1437,14 @@ pub(super) async fn read_usage_state_reset_slots( Ok(slots) } -fn usage_state_reset_floor(slots: &[ScannerUsageStateResetSlot]) -> Result { - let mut floor = PersistedUsageFloor::default(); +enum ScannerUsageResetFloor { + Missing, + Trusted(PersistedUsageFloor), + Corrupt, +} + +fn usage_state_reset_floor(slots: &[ScannerUsageStateResetSlot]) -> Result { + let mut floor = None; for slot in slots { let Some(data) = slot.data.as_deref() else { continue; @@ -1342,9 +1452,21 @@ fn usage_state_reset_floor(slots: &[ScannerUsageStateResetSlot]) -> Result(data) else { continue; }; - update_persisted_usage_floor(&mut floor, &usage, &slot.path)?; + if !data_usage_info_has_persisted_baseline_identity(&usage) + && !(slot.path == DATA_USAGE_OBJ_NAME_PATH.as_str() && data_usage_info_is_bootstrap_pending(&usage)) + && legacy_incomplete_usage_fence(data, &usage) + .and_then(|fence| fence.claimable_epoch()) + .is_none() + { + continue; + } + update_persisted_usage_floor(floor.get_or_insert_with(PersistedUsageFloor::default), &usage, &slot.path)?; } - Ok(floor) + Ok(match floor { + Some(floor) => ScannerUsageResetFloor::Trusted(floor), + None if slots.iter().any(|slot| slot.data.is_some()) => ScannerUsageResetFloor::Corrupt, + None => ScannerUsageResetFloor::Missing, + }) } async fn read_cycle_state_for_usage_reset( @@ -1401,11 +1523,12 @@ async fn delete_usage_state_reset_slot( storeapi: Arc, slot: &ScannerUsageStateResetSlot, expected_epoch: u64, + owns_reset: &(impl Fn() -> bool + Sync), ) -> Result { if matches!(slot.revision, DataUsageCacheRevision::Missing) { return Ok(false); } - let delete_result = delete_config_with_publication_admission_for_epoch( + let delete_result = delete_reset_config( storeapi.clone(), RUSTFS_META_BUCKET, &slot.path, @@ -1415,6 +1538,7 @@ async fn delete_usage_state_reset_slot( ..Default::default() }, expected_epoch, + owns_reset, ) .await; match delete_result { @@ -1486,21 +1610,24 @@ pub(super) async fn publish_scanner_usage_bootstrap_primary( expected_publication_epoch: u64, leader_epoch: Option, context: ScannerUsageBootstrapPublishContext, + owns_publication: impl Fn() -> bool + Sync, ) -> Result<(), ScannerError> { async fn inner( storeapi: Arc, expected_revision: &DataUsageCacheRevision, expected_publication_epoch: u64, leader_epoch: Option, + owns_publication: &(impl Fn() -> bool + Sync), ) -> Result<(), ScannerUsageBootstrapPublishError> { let marker = scanner_usage_bootstrap_marker(std::time::SystemTime::now(), leader_epoch); let data = serde_json::to_vec(&marker).map_err(ScannerUsageBootstrapPublishError::Encode)?; - let save_result = save_config_with_publication_admission_for_epoch( + let save_result = save_reset_config( storeapi.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str(), data.clone(), expected_revision.preconditions(), expected_publication_epoch, + owns_publication, ) .await; if save_result @@ -1524,7 +1651,7 @@ pub(super) async fn publish_scanner_usage_bootstrap_primary( }) } - inner(storeapi, expected_revision, expected_publication_epoch, leader_epoch) + inner(storeapi, expected_revision, expected_publication_epoch, leader_epoch, &owns_publication) .await .map_err(|err| err.into_scanner_error(context)) } @@ -1534,32 +1661,108 @@ pub(super) async fn reset_scanner_usage_state_slots_for_full_rebuild( slots: &[ScannerUsageStateResetSlot], expected_epoch: u64, leader_epoch: u64, + owns_reset: impl Fn() -> bool + Sync, ) -> Result, ScannerError> { let mut reset_paths = Vec::new(); let primary = slots .iter() .find(|slot| slot.path == DATA_USAGE_OBJ_NAME_PATH.as_str()) .ok_or_else(|| ScannerError::Other("scanner usage reset primary slot was not inspected".to_string()))?; - publish_scanner_usage_bootstrap_primary( - storeapi.clone(), - &primary.revision, - expected_epoch, - Some(leader_epoch), - ScannerUsageBootstrapPublishContext::Reset, - ) - .await?; + if !owns_reset() { + return Err(ScannerError::Other("scanner usage reset ownership was lost".to_string())); + } + let resume_epoch = usage_state_reset_resume_epoch(slots)?; + match resume_epoch { + Some(epoch) if epoch == leader_epoch => {} + Some(_) => return Err(ScannerError::Other("scanner usage reset bootstrap epoch changed".to_string())), + None => { + publish_scanner_usage_bootstrap_primary( + storeapi.clone(), + &primary.revision, + expected_epoch, + Some(leader_epoch), + ScannerUsageBootstrapPublishContext::Reset, + &owns_reset, + ) + .await?; + } + } + let (data, intent_revision) = read_config_with_revision(storeapi.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .map_err(|err| ScannerError::Other(format!("failed to inspect scanner usage reset intent: {err}")))?; + data.as_deref() + .and_then(|data| serde_json::from_slice::(data).ok()) + .filter(|usage| data_usage_info_is_bootstrap_pending(usage) && usage.scanner_epoch == Some(leader_epoch)) + .ok_or_else(|| ScannerError::Other("scanner usage reset intent changed before cleanup".to_string()))?; + if !matches!(intent_revision, DataUsageCacheRevision::Etag(_)) + || (resume_epoch.is_some() && intent_revision != primary.revision) + { + return Err(ScannerError::Other("scanner usage reset intent revision changed".to_string())); + } reset_paths.push(DATA_USAGE_OBJ_NAME_PATH.as_str().to_string()); for slot in slots.iter().filter(|slot| slot.path != DATA_USAGE_OBJ_NAME_PATH.as_str()) { - if delete_usage_state_reset_slot(storeapi.clone(), slot, expected_epoch).await? { + if let Some(usage) = slot + .data + .as_deref() + .and_then(|data| serde_json::from_slice::(data).ok()) + && usage_epoch(&usage) >= leader_epoch + { + return Err(ScannerError::Other(format!( + "scanner usage reset slot is not older than its intent: {}", + slot.path + ))); + } + let revision = read_config_revision(storeapi.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .map_err(|err| ScannerError::Other(format!("failed to verify scanner usage reset intent: {err}")))?; + if revision != intent_revision { + return Err(ScannerError::Other("scanner usage reset intent changed during cleanup".to_string())); + } + if !owns_reset() { + return Err(ScannerError::Other("scanner usage reset ownership was lost".to_string())); + } + if delete_usage_state_reset_slot(storeapi.clone(), slot, expected_epoch, &owns_reset).await? { reset_paths.push(slot.path.clone()); } } + let revision = read_config_revision(storeapi.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .map_err(|err| ScannerError::Other(format!("failed to confirm scanner usage reset intent: {err}")))?; + if revision != intent_revision || !owns_reset() { + return Err(ScannerError::Other( + "scanner usage reset intent or ownership changed before completion".to_string(), + )); + } invalidate_admin_data_usage_snapshot_cache().await; invalidate_data_usage_snapshot_cache().await; Ok(reset_paths) } +fn usage_state_reset_resume_epoch(slots: &[ScannerUsageStateResetSlot]) -> Result, ScannerError> { + let primary = slots.iter().find(|slot| slot.path == DATA_USAGE_OBJ_NAME_PATH.as_str()); + let usage = primary + .and_then(|slot| slot.data.as_deref()) + .and_then(|data| serde_json::from_slice::(data).ok()); + match usage { + Some(usage) if usage.usage_snapshot_bootstrap_pending => { + if !data_usage_info_is_bootstrap_pending(&usage) { + return Err(ScannerError::Other("scanner usage reset bootstrap is invalid".to_string())); + } + if usage.scanner_epoch.is_none() { + // Initial bootstrap has no reset owner yet. + return Ok(None); + } + usage + .scanner_epoch + .filter(|epoch| *epoch > 0 && *epoch < u64::MAX) + .map(Some) + .ok_or_else(|| ScannerError::Other("scanner usage reset bootstrap has no valid epoch".to_string())) + } + _ => Ok(None), + } +} + pub async fn reset_scanner_usage_state_for_full_rebuild( ctx: CancellationToken, storeapi: Arc, @@ -1584,12 +1787,31 @@ pub async fn reset_scanner_usage_state_for_full_rebuild( }; let (cycle, cycle_epoch, cycle_revision) = read_cycle_state_for_usage_reset(storeapi.clone()).await?; let slots = read_usage_state_reset_slots(storeapi.clone()).await?; - let usage_floor = usage_state_reset_floor(&slots)?; - let leader_epoch = cycle_epoch - .max(usage_floor.leader_epoch) - .checked_add(1) - .filter(|epoch| *epoch < u64::MAX) - .ok_or_else(|| ScannerError::Other("scanner leader epoch is exhausted".to_string()))?; + let usage_floor = match usage_state_reset_floor(&slots)? { + ScannerUsageResetFloor::Trusted(floor) => floor, + ScannerUsageResetFloor::Corrupt if matches!(cycle_revision, DataUsageCacheRevision::Missing) => { + return Err(ScannerError::Other("scanner usage reset has no trusted cycle or usage floor".to_string())); + } + ScannerUsageResetFloor::Missing | ScannerUsageResetFloor::Corrupt => PersistedUsageFloor { + next_cycle: cycle.next, + leader_epoch: cycle_epoch, + }, + }; + let resume_epoch = usage_state_reset_resume_epoch(&slots)?; + let leader_epoch = if let Some(epoch) = resume_epoch { + if epoch != cycle_epoch || usage_floor.leader_epoch > epoch || usage_floor.next_cycle > cycle.next { + return Err(ScannerError::Other( + "scanner usage reset bootstrap conflicts with the persisted cycle fence".to_string(), + )); + } + epoch + } else { + cycle_epoch + .max(usage_floor.leader_epoch) + .checked_add(1) + .filter(|epoch| *epoch < u64::MAX) + .ok_or_else(|| ScannerError::Other("scanner leader epoch is exhausted".to_string()))? + }; let rebuilt_cycle = CurrentCycle { next: cycle.next.max(usage_floor.next_cycle), ..Default::default() @@ -1602,21 +1824,24 @@ pub async fn reset_scanner_usage_state_for_full_rebuild( "scanner leader lock was lost before fencing usage reset cycle state".to_string(), )); } - save_config_with_publication_admission_for_epoch( - storeapi.clone(), - DATA_USAGE_BLOOM_NAME_PATH.as_str(), - cycle_data, - cycle_revision.preconditions(), - reset_epoch, - ) - .await - .map_err(|err| { - if scanner_publication_epoch_changed(&err) { - ScannerError::Other("scanner usage reset deferred by a movement epoch change".to_string()) - } else { - ScannerError::Other(format!("failed to fence scanner cycle state for usage reset: {err}")) - } - })?; + if resume_epoch.is_none() { + save_reset_config( + storeapi.clone(), + DATA_USAGE_BLOOM_NAME_PATH.as_str(), + cycle_data, + cycle_revision.preconditions(), + reset_epoch, + &|| !guard.is_lock_lost() && !ctx.is_cancelled(), + ) + .await + .map_err(|err| { + if scanner_publication_epoch_changed(&err) { + ScannerError::Other("scanner usage reset deferred by a movement epoch change".to_string()) + } else { + ScannerError::Other(format!("failed to fence scanner cycle state for usage reset: {err}")) + } + })?; + } if guard.is_lock_lost() { return Err(ScannerError::Other( @@ -1624,7 +1849,10 @@ pub async fn reset_scanner_usage_state_for_full_rebuild( )); } let reset_paths = - reset_scanner_usage_state_slots_for_full_rebuild(storeapi.clone(), &slots, reset_epoch, leader_epoch).await?; + reset_scanner_usage_state_slots_for_full_rebuild(storeapi.clone(), &slots, reset_epoch, leader_epoch, || { + !guard.is_lock_lost() && !ctx.is_cancelled() + }) + .await?; if guard.is_lock_lost() { return Err(ScannerError::Other( "scanner leader lock was lost after publishing usage reset marker".to_string(), @@ -2135,6 +2363,7 @@ async fn recover_legacy_incomplete_usage_floor( expected_publication_epoch, Some(primary.epoch), ScannerUsageBootstrapPublishContext::Recovery, + || true, ) .await?; warn!( diff --git a/crates/scanner/src/scanner/leadership.rs b/crates/scanner/src/scanner/leadership.rs index 8283a987a..260476e7a 100644 --- a/crates/scanner/src/scanner/leadership.rs +++ b/crates/scanner/src/scanner/leadership.rs @@ -191,6 +191,7 @@ pub(super) async fn initialize_usage_baseline_bootstrap( expected_epoch, None, ScannerUsageBootstrapPublishContext::Initial, + || true, ) .await } @@ -201,9 +202,10 @@ pub(super) async fn fence_scanner_usage_epoch_with_expected_epoch( claimed_epoch: u64, expected_publication_epoch: Option, allow_bootstrap_pending: bool, + owns_fence: impl Fn() -> bool, ) -> Result<(), ScannerError> { for retry in 0..=SCANNER_PERSIST_CAS_RETRIES { - if ctx.is_cancelled() { + if ctx.is_cancelled() || !owns_fence() { return Err(ScannerError::Other("scanner leadership was cancelled before usage fencing".to_string())); } @@ -264,6 +266,9 @@ pub(super) async fn fence_scanner_usage_epoch_with_expected_epoch( "scanner usage epoch fence changed while preparing its conditional write".to_string(), )); }; + if ctx.is_cancelled() || !owns_fence() { + return Err(ScannerError::Other("scanner leadership was lost before usage fencing".to_string())); + } save_config_with_preconditions(storeapi.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str(), data, revision.preconditions()) .await }; @@ -319,6 +324,7 @@ pub(super) async fn complete_scanner_leadership_claim( claimed_epoch, expected_publication_epoch, allow_bootstrap_pending, + || true, ) .await { diff --git a/crates/scanner/src/scanner/tests.rs b/crates/scanner/src/scanner/tests.rs index 244e9088f..b9b4cd299 100644 --- a/crates/scanner/src/scanner/tests.rs +++ b/crates/scanner/src/scanner/tests.rs @@ -634,6 +634,8 @@ struct MemoryConfigStore { cancel_after_successful_puts: Mutex>, replace_after_successful_puts: Mutex)>>, error_after_commit_deletes: Mutex>, + cancel_after_deletes: Mutex>, + pause_next_publication_admission: Mutex, Arc)>>, put_counts: Mutex>, publication_admission_blocked: AtomicBool, block_publication_after_admissions: AtomicUsize, @@ -4081,6 +4083,9 @@ impl crate::ScannerConfigObjectDelete for MemoryConfigStore { revisions.remove(&key); drop(revisions); drop(objects); + if let Some(token) = self.cancel_after_deletes.lock().await.remove(&key) { + token.cancel(); + } if self.error_after_commit_deletes.lock().await.remove(&key) { return Err(EcstoreError::other("injected delete error after commit")); } @@ -4088,6 +4093,11 @@ impl crate::ScannerConfigObjectDelete for MemoryConfigStore { } async fn scanner_data_usage_publication_admission(&self) -> Option { + let pause = self.pause_next_publication_admission.lock().await.take(); + if let Some((entered, resume)) = pause { + entered.notify_one(); + resume.notified().await; + } if self.publication_admission_blocked.load(Ordering::Acquire) { return None; } @@ -4589,7 +4599,7 @@ async fn scanner_legacy_usage_backup_survives_fencing_and_restart_after_real_met .expect("publication must also read the intact backup"); assert_eq!(baseline.data.as_deref(), Some(data.as_slice())); assert_eq!(baseline.revision, DataUsageCacheRevision::Missing); - fence_scanner_usage_epoch_with_expected_epoch(&CancellationToken::new(), store.clone(), 7, None, false) + fence_scanner_usage_epoch_with_expected_epoch(&CancellationToken::new(), store.clone(), 7, None, false, || true) .await .expect("legacy backup must be fenced into v2"); let fenced = read_config(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) @@ -4818,6 +4828,28 @@ async fn scanner_usage_state_reset_publishes_fenced_bootstrap_marker() { ); } + let cycle_before_retry = read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("cycle should remain before retry"); + let marker_before_retry = read_config_with_revision(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("bootstrap should remain before retry"); + let retry = reset_scanner_usage_state_for_full_rebuild(CancellationToken::new(), store.clone()) + .await + .expect("completed cleanup should be reentrant"); + assert_eq!(retry.leader_epoch, result.leader_epoch); + assert_eq!( + read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("cycle should remain"), + cycle_before_retry + ); + assert_eq!( + read_config_with_revision(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("bootstrap should remain"), + marker_before_retry + ); let (floor, state) = persisted_usage_floor_for_startup(store, false) .await .expect("reset marker should be resumable"); @@ -4973,7 +5005,7 @@ async fn scanner_usage_state_reset_slots_reject_primary_aba() { store.objects.lock().await.insert(key.clone(), b"newer-json".to_vec()); store.revisions.lock().await.insert(key, 2); - let err = reset_scanner_usage_state_slots_for_full_rebuild(store, &slots, 0, 3) + let err = reset_scanner_usage_state_slots_for_full_rebuild(store, &slots, 0, 3, || true) .await .expect_err("stale primary revision must not be overwritten"); assert!( @@ -4983,6 +5015,348 @@ async fn scanner_usage_state_reset_slots_reject_primary_aba() { ); } +#[tokio::test] +async fn scanner_usage_state_reset_resumes_every_cleanup_boundary_without_rewriting_intent() { + for completed in 0..=4 { + let store = Arc::new(MemoryConfigStore::default()); + let primary_path = DATA_USAGE_OBJ_NAME_PATH.as_str(); + let cleanup_paths = [ + format!("{primary_path}.bkp"), + LEGACY_DATA_USAGE_OBJ_NAME_PATH.as_str().to_string(), + format!("{}.bkp", LEGACY_DATA_USAGE_OBJ_NAME_PATH.as_str()), + DATA_USAGE_OBSERVED_OBJ_NAME_PATH.as_str().to_string(), + ]; + for path in std::iter::once(primary_path).chain(cleanup_paths.iter().map(String::as_str)) { + let mut usage = complete_usage_with_bucket_count(Some(std::time::SystemTime::UNIX_EPOCH), 0); + usage.scanner_epoch = Some(1); + save_config(store.clone(), path, serde_json::to_vec(&usage).expect("fixture should encode")) + .await + .expect("fixture should persist"); + } + // These objects belong to other owners, even when reset cleanup resumes. + for path in ["buckets/quota-reservations/ledger", "buckets/example/incarnation"] { + save_config(store.clone(), path, b"retain".to_vec()) + .await + .expect("unrelated state should persist"); + } + let slots = read_usage_state_reset_slots(store.clone()).await.expect("slots should load"); + let cancelled = CancellationToken::new(); + if completed == 0 { + store + .cancel_after_successful_puts + .lock() + .await + .insert(memory_config_key(RUSTFS_META_BUCKET, primary_path), (2, cancelled.clone())); + } else { + store + .cancel_after_deletes + .lock() + .await + .insert(memory_config_key(RUSTFS_META_BUCKET, &cleanup_paths[completed - 1]), cancelled.clone()); + } + let err = reset_scanner_usage_state_slots_for_full_rebuild(store.clone(), &slots, 0, 3, || !cancelled.is_cancelled()) + .await + .expect_err("interruption should stop cleanup"); + assert!(err.to_string().contains("ownership"), "boundary {completed}: {err}"); + for (index, path) in cleanup_paths.iter().enumerate() { + assert_eq!( + store + .objects + .lock() + .await + .contains_key(&memory_config_key(RUSTFS_META_BUCKET, path)), + index >= completed, + "boundary {completed}, slot {index}" + ); + } + let intent = read_config_with_revision(store.clone(), primary_path) + .await + .expect("intent should persist"); + let slots = read_usage_state_reset_slots(store.clone()) + .await + .expect("restart should reload slots"); + reset_scanner_usage_state_slots_for_full_rebuild(store.clone(), &slots, 0, 3, || true) + .await + .expect("restart should complete the same intent"); + assert_eq!( + read_config_with_revision(store.clone(), primary_path) + .await + .expect("intent should remain"), + intent + ); + assert_eq!(store.put_counts.lock().await[&memory_config_key(RUSTFS_META_BUCKET, primary_path)], 2); + for path in cleanup_paths { + assert!( + !store + .objects + .lock() + .await + .contains_key(&memory_config_key(RUSTFS_META_BUCKET, &path)) + ); + } + for path in ["buckets/quota-reservations/ledger", "buckets/example/incarnation"] { + assert_eq!(read_config(store.clone(), path).await.expect("unrelated state should remain"), b"retain"); + } + } +} + +#[tokio::test] +async fn scanner_usage_state_reset_stops_usage_fence_after_owner_loss() { + let store = Arc::new(MemoryConfigStore::default()); + let mut usage = complete_usage_with_bucket_count(Some(std::time::SystemTime::UNIX_EPOCH), 0); + usage.scanner_epoch = Some(1); + let bytes = serde_json::to_vec(&usage).expect("baseline should encode"); + save_config(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str(), bytes.clone()) + .await + .expect("baseline should persist"); + let checks = AtomicUsize::new(0); + let err = fence_scanner_usage_epoch_with_expected_epoch(&CancellationToken::new(), store.clone(), 3, Some(0), false, || { + checks.fetch_add(1, Ordering::SeqCst) == 0 + }) + .await + .expect_err("ownership lost during reads must prevent the write"); + assert!(err.to_string().contains("leadership was lost"), "{err}"); + assert_eq!( + read_config(store, DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("baseline should remain"), + bytes + ); +} + +#[tokio::test] +async fn scanner_usage_state_reset_cancels_during_publication_admission() { + for resuming in [false, true] { + let store = Arc::new(MemoryConfigStore::default()); + let usage = if resuming { + scanner_usage_bootstrap_marker(std::time::SystemTime::UNIX_EPOCH, Some(3)) + } else { + complete_usage_with_bucket_count(Some(std::time::SystemTime::UNIX_EPOCH), 0) + }; + save_config( + store.clone(), + DATA_USAGE_OBJ_NAME_PATH.as_str(), + serde_json::to_vec(&usage).expect("primary should encode"), + ) + .await + .expect("primary should persist"); + save_config(store.clone(), LEGACY_DATA_USAGE_OBJ_NAME_PATH.as_str(), b"corrupt".to_vec()) + .await + .expect("cleanup target should persist"); + let slots = read_usage_state_reset_slots(store.clone()).await.expect("slots should load"); + let before = store.objects.lock().await.clone(); + let revisions_before = store.revisions.lock().await.clone(); + let entered = Arc::new(tokio::sync::Notify::new()); + let resume = Arc::new(tokio::sync::Notify::new()); + *store.pause_next_publication_admission.lock().await = Some((entered.clone(), resume.clone())); + let cancelled = CancellationToken::new(); + let (result, ()) = tokio::join!( + reset_scanner_usage_state_slots_for_full_rebuild(store.clone(), &slots, 0, 3, || !cancelled.is_cancelled()), + async { + entered.notified().await; + cancelled.cancel(); + resume.notify_one(); + } + ); + let err = result.expect_err("losing ownership during admission must prevent mutation"); + assert!(err.to_string().contains("ownership was lost"), "resuming={resuming}: {err}"); + assert_eq!(*store.objects.lock().await, before); + assert_eq!(*store.revisions.lock().await, revisions_before); + } +} + +#[tokio::test] +#[serial] +async fn scanner_usage_state_reset_rejects_corruption_without_a_trusted_floor() { + let (_temp_dir, store) = setup_scanner_cycle_store_with_usage_baseline(false).await; + save_config(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str(), b"{corrupt".to_vec()) + .await + .expect("corrupt primary should persist"); + let before = read_config_with_revision(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("evidence should load"); + let err = reset_scanner_usage_state_for_full_rebuild(CancellationToken::new(), store.clone()) + .await + .expect_err("corruption must not become a zero floor"); + assert!(err.to_string().contains("no trusted cycle or usage floor"), "{err}"); + assert_eq!( + read_config_with_revision(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("evidence should remain"), + before + ); + assert!(matches!( + read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); + let mut backup = complete_usage_with_bucket_count(Some(std::time::SystemTime::UNIX_EPOCH), 0); + backup.scanner_epoch = Some(7); + backup.scanner_cycle = Some(40); + save_config( + store.clone(), + &format!("{}.bkp", DATA_USAGE_OBJ_NAME_PATH.as_str()), + serde_json::to_vec(&backup).expect("backup should encode"), + ) + .await + .expect("valid backup should persist"); + let result = reset_scanner_usage_state_for_full_rebuild(CancellationToken::new(), store) + .await + .expect("valid backup should supply the recovery floor"); + assert_eq!(result.leader_epoch, 8); + assert_eq!(result.next_cycle, 41); +} + +#[tokio::test] +async fn scanner_usage_state_reset_rejects_replaced_intent_and_newer_cleanup_slot() { + let store = Arc::new(MemoryConfigStore::default()); + let marker = scanner_usage_bootstrap_marker(std::time::SystemTime::UNIX_EPOCH, Some(3)); + let bytes = serde_json::to_vec(&marker).expect("marker should encode"); + save_config(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str(), bytes.clone()) + .await + .expect("intent should persist"); + let slots = read_usage_state_reset_slots(store.clone()).await.expect("slots should load"); + save_config(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str(), bytes) + .await + .expect("another intent should persist"); + let err = reset_scanner_usage_state_slots_for_full_rebuild(store.clone(), &slots, 0, 3, || true) + .await + .expect_err("same epoch cannot replace an intent revision"); + assert!(err.to_string().contains("intent revision changed"), "{err}"); + + let mut newer = complete_usage_with_bucket_count(Some(std::time::SystemTime::UNIX_EPOCH), 0); + newer.scanner_epoch = Some(3); + let path = format!("{}.bkp", DATA_USAGE_OBJ_NAME_PATH.as_str()); + let bytes = serde_json::to_vec(&newer).expect("newer snapshot should encode"); + save_config(store.clone(), &path, bytes.clone()) + .await + .expect("newer snapshot should persist"); + let slots = read_usage_state_reset_slots(store.clone()) + .await + .expect("slots should reload"); + let err = reset_scanner_usage_state_slots_for_full_rebuild(store.clone(), &slots, 0, 3, || true) + .await + .expect_err("cleanup cannot delete same-epoch progress"); + assert!(err.to_string().contains("not older than its intent"), "{err}"); + assert_eq!(read_config(store, &path).await.expect("newer snapshot should remain"), bytes); +} + +#[tokio::test] +#[serial] +async fn scanner_usage_state_reset_rejects_decodable_untrusted_floor() { + let (_temp_dir, store) = setup_scanner_cycle_store_with_usage_baseline(false).await; + let invalid_identity = DataUsageInfo { + usage_snapshot_complete: true, + buckets_count: 1, + last_update: Some(std::time::SystemTime::UNIX_EPOCH), + ..Default::default() + }; + for usage in [DataUsageInfo::default(), invalid_identity] { + save_config( + store.clone(), + DATA_USAGE_OBJ_NAME_PATH.as_str(), + serde_json::to_vec(&usage).expect("fixture should encode"), + ) + .await + .expect("untrusted primary should persist"); + let before = read_config_with_revision(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("primary should load"); + let err = reset_scanner_usage_state_for_full_rebuild(CancellationToken::new(), store.clone()) + .await + .expect_err("valid JSON alone cannot prove a usage floor"); + assert!(err.to_string().contains("no trusted cycle or usage floor"), "{err}"); + assert_eq!( + read_config_with_revision(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("evidence should remain"), + before + ); + assert!(matches!( + read_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()).await, + Err(EcstoreError::ConfigNotFound) + )); + } +} + +#[test] +fn full_rescan_reset_rejects_unknown_marker_phase_even_with_invalid_compat_fields() { + for state in [serde_json::json!("rewrite-v2"), serde_json::json!(7), serde_json::Value::Null] { + let marker = serde_json::json!({"state": state, "retry_count": "future-type", "schema_version": 99}); + let err = super::cycle_state::decode_recovery_marker_for_reset( + &serde_json::to_vec(&marker).expect("future marker should encode"), + &DataUsageCacheRevision::Etag("intent-1".to_string()), + ) + .expect_err("unknown persistent phases must remain fenced"); + assert!(err.to_string().contains("state is unsupported"), "{err}"); + } +} + +#[tokio::test] +#[serial] +async fn full_rescan_reset_preserves_unknown_phase_and_retries_completed_cleanup() { + let (_temp_dir, store) = setup_scanner_cycle_store().await; + save_config(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str(), b"corrupt".to_vec()) + .await + .expect("corrupt primary should persist"); + save_config( + store.clone(), + DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), + br#"{"state":"future-rewrite"}"#.to_vec(), + ) + .await + .expect("future marker should persist"); + let primary_before = read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("primary should load"); + let marker_before = read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()) + .await + .expect("marker should load"); + let err = reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect_err("unknown phase must block explicit reset"); + assert!(err.to_string().contains("state is unsupported"), "{err}"); + assert_eq!( + read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("primary should remain"), + primary_before + ); + assert_eq!( + read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str()) + .await + .expect("marker should remain"), + marker_before + ); + + save_config(store.clone(), DATA_USAGE_BLOOM_RECOVERY_PATH.as_str(), b"{malformed".to_vec()) + .await + .expect("recoverable marker should persist"); + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("reset should complete"); + let primary = read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("rebuilt primary should load"); + let usage = read_config_with_revision(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("fenced usage should load"); + reset_scanner_cycle_recovery(CancellationToken::new(), store.clone()) + .await + .expect("retry after marker deletion should complete"); + assert_eq!( + read_config_with_revision(store.clone(), DATA_USAGE_BLOOM_NAME_PATH.as_str()) + .await + .expect("rebuilt primary should remain"), + primary + ); + assert_eq!( + read_config_with_revision(store.clone(), DATA_USAGE_OBJ_NAME_PATH.as_str()) + .await + .expect("fenced usage should remain"), + usage + ); +} + #[tokio::test] async fn scanner_usage_state_reset_slots_defer_when_publication_epoch_moves() { let store = Arc::new(MemoryConfigStore::default()); @@ -4993,7 +5367,7 @@ async fn scanner_usage_state_reset_slots_defer_when_publication_epoch_moves() { .expect("usage reset slots should be inspected"); store.publication_admission_blocked.store(true, Ordering::Release); - let err = reset_scanner_usage_state_slots_for_full_rebuild(store, &slots, 0, 3) + let err = reset_scanner_usage_state_slots_for_full_rebuild(store, &slots, 0, 3, || true) .await .expect_err("movement admission loss must defer reset"); assert!( From 0d1b312673d062e7ca236d2f444728a6afcfed5e Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 5 Sep 2026 16:49:58 +0800 Subject: [PATCH 3/6] fix(ci): require fresh successful scheduled validations (#7192) --- .../scheduled-validation-freshness.yml | 1 + .../check_scheduled_validation_freshness.py | 398 +++++++++++++----- 2 files changed, 293 insertions(+), 106 deletions(-) diff --git a/.github/workflows/scheduled-validation-freshness.yml b/.github/workflows/scheduled-validation-freshness.yml index eef340869..69ddc844c 100644 --- a/.github/workflows/scheduled-validation-freshness.yml +++ b/.github/workflows/scheduled-validation-freshness.yml @@ -42,6 +42,7 @@ jobs: - name: Check latest scheduled runs env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RUSTFS_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | set +e python3 scripts/check_scheduled_validation_freshness.py \ diff --git a/scripts/check_scheduled_validation_freshness.py b/scripts/check_scheduled_validation_freshness.py index a5812c7a3..c694e3c89 100644 --- a/scripts/check_scheduled_validation_freshness.py +++ b/scripts/check_scheduled_validation_freshness.py @@ -1,10 +1,11 @@ #!/usr/bin/env python3 -"""Fail when a critical scheduled validation has not started recently.""" +"""Require recent scheduled attempts and completed successes on the default branch.""" from __future__ import annotations import argparse from datetime import datetime, timedelta, timezone +import io import json import os from pathlib import Path @@ -13,7 +14,7 @@ import sys import tempfile import unittest from unittest import mock -from urllib.parse import quote, urlencode +from urllib.parse import parse_qs, quote, urlencode, urlsplit from urllib.request import Request, urlopen @@ -75,15 +76,8 @@ def stale_reason( run: dict[str, object] | None, now: datetime, max_age_hours: int, - never_ran_grace_until: datetime | None = None, ) -> str | None: if run is None: - # The grace deadline only covers a workflow whose first scheduled slot - # has not arrived yet (for example a monthly cron enabled mid-month). - # A recorded-but-old run proves the schedule used to fire and stopped, - # so the grace never masks that case. - if never_ran_grace_until is not None and now <= never_ran_grace_until: - return None return "no scheduled run has been recorded" created_at = parse_timestamp(run.get("created_at")) age = now - created_at @@ -93,14 +87,23 @@ def stale_reason( def fetch_latest_scheduled_run( - repository: str, workflow: str, token: str, api_url: str + repository: str, + workflow: str, + token: str, + api_url: str, + default_branch: str, + successful: bool = False, ) -> dict[str, object] | None: owner, repo = repository.split("/", 1) workflow_name = Path(workflow).name + query = {"event": "schedule", "branch": default_branch, "per_page": 1} + if successful: + # Filter on the server: the last success may be beyond a page of failures. + query["status"] = "success" endpoint = ( f"{api_url.rstrip('/')}/repos/{quote(owner, safe='')}/{quote(repo, safe='')}" f"/actions/workflows/{quote(workflow_name, safe='')}/runs?" - + urlencode({"event": "schedule", "per_page": 1}) + + urlencode(query) ) request = Request( endpoint, @@ -110,57 +113,104 @@ def fetch_latest_scheduled_run( "X-GitHub-Api-Version": "2022-11-28", }, ) - with urlopen(request, timeout=30) as response: + # Two requests per manifest entry must fit the watchdog's ten-minute job. + with urlopen(request, timeout=15) as response: payload = json.load(response) - runs = payload.get("workflow_runs") + runs = payload.get("workflow_runs") if isinstance(payload, dict) else None if not isinstance(runs, list): raise ValueError(f"GitHub returned no workflow_runs list for {workflow}") + total_count = payload.get("total_count") + if not isinstance(total_count, int) or isinstance(total_count, bool) or total_count < len(runs): + raise ValueError(f"GitHub returned an invalid run count for {workflow}") if not runs: + if total_count: + raise ValueError(f"GitHub returned an empty first page with recorded runs for {workflow}") return None - if not isinstance(runs[0], dict): + run = runs[0] + if not isinstance(run, dict): raise ValueError(f"GitHub returned an invalid workflow run for {workflow}") - return runs[0] + if run.get("event") != "schedule" or run.get("head_branch") != default_branch: + raise ValueError(f"GitHub returned a run outside the scheduled default-branch query for {workflow}") + if not isinstance(run.get("status"), str) or not run["status"]: + raise ValueError(f"GitHub returned no run status for {workflow}") + conclusion = run.get("conclusion") + if (conclusion is not None and not isinstance(conclusion, str)) or ( + run["status"] == "completed" and not conclusion + ): + raise ValueError(f"GitHub returned an invalid run conclusion for {workflow}") + if successful and (run["status"] != "completed" or conclusion != "success"): + raise ValueError(f"GitHub returned a run without a completed success for {workflow}") + parse_timestamp(run.get("created_at")) + if not isinstance(run.get("html_url"), str) or not run["html_url"]: + raise ValueError(f"GitHub returned no run URL for {workflow}") + return run -def write_report(path: Path, failures: list[tuple[str, int, str, str]]) -> None: - lines = ["## Scheduled validation freshness"] - if not failures: - lines.append("") - lines.append("All critical scheduled validations have a recent scheduled run.") - else: - lines.extend( - [ - "", - "The following critical validations are stale or could not be inspected:", - "", - "| Workflow | Limit | Result | Last run |", - "| --- | ---: | --- | --- |", - ] - ) - for workflow, max_age_hours, reason, run_url in failures: - link = f"[open]({run_url})" if run_url else "—" - lines.append(f"| `{workflow}` | {max_age_hours}h | {reason} | {link} |") +def describe_run(run: dict[str, object] | None) -> str: + if run is None: + return "No recorded run" + outcome = run["status"] + if run.get("conclusion"): + outcome = f"{outcome}/{run['conclusion']}" + return f"[{outcome}]({run['html_url']}) — created {run['created_at']}" + + +def write_report(path: Path, rows: list[tuple[str, int, str, str, str]], default_branch: str) -> None: + lines = [ + "## Scheduled validation freshness", + "", + f"Default branch: `{default_branch}`. Ages use scheduled-run creation time; rerunning an old commit does not refresh its evidence.", + "Attempt outcomes are shown independently of successful-run freshness.", + "Success is the GitHub workflow run conclusion; suite completeness remains the responsibility of each workflow.", + "", + "| Workflow | Limit | Freshness | Last attempt | Last completed success |", + "| --- | ---: | --- | --- | --- |", + ] + for workflow, max_age_hours, result, attempt, success in rows: + cells = [f"`{workflow}`", f"{max_age_hours}h", result, attempt, success] + lines.append("| " + " | ".join(cell.replace("|", "\\|").replace("\n", " ") for cell in cells) + " |") path.write_text("\n".join(lines) + "\n") def check_freshness( - config: Path, report: Path, repository: str, token: str, api_url: str + config: Path, report: Path, repository: str, token: str, api_url: str, default_branch: str ) -> int: now = datetime.now(timezone.utc) - failures: list[tuple[str, int, str, str]] = [] + rows: list[tuple[str, int, str, str, str]] = [] + failed = False for workflow, max_age_hours, never_ran_grace_until in load_validations(config): - try: - run = fetch_latest_scheduled_run(repository, workflow, token, api_url) - reason = stale_reason(run, now, max_age_hours, never_ran_grace_until) - if reason is not None: - run_url = str(run.get("html_url", "")) if run else "" - failures.append((workflow, max_age_hours, reason, run_url)) - except Exception as error: - failures.append( - (workflow, max_age_hours, f"inspection failed: {error}", "") - ) - write_report(report, failures) - return 1 if failures else 0 + runs: dict[str, dict[str, object] | None] = {} + reasons: list[str] = [] + for label, successful in (("Last attempt", False), ("Last completed success", True)): + try: + runs[label] = fetch_latest_scheduled_run( + repository, workflow, token, api_url, default_branch, successful + ) + except Exception as error: + reasons.append(f"{label}: inspection failed: {error}") + # A failed inspection or any recorded attempt ends first-run grace. + initial_grace = ( + len(runs) == 2 + and all(run is None for run in runs.values()) + and never_ran_grace_until is not None + and now <= never_ran_grace_until + ) + if not initial_grace: + for label, run in runs.items(): + reason = stale_reason(run, now, max_age_hours) + if reason is not None: + reasons.append(f"{label}: {reason}") + failed |= bool(reasons) + result = "; ".join(reasons) if reasons else "Fresh" + if initial_grace: + result = f"Initial grace until {never_ran_grace_until.isoformat()}" + evidence = [ + describe_run(runs[label]) if label in runs else "Inspection failed" + for label in ("Last attempt", "Last completed success") + ] + rows.append((workflow, max_age_hours, result, *evidence)) + write_report(report, rows, default_branch) + return 1 if failed else 0 class SelfTests(unittest.TestCase): @@ -173,15 +223,6 @@ class SelfTests(unittest.TestCase): self.assertIsNotNone(stale_reason(past_limit, self.NOW, 36)) self.assertIsNotNone(stale_reason(None, self.NOW, 36)) - def test_never_ran_grace_only_covers_missing_runs(self) -> None: - future_grace = self.NOW + timedelta(hours=1) - past_grace = self.NOW - timedelta(seconds=1) - self.assertIsNone(stale_reason(None, self.NOW, 36, future_grace)) - self.assertIsNone(stale_reason(None, self.NOW, 36, self.NOW)) - self.assertIsNotNone(stale_reason(None, self.NOW, 36, past_grace)) - stale_run = {"created_at": "2026-08-20T23:59:59Z"} - self.assertIsNotNone(stale_reason(stale_run, self.NOW, 36, future_grace)) - def test_config_rejects_duplicate_and_invalid_entries(self) -> None: with tempfile.TemporaryDirectory() as tmp: path = Path(tmp) / "validations.json" @@ -238,63 +279,205 @@ class SelfTests(unittest.TestCase): ], ) - def test_check_reports_missing_runs(self) -> None: + @staticmethod + def run_fixture(**overrides: object) -> dict[str, object]: + return { + "status": "completed", + "conclusion": "success", + "event": "schedule", + "head_branch": "release/current", + "created_at": "2026-08-22T00:00:00Z", + "html_url": "https://github.test/rustfs/rustfs/actions/runs/1", + **overrides, + } + + def check_payloads( + self, payloads: list[object], *, grace: str | None = None, workflows: int = 1 + ) -> tuple[int, str, list]: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) config = root / "validations.json" report = root / "report.md" - config.write_text( - json.dumps( - [ - {"workflow": ".github/workflows/ci.yml", "max_age_hours": 36}, - {"workflow": ".github/workflows/fuzz.yml", "max_age_hours": 36}, - {"workflow": ".github/workflows/mint.yml", "max_age_hours": 36}, - ] - ) - ) - with mock.patch( - __name__ + ".fetch_latest_scheduled_run", - side_effect=[ - {"created_at": "2999-01-01T00:00:00Z"}, - None, - RuntimeError("API unavailable"), - ], + entries = [ + {"workflow": f".github/workflows/check-{index}.yml", "max_age_hours": 36} + for index in range(workflows) + ] + if grace is not None: + entries[0]["never_ran_grace_until"] = grace + config.write_text(json.dumps(entries)) + responses = [] + for payload in payloads: + if isinstance(payload, dict) and isinstance(payload.get("workflow_runs"), list): + payload = {"total_count": len(payload["workflow_runs"]), **payload} + responses.append(payload if isinstance(payload, Exception) else io.StringIO(json.dumps(payload))) + with ( + mock.patch(__name__ + ".urlopen", side_effect=responses) as request, + mock.patch(__name__ + ".datetime", wraps=datetime) as clock, ): - self.assertEqual( - check_freshness( - config, - report, - "rustfs/rustfs", - "token", - "https://api.github.test", - ), - 1, + clock.now.return_value = self.NOW + status = check_freshness( + config, report, "rustfs/rustfs", "test-token", + "https://api.github.test", "release/current", ) - contents = report.read_text() - self.assertIn(".github/workflows/fuzz.yml", contents) - self.assertIn("inspection failed: API unavailable", contents) - self.assertNotIn(".github/workflows/ci.yml`", contents) + return status, report.read_text(), request.call_args_list - config.write_text( - json.dumps( - [{"workflow": ".github/workflows/ci.yml", "max_age_hours": 36}] + def test_requests_filter_schedule_default_branch_and_success_on_server(self) -> None: + attempt = self.run_fixture(status="in_progress", conclusion=None) + success = self.run_fixture(html_url="https://github.test/rustfs/rustfs/actions/runs/2") + status, report, calls = self.check_payloads([ + {"workflow_runs": [attempt], "total_count": 1001}, + {"workflow_runs": [success], "total_count": 1}, + ]) + self.assertEqual(status, 0) + self.assertEqual(len(calls), 2) + for call, successful in zip(calls, (False, True)): + request = call.args[0] + url = urlsplit(request.full_url) + self.assertEqual(url.path, "/repos/rustfs/rustfs/actions/workflows/check-0.yml/runs") + expected = {"event": ["schedule"], "branch": ["release/current"], "per_page": ["1"]} + if successful: + expected["status"] = ["success"] + self.assertEqual(parse_qs(url.query), expected) + self.assertEqual(request.get_header("Authorization"), "Bearer test-token") + self.assertEqual(call.kwargs, {"timeout": 15}) + self.assertIn("[in_progress]", report) + self.assertIn(str(attempt["html_url"]), report) + self.assertIn(str(success["html_url"]), report) + + def test_cancelled_attempt_cannot_refresh_expired_success(self) -> None: + attempt = self.run_fixture(conclusion="cancelled") + success = self.run_fixture( + created_at="2026-08-20T23:59:59Z", updated_at="2026-08-22T11:59:59Z", + html_url="https://github.test/rustfs/rustfs/actions/runs/2", + ) + status, report, _ = self.check_payloads([ + {"workflow_runs": [attempt]}, {"workflow_runs": [success]}, + ]) + self.assertEqual(status, 1) + self.assertIn("Last completed success: last scheduled run is", report) + self.assertIn("[completed/cancelled]", report) + for run in (attempt, success): + self.assertIn(str(run["html_url"]), report) + self.assertIn(str(run["created_at"]), report) + + def test_attempt_outcome_does_not_replace_recent_success(self) -> None: + success = self.run_fixture(created_at="2026-08-21T00:00:00Z") + for state, conclusion in ( + ("completed", "failure"), ("completed", "cancelled"), + ("completed", "timed_out"), ("completed", "success"), + ("queued", None), ("in_progress", None), + ): + with self.subTest(state=state, conclusion=conclusion): + status, report, _ = self.check_payloads([ + {"workflow_runs": [self.run_fixture(status=state, conclusion=conclusion)]}, + {"workflow_runs": [success]}, + ]) + self.assertEqual(status, 0) + self.assertIn(f"[{state}" + (f"/{conclusion}" if conclusion else "") + "]", report) + self.assertIn("Fresh", report) + self.assertNotIn("All critical scheduled validations", report) + + def test_grace_requires_two_successful_queries_with_no_history(self) -> None: + for attempt, success, grace, expected in ( + (None, None, "2026-08-22T12:00:00Z", 0), + (None, None, "2026-08-22T11:59:59Z", 1), + (self.run_fixture(conclusion="failure"), None, "2026-08-23T00:00:00Z", 1), + (self.run_fixture(status="queued", conclusion=None), None, "2026-08-23T00:00:00Z", 1), + (None, self.run_fixture(), "2026-08-23T00:00:00Z", 1), + ): + with self.subTest(attempt=attempt, success=success, grace=grace): + status, report, _ = self.check_payloads([ + {"workflow_runs": [] if attempt is None else [attempt]}, + {"workflow_runs": [] if success is None else [success]}, + ], grace=grace) + self.assertEqual(status, expected) + self.assertEqual("Initial grace until" in report, expected == 0) + + def test_api_failures_preserve_other_evidence_and_never_enter_grace(self) -> None: + good = {"workflow_runs": [self.run_fixture()]} + for first, second in ( + (RuntimeError("API unavailable"), good), + (good, RuntimeError("API unavailable")), + (RuntimeError("API unavailable"), {"workflow_runs": []}), + ): + with self.subTest(first=first, second=second): + status, report, calls = self.check_payloads( + [first, second], grace="2026-08-23T00:00:00Z" ) - ) - with mock.patch( - __name__ + ".fetch_latest_scheduled_run", - return_value={"created_at": "2999-01-01T00:00:00Z"}, - ): - self.assertEqual( - check_freshness( - config, - report, - "rustfs/rustfs", - "token", - "https://api.github.test", - ), - 0, - ) - self.assertIn("All critical scheduled validations", report.read_text()) + self.assertEqual(status, 1) + self.assertEqual(len(calls), 2) + self.assertIn("inspection failed: API unavailable", report) + self.assertNotIn("Initial grace until", report) + if first is good or second is good: + self.assertIn(str(self.run_fixture()["html_url"]), report) + + def test_invalid_api_evidence_fails_closed(self) -> None: + malformed = [ + [], {}, {"workflow_runs": {}}, {"workflow_runs": [None]}, + {"workflow_runs": [], "total_count": 1}, + {"workflow_runs": [], "total_count": -1}, + {"workflow_runs": [], "total_count": None}, + {"workflow_runs": [], "total_count": True}, + *({"workflow_runs": [self.run_fixture(**override)]} for override in ( + {"event": "workflow_dispatch"}, {"head_branch": "other"}, + {"created_at": "invalid"}, {"created_at": "2026-08-22T00:00:00"}, + {"status": None}, {"conclusion": None}, {"conclusion": 1}, + {"html_url": ""}, + )), + ] + for payload in malformed: + for index, label in enumerate(("Last attempt", "Last completed success")): + with self.subTest(payload=payload, label=label): + payloads = [{"workflow_runs": [self.run_fixture()]} for _ in range(2)] + payloads[index] = payload + status, report, _ = self.check_payloads(payloads, grace="2026-08-23T00:00:00Z") + self.assertEqual(status, 1) + self.assertIn(f"{label}: inspection failed", report) + self.assertNotIn("Initial grace until", report) + self.assertIn(str(self.run_fixture()["html_url"]), report) + for state, conclusion in (("in_progress", "success"), ("completed", "failure"), ("completed", "skipped")): + with self.subTest(state=state, conclusion=conclusion): + status, report, _ = self.check_payloads([ + {"workflow_runs": [self.run_fixture()]}, + {"workflow_runs": [self.run_fixture(status=state, conclusion=conclusion)]}, + ]) + self.assertEqual(status, 1) + self.assertIn("without a completed success", report) + + def test_report_retains_every_workflow(self) -> None: + status, report, calls = self.check_payloads([ + {"workflow_runs": [self.run_fixture()]}, {"workflow_runs": [self.run_fixture()]}, + {"workflow_runs": []}, {"workflow_runs": []}, + RuntimeError("API unavailable"), {"workflow_runs": [self.run_fixture()]}, + ], workflows=3) + self.assertEqual(status, 1) + self.assertEqual(len(calls), 6) + for index in range(3): + self.assertEqual(report.count(f"`.github/workflows/check-{index}.yml`"), 1) + self.assertIn("No recorded run", report) + self.assertIn("Inspection failed", report) + + def test_cli_requires_the_repository_default_branch(self) -> None: + from check_test_wiring import yaml_block + + workflow = (ROOT / ".github/workflows/scheduled-validation-freshness.yml").read_text().splitlines() + job = yaml_block(workflow, "check-freshness", 2) + self.assertIsNotNone(job) + start = job.index(" - name: Check latest scheduled runs") + end = next((index for index in range(start + 1, len(job)) if job[index].startswith(" - ")), len(job)) + environment = yaml_block(job[start:end], "env", 8) + self.assertIsNotNone(environment) + self.assertIn(" RUSTFS_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}", environment) + + with ( + mock.patch.dict(os.environ, {"GITHUB_REPOSITORY": "rustfs/rustfs", "GH_TOKEN": "test-token"}, clear=True), + mock.patch.object(sys, "argv", ["checker", "--report", "unused.md"]), + mock.patch("sys.stderr", new=io.StringIO()) as stderr, + self.assertRaises(SystemExit) as error, + ): + main() + self.assertEqual(error.exception.code, 2) + self.assertIn("RUSTFS_DEFAULT_BRANCH", stderr.getvalue()) def main() -> int: @@ -318,11 +501,14 @@ def main() -> int: repository = os.environ.get("GITHUB_REPOSITORY", "") token = os.environ.get("GH_TOKEN", "") api_url = os.environ.get("GITHUB_API_URL", "https://api.github.com") + default_branch = os.environ.get("RUSTFS_DEFAULT_BRANCH", "") if not re.fullmatch(r"[^/\s]+/[^/\s]+", repository): parser.error("GITHUB_REPOSITORY must be owner/repository") if not token: parser.error("GH_TOKEN is required") - return check_freshness(args.config, args.report, repository, token, api_url) + if not default_branch or any(character.isspace() for character in default_branch): + parser.error("RUSTFS_DEFAULT_BRANCH is required and must name the repository default branch") + return check_freshness(args.config, args.report, repository, token, api_url, default_branch) if __name__ == "__main__": From acfeef55abc1a68c285d23b5dd301ed08f822337 Mon Sep 17 00:00:00 2001 From: houseme Date: Sat, 5 Sep 2026 16:54:55 +0800 Subject: [PATCH 4/6] feat(scanner): add bounded incarnation-scoped ACK receiver (#7182) * chore(deps): refresh SDKs and pin clock skew regression coverage Refresh compatible dependencies for Scanner/Heal V2 batch 1 and verify the production S3 retry/signing path with a deterministic clock. Co-Authored-By: heihutu Co-Authored-By: zhi22915 * feat(scanner): add bounded incarnation-scoped ACK receiver Refs rustfs/backlog#2265 and rustfs/backlog#2240. Co-Authored-By: heihutu Co-Authored-By: zhi22915 --------- Co-authored-by: heihutu Co-authored-by: zhi22915 --- crates/ecstore/src/api/mod.rs | 19 +- crates/ecstore/src/bucket/metadata_sys.rs | 121 +++++++- crates/ecstore/src/cluster/rpc/client.rs | 19 ++ .../src/cluster/rpc/peer_rest_client.rs | 47 +++ .../src/generated/proto_gen/node_service.rs | 286 ++++++++++++++++++ crates/protos/src/lib.rs | 2 + crates/protos/src/node.proto | 34 +++ crates/protos/src/scoped_dirty_usage.rs | 213 +++++++++++++ crates/scanner/src/lib.rs | 5 +- crates/scanner/src/scanner_io.rs | 5 +- crates/scanner/src/scanner_io/dirty_usage.rs | 106 +++++++ crates/scanner/src/storage_api.rs | 4 +- rustfs/src/server/http.rs | 47 +++ rustfs/src/storage/rpc/node_service.rs | 200 ++++++++++++ rustfs/src/storage/storage_api.rs | 10 +- rustfs/src/storage/tonic_service.rs | 1 + rustfs/src/storage_api.rs | 2 +- 17 files changed, 1097 insertions(+), 24 deletions(-) create mode 100644 crates/protos/src/scoped_dirty_usage.rs diff --git a/crates/ecstore/src/api/mod.rs b/crates/ecstore/src/api/mod.rs index 34e784288..437f5c458 100644 --- a/crates/ecstore/src/api/mod.rs +++ b/crates/ecstore/src/api/mod.rs @@ -196,15 +196,16 @@ pub mod bucket { pub use crate::bucket::metadata_sys::ConfigWriteLockProbe; pub use crate::bucket::metadata_sys::{ BucketMetadataMutationGuard, BucketMetadataSys, ObjectLockConfigState, acquire_bucket_metadata_transaction_lock, - acquire_bucket_metadata_transaction_lock_for_incarnation, capture_bucket_metadata_incarnation, delete, - delete_if_incarnation, delete_under_transaction_lock, get, get_accelerate_config, get_bucket_policy, - get_bucket_policy_raw, get_bucket_targets_config, get_config_from_disk, get_cors_config, get_durability_config, - get_global_bucket_metadata_sys, get_lifecycle_config, get_logging_config, get_notification_config, - get_object_lock_config, get_object_lock_config_state, get_on_demand_migration_config, get_public_access_block_config, - get_quota_config, get_replication_config, get_request_payment_config, get_sse_config, get_tagging_config, - get_versioning_config, get_website_config, init_bucket_metadata_sys, list_bucket_targets, reload_bucket_metadata, - remove_bucket_metadata, set_bucket_metadata, update, update_bucket_targets_under_transaction_lock, - update_config_with, update_if_incarnation, update_quota_if_incarnation, update_under_transaction_lock, + acquire_bucket_metadata_transaction_lock_for_incarnation, acquire_scanner_bucket_incarnation_fence, + capture_bucket_metadata_incarnation, delete, delete_if_incarnation, delete_under_transaction_lock, get, + get_accelerate_config, get_bucket_policy, get_bucket_policy_raw, get_bucket_targets_config, get_config_from_disk, + get_cors_config, get_durability_config, get_global_bucket_metadata_sys, get_lifecycle_config, get_logging_config, + get_notification_config, get_object_lock_config, get_object_lock_config_state, get_on_demand_migration_config, + get_public_access_block_config, get_quota_config, get_replication_config, get_request_payment_config, get_sse_config, + get_tagging_config, get_versioning_config, get_website_config, init_bucket_metadata_sys, list_bucket_targets, + reload_bucket_metadata, remove_bucket_metadata, set_bucket_metadata, update, + update_bucket_targets_under_transaction_lock, update_config_with, update_if_incarnation, update_quota_if_incarnation, + update_under_transaction_lock, }; } diff --git a/crates/ecstore/src/bucket/metadata_sys.rs b/crates/ecstore/src/bucket/metadata_sys.rs index eb40ed8c9..4a53927f9 100644 --- a/crates/ecstore/src/bucket/metadata_sys.rs +++ b/crates/ecstore/src/bucket/metadata_sys.rs @@ -655,6 +655,12 @@ pub struct BucketMetadataMutationGuard { } impl BucketMetadataMutationGuard { + /// Returns the storage-verified identity while both incarnation fences remain valid. + pub fn checked_bucket_incarnation(&self) -> Result<(&str, Uuid)> { + self.ensure_valid(&self.bucket)?; + Ok((&self.bucket, self.incarnation_id)) + } + fn ensure_valid(&self, bucket: &str) -> Result<()> { if self.bucket != bucket { return Err(Error::other("bucket metadata mutation guard does not match bucket")); @@ -674,6 +680,29 @@ async fn acquire_config_write_guard_for_incarnation( sys: Arc>, bucket: &str, expected_incarnation_id: Option, +) -> Result { + acquire_config_write_guard_with_migration(sys, bucket, expected_incarnation_id, true).await +} + +/// Scanner probes must not create an incarnation to make a capability available. +pub async fn acquire_scanner_bucket_incarnation_fence( + bucket: &str, + expected_incarnation_id: Uuid, + expected_owner_id: Uuid, +) -> Result { + super::utils::check_valid_bucket_name(bucket)?; + let sys = get_bucket_metadata_sys()?; + if expected_owner_id.is_nil() || sys.read().await.api.id != expected_owner_id || expected_incarnation_id.is_nil() { + return Err(Error::other("scanner bucket incarnation owner does not match")); + } + acquire_config_write_guard_with_migration(sys, bucket, Some(expected_incarnation_id), false).await +} + +async fn acquire_config_write_guard_with_migration( + sys: Arc>, + bucket: &str, + expected_incarnation_id: Option, + migrate: bool, ) -> Result { let metadata_sys = sys.read().await.clone(); let lifecycle_guard = metadata_sys.api.acquire_bucket_lifecycle_read_lock(bucket).await?; @@ -681,13 +710,15 @@ async fn acquire_config_write_guard_for_incarnation( // Legacy buckets are migrated while the lifecycle fence prevents a // same-name replacement. The second read under the write transaction is // the CAS source of truth for the actual rewrite. - await_bucket_namespace_operation( - Some(&lifecycle_guard), - bucket, - "bucket config incarnation migration", - metadata_sys.get_bucket_incarnation_id(bucket), - ) - .await?; + if migrate { + await_bucket_namespace_operation( + Some(&lifecycle_guard), + bucket, + "bucket config incarnation migration", + metadata_sys.get_bucket_incarnation_id(bucket), + ) + .await?; + } let transaction_guard = await_bucket_namespace_operation( Some(&lifecycle_guard), bucket, @@ -3176,6 +3207,82 @@ mod tests { ); } + #[tokio::test] + async fn scoped_dirty_usage_incarnation_probe_does_not_migrate_legacy_metadata() { + let (dirs, store) = isolated_store_over_temp_disks().await; + let sys = Arc::new(RwLock::new(BucketMetadataSys::new(store.clone()))); + let bucket = "scoped-ack-legacy"; + for dir in &dirs { + std::fs::create_dir_all(dir.path().join(bucket)).expect("create legacy bucket"); + } + let mut metadata = BucketMetadata::new(bucket); + metadata.bucket_incarnation_id = Uuid::nil(); + sys.read() + .await + .persist_and_set(metadata) + .await + .expect("persist legacy metadata"); + assert!( + acquire_config_write_guard_with_migration(sys.clone(), bucket, Some(Uuid::new_v4()), false) + .await + .is_err() + ); + assert!(load_bucket_incarnation(store, bucket).await.expect("read sidecar").is_none()); + assert!( + sys.read() + .await + .get_config_from_disk(bucket) + .await + .expect("read metadata") + .bucket_incarnation_id + .is_nil() + ); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + #[serial] + async fn scoped_dirty_usage_incarnation_rejects_deleted_and_recreated_bucket() { + let (_dirs, store) = isolated_store_over_temp_disks().await; + init_bucket_metadata_sys(store.clone(), Vec::new()).await; + let sys = bucket_metadata_sys_of(&store.ctx).expect("metadata owner"); + let bucket = "scoped-ack-recreated"; + store + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("create bucket"); + let old = store.bucket_incarnation_id_from_disk(bucket).await.expect("old incarnation"); + let guard = acquire_config_write_guard_with_migration(sys.clone(), bucket, Some(old), false) + .await + .expect("trusted incarnation fence"); + assert_eq!(guard.checked_bucket_incarnation().expect("valid fences"), (bucket, old)); + drop(guard); + store + .delete_bucket(bucket, &DeleteBucketOptions::default()) + .await + .expect("delete bucket"); + assert!( + acquire_config_write_guard_with_migration(sys.clone(), bucket, Some(old), false) + .await + .is_err() + ); + store + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("recreate bucket"); + let new = store.bucket_incarnation_id_from_disk(bucket).await.expect("new incarnation"); + assert_ne!(old, new); + assert!( + acquire_config_write_guard_with_migration(sys.clone(), bucket, Some(old), false) + .await + .is_err() + ); + assert!( + acquire_config_write_guard_with_migration(sys, bucket, Some(new), false) + .await + .is_ok() + ); + } + #[tokio::test] async fn old_node_metadata_rewrite_cannot_replace_bucket_incarnation_sidecar() { let (dirs, ecstore) = isolated_store_over_temp_disks().await; diff --git a/crates/ecstore/src/cluster/rpc/client.rs b/crates/ecstore/src/cluster/rpc/client.rs index 435cbf64c..c4bf786f9 100644 --- a/crates/ecstore/src/cluster/rpc/client.rs +++ b/crates/ecstore/src/cluster/rpc/client.rs @@ -30,6 +30,7 @@ use rustfs_protos::{ ChannelClass, create_new_channel, get_channel_for_class, proto_gen::node_service::{ heal_control_service_client::HealControlServiceClient, node_service_client::NodeServiceClient, + scanner_control_service_client::ScannerControlServiceClient, tier_mutation_control_service_client::TierMutationControlServiceClient, }, }; @@ -60,6 +61,24 @@ pub async fn node_service_time_out_client( node_service_time_out_client_for_class(addr, interceptor, ChannelClass::Control).await } +pub(crate) async fn scanner_control_time_out_client( + addr: &str, + interceptor: TonicInterceptor, +) -> crate::error::Result>> { + let interceptor = interceptor.with_rpc_audience(addr)?; + let channel = match runtime_sources::cached_node_channel(addr).await { + Some(channel) => channel, + None => create_new_channel(addr) + .await + .map_err(|err| crate::error::Error::other(err.to_string()))?, + }; + let channel = ReplayScopeChannel::new(channel, interceptor.replay_scope_audience()); + let limit = rustfs_protos::scoped_dirty_usage::SCOPED_DIRTY_USAGE_MAX_REQUEST_BYTES as usize; + Ok(ScannerControlServiceClient::with_interceptor(channel, interceptor) + .max_decoding_message_size(limit) + .max_encoding_message_size(limit)) +} + pub async fn heal_control_time_out_client( addr: &str, interceptor: TonicInterceptor, diff --git a/crates/ecstore/src/cluster/rpc/peer_rest_client.rs b/crates/ecstore/src/cluster/rpc/peer_rest_client.rs index 5316199e3..9d57c4647 100644 --- a/crates/ecstore/src/cluster/rpc/peer_rest_client.rs +++ b/crates/ecstore/src/cluster/rpc/peer_rest_client.rs @@ -2050,6 +2050,53 @@ impl PeerRestClient { .await } + /// Probe only: scoped ACK production requires a durable per-bucket proof. + pub async fn scanner_scoped_dirty_usage_capability( + &self, + owner_id: String, + instance_id: String, + entries: Vec, + ) -> Result { + use rustfs_protos::scoped_dirty_usage::*; + let payload = rustfs_protos::proto_gen::node_service::ScannerScopedDirtyUsageAckRequest { + challenge: Uuid::new_v4().as_bytes().to_vec().into(), + protocol_version: SCOPED_DIRTY_USAGE_PROTOCOL_VERSION, + owner_id, + instance_id, + scope: SCOPED_DIRTY_USAGE_BUCKET_SCOPE, + probe_only: true, + entries, + }; + let canonical = canonical_scoped_dirty_usage_request(&payload).map_err(|err| Error::other(err.to_string()))?; + self.finalize_result( + async { + let mut client = super::client::scanner_control_time_out_client( + &self.grid_host, + TonicInterceptor::Signature(gen_tonic_signature_interceptor()), + ) + .await?; + let mut request = Request::new(payload.clone()); + set_tonic_canonical_body_digest(&mut request, &canonical)?; + let response = client.scanner_scoped_dirty_usage_ack(request).await?.into_inner(); + let body = canonical_scoped_dirty_usage_response(&canonical, &response) + .map_err(|_| Error::other("scoped dirty usage capability response is too large"))?; + verify_tonic_rpc_response_proof(&body, response.response_proof.as_ref())?; + if response.protocol_version != SCOPED_DIRTY_USAGE_PROTOCOL_VERSION + || response.owner_id != payload.owner_id + || response.instance_id != payload.instance_id + || response.max_entries != SCOPED_DIRTY_USAGE_MAX_ENTRIES + || response.max_request_bytes != SCOPED_DIRTY_USAGE_MAX_REQUEST_BYTES + || response.cleared != 0 + { + return Err(Error::other("scoped dirty usage capability response does not match request")); + } + Ok(response.supported) + } + .await, + ) + .await + } + pub async fn acknowledge_scanner_dirty_usage(&self, instance_id: String, generation: u64) -> Result { let result = self .scanner_activity_request_with_protocol(instance_id.clone(), generation, SCANNER_ACTIVITY_PROTOCOL_VERSION) diff --git a/crates/protos/src/generated/proto_gen/node_service.rs b/crates/protos/src/generated/proto_gen/node_service.rs index f015305f0..fe04d93ab 100644 --- a/crates/protos/src/generated/proto_gen/node_service.rs +++ b/crates/protos/src/generated/proto_gen/node_service.rs @@ -1283,6 +1283,54 @@ pub struct ScannerDirtyUsageSnapshotResponse { #[prost(bytes = "bytes", tag = "7")] pub response_proof: ::prost::bytes::Bytes, } +/// Receiver-only protocol. Producers must retain whole-cycle ACK until they +/// have a durable per-bucket publication proof. +#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] +pub struct ScannerScopedDirtyUsageEntry { + #[prost(string, tag = "1")] + pub bucket: ::prost::alloc::string::String, + #[prost(bytes = "bytes", tag = "2")] + pub bucket_incarnation: ::prost::bytes::Bytes, + #[prost(uint64, tag = "3")] + pub generation: u64, +} +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct ScannerScopedDirtyUsageAckRequest { + #[prost(bytes = "bytes", tag = "1")] + pub challenge: ::prost::bytes::Bytes, + #[prost(uint32, tag = "2")] + pub protocol_version: u32, + #[prost(string, tag = "3")] + pub owner_id: ::prost::alloc::string::String, + #[prost(string, tag = "4")] + pub instance_id: ::prost::alloc::string::String, + /// Only scope 1 (a complete bucket) is supported; zero is invalid. + #[prost(uint32, tag = "5")] + pub scope: u32, + #[prost(bool, tag = "6")] + pub probe_only: bool, + #[prost(message, repeated, tag = "7")] + pub entries: ::prost::alloc::vec::Vec, +} +#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] +pub struct ScannerScopedDirtyUsageAckResponse { + #[prost(uint32, tag = "1")] + pub protocol_version: u32, + #[prost(string, tag = "2")] + pub owner_id: ::prost::alloc::string::String, + #[prost(string, tag = "3")] + pub instance_id: ::prost::alloc::string::String, + #[prost(bool, tag = "4")] + pub supported: bool, + #[prost(uint32, tag = "5")] + pub max_entries: u32, + #[prost(uint32, tag = "6")] + pub max_request_bytes: u32, + #[prost(uint64, tag = "7")] + pub cleared: u64, + #[prost(bytes = "bytes", tag = "8")] + pub response_proof: ::prost::bytes::Bytes, +} /// A short-lived storage-owned read admission used only around a final /// scanner metadata publication. It is intentionally separate from the /// ScannerActivity observation wire so v6/v7 rolling compatibility remains @@ -6282,6 +6330,244 @@ pub mod node_service_server { } } /// Generated client implementations. +pub mod scanner_control_service_client { + #![allow(unused_variables, dead_code, missing_docs, clippy::wildcard_imports, clippy::let_unit_value)] + use tonic::codegen::http::Uri; + use tonic::codegen::*; + #[derive(Debug, Clone)] + pub struct ScannerControlServiceClient { + inner: tonic::client::Grpc, + } + impl ScannerControlServiceClient { + /// Attempt to create a new client by connecting to a given endpoint. + pub async fn connect(dst: D) -> Result + where + D: TryInto, + D::Error: Into, + { + let conn = tonic::transport::Endpoint::new(dst)?.connect().await?; + Ok(Self::new(conn)) + } + } + impl ScannerControlServiceClient + where + T: tonic::client::GrpcService, + T::Error: Into, + T::ResponseBody: Body + std::marker::Send + 'static, + ::Error: Into + std::marker::Send, + { + pub fn new(inner: T) -> Self { + let inner = tonic::client::Grpc::new(inner); + Self { inner } + } + pub fn with_origin(inner: T, origin: Uri) -> Self { + let inner = tonic::client::Grpc::with_origin(inner, origin); + Self { inner } + } + pub fn with_interceptor(inner: T, interceptor: F) -> ScannerControlServiceClient> + where + F: tonic::service::Interceptor, + T::ResponseBody: Default, + T: tonic::codegen::Service< + http::Request, + Response = http::Response<>::ResponseBody>, + >, + >>::Error: + Into + std::marker::Send + std::marker::Sync, + { + ScannerControlServiceClient::new(InterceptedService::new(inner, interceptor)) + } + /// Compress requests with the given encoding. + /// + /// This requires the server to support it otherwise it might respond with an + /// error. + #[must_use] + pub fn send_compressed(mut self, encoding: CompressionEncoding) -> Self { + self.inner = self.inner.send_compressed(encoding); + self + } + /// Enable decompressing responses. + #[must_use] + pub fn accept_compressed(mut self, encoding: CompressionEncoding) -> Self { + self.inner = self.inner.accept_compressed(encoding); + self + } + /// Limits the maximum size of a decoded message. + /// + /// Default: `4MB` + #[must_use] + pub fn max_decoding_message_size(mut self, limit: usize) -> Self { + self.inner = self.inner.max_decoding_message_size(limit); + self + } + /// Limits the maximum size of an encoded message. + /// + /// Default: `usize::MAX` + #[must_use] + pub fn max_encoding_message_size(mut self, limit: usize) -> Self { + self.inner = self.inner.max_encoding_message_size(limit); + self + } + pub async fn scanner_scoped_dirty_usage_ack( + &mut self, + request: impl tonic::IntoRequest, + ) -> std::result::Result, tonic::Status> { + self.inner + .ready() + .await + .map_err(|e| tonic::Status::unknown(format!("Service was not ready: {}", e.into())))?; + let codec = tonic_prost::ProstCodec::default(); + let path = http::uri::PathAndQuery::from_static("/node_service.ScannerControlService/ScannerScopedDirtyUsageAck"); + let mut req = request.into_request(); + req.extensions_mut() + .insert(GrpcMethod::new("node_service.ScannerControlService", "ScannerScopedDirtyUsageAck")); + self.inner.unary(req, path, codec).await + } + } +} +/// Generated server implementations. +pub mod scanner_control_service_server { + #![allow(unused_variables, dead_code, missing_docs, clippy::wildcard_imports, clippy::let_unit_value)] + use tonic::codegen::*; + /// Generated trait containing gRPC methods that should be implemented for use with ScannerControlServiceServer. + #[async_trait] + pub trait ScannerControlService: std::marker::Send + std::marker::Sync + 'static { + async fn scanner_scoped_dirty_usage_ack( + &self, + request: tonic::Request, + ) -> std::result::Result, tonic::Status>; + } + #[derive(Debug)] + pub struct ScannerControlServiceServer { + inner: Arc, + accept_compression_encodings: EnabledCompressionEncodings, + send_compression_encodings: EnabledCompressionEncodings, + max_decoding_message_size: Option, + max_encoding_message_size: Option, + } + impl ScannerControlServiceServer { + pub fn new(inner: T) -> Self { + Self::from_arc(Arc::new(inner)) + } + pub fn from_arc(inner: Arc) -> Self { + Self { + inner, + accept_compression_encodings: Default::default(), + send_compression_encodings: Default::default(), + max_decoding_message_size: None, + max_encoding_message_size: None, + } + } + pub fn with_interceptor(inner: T, interceptor: F) -> InterceptedService + where + F: tonic::service::Interceptor, + { + InterceptedService::new(Self::new(inner), interceptor) + } + /// Enable decompressing requests with the given encoding. + #[must_use] + pub fn accept_compressed(mut self, encoding: CompressionEncoding) -> Self { + self.accept_compression_encodings.enable(encoding); + self + } + /// Compress responses with the given encoding, if the client supports it. + #[must_use] + pub fn send_compressed(mut self, encoding: CompressionEncoding) -> Self { + self.send_compression_encodings.enable(encoding); + self + } + /// Limits the maximum size of a decoded message. + /// + /// Default: `4MB` + #[must_use] + pub fn max_decoding_message_size(mut self, limit: usize) -> Self { + self.max_decoding_message_size = Some(limit); + self + } + /// Limits the maximum size of an encoded message. + /// + /// Default: `usize::MAX` + #[must_use] + pub fn max_encoding_message_size(mut self, limit: usize) -> Self { + self.max_encoding_message_size = Some(limit); + self + } + } + impl tonic::codegen::Service> for ScannerControlServiceServer + where + T: ScannerControlService, + B: Body + std::marker::Send + 'static, + B::Error: Into + std::marker::Send + 'static, + { + type Response = http::Response; + type Error = std::convert::Infallible; + type Future = BoxFuture; + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + fn call(&mut self, req: http::Request) -> Self::Future { + match req.uri().path() { + "/node_service.ScannerControlService/ScannerScopedDirtyUsageAck" => { + #[allow(non_camel_case_types)] + struct ScannerScopedDirtyUsageAckSvc(pub Arc); + impl tonic::server::UnaryService + for ScannerScopedDirtyUsageAckSvc + { + type Response = super::ScannerScopedDirtyUsageAckResponse; + type Future = BoxFuture, tonic::Status>; + fn call(&mut self, request: tonic::Request) -> Self::Future { + let inner = Arc::clone(&self.0); + let fut = async move { + ::scanner_scoped_dirty_usage_ack(&inner, request).await + }; + Box::pin(fut) + } + } + let accept_compression_encodings = self.accept_compression_encodings; + let send_compression_encodings = self.send_compression_encodings; + let max_decoding_message_size = self.max_decoding_message_size; + let max_encoding_message_size = self.max_encoding_message_size; + let inner = self.inner.clone(); + let fut = async move { + let method = ScannerScopedDirtyUsageAckSvc(inner); + let codec = tonic_prost::ProstCodec::default(); + let mut grpc = tonic::server::Grpc::new(codec) + .apply_compression_config(accept_compression_encodings, send_compression_encodings) + .apply_max_message_size_config(max_decoding_message_size, max_encoding_message_size); + let res = grpc.unary(method, req).await; + Ok(res) + }; + Box::pin(fut) + } + _ => Box::pin(async move { + let mut response = http::Response::new(tonic::body::Body::default()); + let headers = response.headers_mut(); + headers.insert(tonic::Status::GRPC_STATUS, (tonic::Code::Unimplemented as i32).into()); + headers.insert(http::header::CONTENT_TYPE, tonic::metadata::GRPC_CONTENT_TYPE); + Ok(response) + }), + } + } + } + impl Clone for ScannerControlServiceServer { + fn clone(&self) -> Self { + let inner = self.inner.clone(); + Self { + inner, + accept_compression_encodings: self.accept_compression_encodings, + send_compression_encodings: self.send_compression_encodings, + max_decoding_message_size: self.max_decoding_message_size, + max_encoding_message_size: self.max_encoding_message_size, + } + } + } + /// Generated gRPC service name + pub const SERVICE_NAME: &str = "node_service.ScannerControlService"; + impl tonic::server::NamedService for ScannerControlServiceServer { + const NAME: &'static str = SERVICE_NAME; + } +} +/// Generated client implementations. pub mod heal_control_service_client { #![allow(unused_variables, dead_code, missing_docs, clippy::wildcard_imports, clippy::let_unit_value)] use tonic::codegen::http::Uri; diff --git a/crates/protos/src/lib.rs b/crates/protos/src/lib.rs index fd8704382..81ead7d65 100644 --- a/crates/protos/src/lib.rs +++ b/crates/protos/src/lib.rs @@ -541,6 +541,8 @@ pub fn canonical_scanner_activity_v7_response_body( Ok(body) } +pub mod scoped_dirty_usage; + pub fn canonical_scanner_dirty_usage_snapshot_request_body( request: &proto_gen::node_service::ScannerDirtyUsageSnapshotRequest, ) -> Result, std::num::TryFromIntError> { diff --git a/crates/protos/src/node.proto b/crates/protos/src/node.proto index d3796991c..abc030adc 100644 --- a/crates/protos/src/node.proto +++ b/crates/protos/src/node.proto @@ -903,6 +903,36 @@ message ScannerDirtyUsageSnapshotResponse { bytes response_proof = 7; } +// Receiver-only protocol. Producers must retain whole-cycle ACK until they +// have a durable per-bucket publication proof. +message ScannerScopedDirtyUsageEntry { + string bucket = 1; + bytes bucket_incarnation = 2; + uint64 generation = 3; +} + +message ScannerScopedDirtyUsageAckRequest { + bytes challenge = 1; + uint32 protocol_version = 2; + string owner_id = 3; + string instance_id = 4; + // Only scope 1 (a complete bucket) is supported; zero is invalid. + uint32 scope = 5; + bool probe_only = 6; + repeated ScannerScopedDirtyUsageEntry entries = 7; +} + +message ScannerScopedDirtyUsageAckResponse { + uint32 protocol_version = 1; + string owner_id = 2; + string instance_id = 3; + bool supported = 4; + uint32 max_entries = 5; + uint32 max_request_bytes = 6; + uint64 cleared = 7; + bytes response_proof = 8; +} + // A short-lived storage-owned read admission used only around a final // scanner metadata publication. It is intentionally separate from the // ScannerActivity observation wire so v6/v7 rolling compatibility remains @@ -1245,6 +1275,10 @@ service NodeService { rpc GetLiveEvents(GetLiveEventsRequest) returns (GetLiveEventsResponse) {}; // auth-policy: read-only } +service ScannerControlService { + rpc ScannerScopedDirtyUsageAck(ScannerScopedDirtyUsageAckRequest) returns (ScannerScopedDirtyUsageAckResponse) {}; // auth-policy: body-bound +} + service HealControlService { rpc HealControl(HealControlRequest) returns (HealControlResponse) {}; } diff --git a/crates/protos/src/scoped_dirty_usage.rs b/crates/protos/src/scoped_dirty_usage.rs new file mode 100644 index 000000000..ac3effaf7 --- /dev/null +++ b/crates/protos/src/scoped_dirty_usage.rs @@ -0,0 +1,213 @@ +// Copyright 2024 RustFS Team +// Licensed under the Apache License, Version 2.0. + +//! Bounded, authenticated receiver contract for per-bucket dirty acknowledgements. + +use crate::CanonicalBodyBuilder; +use crate::proto_gen::node_service::{ScannerScopedDirtyUsageAckRequest, ScannerScopedDirtyUsageAckResponse}; +use prost::Message; + +pub const SCOPED_DIRTY_USAGE_PROTOCOL_VERSION: u32 = 1; +pub const SCOPED_DIRTY_USAGE_BUCKET_SCOPE: u32 = 1; +pub const SCOPED_DIRTY_USAGE_MAX_ENTRIES: u32 = 32; +pub const SCOPED_DIRTY_USAGE_MAX_REQUEST_BYTES: u32 = 8192; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ScopedDirtyUsageRequestError { + UnsupportedProtocol, + UnsupportedScope, + InvalidIdentity, + InvalidGeneration, + InvalidEntries, + TooLarge, +} + +impl std::fmt::Display for ScopedDirtyUsageRequestError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::UnsupportedProtocol => "unsupported scoped dirty usage protocol", + Self::UnsupportedScope => "unsupported scoped dirty usage scope", + Self::InvalidIdentity => "invalid scoped dirty usage identity", + Self::InvalidGeneration => "invalid scoped dirty usage generation", + Self::InvalidEntries => "scoped dirty usage entries must be nonempty and strictly ordered", + Self::TooLarge => "scoped dirty usage request exceeds its budget", + }) + } +} + +impl std::error::Error for ScopedDirtyUsageRequestError {} + +pub fn validate_scoped_dirty_usage_request( + request: &ScannerScopedDirtyUsageAckRequest, +) -> Result<(), ScopedDirtyUsageRequestError> { + use ScopedDirtyUsageRequestError as E; + if request.entries.len() > SCOPED_DIRTY_USAGE_MAX_ENTRIES as usize + || request.encoded_len() > SCOPED_DIRTY_USAGE_MAX_REQUEST_BYTES as usize + { + return Err(E::TooLarge); + } + if request.protocol_version != SCOPED_DIRTY_USAGE_PROTOCOL_VERSION { + return Err(E::UnsupportedProtocol); + } + if request.scope != SCOPED_DIRTY_USAGE_BUCKET_SCOPE { + return Err(E::UnsupportedScope); + } + if request.challenge.len() != 16 || request.owner_id.len() != 36 || request.instance_id.len() != 32 { + return Err(E::InvalidIdentity); + } + if request.entries.is_empty() || request.entries.windows(2).any(|pair| pair[0].bucket >= pair[1].bucket) { + return Err(E::InvalidEntries); + } + for entry in &request.entries { + if entry.bucket.is_empty() + || entry.bucket.len() > 63 + || entry.bucket_incarnation.len() != 16 + || entry.bucket_incarnation.iter().all(|byte| *byte == 0) + { + return Err(E::InvalidIdentity); + } + if entry.generation == 0 || entry.generation == u64::MAX { + return Err(E::InvalidGeneration); + } + } + Ok(()) +} + +pub fn canonical_scoped_dirty_usage_request( + request: &ScannerScopedDirtyUsageAckRequest, +) -> Result, ScopedDirtyUsageRequestError> { + validate_scoped_dirty_usage_request(request)?; + let mut body = CanonicalBodyBuilder::new(b"rustfs-scoped-dirty-usage-ack-request-v1\0"); + let encode = |_: std::num::TryFromIntError| ScopedDirtyUsageRequestError::TooLarge; + body.push_bytes(request.challenge.as_ref()).map_err(encode)?; + body.push_u32(request.protocol_version); + body.push_str(&request.owner_id).map_err(encode)?; + body.push_str(&request.instance_id).map_err(encode)?; + body.push_u32(request.scope); + body.push_bool(request.probe_only); + body.push_count(request.entries.len()).map_err(encode)?; + for entry in &request.entries { + body.push_str(&entry.bucket).map_err(encode)?; + body.push_bytes(entry.bucket_incarnation.as_ref()).map_err(encode)?; + body.push_u64(entry.generation); + } + Ok(body.finish()) +} + +pub fn canonical_scoped_dirty_usage_response( + request_body: &[u8], + response: &ScannerScopedDirtyUsageAckResponse, +) -> Result, std::num::TryFromIntError> { + let mut body = CanonicalBodyBuilder::new(b"rustfs-scoped-dirty-usage-ack-response-v1\0"); + body.push_bytes(request_body)?; + body.push_u32(response.protocol_version); + body.push_str(&response.owner_id)?; + body.push_str(&response.instance_id)?; + body.push_bool(response.supported); + body.push_u32(response.max_entries); + body.push_u32(response.max_request_bytes); + body.push_u64(response.cleared); + Ok(body.finish()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::proto_gen::node_service::ScannerScopedDirtyUsageEntry; + + fn request() -> ScannerScopedDirtyUsageAckRequest { + ScannerScopedDirtyUsageAckRequest { + challenge: vec![1; 16].into(), + protocol_version: 1, + owner_id: "11111111-1111-1111-1111-111111111111".into(), + instance_id: "a".repeat(32), + scope: 1, + probe_only: false, + entries: vec![ScannerScopedDirtyUsageEntry { + bucket: "photos".into(), + bucket_incarnation: vec![2; 16].into(), + generation: 8, + }], + } + } + + #[test] + fn scoped_dirty_usage_binds_every_request_field() { + let base = request(); + let baseline = canonical_scoped_dirty_usage_request(&base).expect("valid request"); + for field in 0..9 { + let mut changed = base.clone(); + match field { + 0 => changed.challenge = vec![3; 16].into(), + 1 => changed.protocol_version += 1, + 2 => changed.owner_id = "22222222-2222-2222-2222-222222222222".into(), + 3 => changed.instance_id = "b".repeat(32), + 4 => changed.scope += 1, + 5 => changed.probe_only = true, + 6 => changed.entries[0].bucket = "videos".into(), + 7 => changed.entries[0].bucket_incarnation = vec![3; 16].into(), + _ => changed.entries[0].generation += 1, + } + assert!(canonical_scoped_dirty_usage_request(&changed).map_or(true, |body| body != baseline)); + } + } + + #[test] + fn scoped_dirty_usage_binds_capability_and_ack_to_exact_request() { + let request = canonical_scoped_dirty_usage_request(&request()).expect("valid request"); + let response = ScannerScopedDirtyUsageAckResponse { + protocol_version: 1, + owner_id: "owner".into(), + instance_id: "process".into(), + supported: true, + max_entries: 32, + max_request_bytes: 8192, + cleared: 1, + response_proof: vec![1; 32].into(), + }; + let baseline = canonical_scoped_dirty_usage_response(&request, &response).expect("valid response"); + for field in 0..7 { + let mut changed = response.clone(); + match field { + 0 => changed.protocol_version += 1, + 1 => changed.owner_id.push('x'), + 2 => changed.instance_id.push('x'), + 3 => changed.supported = false, + 4 => changed.max_entries += 1, + 5 => changed.max_request_bytes += 1, + _ => changed.cleared += 1, + } + assert_ne!( + canonical_scoped_dirty_usage_response(&request, &changed).expect("response variant"), + baseline + ); + } + assert_ne!( + canonical_scoped_dirty_usage_response(b"another request", &response).expect("request variant"), + baseline + ); + } + + #[test] + fn scoped_dirty_usage_rejects_overflow_unknown_and_duplicate_entries() { + let base = request(); + let mut invalid = base.clone(); + invalid.entries = vec![base.entries[0].clone(); SCOPED_DIRTY_USAGE_MAX_ENTRIES as usize + 1]; + assert_eq!(validate_scoped_dirty_usage_request(&invalid), Err(ScopedDirtyUsageRequestError::TooLarge)); + invalid = base.clone(); + invalid.entries[0].bucket = "x".repeat(SCOPED_DIRTY_USAGE_MAX_REQUEST_BYTES as usize); + assert_eq!(validate_scoped_dirty_usage_request(&invalid), Err(ScopedDirtyUsageRequestError::TooLarge)); + invalid = base.clone(); + invalid.entries.push(base.entries[0].clone()); + assert_eq!( + validate_scoped_dirty_usage_request(&invalid), + Err(ScopedDirtyUsageRequestError::InvalidEntries) + ); + invalid = base; + invalid.entries[0].bucket_incarnation = vec![0; 16].into(); + assert_eq!( + validate_scoped_dirty_usage_request(&invalid), + Err(ScopedDirtyUsageRequestError::InvalidIdentity) + ); + } +} diff --git a/crates/scanner/src/lib.rs b/crates/scanner/src/lib.rs index f85a2e0b6..28cf638e9 100644 --- a/crates/scanner/src/lib.rs +++ b/crates/scanner/src/lib.rs @@ -90,8 +90,9 @@ pub use scanner::{ }; pub use scanner_io::{ ScannerDirtyUsageAckError, ScannerDirtyUsageBucket, ScannerDirtyUsageSnapshot, ScannerDirtyUsageState, - acknowledge_dirty_usage_generation, clear_dirty_usage_bucket, record_dirty_usage_bucket, record_scanner_maintenance_change, - scanner_activity_epoch, scanner_dirty_usage_snapshot, scanner_dirty_usage_state, scanner_maintenance_generation, + acknowledge_dirty_usage_generation, acknowledge_scoped_dirty_usage, clear_dirty_usage_bucket, record_dirty_usage_bucket, + record_scanner_maintenance_change, scanner_activity_epoch, scanner_dirty_usage_snapshot, scanner_dirty_usage_state, + scanner_maintenance_generation, }; pub use sleeper::{DynamicSleeper, SCANNER_IDLE_MODE, SCANNER_SLEEPER}; use std::sync::atomic::{AtomicU64, Ordering}; diff --git a/crates/scanner/src/scanner_io.rs b/crates/scanner/src/scanner_io.rs index c8353bae9..0d5e82eeb 100644 --- a/crates/scanner/src/scanner_io.rs +++ b/crates/scanner/src/scanner_io.rs @@ -883,8 +883,9 @@ pub(crate) use cache::{ }; pub use dirty_usage::{ ScannerDirtyUsageAckError, ScannerDirtyUsageBucket, ScannerDirtyUsageSnapshot, ScannerDirtyUsageState, - acknowledge_dirty_usage_generation, clear_dirty_usage_bucket, record_dirty_usage_bucket, record_scanner_maintenance_change, - scanner_activity_epoch, scanner_dirty_usage_snapshot, scanner_dirty_usage_state, scanner_maintenance_generation, + acknowledge_dirty_usage_generation, acknowledge_scoped_dirty_usage, clear_dirty_usage_bucket, record_dirty_usage_bucket, + record_scanner_maintenance_change, scanner_activity_epoch, scanner_dirty_usage_snapshot, scanner_dirty_usage_state, + scanner_maintenance_generation, }; #[cfg(test)] pub(crate) use dirty_usage::{clear_dirty_usage_buckets_for_tests, dirty_usage_buckets_for_tests}; diff --git a/crates/scanner/src/scanner_io/dirty_usage.rs b/crates/scanner/src/scanner_io/dirty_usage.rs index a5978e263..18db78a81 100644 --- a/crates/scanner/src/scanner_io/dirty_usage.rs +++ b/crates/scanner/src/scanner_io/dirty_usage.rs @@ -52,6 +52,112 @@ pub enum ScannerDirtyUsageAckError { ProcessChanged, #[error("scanner dirty usage generation cannot be acknowledged")] InvalidGeneration, + #[error("scanner dirty usage bucket incarnation fence is unavailable")] + IncarnationUnavailable, +} + +/// A scoped ACK requires storage-owned lifecycle and incarnation fences. +/// Callers must only send ACKs backed by durable per-bucket publication. +pub fn acknowledge_scoped_dirty_usage( + instance_id: &str, + entries: &[(&crate::storage_api::EcstoreBucketMetadataMutationGuard, u64)], + probe_only: bool, +) -> std::result::Result { + // Lock order: sorted bucket lifecycle/metadata fences (caller), then dirty map. + // No await or storage operation occurs while the dirty map is locked. + let (cleared, pending) = { + let mut dirty = dirty_usage_buckets(); + let checked = entries + .iter() + .map(|(guard, generation)| { + guard + .checked_bucket_incarnation() + .map(|(bucket, _)| (bucket, *generation)) + .map_err(|_| ScannerDirtyUsageAckError::IncarnationUnavailable) + }) + .collect::, _>>()?; + let cleared = apply_scoped_dirty_usage_ack( + instance_id, + scanner_activity_epoch(), + DIRTY_USAGE_BUCKET_GENERATION.load(Ordering::Acquire), + &mut dirty, + &checked, + probe_only, + )?; + if cleared > 0 { + advance_generation(&DIRTY_USAGE_BUCKET_GENERATION); + } + (cleared, dirty.len()) + }; + if !probe_only { + global_metrics().record_scanner_dirty_usage_cycle_clear(usize_to_u64_saturated(cleared), usize_to_u64_saturated(pending)); + } + Ok(usize_to_u64_saturated(cleared)) +} + +fn apply_scoped_dirty_usage_ack( + instance_id: &str, + current_instance: &str, + current_generation: u64, + dirty: &mut DirtyUsageBuckets, + entries: &[(&str, u64)], + probe_only: bool, +) -> std::result::Result { + if instance_id != current_instance { + return Err(ScannerDirtyUsageAckError::ProcessChanged); + } + if current_generation == u64::MAX + || entries + .iter() + .any(|(_, generation)| *generation == 0 || *generation == u64::MAX || *generation > current_generation) + { + return Err(ScannerDirtyUsageAckError::InvalidGeneration); + } + let mut cleared = 0; + if !probe_only { + for (bucket, generation) in entries { + if dirty.get(*bucket) == Some(generation) { + dirty.remove(*bucket); + cleared += 1; + } + } + } + Ok(cleared) +} + +#[cfg(test)] +mod scoped_dirty_usage_tests { + use super::*; + + #[test] + fn scoped_dirty_usage_preserves_uncovered_newer_and_replayed_generations() { + let mut dirty = HashMap::from([("hot".to_string(), 7), ("cold".to_string(), 8)]); + assert_eq!(apply_scoped_dirty_usage_ack("p", "p", 8, &mut dirty, &[("cold", 8)], true), Ok(0)); + assert_eq!(dirty.len(), 2); + assert_eq!(apply_scoped_dirty_usage_ack("p", "p", 8, &mut dirty, &[("cold", 8)], false), Ok(1)); + assert_eq!(dirty.get("hot"), Some(&7)); + assert_eq!(apply_scoped_dirty_usage_ack("p", "p", 8, &mut dirty, &[("cold", 8)], false), Ok(0)); + dirty.insert("cold".to_string(), 9); + assert_eq!(apply_scoped_dirty_usage_ack("p", "p", 9, &mut dirty, &[("cold", 8)], false), Ok(0)); + assert_eq!(dirty.get("cold"), Some(&9)); + } + + #[test] + fn scoped_dirty_usage_rejects_restart_and_invalid_batch_before_clearing() { + let original = HashMap::from([("hot".to_string(), 7), ("cold".to_string(), 8)]); + let mut dirty = original.clone(); + assert_eq!( + apply_scoped_dirty_usage_ack("old", "new", 8, &mut dirty, &[("cold", 8)], false), + Err(ScannerDirtyUsageAckError::ProcessChanged) + ); + for generation in [0, 9, u64::MAX] { + assert_eq!( + apply_scoped_dirty_usage_ack("p", "p", 8, &mut dirty, &[("cold", 8), ("hot", generation)], false), + Err(ScannerDirtyUsageAckError::InvalidGeneration) + ); + assert_eq!(dirty, original); + } + } } pub(super) fn dirty_usage_buckets() -> MutexGuard<'static, DirtyUsageBuckets> { diff --git a/crates/scanner/src/storage_api.rs b/crates/scanner/src/storage_api.rs index f81065d59..7b2cda493 100644 --- a/crates/scanner/src/storage_api.rs +++ b/crates/scanner/src/storage_api.rs @@ -38,8 +38,8 @@ pub(crate) use rustfs_ecstore::api::bucket::lifecycle::lifecycle::object_opts_fr #[cfg(test)] pub(crate) use rustfs_ecstore::api::bucket::metadata_sys::init_bucket_metadata_sys as ecstore_init_bucket_metadata_sys; pub(crate) use rustfs_ecstore::api::bucket::metadata_sys::{ - get_lifecycle_config as ecstore_get_lifecycle_config, get_object_lock_config as ecstore_get_object_lock_config, - get_replication_config as ecstore_get_replication_config, + BucketMetadataMutationGuard as EcstoreBucketMetadataMutationGuard, get_lifecycle_config as ecstore_get_lifecycle_config, + get_object_lock_config as ecstore_get_object_lock_config, get_replication_config as ecstore_get_replication_config, }; pub(crate) use rustfs_ecstore::api::bucket::replication::{ ReplicateObjectInfo, ReplicationConfig as EcstoreReplicationConfig, diff --git a/rustfs/src/server/http.rs b/rustfs/src/server/http.rs index 78ce81e5b..d82d67f20 100644 --- a/rustfs/src/server/http.rs +++ b/rustfs/src/server/http.rs @@ -208,6 +208,7 @@ const EVENT_PEER_ADDR_UNAVAILABLE: &str = "peer_addr_unavailable"; const EVENT_RPC_SIGNATURE_VERIFICATION_FAILED: &str = "rpc_signature_verification_failed"; const EVENT_GRPC_TRACE_CONTEXT_PROPAGATION_FAILED: &str = "grpc_trace_context_propagation_failed"; const HEAL_CONTROL_TONIC_RPC_PATH: &str = "/node_service.HealControlService/HealControl"; +const SCANNER_SCOPED_DIRTY_USAGE_ACK_TONIC_RPC_PATH: &str = "/node_service.ScannerControlService/ScannerScopedDirtyUsageAck"; const TIER_MUTATION_PREPARE_TONIC_RPC_PATH: &str = "/node_service.TierMutationControlService/PrepareTierMutation"; const TIER_MUTATION_COMMIT_TONIC_RPC_PATH: &str = "/node_service.TierMutationControlService/CommitTierMutation"; const TIER_MUTATION_ABORT_TONIC_RPC_PATH: &str = "/node_service.TierMutationControlService/AbortTierMutation"; @@ -1856,6 +1857,7 @@ fn process_connection( ); let rpc_service = RpcRequestPathService::new( Routes::new(node_service) + .add_service(InterceptedService::new(storage::tonic_service::make_scanner_control_server(), check_auth)) .add_service(heal_control_service) .add_service(tier_mutation_control_service) .prepare(), @@ -2259,6 +2261,7 @@ fn check_auth(req: Request<()>) -> std::result::Result, Status> { .strip_prefix(TONIC_RPC_PREFIX) .and_then(|suffix| suffix.strip_prefix('/')) .or_else(|| (target.uri.path() == HEAL_CONTROL_TONIC_RPC_PATH).then_some("HealControl")) + .or_else(|| (target.uri.path() == SCANNER_SCOPED_DIRTY_USAGE_ACK_TONIC_RPC_PATH).then_some("ScannerScopedDirtyUsageAck")) .or_else(|| (target.uri.path() == TIER_MUTATION_PREPARE_TONIC_RPC_PATH).then_some("PrepareTierMutation")) .or_else(|| (target.uri.path() == TIER_MUTATION_COMMIT_TONIC_RPC_PATH).then_some("CommitTierMutation")) .or_else(|| (target.uri.path() == TIER_MUTATION_ABORT_TONIC_RPC_PATH).then_some("AbortTierMutation")) @@ -3427,6 +3430,50 @@ mod tests { rustfs_common::set_global_local_node_name(&previous_node_name).await; } + #[tokio::test] + #[serial_test::serial] + async fn scoped_dirty_usage_peer_probe_reaches_handler_through_production_auth() { + let _ = rustfs_credentials::set_global_rpc_secret("rpc-http-test-secret".to_string()); + let listener = TcpListener::bind("127.0.0.1:0").await.expect("bind scoped ACK auth test"); + let addr = listener.local_addr().expect("listener address"); + let previous_node_name = rustfs_common::get_global_local_node_name().await; + rustfs_common::set_global_local_node_name(&addr.to_string()).await; + let node = InterceptedService::new(NodeServiceServer::new(make_server()), check_auth); + let scanner = InterceptedService::new(storage::tonic_service::make_scanner_control_server(), check_auth); + let service = RpcRequestPathService::new(Routes::new(node).add_service(scanner).prepare()); + let server = tokio::spawn(async move { + let (socket, _) = listener.accept().await.expect("accept test connection"); + ConnBuilder::new(TokioExecutor::new()) + .serve_connection(TokioIo::new(socket), TowerToHyperService::new(service)) + .await + .expect("serve scoped ACK auth test"); + }); + let host = rustfs_utils::XHost::try_from(addr.to_string()).expect("peer address"); + let client = storage::PeerRestClient::new(host, format!("http://{addr}")); + let result = client + .scanner_scoped_dirty_usage_capability( + "11111111-1111-1111-1111-111111111111".to_string(), + "a".repeat(32), + vec![rustfs_protos::proto_gen::node_service::ScannerScopedDirtyUsageEntry { + bucket: "photos".into(), + bucket_incarnation: vec![1; 16].into(), + generation: 8, + }], + ) + .await; + client.evict_connection().await; + server.abort(); + let _ = server.await; + rustfs_common::set_global_local_node_name(&previous_node_name).await; + let error = result + .expect_err("probe must fail closed without the requested storage owner") + .to_string(); + assert!( + error.contains("storage layer is not initialized") || error.contains("scoped dirty usage peer or process changed"), + "signed probe must pass production path authentication and reach owner validation: {error}" + ); + } + #[tokio::test] #[serial_test::serial] async fn peer_rest_heal_control_uses_production_auth_and_keeps_validation_errors_online() { diff --git a/rustfs/src/storage/rpc/node_service.rs b/rustfs/src/storage/rpc/node_service.rs index 0dd18424e..111726af3 100644 --- a/rustfs/src/storage/rpc/node_service.rs +++ b/rustfs/src/storage/rpc/node_service.rs @@ -493,6 +493,13 @@ impl std::fmt::Debug for NodeService { } } +pub(crate) fn make_scanner_control_server() -> scanner_control_service_server::ScannerControlServiceServer { + let limit = rustfs_protos::scoped_dirty_usage::SCOPED_DIRTY_USAGE_MAX_REQUEST_BYTES as usize; + scanner_control_service_server::ScannerControlServiceServer::new(make_server()) + .max_decoding_message_size(limit) + .max_encoding_message_size(limit) +} + pub fn make_server() -> NodeService { let context = runtime_sources::current_app_context(); make_server_for_context(context) @@ -1087,6 +1094,74 @@ impl NodeService { } } +#[tonic::async_trait] +impl scanner_control_service_server::ScannerControlService for NodeService { + async fn scanner_scoped_dirty_usage_ack( + &self, + request: Request, + ) -> Result, Status> { + use rustfs_protos::scoped_dirty_usage::*; + static ADMISSION: tokio::sync::Semaphore = tokio::sync::Semaphore::const_new(4); + + let canonical = + canonical_scoped_dirty_usage_request(request.get_ref()).map_err(|err| Status::invalid_argument(err.to_string()))?; + verify_tonic_canonical_body_digest(&request, &canonical) + .map_err(|_| Status::permission_denied("scoped dirty usage authentication failed"))?; + let _admission = ADMISSION + .try_acquire() + .map_err(|_| Status::resource_exhausted("scoped dirty usage receiver is busy"))?; + let request = request.into_inner(); + let store = self + .resolve_object_store() + .ok_or_else(|| Status::unavailable("storage layer is not initialized"))?; + if store.id.is_nil() + || request.owner_id != store.id.to_string() + || request.instance_id != rustfs_scanner::scanner_activity_epoch() + { + return Err(Status::failed_precondition("scoped dirty usage peer or process changed")); + } + let cleared = timeout(Duration::from_secs(30), async { + // Strict bucket order is validated before admission. Acquire every + // lifecycle/metadata fence before clearing any dirty record. + let mut guards = Vec::with_capacity(request.entries.len()); + for entry in &request.entries { + let incarnation = Uuid::from_slice(entry.bucket_incarnation.as_ref()) + .map_err(|_| Status::invalid_argument("invalid bucket incarnation"))?; + let guard = + crate::storage::storage_api::acquire_scanner_bucket_incarnation_fence(&entry.bucket, incarnation, store.id) + .await + .map_err(|_| Status::failed_precondition("trusted bucket incarnation is unavailable"))?; + guards.push(guard); + } + let entries = guards + .iter() + .zip(&request.entries) + .map(|(guard, entry)| (guard, entry.generation)) + .collect::>(); + rustfs_scanner::acknowledge_scoped_dirty_usage(&request.instance_id, &entries, request.probe_only) + .map_err(|err| Status::failed_precondition(err.to_string())) + }) + .await + .map_err(|_| Status::deadline_exceeded("scoped dirty usage incarnation validation timed out"))??; + let mut response = ScannerScopedDirtyUsageAckResponse { + protocol_version: SCOPED_DIRTY_USAGE_PROTOCOL_VERSION, + owner_id: request.owner_id, + instance_id: request.instance_id, + supported: true, + max_entries: SCOPED_DIRTY_USAGE_MAX_ENTRIES, + max_request_bytes: SCOPED_DIRTY_USAGE_MAX_REQUEST_BYTES, + cleared, + response_proof: Bytes::new(), + }; + let body = canonical_scoped_dirty_usage_response(&canonical, &response) + .map_err(|_| Status::internal("scoped dirty usage response is too large"))?; + response.response_proof = sign_tonic_rpc_response_proof(&body) + .map_err(|_| Status::unavailable("scoped dirty usage response authentication is unavailable"))? + .into(); + Ok(Response::new(response)) + } +} + #[tonic::async_trait] impl Node for NodeService { async fn ping(&self, request: Request) -> Result, Status> { @@ -2623,6 +2698,7 @@ mod tests { use rustfs_kms::KmsServiceManager; use rustfs_protos::CanonicalMutationBody as _; use rustfs_protos::models::PingBodyBuilder; + use rustfs_protos::proto_gen::node_service::scanner_control_service_server::ScannerControlService as _; use rustfs_protos::proto_gen::node_service::{ BackgroundHealStatusRequest, BatchGenerallyLockRequest, CancelDecommissionRequest, CheckPartsRequest, ClearDecommissionRequest, ControlPlaneErrorCode, DeleteBucketMetadataRequest, DeleteBucketRequest, DeletePathsRequest, @@ -5990,6 +6066,74 @@ mod tests { ); } + fn scoped_dirty_usage_request() -> rustfs_protos::proto_gen::node_service::ScannerScopedDirtyUsageAckRequest { + rustfs_protos::proto_gen::node_service::ScannerScopedDirtyUsageAckRequest { + challenge: vec![7; 16].into(), + protocol_version: 1, + owner_id: "11111111-1111-1111-1111-111111111111".into(), + instance_id: "a".repeat(32), + scope: 1, + probe_only: false, + entries: vec![rustfs_protos::proto_gen::node_service::ScannerScopedDirtyUsageEntry { + bucket: "photos".into(), + bucket_incarnation: vec![1; 16].into(), + generation: 8, + }], + } + } + + #[tokio::test] + async fn scoped_dirty_usage_authenticates_before_storage_and_rejects_tampering() { + use rustfs_protos::scoped_dirty_usage::canonical_scoped_dirty_usage_request; + let service = create_test_node_service(); + let unsigned = service + .scanner_scoped_dirty_usage_ack(Request::new(scoped_dirty_usage_request())) + .await + .expect_err("unsigned ACK must not access storage"); + assert_eq!(unsigned.code(), tonic::Code::PermissionDenied); + for field in 0..9 { + let mut signed = Request::new(scoped_dirty_usage_request()); + let canonical = canonical_scoped_dirty_usage_request(signed.get_ref()).expect("canonical request"); + set_tonic_canonical_body_digest(&mut signed, &canonical).expect("digest"); + mark_v2_authenticated(&mut signed); + match field { + 0 => signed.get_mut().challenge = vec![3; 16].into(), + 1 => signed.get_mut().owner_id = "22222222-2222-2222-2222-222222222222".into(), + 2 => signed.get_mut().instance_id = "b".repeat(32), + 3 => signed.get_mut().probe_only = true, + 4 => signed.get_mut().entries[0].bucket = "videos".into(), + 5 => signed.get_mut().entries[0].bucket_incarnation = vec![2; 16].into(), + 6 => signed.get_mut().entries[0].generation += 1, + 7 => signed.get_mut().scope += 1, + _ => signed.get_mut().protocol_version += 1, + } + let error = service + .scanner_scoped_dirty_usage_ack(signed) + .await + .expect_err("tampered ACK must fail"); + assert_eq!( + error.code(), + if field < 7 { + tonic::Code::PermissionDenied + } else { + tonic::Code::InvalidArgument + } + ); + } + let mut signed = Request::new(scoped_dirty_usage_request()); + let canonical = canonical_scoped_dirty_usage_request(signed.get_ref()).expect("canonical request"); + set_tonic_canonical_body_digest(&mut signed, &canonical).expect("digest"); + mark_v2_authenticated(&mut signed); + assert_eq!( + service + .scanner_scoped_dirty_usage_ack(signed) + .await + .expect_err("missing owner cannot advertise capability") + .code(), + tonic::Code::Unavailable + ); + } + #[tokio::test] async fn test_scanner_activity_requires_body_bound_auth_before_storage_lookup() { let service = create_test_node_service(); @@ -6485,6 +6629,62 @@ mod tests { ) } + #[tokio::test] + async fn scoped_dirty_usage_transport_rejects_oversized_unknown_and_duplicate_fields() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind scoped ACK transport test"); + let addr = listener.local_addr().expect("test listener address"); + let (shutdown, stopped) = tokio::sync::oneshot::channel(); + let server = tokio::spawn(async move { + tonic::transport::Server::builder() + .add_service(super::make_scanner_control_server()) + .serve_with_incoming_shutdown(TcpListenerStream::new(listener), async { + let _ = stopped.await; + }) + .await + .expect("scoped ACK transport server"); + }); + let client = reqwest::Client::builder() + .no_proxy() + .http2_prior_knowledge() + .build() + .expect("HTTP/2 client"); + let limit = rustfs_protos::scoped_dirty_usage::SCOPED_DIRTY_USAGE_MAX_REQUEST_BYTES as usize; + for tag in [0x78, 0x0a] { + // Unknown varint field 15, or repeated empty singular challenge: + // both decode to a tiny default struct despite the large wire body. + for oversized in [false, true] { + let mut payload = [tag, 0].repeat(if oversized { (limit - 4) / 2 } else { limit / 2 }); + if oversized { + // Unknown fixed32 field 15 makes a valid cap+1 protobuf. + payload.extend_from_slice(&[0x7d, 0, 0, 0, 0]); + } + assert_eq!(payload.len(), limit + usize::from(oversized)); + let mut frame = vec![0]; + frame.extend_from_slice(&u32::try_from(payload.len()).expect("bounded test payload").to_be_bytes()); + frame.extend_from_slice(&payload); + let response = client + .post(format!("http://{addr}/node_service.ScannerControlService/ScannerScopedDirtyUsageAck")) + .header("content-type", "application/grpc") + .header("te", "trailers") + .body(frame) + .send() + .await + .expect("send raw protobuf frame"); + let status = response.headers().get("grpc-status").expect("gRPC failure status"); + assert_eq!( + status.to_str().expect("status text"), + if oversized { "11" } else { "3" }, + "cap+1 must fail in the codec, while cap bytes reach request validation" + ); + } + } + drop(client); + shutdown.send(()).expect("stop test server"); + server.await.expect("join test server"); + } + #[tokio::test] async fn heal_control_transport_enforces_codec_limit_and_fails_closed() { let Some(mut client) = connect_test_heal_control_client().await else { diff --git a/rustfs/src/storage/storage_api.rs b/rustfs/src/storage/storage_api.rs index 9e1e92dc4..2216db7f4 100644 --- a/rustfs/src/storage/storage_api.rs +++ b/rustfs/src/storage/storage_api.rs @@ -379,7 +379,7 @@ pub(crate) mod tonic_service_consumer { #[cfg(test)] pub(crate) use super::super::tonic_service::{heal_topology_fingerprint, make_heal_control_server_for_source}; pub(crate) use super::super::tonic_service::{ - make_heal_control_server_with_cache, make_server, make_tier_mutation_control_server, + make_heal_control_server_with_cache, make_scanner_control_server, make_server, make_tier_mutation_control_server, }; } @@ -1704,6 +1704,14 @@ pub(crate) async fn acquire_bucket_metadata_transaction_lock( ecstore_bucket::metadata_sys::acquire_bucket_metadata_transaction_lock(bucket).await } +pub(crate) async fn acquire_scanner_bucket_incarnation_fence( + bucket: &str, + incarnation: uuid::Uuid, + owner_id: uuid::Uuid, +) -> Result { + ecstore_bucket::metadata_sys::acquire_scanner_bucket_incarnation_fence(bucket, incarnation, owner_id).await +} + pub(crate) async fn update_bucket_targets_under_transaction_lock( guard: &ecstore_bucket::metadata_sys::BucketMetadataMutationGuard, bucket: &str, diff --git a/rustfs/src/storage/tonic_service.rs b/rustfs/src/storage/tonic_service.rs index 539361507..c7d2ee28a 100644 --- a/rustfs/src/storage/tonic_service.rs +++ b/rustfs/src/storage/tonic_service.rs @@ -13,6 +13,7 @@ // limitations under the License. pub(crate) use crate::storage::rpc::node_service::make_heal_control_server_with_cache; +pub(crate) use crate::storage::rpc::node_service::make_scanner_control_server; #[cfg(test)] pub(crate) use crate::storage::rpc::node_service::{heal::heal_topology_fingerprint, make_heal_control_server_for_source}; pub use crate::storage::rpc::{make_heal_control_server, make_server, make_tier_mutation_control_server}; diff --git a/rustfs/src/storage_api.rs b/rustfs/src/storage_api.rs index db459b72d..4f824f8da 100644 --- a/rustfs/src/storage_api.rs +++ b/rustfs/src/storage_api.rs @@ -176,7 +176,7 @@ pub(crate) mod server { heal_topology_fingerprint, make_heal_control_server_for_source, }; pub(crate) use crate::storage::storage_api::tonic_service_consumer::{ - make_heal_control_server_with_cache, make_server, make_tier_mutation_control_server, + make_heal_control_server_with_cache, make_scanner_control_server, make_server, make_tier_mutation_control_server, }; } } From 7ba5cd6888b367423357ef3e5211798c87bab3be Mon Sep 17 00:00:00 2001 From: houseme Date: Sat, 5 Sep 2026 17:06:52 +0800 Subject: [PATCH 5/6] chore(deps): refresh SDKs and verify clock skew behavior (#7174) chore(deps): refresh SDKs and pin clock skew regression coverage Refresh compatible dependencies for Scanner/Heal V2 batch 1 and verify the production S3 retry/signing path with a deterministic clock. Co-authored-by: heihutu Co-authored-by: zhi22915 From e8a7f4bc4ab9f8e22f6234927f0dde9898d326d7 Mon Sep 17 00:00:00 2001 From: houseme Date: Sat, 5 Sep 2026 18:44:01 +0800 Subject: [PATCH 6/6] fix(ecstore): remove duplicate local rename implementation (#7190) * fix(ecstore): remove duplicate local rename implementation Keep the canonical commit module after concurrent storage changes merged. The control-write and rollback changes are already present there. Co-Authored-By: heihutu Co-Authored-By: zhi22915 * fix(ci): satisfy new clippy lints --------- Co-authored-by: heihutu Co-authored-by: zhi22915 Co-authored-by: Zhengchao An --- crates/ecstore/src/disk/local.rs | 997 --------------------------- rustfs/src/app/object/shared.rs | 2 +- rustfs/src/site_replication/tests.rs | 5 +- 3 files changed, 3 insertions(+), 1001 deletions(-) diff --git a/crates/ecstore/src/disk/local.rs b/crates/ecstore/src/disk/local.rs index bf2239a9f..9cd683578 100644 --- a/crates/ecstore/src/disk/local.rs +++ b/crates/ecstore/src/disk/local.rs @@ -9862,1004 +9862,7 @@ fn should_read_legacy_inline_part(fi: &FileInfo, storage_class_config: &crate::c storage_class_config.should_inline(shard_size, fi.erasure.data_blocks, versioned) } -/// Proof produced only when the local rename returns at an existing access -/// preflight, before metadata, backups, or object data can be published. -#[derive(Debug)] -pub(in crate::disk) struct LocalRenamePreflightRejection(()); - impl LocalDisk { - #[tracing::instrument(name = "rename_data", level = "trace", skip_all)] - async fn rename_data_inner( - &self, - src_volume: &str, - src_path: &str, - fi: FileInfo, - dst_volume: &str, - dst_path: &str, - preflight_rejection: &mut Option, - ) -> Result { - crate::hp_guard!("LocalDisk::rename_data"); - let mut fi = fi; - // A non-force DeleteBucket must not remove a directory while a local - // object commit is publishing into it. The peer's empty scan remains - // optimistic; this lease establishes the local commit/delete order and - // remains owned by any blocking syscall that outlives async cancellation. - let destination_object_path = self.io_get_object_path(dst_volume, dst_path)?; - let quota_fence_token = - match rustfs_utils::http::metadata_compat::get_consistent_str(&fi.metadata, QUOTA_MUTATION_FENCE_METADATA_SUFFIX) { - Some(value) => { - let token = Uuid::parse_str(value).map_err(|_| DiskError::FileCorrupt)?; - Some(SnapshotLeaseToken::from_slice(token.as_bytes())?) - } - None if rustfs_utils::http::metadata_compat::contains_key_str( - &fi.metadata, - QUOTA_MUTATION_FENCE_METADATA_SUFFIX, - ) => - { - return Err(DiskError::FileCorrupt); - } - None => None, - }; - rustfs_utils::http::metadata_compat::remove_str(&mut fi.metadata, QUOTA_MUTATION_FENCE_METADATA_SUFFIX); - let quota_fence_claim = match quota_fence_token { - Some(token) => Some(self.claim_quota_mutation_fence(dst_volume, dst_path, token).await?), - None => None, - }; - let mutation_lease = os::acquire_rename_data_mutation_lease(&self.root, dst_volume, &destination_object_path).await; - if let Some(claim) = quota_fence_claim { - mutation_lease.attach_external_guard(claim); - } - if fi.is_legacy_indexed_delete_marker() { - fi.erasure.index = 0; - } - fi.validate_for_metadata_read()?; - // Snapshot the destination part paths before `fi` is consumed below. These - // are the descriptors a reader may hold for the version this call is about - // to replace (backlog#1145); readers build the identical string in - // `io_primitives`. An inline-data version has no parts and yields none. - let invalidate_part_paths: Vec = { - let data_dir = fi.data_dir.unwrap_or_default(); - fi.parts - .iter() - .map(|part| format!("{dst_path}/{data_dir}/part.{}", part.number)) - .collect() - }; - let src_volume_dir = self.io_get_bucket_path(src_volume)?; - if !skip_access_checks(src_volume) - && let Err(e) = super::fs::access_std(&src_volume_dir) - { - info!( - event = EVENT_DISK_LOCAL_ACCESS_FAILED, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_DISK_LOCAL, - path = ?src_volume_dir, - operation = "rename_data_src_access", - error = %e, - "Disk local access check failed" - ); - *preflight_rejection = Some(LocalRenamePreflightRejection(())); - return Err(to_access_error(e, DiskError::VolumeAccessDenied).into()); - } - - let dst_volume_dir = self.io_get_bucket_path(dst_volume)?; - if !skip_access_checks(dst_volume) - && let Err(e) = super::fs::access_std(&dst_volume_dir) - { - info!( - event = EVENT_DISK_LOCAL_ACCESS_FAILED, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_DISK_LOCAL, - path = ?dst_volume_dir, - operation = "rename_data_dst_access", - error = %e, - "Disk local access check failed" - ); - *preflight_rejection = Some(LocalRenamePreflightRejection(())); - return Err(to_access_error(e, DiskError::VolumeAccessDenied).into()); - } - - // xl.meta path - let src_file_path = self.io_get_object_path(src_volume, format!("{}/{}", src_path, STORAGE_FORMAT_FILE).as_str())?; - let dst_file_path = self.io_get_object_path(dst_volume, format!("{}/{}", dst_path, STORAGE_FORMAT_FILE).as_str())?; - - // data_dir path - let has_data_dir_path = { - let has_data_dir = { - if !fi.is_remote() { - fi.data_dir - .map(|dir| rustfs_utils::path::retain_slash(dir.to_string().as_str())) - } else { - None - } - }; - - if let Some(data_dir) = has_data_dir { - let src_data_path = self.io_get_object_path( - src_volume, - rustfs_utils::path::retain_slash(format!("{}/{}", src_path, data_dir).as_str()).as_str(), - )?; - let dst_data_path = self.io_get_object_path( - dst_volume, - rustfs_utils::path::retain_slash(format!("{}/{}", dst_path, data_dir).as_str()).as_str(), - )?; - - Some((src_data_path, dst_data_path)) - } else { - None - } - }; - - check_path_length(src_file_path.to_string_lossy().to_string().as_str())?; - check_path_length(dst_file_path.to_string_lossy().to_string().as_str())?; - - let no_inline = fi.data.is_none() && fi.size > 0; - // Captured before `fi` is consumed by add_version; gates the stale - // destination purge below. - let fi_healing = fi.is_healing(); - - // Resolved once for the whole commit so a concurrent configuration - // change can never leave a single rename_data half-synced. The tier is - // keyed on the destination volume: user data staged in scratch - // namespaces follows the configured tier, while commits into - // system-critical namespaces (IAM, config, bucket metadata) stay - // pinned to strict. - let durability = effective_durability(dst_volume); - - let src_file_parent = src_file_path - .parent() - .ok_or_else(|| DiskError::other("missing staged metadata parent"))?; - let dst_file_parent = dst_file_path - .parent() - .ok_or_else(|| DiskError::other("missing object metadata parent"))?; - if !no_inline { - fs::create_dir_all(src_file_parent).await.map_err(to_file_error)?; - } - // Acquire the common trees before reading destination metadata. On - // Windows this pins the object directory identity across metadata - // preparation, data publication, rollback backup, and final commit. - let rename_commit_guard = lock_rename_commit_directories( - src_file_parent, - dst_file_parent, - &dst_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - let has_dst_buf = read_rename_destination_metadata(&dst_file_path, &rename_commit_guard, mutation_lease.clone()).await?; - - if no_inline { - // Non-inline: read xl.meta, parse, write, rename data dir, rename xl.meta - let mut xlmeta = FileMeta::new(); - // An existing dst xl.meta that fails to parse leaves `xlmeta` empty - // and gets overwritten by the commit below (pre-existing behavior); - // track that so the old-size observation reports unknown instead of - // a false `Absent` (rustfs/backlog#1009). - let mut dst_meta_unparsable = false; - if let Some(dst_buf) = has_dst_buf.as_ref() { - if FileMeta::is_xl2_v1_format(dst_buf) - && let Ok(nmeta) = FileMeta::load(dst_buf) - { - xlmeta = nmeta - } else { - dst_meta_unparsable = true; - } - } - - let old_current_size = if dst_meta_unparsable { - None - } else { - observe_old_current_size(has_dst_buf.is_some(), &xlmeta) - }; - - let mut skip_parent = dst_volume_dir.clone(); - if has_dst_buf.as_ref().is_some() - && let Some(parent) = dst_file_path.parent() - { - skip_parent = parent.to_path_buf(); - } - - let version_id = fi.version_id.unwrap_or_default(); - let has_old_data_dir = xlmeta.find_unshared_data_dir_for_version(Some(version_id)); - let old_version_exists = xlmeta.find_version(Some(version_id)).is_ok(); - let rollback_data_dir = has_old_data_dir.or_else(|| { - if old_version_exists && has_dst_buf.is_some() { - Some(inline_metadata_rollback_dir(version_id, &xlmeta)) - } else { - None - } - }); - if let Some(old_data_dir) = has_old_data_dir.as_ref() { - let _ = xlmeta.data.remove_two(version_id, *old_data_dir); - } - xlmeta.add_version(fi)?; - let version_signature = rename_data_versions_signature(&xlmeta); - let new_dst_buf = xlmeta.marshal_msg()?; - - // This tmp xl.meta is renamed onto dst_file_path at the commit - // point below, so only its contents must be durable before the - // rename (SyncMode::FileOnly); the dst parent directory is fsynced - // after the commit rename, and a crash before the rename means the - // PUT was never acknowledged. A metadata commit: relaxed tiers - // leave it to the page cache. - let tmp_meta_sync = if durability.syncs_commit_metadata() { - SyncMode::FileOnly - } else { - SyncMode::None - }; - // The tmp xl.meta write and the shard-file fdatasync are independent - // (disjoint paths) and both only need to be durable before the commit - // renames below, so run them concurrently to drop a blocking - // round-trip from the PUT commit critical path (rustfs/backlog#922 - // step 2). The "contents durable -> rename -> dst dir fsync" ordering - // is unchanged — both futures complete before any rename — which the - // rename_data crash-consistency harness (backlog#935) exercises. - // - // Shard durability: once rename_data succeeds the write is - // acknowledged, so data must not live only in the page cache. - // Multipart parts were already synced during rename_part, so their - // fdatasync here is a cheap no-op. A missing source dir is left for the - // rename below to report through the existing rollback path. Payload - // durability is kept by both strict and relaxed. - let tmp_meta_write = { - let src_file_path = src_file_path.clone(); - let dst_file_path = dst_file_path.clone(); - let rename_commit_guard = rename_commit_guard.clone(); - let mutation_lease = mutation_lease.clone(); - async move { - os::run_blocking_namespace_operation(mutation_lease, move || { - #[cfg(test)] - run_owned_file_write_before_open(&src_file_path); - let mut prepared_metadata_source = os::create_prepared_rename_source_with_commit_guard( - &src_file_path, - &dst_file_path, - &rename_commit_guard, - )?; - prepared_metadata_source.write_all(&new_dst_buf, tmp_meta_sync != SyncMode::None)?; - Ok(prepared_metadata_source) - }) - .await - .map_err(to_file_error) - .map_err(DiskError::from) - } - }; - let shard_sync = async { - if durability.syncs_data_shards() - && let Some((src_data_path, _)) = has_data_dir_path.as_ref() - && let Err(err) = os::sync_dir_files_with_limiter(src_data_path, self.file_sync_permits.clone()).await - && err.kind() != ErrorKind::NotFound - { - return Err::<(), DiskError>(to_file_error(err).into()); - } - Ok(()) - }; - let (tmp_meta_res, shard_sync_res) = tokio::join!(tmp_meta_write, shard_sync); - // Surface a tmp-meta failure first (its prior serial position), then a - // shard-sync failure; either aborts before any rename, exactly as the - // sequential version did. - let prepared_metadata_source = tmp_meta_res?; - shard_sync_res?; - let rename_commit_guard = remove_dst_base_before_commit( - dst_path, - rename_commit_guard, - src_file_parent, - dst_file_parent, - &dst_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - if should_remove_staged_meta_before_commit(dst_path) { - drop(prepared_metadata_source); - std::fs::remove_file(&src_file_path).map_err(to_file_error)?; - return Err(DiskError::FileNotFound); - } - - // Heal reuses the version's data_dir, so for in-place corruption - // the destination dir still exists — and rename(2) cannot replace - // a non-empty directory (EEXIST on XFS, ENOTEMPTY on ext4). Purge - // it first, healing commits only; fresh PUTs mint a new data_dir - // and never collide. Best effort: a real failure surfaces in the - // rename below. - if fi_healing - && let Some((_, dst_data_path)) = has_data_dir_path.as_ref() - && let Err(err) = self.move_to_trash(dst_data_path, true, false).await - { - warn!( - event = EVENT_DISK_LOCAL_HEAL_PURGE_FAILED, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_DISK_LOCAL, - dst_path = ?dst_data_path, - error = ?err, - "Healing commit could not purge the stale destination data dir" - ); - } - if let Some((src_data_path, dst_data_path)) = has_data_dir_path.as_ref() - && let Err(err) = os::rename_all_with_commit_guard( - src_data_path, - dst_data_path, - &skip_parent, - &self.publication_root, - &rename_commit_guard, - mutation_lease.clone(), - ) - .await - { - info!( - event = EVENT_DISK_LOCAL_RENAME_REJECTED, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_DISK_LOCAL, - reason = "rename_all_data_path_failed", - src_path = ?src_data_path, - dst_path = ?dst_data_path, - error = ?err, - "Disk local rename flow failed" - ); - restore_published_data_source( - has_data_dir_path.as_ref(), - &src_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - return Err(err); - } - #[cfg(test)] - if has_data_dir_path.is_some() { - run_rename_data_after_first_publication(&self.root, dst_volume, dst_path); - } - - // Crash-consistency injection: hard power loss after the data dir - // is in place but before xl.meta commits. No cleanup — the harness - // reopens the disk and asserts the object still reads as the old - // version (the staged data dir is a harmless orphan for GC). - if crash_inject::should_crash_at(CrashPoint::RenameAfterDataRename, dst_path) { - return Err(DiskError::Unexpected); - } - - if should_fail_before_old_metadata_backup(dst_path) { - info!( - event = EVENT_DISK_LOCAL_RENAME_REJECTED, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_DISK_LOCAL, - reason = "test_fail_before_old_metadata_backup", - "Disk local rename flow failed before metadata commit" - ); - restore_published_data_source( - has_data_dir_path.as_ref(), - &src_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - return Err(DiskError::Unexpected); - } - - // The rollback backup stays where it is written (no rename) and is - // the sole restore source for a later undo_write, so under strict - // it keeps SyncMode::FileAndDir: contents and directory entry both - // durable. It is part of the metadata commit machinery, so relaxed - // tiers leave it to the page cache like the xl.meta it mirrors. - let backup_sync = if durability.syncs_commit_metadata() { - SyncMode::FileAndDir - } else { - SyncMode::None - }; - if let (Some(old_data_dir), Some(dst_buf)) = (rollback_data_dir, has_dst_buf.as_ref()) { - let backup_parent = dst_file_parent.join(old_data_dir.to_string()); - #[cfg(not(windows))] - if let Err(err) = os::make_dir_all(&backup_parent, &skip_parent).await { - restore_published_data_source( - has_data_dir_path.as_ref(), - &src_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - return Err(err); - } - let backup_path_guard = match rename_commit_guard.create_destination_directory_for_path_access(&backup_parent) { - Ok(guard) => guard, - Err(err) => { - restore_published_data_source( - has_data_dir_path.as_ref(), - &src_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - return Err(DiskError::from(to_file_error(err))); - } - }; - let backup_path = backup_parent.join(STORAGE_FORMAT_FILE_BACKUP); - if let Err(err) = check_path_length(backup_path.to_string_lossy().as_ref()) { - #[cfg(windows)] - drop(backup_path_guard); - restore_published_data_source( - has_data_dir_path.as_ref(), - &src_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - return Err(err); - } - let backup_bytes = dst_buf.clone(); - // Keep the volume, commit-tree, and exact destination-path - // guards in this task until the backup write and durability - // sync finish. A detached spawn_blocking writer could survive - // cancellation and later truncate a newer transaction's - // deterministic rollback backup. - let write_result = os::run_blocking_namespace_operation(mutation_lease.clone(), move || { - #[cfg(test)] - run_owned_file_write_before_open(&backup_path); - backup_path_guard.write_file_for_path_access( - &backup_path, - backup_bytes.as_ref(), - backup_sync != SyncMode::None, - backup_sync == SyncMode::FileAndDir, - ) - }) - .await - .map_err(to_file_error) - .map_err(DiskError::from); - if let Err(err) = write_result { - info!( - event = EVENT_DISK_LOCAL_RENAME_REJECTED, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_DISK_LOCAL, - reason = "write_old_metadata_backup_failed", - error = ?err, - "Disk local rename flow failed" - ); - restore_published_data_source( - has_data_dir_path.as_ref(), - &src_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - return Err(err); - } - } - - // Crash-consistency injection: hard power loss after the rollback - // backup is durable but before the xl.meta commit rename. No - // cleanup — the harness asserts the object still reads as the old - // version, since the destination xl.meta is untouched here. - if crash_inject::should_crash_at(CrashPoint::RenameAfterBackupBeforeMetaCommit, dst_path) { - return Err(DiskError::Unexpected); - } - - if let Err(err) = os::rename_all_with_prepared_source( - prepared_metadata_source, - &src_file_path, - &dst_file_path, - &skip_parent, - &self.publication_root, - &rename_commit_guard, - mutation_lease.clone(), - ) - .await - { - info!( - event = EVENT_DISK_LOCAL_RENAME_REJECTED, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_DISK_LOCAL, - reason = "rename_all_metadata_failed", - src_path = ?src_file_path, - dst_path = ?dst_file_path, - error = ?err, - "Disk local rename flow failed" - ); - restore_published_data_source( - has_data_dir_path.as_ref(), - &src_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - return Err(err); - } - - let committed_new_data_path = has_data_dir_path.as_ref().map(|(_, dst_data_path)| dst_data_path.as_path()); - if should_fail_after_metadata_commit(dst_path) { - rollback_committed_rename_std(&dst_file_path, committed_new_data_path, rollback_data_dir) - .map_err(to_file_error)?; - return Err(DiskError::Unexpected); - } - - // Crash-consistency injection: hard power loss immediately after the - // xl.meta commit rename but before the durability fsync. Unlike the - // graceful failpoint above, no rollback runs — the commit rename is - // already on disk, so the harness asserts the object reads back as - // the new version. - if crash_inject::should_crash_at(CrashPoint::RenameAfterMetaCommit, dst_path) { - return Err(DiskError::Unexpected); - } - - // Persist the directory entries for both the data dir and xl.meta renames; - // without this the commit itself can vanish on power loss. Relaxed tiers - // accept that window (documented in docs/operations/durability-modes.md). - if durability.syncs_commit_metadata() - && let Some(parent) = dst_file_path.parent() - { - let fsync_started = rustfs_io_metrics::put_stage_timer(); - if let Err(err) = os::fsync_dst_dir_group_commit(parent).await { - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_DST_DIR_FSYNC, - fsync_started, - ); - rollback_committed_rename_std(&dst_file_path, committed_new_data_path, rollback_data_dir) - .map_err(to_file_error)?; - // The commit rename changed the dst part inodes before this fsync - // failed and rolled them back; drop any fd cached during that - // window so readers re-open the restored inode (rustfs/backlog#1177). - for part_path in &invalidate_part_paths { - self.io_backend.invalidate_cached_fd(dst_volume, part_path).await; - } - return Err(to_file_error(err).into()); - } - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_DST_DIR_FSYNC, - fsync_started, - ); - } - - // First PUT of an object creates its directory (and any missing prefix - // dirs) via reliable_mkdir_all, which never fsyncs the parent chain. The - // commit fsync above persists the object dir's *contents*, not its own - // entry in the bucket/prefix dir, so on power loss after ack the whole - // object dir could vanish (rustfs/backlog#922 step 4). For a new object - // (no prior xl.meta) fsync the ancestor chain from the object dir's - // parent up to and including the bucket so those new directory entries - // are durable. Overwrites already have a durable object dir. The - // starts_with guard bounds the walk to the bucket subtree. Relaxed/none - // accept the wider window, like the commit fsync above. - if has_dst_buf.is_none() && durability.syncs_commit_metadata() { - let mut ancestor = dst_file_path.parent().and_then(|object_dir| object_dir.parent()); - while let Some(dir) = ancestor { - if !dir.starts_with(&dst_volume_dir) { - break; - } - let fsync_started = rustfs_io_metrics::put_stage_timer(); - if let Err(err) = os::fsync_dir(dir).await { - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_ANCESTOR_DIR_FSYNC, - fsync_started, - ); - rollback_committed_rename_std(&dst_file_path, committed_new_data_path, rollback_data_dir) - .map_err(to_file_error)?; - // Same post-commit rollback window as above — drop cached - // dst part fds so readers re-open the restored inode - // (rustfs/backlog#1177). - for part_path in &invalidate_part_paths { - self.io_backend.invalidate_cached_fd(dst_volume, part_path).await; - } - return Err(to_file_error(err).into()); - } - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_ANCESTOR_DIR_FSYNC, - fsync_started, - ); - if dir == dst_volume_dir.as_path() { - break; - } - ancestor = dir.parent(); - } - } - - // Publication and every rollback-capable durability step are now - // complete. Do not retain the Windows object identity guard while - // cleaning staging paths or invalidating cached descriptors. - #[cfg(windows)] - drop(rename_commit_guard); - - if let Some(src_file_path_parent) = src_file_path.parent() { - if src_volume != super::RUSTFS_META_MULTIPART_BUCKET { - let _ = std::fs::remove_dir(src_file_path_parent); - } else { - let _ = self - .delete_file(&dst_volume_dir, &src_file_path_parent.to_path_buf(), true, false) - .await; - } - } - - // Heal reuses a version's `data_dir` and lands the rebuilt shard on - // the SAME `//part.N` path. Without this, a cached - // descriptor would keep serving the pre-heal inode, defeating the heal - // and eroding read quorum (backlog#1145). - // - // The exact keys are derivable here, and this runs on every write, so - // use them rather than registering a predicate the read path would then - // have to evaluate. Readers build the same string - // (`{object}/{data_dir}/part.{n}`), and `fi.parts` enumerates every - // part of the version now at `dst_path` — any part path absent from it - // no longer exists for readers to ask for. - for part_path in &invalidate_part_paths { - self.io_backend.invalidate_cached_fd(dst_volume, part_path).await; - } - - Ok(RenameDataResp { - old_data_dir: has_old_data_dir, - rollback_data_dir, - cleanup_data_dir: has_old_data_dir, - sign: version_signature, - old_current_size, - }) - } else { - // Inline metadata preparation is blocking. The transaction lease is - // moved into that work so a timeout can release the async waiter without - // allowing a retry to reuse the deterministic staging path too early. - let src = src_file_path.clone(); - let dst = dst_file_path.clone(); - let cleanup_path = if src_volume == super::RUSTFS_META_MULTIPART_BUCKET { - src_file_path.parent().map(|p| p.to_path_buf()) - } else { - None - }; - let dst_path_for_failpoint = dst_path.to_string(); - #[cfg(windows)] - let source_parent = src_file_parent.to_path_buf(); - let rename_commit_guard_for_preparation = rename_commit_guard.clone(); - let sync = durability.syncs_commit_metadata(); - #[cfg(test)] - run_inline_before_file_sync_admission(dst_path); - let mut file_sync_admission = if sync { - Some( - os::acquire_file_sync_admission(self.file_sync_permits.clone()) - .await - .map_err(to_file_error) - .map_err(DiskError::from)?, - ) - } else { - None - }; - let prepare_inline_metadata = move || { - let mut prepared_metadata_source = - os::create_prepared_rename_source_with_commit_guard(&src, &dst, &rename_commit_guard_for_preparation)?; - #[cfg(windows)] - let source_metadata_guard = - rename_commit_guard_for_preparation.lock_source_directory_for_path_access(&source_parent)?; - let mut xlmeta = FileMeta::new(); - // Same as the non-inline branch: an unparsable existing dst - // xl.meta must surface as unknown, not `Absent` - // (rustfs/backlog#1009). - let mut dst_meta_unparsable = false; - if let Some(ref buf) = has_dst_buf { - if FileMeta::is_xl2_v1_format(buf) - && let Ok(nmeta) = FileMeta::load(buf) - { - xlmeta = nmeta - } else { - dst_meta_unparsable = true; - } - } - - let old_current_size = if dst_meta_unparsable { - None - } else { - observe_old_current_size(has_dst_buf.is_some(), &xlmeta) - }; - - let version_id = fi.version_id.unwrap_or_default(); - let old_data_dir = xlmeta.find_unshared_data_dir_for_version(Some(version_id)); - let old_version_exists = xlmeta.find_version(Some(version_id)).is_ok(); - let rollback_data_dir = old_data_dir.or_else(|| { - if old_version_exists && has_dst_buf.is_some() { - Some(inline_metadata_rollback_dir(version_id, &xlmeta)) - } else { - None - } - }); - let mut staged_rollback_path = None; - if let Some(d) = old_data_dir.as_ref() { - let _ = xlmeta.data.remove_two(version_id, *d); - } - xlmeta.add_version(fi)?; - let version_signature = rename_data_versions_signature(&xlmeta); - let new_buf = xlmeta.marshal_msg()?; - // Write the staged xl.meta. Inline objects carry their data inside - // xl.meta, so this is the durable preparation for the metadata commit: - // relaxed tiers do no per-object fsync here at all (aligned - // with MinIO's default), trading a documented power-loss - // window for latency. - prepared_metadata_source.write_all(&new_buf, sync)?; - run_inline_preparation_before_backup(&dst_path_for_failpoint); - if let Some(ref old_metadata) = has_dst_buf - && (rollback_data_dir.is_some() || sync || cfg!(test)) - { - #[cfg(windows)] - let backup_path = { - let backup_path = src - .parent() - .ok_or_else(|| std::io::Error::new(ErrorKind::InvalidInput, "missing staging metadata parent"))? - .join(STORAGE_FORMAT_FILE_BACKUP); - source_metadata_guard.write_file_for_path_access(&backup_path, old_metadata, sync, false)?; - backup_path - }; - #[cfg(not(windows))] - let backup_path = create_local_inline_rollback_backup(&dst, &src, old_metadata)?; - #[cfg(not(windows))] - if sync { - std::fs::File::open(&backup_path)?.sync_data()?; - } - staged_rollback_path = Some(backup_path); - } - - Ok::<_, std::io::Error>(( - rollback_data_dir, - old_data_dir, - version_signature, - old_current_size, - staged_rollback_path, - has_dst_buf.is_none(), - prepared_metadata_source, - )) - }; - let inline_preparation = if let Some(admission) = file_sync_admission.as_ref() { - os::run_blocking_namespace_file_sync_operation(mutation_lease.clone(), admission, prepare_inline_metadata).await - } else { - os::run_blocking_namespace_operation(mutation_lease.clone(), prepare_inline_metadata).await - } - .map_err(to_file_error) - .map_err(DiskError::from); - - let ( - rollback_data_dir, - cleanup_data_dir, - version_signature, - old_current_size, - mut local_rollback_path, - destination_was_absent, - prepared_metadata_source, - ) = match inline_preparation { - Ok(prepared) => prepared, - Err(err) => { - for part_path in &invalidate_part_paths { - self.io_backend.invalidate_cached_fd(dst_volume, part_path).await; - } - return Err(err); - } - }; - - let rename_commit_guard = remove_dst_base_before_commit( - dst_path, - rename_commit_guard, - src_file_parent, - dst_file_parent, - &dst_volume_dir, - &self.publication_root, - mutation_lease.clone(), - ) - .await?; - - if should_remove_staged_meta_before_commit(dst_path) { - drop(prepared_metadata_source); - let remove_result = std::fs::remove_file(&src_file_path); - if let Some(backup_path) = local_rollback_path.as_deref() { - let _ = remove_file_if_exists(backup_path); - } - remove_result.map_err(to_file_error)?; - return Err(DiskError::FileNotFound); - } - - if let (Some(rollback_data_dir), Some(staged_backup)) = (rollback_data_dir, local_rollback_path.as_deref()) { - let Some(dst_parent) = dst_file_path.parent() else { - return Err(DiskError::other("missing object metadata parent")); - }; - let backup_path = dst_parent - .join(rollback_data_dir.to_string()) - .join(STORAGE_FORMAT_FILE_BACKUP); - // rename_all acquires the backup path's namespace lease. Do not - // hold a disk admission while acquiring another namespace lock. - drop(file_sync_admission.take()); - if let Err(err) = rename_all(staged_backup, &backup_path, &dst_volume_dir, &self.publication_root).await { - let _ = remove_file_if_exists(staged_backup); - return Err(err); - } - #[cfg(test)] - run_rename_data_after_first_publication(&self.root, dst_volume, dst_path); - if sync { - file_sync_admission = Some( - os::acquire_file_sync_admission(self.file_sync_permits.clone()) - .await - .map_err(to_file_error) - .map_err(DiskError::from)?, - ); - } - if let Some(admission) = file_sync_admission.as_ref() - && let Some(backup_parent) = backup_path.parent() - { - let fsync_started = rustfs_io_metrics::put_stage_timer(); - if let Err(err) = - os::fsync_dir_with_namespace_file_sync_limit(backup_parent, mutation_lease.clone(), admission).await - { - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_BACKUP_DIR_FSYNC, - fsync_started, - ); - return Err(DiskError::from(to_file_error(err))); - } - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_BACKUP_DIR_FSYNC, - fsync_started, - ); - } - local_rollback_path = None; - } - - let commit_result = if should_fail_commit_rename(dst_path) { - Err(DiskError::other("test fail during metadata commit rename")) - } else { - os::rename_all_with_prepared_source( - prepared_metadata_source, - &src_file_path, - &dst_file_path, - &dst_volume_dir, - &self.publication_root, - &rename_commit_guard, - mutation_lease.clone(), - ) - .await - }; - if let Err(err) = commit_result { - if let Some(backup_path) = local_rollback_path.as_deref() { - let _ = remove_file_if_exists(backup_path); - } - for part_path in &invalidate_part_paths { - self.io_backend.invalidate_cached_fd(dst_volume, part_path).await; - } - return Err(err); - } - - let post_commit = async { - if should_fail_after_metadata_commit(dst_path) { - rollback_inline_metadata_commit_std(&dst_file_path, rollback_data_dir, local_rollback_path.as_deref())?; - return Err(std::io::Error::other("test fail after metadata commit")); - } - - // Persist the commit rename's directory entry across power loss. - if let Some(admission) = file_sync_admission.as_ref() - && let Some(dst_parent) = dst_file_path.parent() - { - let fsync_started = rustfs_io_metrics::put_stage_timer(); - if let Err(err) = - os::fsync_dst_dir_group_commit_or_namespace_file_sync_limit(dst_parent, mutation_lease.clone(), admission) - .await - { - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_DST_DIR_FSYNC, - fsync_started, - ); - rollback_inline_metadata_commit_std(&dst_file_path, rollback_data_dir, local_rollback_path.as_deref())?; - return Err(err); - } - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_DST_DIR_FSYNC, - fsync_started, - ); - } - - // Same power-loss gap as the non-inline path (rustfs/backlog#922 - // step 4): a first PUT creates the object dir (and any missing - // prefix dirs) whose entry in the bucket/prefix dir reliable_mkdir_all - // never fsynced. The fsync above persists the object dir's contents, - // not its own entry, so for a new inline object fsync the ancestor - // chain up to and including the bucket. Overwrites already have a - // durable object dir; the starts_with guard bounds the walk. - if let Some(admission) = file_sync_admission.as_ref() - && destination_was_absent - { - let mut ancestor = dst_file_path.parent().and_then(|object_dir| object_dir.parent()); - while let Some(ancestor_dir) = ancestor { - if !ancestor_dir.starts_with(&dst_volume_dir) { - break; - } - let fsync_started = rustfs_io_metrics::put_stage_timer(); - if let Err(err) = - os::fsync_dir_with_namespace_file_sync_limit(ancestor_dir, mutation_lease.clone(), admission).await - { - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_ANCESTOR_DIR_FSYNC, - fsync_started, - ); - rollback_inline_metadata_commit_std( - &dst_file_path, - rollback_data_dir, - local_rollback_path.as_deref(), - )?; - return Err(err); - } - rustfs_io_metrics::record_put_object_stage_duration_from( - rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_ANCESTOR_DIR_FSYNC, - fsync_started, - ); - if ancestor_dir == dst_volume_dir.as_path() { - break; - } - ancestor = ancestor_dir.parent(); - } - } - - Ok::<(), std::io::Error>(()) - } - .await; - - // The disk admission protects the durability chain, not staging - // cleanup or cache invalidation after that chain has completed. - drop(file_sync_admission.take()); - - // A post-commit rollback (for example, a commit-metadata fsync - // failure under strict durability) restores the old metadata; drop any - // descriptors cached during the committed window before propagating the - // error (rustfs/backlog#1177). Inline objects carry data in xl.meta, so - // this is mostly defensive and keeps both commit branches consistent. - if let Err(err) = post_commit { - for part_path in &invalidate_part_paths { - self.io_backend.invalidate_cached_fd(dst_volume, part_path).await; - } - return Err(DiskError::from(err)); - } - - // The commit no longer has a rollback path. Release the Windows - // object identity guard before best-effort staging cleanup. - #[cfg(windows)] - drop(rename_commit_guard); - - if let Some(backup_path) = local_rollback_path.as_deref() { - let _ = remove_file_if_exists(backup_path); - } - - // Cleanup - if let Some(ref cleanup) = cleanup_path { - let _ = self.delete_file(&dst_volume_dir, cleanup, true, false).await; - } else if let Some(parent) = src_file_path.parent() { - let _ = std::fs::remove_dir(parent); - } - - // Heal reuses a version's `data_dir` and lands the rebuilt shard on - // the SAME `//part.N` path. Without this, a cached - // descriptor would keep serving the pre-heal inode, defeating the heal - // and eroding read quorum (backlog#1145). - // - // The exact keys are derivable here, and this runs on every write, so - // use them rather than registering a predicate the read path would then - // have to evaluate. Readers build the same string - // (`{object}/{data_dir}/part.{n}`), and `fi.parts` enumerates every - // part of the version now at `dst_path` — any part path absent from it - // no longer exists for readers to ask for. - for part_path in &invalidate_part_paths { - self.io_backend.invalidate_cached_fd(dst_volume, part_path).await; - } - - Ok(RenameDataResp { - old_data_dir: cleanup_data_dir, - rollback_data_dir, - cleanup_data_dir, - sign: version_signature, - old_current_size, - }) - } - } - - pub(in crate::disk) async fn rename_data_observed( - &self, - src_volume: &str, - src_path: &str, - fi: &FileInfo, - dst_volume: &str, - dst_path: &str, - ) -> super::RenameDataObservation { - let mut preflight_rejection = None; - let result = self - .rename_data_inner(src_volume, src_path, fi.clone(), dst_volume, dst_path, &mut preflight_rejection) - .await; - super::RenameDataObservation { - result, - preflight_rejection, - } - } - pub(crate) async fn rename_data_borrowed( &self, src_volume: &str, diff --git a/rustfs/src/app/object/shared.rs b/rustfs/src/app/object/shared.rs index c35619edb..ffc9e7db2 100644 --- a/rustfs/src/app/object/shared.rs +++ b/rustfs/src/app/object/shared.rs @@ -309,7 +309,7 @@ fn classify_bucket_default_sse_lookup( ) -> S3Result> { match lookup { Ok(config) => Ok(Some(config)), - Err(err) if err == StorageError::ConfigNotFound => Ok(None), + Err(StorageError::ConfigNotFound) => Ok(None), Err(err) => { let api_error = ApiError::from(err); error!( diff --git a/rustfs/src/site_replication/tests.rs b/rustfs/src/site_replication/tests.rs index e2ab27bb7..3b2c48666 100644 --- a/rustfs/src/site_replication/tests.rs +++ b/rustfs/src/site_replication/tests.rs @@ -867,7 +867,6 @@ fn test_retry_drain_bounds_each_peer_round_to_one_small_request_chain() { r#type: "tags".to_string(), ..Default::default() }], - ..Default::default() }; let make = RetryDrainAction::BucketOpReplay { operation: SITE_REPLICATION_BUCKET_OP_MAKE_WITH_VERSIONING.to_string(), @@ -973,7 +972,7 @@ fn test_lightweight_bucket_retry_plan_orders_real_metadata_and_counts_it() { operator_replication.rules.push(operator_rule("operator-backup")); let mut bucket_with_operator_rule = bucket; bucket_with_operator_rule.replication_config = - Some(BASE64_STANDARD.encode_to_string(&serialize(&operator_replication).expect("operator replication config"))); + Some(BASE64_STANDARD.encode_to_string(serialize(&operator_replication).expect("operator replication config"))); let plan = site_replication_bucket_retry_plan_from_info(&bucket_with_operator_rule, false).expect("targeted retry plan"); assert!( plan.bucket_items.iter().any(|item| item.r#type == "replication-config"), @@ -1050,7 +1049,7 @@ fn test_reachable_probe_promotion_is_fenced_by_the_observed_event() { .peers .insert("remote".to_string(), peer("remote", "https://remote.example.com")); - assert_eq!(mark_reachable_deferred_retry_events(&mut state, &[recovered.clone()]), 1); + assert_eq!(mark_reachable_deferred_retry_events(&mut state, std::slice::from_ref(&recovered)), 1); assert_eq!(state.retry_queue[0].updated_at, None); assert!(!state.retry_queue[0].peer_unreachable); assert_eq!(