From cf2da0c44d630c6df1ff179caa0bda656e3d29f6 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Thu, 9 Jul 2026 04:55:09 +0800 Subject: [PATCH] refactor(object-capacity): remove the decorative SymlinkTracker and its no-op depth knob (#4571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tracker never influenced traversal: walkdir's follow behavior is fixed up front by follow_links(), and should_follow() only gated the tracker's own bookkeeping. Its 'depth limit' compared tree depth (not symlink chain depth), so RUSTFS_CAPACITY_MAX_SYMLINK_DEPTH was a complete no-op while its telemetry claimed symlinks were skipped that walkdir had in fact followed and counted; record_symlink was always called with size 0, so tracked_bytes never left zero (S12). Remove the tracker, its skipped/summary events, the symlink metric and the depth env knob end to end (the env's 'as u8' truncation goes with it), and document the real semantics at the walker: follow_links(true) counts targets with walkdir's ancestor-loop detection breaking cycles, follow_links(false) — the default — counts no symlink targets. The scan root itself is pre-resolved since backlog#1015. Ref: rustfs/backlog#1018 (S12 from audit rustfs/backlog#1010) --- crates/config/src/constants/capacity.rs | 8 -- crates/io-metrics/src/capacity_metrics.rs | 7 -- crates/io-metrics/src/lib.rs | 2 +- crates/object-capacity/README.md | 3 +- crates/object-capacity/README_ZH.md | 3 +- .../object-capacity/src/capacity_manager.rs | 30 ++--- crates/object-capacity/src/scan.rs | 110 ++---------------- rustfs/src/capacity/mod.rs | 1 - 8 files changed, 22 insertions(+), 142 deletions(-) diff --git a/crates/config/src/constants/capacity.rs b/crates/config/src/constants/capacity.rs index cc9dad9af..3c6f26843 100644 --- a/crates/config/src/constants/capacity.rs +++ b/crates/config/src/constants/capacity.rs @@ -45,9 +45,6 @@ pub const ENV_CAPACITY_METRICS_INTERVAL: &str = "RUSTFS_CAPACITY_METRICS_INTERVA /// Environment variable for following symbolic links during capacity calculation pub const ENV_CAPACITY_FOLLOW_SYMLINKS: &str = "RUSTFS_CAPACITY_FOLLOW_SYMLINKS"; -/// Environment variable for maximum symlink follow depth -pub const ENV_CAPACITY_MAX_SYMLINK_DEPTH: &str = "RUSTFS_CAPACITY_MAX_SYMLINK_DEPTH"; - /// Environment variable for enabling dynamic timeout calculation pub const ENV_CAPACITY_ENABLE_DYNAMIC_TIMEOUT: &str = "RUSTFS_CAPACITY_ENABLE_DYNAMIC_TIMEOUT"; @@ -97,10 +94,6 @@ pub const DEFAULT_CAPACITY_METRICS_INTERVAL_SECS: u64 = 600; /// Default: false (disabled for safety) pub const DEFAULT_CAPACITY_FOLLOW_SYMLINKS: bool = false; -/// Maximum symlink follow depth -/// Default: 3 levels -pub const DEFAULT_CAPACITY_MAX_SYMLINK_DEPTH: u8 = 3; - /// Enable dynamic timeout calculation based on directory characteristics /// Default: true (enabled) pub const DEFAULT_CAPACITY_ENABLE_DYNAMIC_TIMEOUT: bool = true; @@ -132,7 +125,6 @@ mod tests { assert_eq!(ENV_CAPACITY_SAMPLE_RATE, "RUSTFS_CAPACITY_SAMPLE_RATE"); assert_eq!(ENV_CAPACITY_METRICS_INTERVAL, "RUSTFS_CAPACITY_METRICS_INTERVAL"); assert_eq!(ENV_CAPACITY_FOLLOW_SYMLINKS, "RUSTFS_CAPACITY_FOLLOW_SYMLINKS"); - assert_eq!(ENV_CAPACITY_MAX_SYMLINK_DEPTH, "RUSTFS_CAPACITY_MAX_SYMLINK_DEPTH"); assert_eq!(ENV_CAPACITY_ENABLE_DYNAMIC_TIMEOUT, "RUSTFS_CAPACITY_ENABLE_DYNAMIC_TIMEOUT"); assert_eq!(ENV_CAPACITY_MIN_TIMEOUT, "RUSTFS_CAPACITY_MIN_TIMEOUT"); assert_eq!(ENV_CAPACITY_MAX_TIMEOUT, "RUSTFS_CAPACITY_MAX_TIMEOUT"); diff --git a/crates/io-metrics/src/capacity_metrics.rs b/crates/io-metrics/src/capacity_metrics.rs index 000f433cd..f2df23014 100644 --- a/crates/io-metrics/src/capacity_metrics.rs +++ b/crates/io-metrics/src/capacity_metrics.rs @@ -113,13 +113,6 @@ pub fn record_capacity_write_operation(write_frequency: usize) { gauge!("rustfs_capacity_write_frequency").set(write_frequency as f64); } -/// Record symlink accounting. -#[inline(always)] -pub fn record_capacity_symlink(size_bytes: u64) { - counter!("rustfs_capacity_symlinks_encountered").increment(1); - histogram!("rustfs_capacity_symlinks_size_bytes").record(size_bytes as f64); -} - /// Record timeout fallback event. #[inline(always)] pub fn record_capacity_timeout_fallback() { diff --git a/crates/io-metrics/src/lib.rs b/crates/io-metrics/src/lib.rs index 16491b33a..4815d4e7b 100644 --- a/crates/io-metrics/src/lib.rs +++ b/crates/io-metrics/src/lib.rs @@ -121,7 +121,7 @@ pub use capacity_metrics::{ record_capacity_dirty_disk_count, record_capacity_dynamic_timeout, record_capacity_refresh_inflight, record_capacity_refresh_joiner, record_capacity_refresh_request, record_capacity_refresh_result, record_capacity_refresh_scope, record_capacity_scan_disk, record_capacity_scan_mode, record_capacity_scan_sampling, - record_capacity_symlink, record_capacity_timeout_fallback, record_capacity_update_completed, record_capacity_update_failed, + record_capacity_timeout_fallback, record_capacity_update_completed, record_capacity_update_failed, record_capacity_write_operation, record_old_data_dir_cleanup, }; diff --git a/crates/object-capacity/README.md b/crates/object-capacity/README.md index 621f176d9..14a7dace9 100644 --- a/crates/object-capacity/README.md +++ b/crates/object-capacity/README.md @@ -99,7 +99,7 @@ This crate is intentionally not "timeout means hard failure": ### Symlink Handling -- Symlinks are not followed by default: `RUSTFS_CAPACITY_FOLLOW_SYMLINKS=false`. +- Symlinks are not followed by default: `RUSTFS_CAPACITY_FOLLOW_SYMLINKS=false`. When enabled, symlink targets are counted and cycles are broken by the walker's ancestor-loop detection. - If enabled, the scan applies circular-reference detection and a maximum follow depth. - The default maximum depth is `3`. @@ -289,7 +289,6 @@ The configuration constants are defined in `crates/config/src/constants/capacity | `RUSTFS_CAPACITY_SAMPLE_RATE` | `200` | Overflow-file sampling interval | | `RUSTFS_CAPACITY_METRICS_INTERVAL` | `600s` | Runtime summary emission interval | | `RUSTFS_CAPACITY_FOLLOW_SYMLINKS` | `false` | Whether to follow symlinks | -| `RUSTFS_CAPACITY_MAX_SYMLINK_DEPTH` | `3` | Maximum symlink follow depth | | `RUSTFS_CAPACITY_ENABLE_DYNAMIC_TIMEOUT` | `true` | Whether to enable dynamic timeout scaling | | `RUSTFS_CAPACITY_MIN_TIMEOUT` | `2s` | Dynamic-timeout lower bound | | `RUSTFS_CAPACITY_MAX_TIMEOUT` | `15s` | Dynamic-timeout upper bound | diff --git a/crates/object-capacity/README_ZH.md b/crates/object-capacity/README_ZH.md index a97b8f253..3df85a2ca 100644 --- a/crates/object-capacity/README_ZH.md +++ b/crates/object-capacity/README_ZH.md @@ -99,7 +99,7 @@ crate 不是“超时就直接失败”的设计： ### 符号链接处理 -- 默认不跟随符号链接：`RUSTFS_CAPACITY_FOLLOW_SYMLINKS=false`。 +- 默认不跟随符号链接：`RUSTFS_CAPACITY_FOLLOW_SYMLINKS=false`。开启后会统计符号链接目标,循环由遍历器的祖先环检测阻断。 - 开启后会做循环引用检测和最大深度限制。 - 默认最大深度是 `3`。 @@ -289,7 +289,6 @@ get_capacity_manager() | `RUSTFS_CAPACITY_SAMPLE_RATE` | `200` | overflow 文件采样间隔 | | `RUSTFS_CAPACITY_METRICS_INTERVAL` | `600s` | runtime summary 打点间隔 | | `RUSTFS_CAPACITY_FOLLOW_SYMLINKS` | `false` | 是否跟随符号链接 | -| `RUSTFS_CAPACITY_MAX_SYMLINK_DEPTH` | `3` | 符号链接最大跟随深度 | | `RUSTFS_CAPACITY_ENABLE_DYNAMIC_TIMEOUT` | `true` | 是否启用动态超时 | | `RUSTFS_CAPACITY_MIN_TIMEOUT` | `2s` | 动态超时下界 | | `RUSTFS_CAPACITY_MAX_TIMEOUT` | `15s` | 动态超时上界 | diff --git a/crates/object-capacity/src/capacity_manager.rs b/crates/object-capacity/src/capacity_manager.rs index 1090f3aad..eeab34d90 100644 --- a/crates/object-capacity/src/capacity_manager.rs +++ b/crates/object-capacity/src/capacity_manager.rs @@ -19,14 +19,13 @@ use super::types::CapacityDiskRef; use crate::capacity_scope::{CapacityScope, CapacityScopeDisk, drain_global_dirty_scopes, take_capacity_scope}; use futures::FutureExt; use rustfs_config::{ - DEFAULT_CAPACITY_ENABLE_DYNAMIC_TIMEOUT, DEFAULT_CAPACITY_FOLLOW_SYMLINKS, DEFAULT_CAPACITY_MAX_SYMLINK_DEPTH, - DEFAULT_CAPACITY_MAX_TIMEOUT_SECS, DEFAULT_CAPACITY_METRICS_INTERVAL_SECS, DEFAULT_CAPACITY_MIN_TIMEOUT_SECS, - DEFAULT_FAST_UPDATE_THRESHOLD_SECS, DEFAULT_MAX_FILES_THRESHOLD, DEFAULT_SAMPLE_RATE, DEFAULT_SCHEDULED_UPDATE_INTERVAL_SECS, - DEFAULT_STAT_TIMEOUT_SECS, DEFAULT_WRITE_FREQUENCY_THRESHOLD, DEFAULT_WRITE_TRIGGER_DELAY_SECS, - ENV_CAPACITY_ENABLE_DYNAMIC_TIMEOUT, ENV_CAPACITY_FAST_UPDATE_THRESHOLD, ENV_CAPACITY_FOLLOW_SYMLINKS, - ENV_CAPACITY_MAX_FILES_THRESHOLD, ENV_CAPACITY_MAX_SYMLINK_DEPTH, ENV_CAPACITY_MAX_TIMEOUT, ENV_CAPACITY_METRICS_INTERVAL, - ENV_CAPACITY_MIN_TIMEOUT, ENV_CAPACITY_SAMPLE_RATE, ENV_CAPACITY_SCHEDULED_INTERVAL, ENV_CAPACITY_STAT_TIMEOUT, - ENV_CAPACITY_WRITE_FREQUENCY_THRESHOLD, ENV_CAPACITY_WRITE_TRIGGER_DELAY, + DEFAULT_CAPACITY_ENABLE_DYNAMIC_TIMEOUT, DEFAULT_CAPACITY_FOLLOW_SYMLINKS, DEFAULT_CAPACITY_MAX_TIMEOUT_SECS, + DEFAULT_CAPACITY_METRICS_INTERVAL_SECS, DEFAULT_CAPACITY_MIN_TIMEOUT_SECS, DEFAULT_FAST_UPDATE_THRESHOLD_SECS, + DEFAULT_MAX_FILES_THRESHOLD, DEFAULT_SAMPLE_RATE, DEFAULT_SCHEDULED_UPDATE_INTERVAL_SECS, DEFAULT_STAT_TIMEOUT_SECS, + DEFAULT_WRITE_FREQUENCY_THRESHOLD, DEFAULT_WRITE_TRIGGER_DELAY_SECS, ENV_CAPACITY_ENABLE_DYNAMIC_TIMEOUT, + ENV_CAPACITY_FAST_UPDATE_THRESHOLD, ENV_CAPACITY_FOLLOW_SYMLINKS, ENV_CAPACITY_MAX_FILES_THRESHOLD, ENV_CAPACITY_MAX_TIMEOUT, + ENV_CAPACITY_METRICS_INTERVAL, ENV_CAPACITY_MIN_TIMEOUT, ENV_CAPACITY_SAMPLE_RATE, ENV_CAPACITY_SCHEDULED_INTERVAL, + ENV_CAPACITY_STAT_TIMEOUT, ENV_CAPACITY_WRITE_FREQUENCY_THRESHOLD, ENV_CAPACITY_WRITE_TRIGGER_DELAY, }; use rustfs_io_metrics::capacity_metrics::{ record_capacity_current_bytes, record_capacity_degraded_reading, record_capacity_dirty_disk_count, @@ -82,8 +81,6 @@ struct CachedCapacityConfig { metrics_interval: Duration, /// Follow symlinks flag follow_symlinks: bool, - /// Max symlink depth - max_symlink_depth: u8, /// Enable dynamic timeout flag enable_dynamic_timeout: bool, /// Min timeout @@ -138,7 +135,6 @@ impl CachedCapacityConfig { DEFAULT_CAPACITY_METRICS_INTERVAL_SECS, )), follow_symlinks: get_env_bool(ENV_CAPACITY_FOLLOW_SYMLINKS, DEFAULT_CAPACITY_FOLLOW_SYMLINKS), - max_symlink_depth: get_env_u64(ENV_CAPACITY_MAX_SYMLINK_DEPTH, DEFAULT_CAPACITY_MAX_SYMLINK_DEPTH as u64) as u8, enable_dynamic_timeout: get_env_bool(ENV_CAPACITY_ENABLE_DYNAMIC_TIMEOUT, DEFAULT_CAPACITY_ENABLE_DYNAMIC_TIMEOUT), min_timeout: Duration::from_secs(env_u64_at_least(ENV_CAPACITY_MIN_TIMEOUT, DEFAULT_CAPACITY_MIN_TIMEOUT_SECS, 1)), max_timeout: Duration::from_secs(env_u64_at_least(ENV_CAPACITY_MAX_TIMEOUT, DEFAULT_CAPACITY_MAX_TIMEOUT_SECS, 1)), @@ -267,18 +263,6 @@ pub fn get_follow_symlinks() -> bool { get_cached_config().follow_symlinks } -/// Get max symlink depth from environment or default -#[cfg(not(test))] -pub fn get_max_symlink_depth() -> u8 { - get_cached_config().max_symlink_depth -} - -/// Get max symlink depth from environment or default (test mode) -#[cfg(test)] -pub fn get_max_symlink_depth() -> u8 { - get_cached_config().max_symlink_depth -} - /// Get enable dynamic timeout flag from environment or default #[cfg(not(test))] pub fn get_enable_dynamic_timeout() -> bool { diff --git a/crates/object-capacity/src/scan.rs b/crates/object-capacity/src/scan.rs index 4e29798c1..2021dd84b 100644 --- a/crates/object-capacity/src/scan.rs +++ b/crates/object-capacity/src/scan.rs @@ -14,18 +14,18 @@ use super::capacity_manager::{ CapacityUpdate, DiskCapacityUpdate, HybridCapacityManager, get_enable_dynamic_timeout, get_follow_symlinks, - get_max_files_threshold, get_max_symlink_depth, get_max_timeout, get_min_timeout, get_sample_rate, get_stat_timeout, + get_max_files_threshold, get_max_timeout, get_min_timeout, get_sample_rate, get_stat_timeout, }; use super::types::{CapacityDiskRef, CapacityScanResult, CapacityScanSummary}; use crate::capacity_scope::CapacityScopeDisk; use futures::{StreamExt, stream}; use rustfs_io_metrics::capacity_metrics::{ record_capacity_dynamic_timeout, record_capacity_scan_disk, record_capacity_scan_mode, record_capacity_scan_sampling, - record_capacity_symlink, record_capacity_timeout_fallback, + record_capacity_timeout_fallback, }; use std::collections::HashSet; use std::ffi::OsStr; -use std::path::{Component, Path, PathBuf}; +use std::path::{Component, Path}; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::{Duration, Instant}; @@ -41,8 +41,6 @@ const LOG_SUBSYSTEM_SAMPLING: &str = "sampling"; const EVENT_CAPACITY_SCAN_DISK_COMPLETED: &str = "capacity_scan_disk_completed"; const EVENT_CAPACITY_SCAN_DISK_FAILED: &str = "capacity_scan_disk_failed"; const EVENT_CAPACITY_SCAN_SUMMARY: &str = "capacity_scan_summary"; -const EVENT_CAPACITY_SCAN_SYMLINK_SKIPPED: &str = "capacity_scan_symlink_skipped"; -const EVENT_CAPACITY_SCAN_SYMLINK_SUMMARY: &str = "capacity_scan_symlink_summary"; const EVENT_CAPACITY_SCAN_DYNAMIC_TIMEOUT: &str = "capacity_scan_dynamic_timeout"; const EVENT_CAPACITY_SCAN_TIMEOUT: &str = "capacity_scan_timeout"; const EVENT_CAPACITY_SCAN_SAMPLING_CLAMPED: &str = "capacity_scan_sampling_clamped"; @@ -365,69 +363,6 @@ pub async fn scan_used_capacity_disks( Ok(calculate_data_dir_used_capacity(disks).await?.into()) } -/// Tracker for symlink resolution with circular reference detection. -struct SymlinkTracker { - visited: HashSet, - symlink_count: usize, - symlink_size: u64, - max_depth: u8, -} - -impl SymlinkTracker { - fn new(max_depth: u8) -> Self { - Self { - visited: HashSet::new(), - symlink_count: 0, - symlink_size: 0, - max_depth, - } - } - - fn should_follow(&self, path: &Path, depth: u8) -> bool { - if depth >= self.max_depth { - debug!( - event = EVENT_CAPACITY_SCAN_SYMLINK_SKIPPED, - component = LOG_COMPONENT_CAPACITY, - subsystem = LOG_SUBSYSTEM_SCAN, - result = "skipped", - reason = "depth_limit", - depth, - max_depth = self.max_depth, - path = ?path, - "capacity scan symlink skipped" - ); - return false; - } - - if self.visited.contains(path) { - warn!( - event = EVENT_CAPACITY_SCAN_SYMLINK_SKIPPED, - component = LOG_COMPONENT_CAPACITY, - subsystem = LOG_SUBSYSTEM_SCAN, - result = "skipped", - reason = "cycle_detected", - path = ?path, - "capacity scan symlink skipped" - ); - return false; - } - - true - } - - fn record_symlink(&mut self, path: PathBuf, size: u64) { - if self.visited.insert(path) { - self.symlink_count += 1; - self.symlink_size += size; - record_capacity_symlink(size); - } - } - - fn get_stats(&self) -> (usize, u64) { - (self.symlink_count, self.symlink_size) - } -} - /// Monitor for directory traversal progress with a cooperative timeout. /// /// The old in-loop stall detector was structurally unreachable: cooperative @@ -549,7 +484,6 @@ struct ScanLimits { sample_rate: usize, enable_dynamic_timeout: bool, follow_symlinks: bool, - max_symlink_depth: u8, } impl ScanLimits { @@ -579,7 +513,6 @@ impl ScanLimits { sample_rate: effective_sample_rate, enable_dynamic_timeout: get_enable_dynamic_timeout(), follow_symlinks: get_follow_symlinks(), - max_symlink_depth: get_max_symlink_depth(), } } } @@ -723,7 +656,6 @@ fn scan_dir_blocking(path: &Path, limits: &ScanLimits, cancelled: &AtomicBool) - sample_rate: effective_sample_rate, enable_dynamic_timeout, follow_symlinks, - max_symlink_depth, } = *limits; { if !path.exists() { @@ -756,13 +688,17 @@ fn scan_dir_blocking(path: &Path, limits: &ScanLimits, cancelled: &AtomicBool) - // elapses before the exact prefix fills (early sampling entry). let mut effective_threshold = max_files_threshold; - let mut symlink_tracker = SymlinkTracker::new(max_symlink_depth); let mut progress_monitor = ProgressMonitor::new(base_timeout, min_timeout, max_timeout, enable_dynamic_timeout); - let walker = WalkDir::new(path) - .follow_links(follow_symlinks) - .follow_root_links(follow_symlinks) - .into_iter(); + // With follow_links(true), symlink targets are counted and walkdir's + // own ancestor-loop detection breaks cycles (diamond-shaped links may + // still be counted once per path). With the default (false), symlink + // targets are not counted. The old SymlinkTracker layered on top of + // this never influenced traversal, reported tree depth as chain depth + // and always tracked 0 bytes, so it was removed along with the no-op + // RUSTFS_CAPACITY_MAX_SYMLINK_DEPTH knob (backlog#1018 S12). The root + // itself is pre-resolved above (backlog#1015). + let walker = WalkDir::new(path).follow_links(follow_symlinks).into_iter(); for entry_result in walker { if cancelled.load(Ordering::Relaxed) { @@ -864,14 +800,6 @@ fn scan_dir_blocking(path: &Path, limits: &ScanLimits, cancelled: &AtomicBool) - } }; - if follow_symlinks - && entry.path_is_symlink() - && let Ok(target) = std::fs::read_link(entry.path()) - && symlink_tracker.should_follow(&target, entry.depth().min(u8::MAX as usize) as u8) - { - symlink_tracker.record_symlink(target, 0); - } - let file_type = entry.file_type(); if file_type.is_dir() { continue; @@ -977,19 +905,6 @@ fn scan_dir_blocking(path: &Path, limits: &ScanLimits, cancelled: &AtomicBool) - } } - let (symlink_count, symlink_size) = symlink_tracker.get_stats(); - if symlink_count > 0 { - info!( - event = EVENT_CAPACITY_SCAN_SYMLINK_SUMMARY, - component = LOG_COMPONENT_CAPACITY, - subsystem = LOG_SUBSYSTEM_SCAN, - result = "observed", - symlink_count, - tracked_bytes = symlink_size, - "capacity scan symlink summary" - ); - } - if file_count > effective_threshold && sampled_count > 0 { let overflow_count = file_count - effective_threshold; let estimated_overflow = estimate_overflow_bytes(overflow_sampled_bytes, overflow_count as u64, sampled_count as u64); @@ -1369,7 +1284,6 @@ mod tests { sample_rate: 1, enable_dynamic_timeout: false, follow_symlinks: false, - max_symlink_depth: 40, } } diff --git a/rustfs/src/capacity/mod.rs b/rustfs/src/capacity/mod.rs index cf8dc9a29..71440cb1f 100644 --- a/rustfs/src/capacity/mod.rs +++ b/rustfs/src/capacity/mod.rs @@ -32,7 +32,6 @@ //! - `RUSTFS_CAPACITY_SAMPLE_RATE` - Sampling rate for metrics (default: 200) //! - `RUSTFS_CAPACITY_METRICS_INTERVAL` - Metrics summary logging interval (default: 600s) //! - `RUSTFS_CAPACITY_FOLLOW_SYMLINKS` - Follow symlinks during traversal (default: false) -//! - `RUSTFS_CAPACITY_MAX_SYMLINK_DEPTH` - Max symlink depth (default: 3) //! - `RUSTFS_CAPACITY_ENABLE_DYNAMIC_TIMEOUT` - Enable dynamic timeout (default: true) //! - `RUSTFS_CAPACITY_MIN_TIMEOUT` - Minimum timeout (default: 2s) //! - `RUSTFS_CAPACITY_MAX_TIMEOUT` - Maximum timeout (default: 15s)