From 6ab8636734cdd70e040bf57daa67472959b76c7d Mon Sep 17 00:00:00 2001 From: houseme Date: Thu, 9 Jul 2026 00:50:51 +0800 Subject: [PATCH] fix(obs): count scanned versions independent of ILM (#4516) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit versions_scanned for both the scanner and ILM collectors was read from the Lifecycle work source's `checked` counter, which is never recorded on the production scan path — so rustfs_scanner_versions_scanned_total and rustfs_ilm_versions_scanned_total sat at zero even while objects_scanned climbed. The two metrics also have distinct intended meanings that were conflated: "versions scanned" (all versions, any bucket) vs "versions checked for ILM actions" (lifecycle-configured buckets only). Add a lifetime `versions_scanned` counter recorded for every version the scanner walks (independent of ILM), and record the Lifecycle source's `checked` counter from the ILM evaluator so the ILM metric reflects the real checked subset. The scanner collector now reports total scanned versions; the ILM collector keeps the ILM-checked subset. Closes backlog#995 (OBS-09). Co-authored-by: heihutu --- crates/common/src/metrics.rs | 65 +++++++++++++++++++++++ crates/obs/src/metrics/stats_collector.rs | 7 ++- crates/scanner/src/scanner_folder.rs | 8 +++ crates/scanner/src/scanner_io.rs | 6 +++ 4 files changed, 85 insertions(+), 1 deletion(-) diff --git a/crates/common/src/metrics.rs b/crates/common/src/metrics.rs index e9667d767..aea19129a 100644 --- a/crates/common/src/metrics.rs +++ b/crates/common/src/metrics.rs @@ -750,6 +750,11 @@ pub struct Metrics { last_scan_cycle_duration_millis: AtomicU64, last_scan_cycle_objects_scanned: AtomicU64, last_scan_cycle_directories_scanned: AtomicU64, + /// Lifetime count of object versions walked by the data scanner, counted + /// for every version regardless of whether any lifecycle rule applies. + /// This is the honest source for `rustfs_scanner_versions_scanned_total`; + /// ILM-checked versions are tracked separately on the `Lifecycle` source. + lifetime_versions_scanned: AtomicU64, last_scan_cycle_bucket_drive_scans: AtomicU64, last_scan_cycle_bucket_drive_failures: AtomicU64, last_scan_cycle_yield_events: AtomicU64, @@ -1106,6 +1111,9 @@ pub struct ScannerMetricsReport { pub oldest_active_path_age_seconds: u64, pub life_time_ops: HashMap, pub life_time_ilm: HashMap, + /// Lifetime object versions scanned, independent of ILM configuration. + #[serde(default)] + pub versions_scanned: u64, pub last_minute: ScannerLastMinute, pub active_paths: Vec, pub current_scan_mode: String, @@ -1704,6 +1712,7 @@ impl Metrics { last_scan_cycle_duration_millis: AtomicU64::new(0), last_scan_cycle_objects_scanned: AtomicU64::new(0), last_scan_cycle_directories_scanned: AtomicU64::new(0), + lifetime_versions_scanned: AtomicU64::new(0), last_scan_cycle_bucket_drive_scans: AtomicU64::new(0), last_scan_cycle_bucket_drive_failures: AtomicU64::new(0), last_scan_cycle_yield_events: AtomicU64::new(0), @@ -2112,6 +2121,17 @@ impl Metrics { self.record_scanner_source_work(source, ScannerSourceWorkUpdate::checked(count)); } + /// Record `count` object versions walked by the data scanner. + /// + /// Every version the scanner visits is counted here, independent of + /// lifecycle configuration, so `rustfs_scanner_versions_scanned_total` + /// reflects real scan coverage even on clusters with no ILM rules. ILM + /// evaluation coverage is recorded separately via the `Lifecycle` source's + /// `checked` counter and surfaces as `rustfs_ilm_versions_scanned_total`. + pub fn record_scanner_versions_scanned(&self, count: u64) { + self.lifetime_versions_scanned.fetch_add(count, Ordering::Relaxed); + } + pub fn record_scanner_source_queued(&self, source: ScannerWorkSource, count: u64) { self.record_scanner_source_work(source, ScannerSourceWorkUpdate::queued(count)); } @@ -2695,6 +2715,7 @@ impl Metrics { .map(|(disk, state)| format!("{disk}/{}", state.path)) .collect(); m.current_scan_mode = self.current_scan_mode().as_str().to_string(); + m.versions_scanned = self.lifetime_versions_scanned.load(Ordering::Relaxed); m.leader_lock_state = self.scanner_leader_lock_state.read().await.clone(); m.leader_lock_held_by_this_process = self.scanner_leader_lock_held.load(Ordering::Relaxed); m.leader_lock_last_error = self.scanner_leader_lock_last_error.read().await.clone(); @@ -3054,6 +3075,50 @@ mod tests { assert_eq!(report.current_disk_bucket_scans_active, 0); } + #[tokio::test] + async fn report_counts_scanned_versions_independent_of_lifecycle() { + let metrics = Metrics::new(); + + // No ILM configuration touched: scanned versions must still accrue so + // rustfs_scanner_versions_scanned_total reflects real coverage. + metrics.record_scanner_versions_scanned(3); + metrics.record_scanner_versions_scanned(5); + + let report = metrics.report().await; + + assert_eq!(report.versions_scanned, 8); + // ILM-checked versions live on the Lifecycle source and stay zero when + // no lifecycle evaluation ran. + let lifecycle_checked = report + .source_work + .iter() + .find(|s| s.source == "lifecycle") + .map(|s| s.checked) + .unwrap_or_default(); + assert_eq!(lifecycle_checked, 0); + } + + #[tokio::test] + async fn report_tracks_lifecycle_checked_versions_separately() { + let metrics = Metrics::new(); + + // Simulate the scanner walking versions and, on a lifecycle-configured + // bucket, handing a subset to the ILM evaluator. + metrics.record_scanner_versions_scanned(10); + metrics.record_scanner_source_checked(ScannerWorkSource::Lifecycle, 4); + + let report = metrics.report().await; + + assert_eq!(report.versions_scanned, 10, "total scanned versions independent of ILM"); + let lifecycle_checked = report + .source_work + .iter() + .find(|s| s.source == "lifecycle") + .map(|s| s.checked) + .unwrap_or_default(); + assert_eq!(lifecycle_checked, 4, "ILM-checked versions tracked on the Lifecycle source"); + } + #[tokio::test] async fn report_derives_scanner_pacing_pressure() { let metrics = Metrics::new(); diff --git a/crates/obs/src/metrics/stats_collector.rs b/crates/obs/src/metrics/stats_collector.rs index 47956be1e..45a9d7f40 100644 --- a/crates/obs/src/metrics/stats_collector.rs +++ b/crates/obs/src/metrics/stats_collector.rs @@ -990,7 +990,12 @@ pub async fn collect_scanner_metric_stats() -> Option { let completed_cycles = metrics.life_time_ops.get("scan_cycle").copied().unwrap_or_default(); let directories_scanned = metrics.life_time_ops.get("scan_folder").copied().unwrap_or_default(); let objects_scanned = metrics.life_time_ops.get("scan_object").copied().unwrap_or_default(); - let versions_scanned = scanner_lifecycle_checked_versions(&metrics); + // Real scan coverage: every version the scanner walked, independent of ILM + // rules. The ILM-checked subset (`scanner_lifecycle_checked_versions`) still + // feeds the ILM collector's versions_scanned, but the scanner collector must + // report the total scanned versions — otherwise clusters without lifecycle + // rules report zero here while objects_scanned keeps climbing. + let versions_scanned = metrics.versions_scanned; let reference_time = metrics.cycles_completed_at.last().copied().unwrap_or(metrics.current_started); let last_activity_seconds = now.signed_duration_since(reference_time).num_seconds().max(0) as u64; let active_paths = metrics.active_scan_paths as u64; diff --git a/crates/scanner/src/scanner_folder.rs b/crates/scanner/src/scanner_folder.rs index 706d74f4d..bb5c67c15 100644 --- a/crates/scanner/src/scanner_folder.rs +++ b/crates/scanner/src/scanner_folder.rs @@ -821,6 +821,14 @@ impl ScannerItem { return; } }; + + // Every version handed to the evaluator counts as an ILM-checked + // version, whether or not a rule matched (NoneAction included). This is + // reached only for buckets with lifecycle rules; it feeds + // rustfs_ilm_versions_scanned_total via the Lifecycle source's checked + // counter. + global_metrics().record_scanner_source_checked(ScannerWorkSource::Lifecycle, object_opts.len() as u64); + let mut to_delete_objs: Vec = Vec::new(); let mut noncurrent_events: Vec = Vec::new(); let mut noncurrent_accounting: Vec> = Vec::new(); diff --git a/crates/scanner/src/scanner_io.rs b/crates/scanner/src/scanner_io.rs index 5c58c1384..9f4811689 100644 --- a/crates/scanner/src/scanner_io.rs +++ b/crates/scanner/src/scanner_io.rs @@ -1535,6 +1535,12 @@ impl ScannerIODisk for Disk { let lock_config = object_lock_config_for_scanner_item(&item).await; + // Count every version this object contributes to the scan, independent + // of any lifecycle configuration, so scan-coverage metrics stay honest + // on clusters without ILM rules. Recorded before `apply_actions` moves + // `object_infos`. + global_metrics().record_scanner_versions_scanned(object_infos.len() as u64); + item.apply_actions(object_infos, lock_config, &mut size_summary).await; if !free_version_infos.is_empty() {