From 08ddffdf424233c93003ddad35d32404c9fc38c9 Mon Sep 17 00:00:00 2001 From: overtrue Date: Sat, 22 Aug 2026 00:10:33 +0800 Subject: [PATCH] refactor(data-usage): ReplicationStats -> ReplicationTargetUsage Rename the data-usage crate's ReplicationStats to ReplicationTargetUsage. Serde field names are byte-identical (only the Rust type name changed; field identifiers that rmp encodes are untouched). An rmp round-trip test guards against future drift. Scanner test imports updated to match. --- crates/data-usage/src/data_usage.rs | 75 ++++++++++++++----- crates/scanner/src/data_usage_define/tests.rs | 4 +- .../src/scanner_io/publish_gate_tests.rs | 4 +- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/crates/data-usage/src/data_usage.rs b/crates/data-usage/src/data_usage.rs index 9c4692a21..8950b37a8 100644 --- a/crates/data-usage/src/data_usage.rs +++ b/crates/data-usage/src/data_usage.rs @@ -585,9 +585,12 @@ impl VersionsHistogram { } } -/// Replication statistics for a single target -#[derive(Debug, Default, Clone, Serialize, Deserialize)] -pub struct ReplicationStats { +/// Replication statistics for a single target. +/// +/// Renamed from `ReplicationStats`; serde field names are preserved +/// byte-identically to maintain wire compatibility with existing snapshots. +#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct ReplicationTargetUsage { pub pending_size: u64, pub replicated_size: u64, pub failed_size: u64, @@ -600,7 +603,7 @@ pub struct ReplicationStats { pub replicated_count: u64, } -impl ReplicationStats { +impl ReplicationTargetUsage { pub fn is_empty(&self) -> bool { let Self { pending_size, @@ -636,7 +639,7 @@ impl ReplicationStats { /// Replication statistics for all targets #[derive(Debug, Default, Clone, Serialize, Deserialize)] pub struct ReplicationAllStats { - pub targets: HashMap, + pub targets: HashMap, pub replica_size: u64, pub replica_count: u64, } @@ -649,7 +652,7 @@ impl ReplicationAllStats { targets, } = self; - *replica_size == 0 && *replica_count == 0 && targets.values().all(ReplicationStats::is_empty) + *replica_size == 0 && *replica_count == 0 && targets.values().all(ReplicationTargetUsage::is_empty) } #[deprecated(note = "use is_empty instead")] @@ -2466,7 +2469,7 @@ mod tests { #[test] fn replication_stats_empty_checks_every_field() { - type SetField = fn(&mut ReplicationStats); + type SetField = fn(&mut ReplicationTargetUsage); let cases: [(&str, SetField); 10] = [ ("pending_size", |stats| stats.pending_size = 1), @@ -2481,9 +2484,9 @@ mod tests { ("replicated_count", |stats| stats.replicated_count = 1), ]; - assert!(ReplicationStats::default().is_empty()); + assert!(ReplicationTargetUsage::default().is_empty()); for (field, set_nonzero) in cases { - let mut stats = ReplicationStats::default(); + let mut stats = ReplicationTargetUsage::default(); set_nonzero(&mut stats); assert!(!stats.is_empty(), "{field} must make replication stats non-empty"); } @@ -2514,17 +2517,17 @@ mod tests { } let empty_targets = ReplicationAllStats { - targets: HashMap::from([("arn:test:empty".to_string(), ReplicationStats::default())]), + targets: HashMap::from([("arn:test:empty".to_string(), ReplicationTargetUsage::default())]), ..Default::default() }; assert!(empty_targets.is_empty(), "all-empty targets must keep aggregate stats empty"); let stats = ReplicationAllStats { targets: HashMap::from([ - ("arn:test:empty".to_string(), ReplicationStats::default()), + ("arn:test:empty".to_string(), ReplicationTargetUsage::default()), ( "arn:test:non-empty".to_string(), - ReplicationStats { + ReplicationTargetUsage { pending_count: 1, ..Default::default() }, @@ -2565,7 +2568,7 @@ mod tests { replication_stats: Some(ReplicationAllStats { targets: HashMap::from([( "arn:test:pending".to_string(), - ReplicationStats { + ReplicationTargetUsage { pending_count: 1, ..Default::default() }, @@ -2714,7 +2717,7 @@ mod tests { targets: HashMap::from([ ( "arn:self-only".to_string(), - ReplicationStats { + ReplicationTargetUsage { pending_size: 7, pending_count: 1, ..Default::default() @@ -2722,7 +2725,7 @@ mod tests { ), ( "arn:shared".to_string(), - ReplicationStats { + ReplicationTargetUsage { failed_size: 3, failed_count: 1, missed_threshold_size: 2, @@ -2741,7 +2744,7 @@ mod tests { targets: HashMap::from([ ( "arn:shared".to_string(), - ReplicationStats { + ReplicationTargetUsage { failed_size: 5, failed_count: 2, after_threshold_size: 4, @@ -2751,7 +2754,7 @@ mod tests { ), ( "arn:other-only".to_string(), - ReplicationStats { + ReplicationTargetUsage { replicated_size: 11, replicated_count: 3, ..Default::default() @@ -2993,7 +2996,7 @@ mod tests { fn replication_target_deserialization_preserves_large_historical_maps() { let mut stats = ReplicationAllStats::default(); for index in 0..=1024 { - stats.targets.insert(format!("target-{index}"), ReplicationStats::default()); + stats.targets.insert(format!("target-{index}"), ReplicationTargetUsage::default()); } let encoded = rmp_serde::to_vec_named(&stats).expect("large replication target fixture should encode"); let decoded = rmp_serde::from_slice::(&encoded) @@ -3002,6 +3005,42 @@ mod tests { assert_eq!(decoded.targets.len(), stats.targets.len()); } + /// Round-trip test: encoding a [`ReplicationTargetUsage`] and decoding it back + /// must produce the exact same value. This guards against accidental serde + /// field-name drift during the `ReplicationStats` -> `ReplicationTargetUsage` + /// rename. Wire-level field names are the serialized Rust field identifiers, + /// which must remain byte-identical. + #[test] + fn replication_target_usage_rmp_round_trip() { + let original = ReplicationTargetUsage { + pending_size: 100, + replicated_size: 2_000, + failed_size: 50, + failed_count: 3, + pending_count: 7, + missed_threshold_size: 11, + after_threshold_size: 22, + missed_threshold_count: 1, + after_threshold_count: 2, + replicated_count: 99, + }; + + let buf = rmp_serde::to_vec_named(&original).expect("encode ReplicationTargetUsage to msgpack"); + let decoded: ReplicationTargetUsage = + rmp_serde::from_slice(&buf).expect("decode ReplicationTargetUsage from msgpack"); + assert_eq!(original, decoded, "round-trip through rmp must preserve every field"); + + // Also verify that encoding as an unnamed sequence and then decoding + // with named fields produces the correct mapping (this catches reordering). + let named_buf = rmp_serde::to_vec_named(&original).expect("re-encode for field-name pinning"); + // Spot-check that known field names appear in the named encoding. + let named_str = String::from_utf8_lossy(&named_buf); + assert!(named_str.contains("pending_size"), "field 'pending_size' must survive the rename"); + assert!(named_str.contains("replicated_size"), "field 'replicated_size' must survive the rename"); + assert!(named_str.contains("missed_threshold_size"), "field 'missed_threshold_size' must survive the rename"); + assert!(named_str.contains("after_threshold_count"), "field 'after_threshold_count' must survive the rename"); + } + #[test] fn checked_merge_rejects_noncanonical_histograms_without_mutation() { let mut entry = DataUsageEntry { diff --git a/crates/scanner/src/data_usage_define/tests.rs b/crates/scanner/src/data_usage_define/tests.rs index ed3fcd544..bdccb11c1 100644 --- a/crates/scanner/src/data_usage_define/tests.rs +++ b/crates/scanner/src/data_usage_define/tests.rs @@ -16,7 +16,7 @@ use super::persistence::DataUsageCacheLoadAttempt; use super::*; use crate::storage_api::scanner_io::{HTTPRangeSpec, ObjectIO}; use crate::{ScannerGetObjectReader, ScannerPutObjReader}; -use rustfs_data_usage::{ReplicationAllStats, ReplicationStats}; +use rustfs_data_usage::{ReplicationAllStats, ReplicationTargetUsage}; use serde_json::Value; use std::io::Cursor; use std::pin::Pin; @@ -1636,7 +1636,7 @@ fn size_recursive_prunes_empty_and_preserves_threshold_replication_stats() { replication_stats: Some(ReplicationAllStats { targets: HashMap::from([( "arn:test:threshold".to_string(), - ReplicationStats { + ReplicationTargetUsage { after_threshold_count: 1, ..Default::default() }, diff --git a/crates/scanner/src/scanner_io/publish_gate_tests.rs b/crates/scanner/src/scanner_io/publish_gate_tests.rs index 56961c89e..c6acdea1a 100644 --- a/crates/scanner/src/scanner_io/publish_gate_tests.rs +++ b/crates/scanner/src/scanner_io/publish_gate_tests.rs @@ -13,7 +13,7 @@ // limitations under the License. use super::*; -use rustfs_data_usage::{ReplicationAllStats, ReplicationStats}; +use rustfs_data_usage::{ReplicationAllStats, ReplicationTargetUsage}; const TEST_PLAN_DIGEST: DataUsageScanPlanDigest = DataUsageScanPlanDigest([7; 32]); @@ -271,7 +271,7 @@ fn completed_data_usage_info_flattens_nested_bucket_entries() { replication_stats: Some(ReplicationAllStats { targets: HashMap::from([( "arn:target".to_string(), - ReplicationStats { + ReplicationTargetUsage { replicated_size: 2048, replicated_count: 2, ..Default::default()