From a85864067ed500299b6b724d1d078860dc1dc407 Mon Sep 17 00:00:00 2001 From: shuiyisong <113876041+shuiyisong@users.noreply.github.com> Date: Thu, 18 Dec 2025 17:39:10 +0800 Subject: [PATCH] chore: remove `canonicalize` (#7430) * chore: remove canonicalize Signed-off-by: shuiyisong * chore: add match file name option Signed-off-by: shuiyisong * chore: update field name Signed-off-by: shuiyisong * chore: modify tls option Signed-off-by: shuiyisong * chore: update config file Signed-off-by: shuiyisong * chore: update config md Signed-off-by: shuiyisong * chore: update option to `enable_filename_match` Signed-off-by: shuiyisong * chore: address CR issues Signed-off-by: shuiyisong * chore: remove option Signed-off-by: shuiyisong * chore: remove unused test Signed-off-by: shuiyisong --------- Signed-off-by: shuiyisong --- config/frontend.example.toml | 1 - src/common/config/src/error.rs | 12 +--- src/common/config/src/file_watcher.rs | 90 ++------------------------- src/servers/src/tls.rs | 4 +- 4 files changed, 9 insertions(+), 98 deletions(-) diff --git a/config/frontend.example.toml b/config/frontend.example.toml index 701cb0b087..5b9fe9f27b 100644 --- a/config/frontend.example.toml +++ b/config/frontend.example.toml @@ -131,7 +131,6 @@ key_path = "" ## For now, gRPC tls config does not support auto reload. watch = false - ## MySQL server options. [mysql] ## Whether to enable. diff --git a/src/common/config/src/error.rs b/src/common/config/src/error.rs index 82abd8a9b8..9e788d1d30 100644 --- a/src/common/config/src/error.rs +++ b/src/common/config/src/error.rs @@ -59,15 +59,6 @@ pub enum Error { location: Location, }, - #[snafu(display("Failed to canonicalize path: {}", path))] - CanonicalizePath { - path: String, - #[snafu(source)] - error: std::io::Error, - #[snafu(implicit)] - location: Location, - }, - #[snafu(display("Invalid path '{}': expected a file, not a directory", path))] InvalidPath { path: String, @@ -82,8 +73,7 @@ impl ErrorExt for Error { Error::TomlFormat { .. } | Error::LoadLayeredConfig { .. } | Error::FileWatch { .. } - | Error::InvalidPath { .. } - | Error::CanonicalizePath { .. } => StatusCode::InvalidArguments, + | Error::InvalidPath { .. } => StatusCode::InvalidArguments, Error::SerdeJson { .. } => StatusCode::Unexpected, } } diff --git a/src/common/config/src/file_watcher.rs b/src/common/config/src/file_watcher.rs index 2507af024a..d076692019 100644 --- a/src/common/config/src/file_watcher.rs +++ b/src/common/config/src/file_watcher.rs @@ -30,7 +30,7 @@ use common_telemetry::{error, info, warn}; use notify::{EventKind, RecursiveMode, Watcher}; use snafu::ResultExt; -use crate::error::{CanonicalizePathSnafu, FileWatchSnafu, InvalidPathSnafu, Result}; +use crate::error::{FileWatchSnafu, InvalidPathSnafu, Result}; /// Configuration for the file watcher behavior. #[derive(Debug, Clone, Default)] @@ -41,15 +41,10 @@ pub struct FileWatcherConfig { impl FileWatcherConfig { pub fn new() -> Self { - Self::default() + Default::default() } - pub fn with_modify_and_create(mut self) -> Self { - self.include_remove_events = false; - self - } - - pub fn with_remove_events(mut self) -> Self { + pub fn include_remove_events(mut self) -> Self { self.include_remove_events = true; self } @@ -93,11 +88,8 @@ impl FileWatcherBuilder { path: path.display().to_string(), } ); - // Canonicalize the path for reliable comparison with event paths - let canonical = path.canonicalize().context(CanonicalizePathSnafu { - path: path.display().to_string(), - })?; - self.file_paths.push(canonical); + + self.file_paths.push(path.to_path_buf()); Ok(self) } @@ -144,7 +136,6 @@ impl FileWatcherBuilder { } let config = self.config; - let watched_files: HashSet = self.file_paths.iter().cloned().collect(); info!( "Spawning file watcher for paths: {:?} (watching parent directories)", @@ -165,25 +156,7 @@ impl FileWatcherBuilder { continue; } - // Check if any of the event paths match our watched files - let is_watched_file = event.paths.iter().any(|event_path| { - // Try to canonicalize the event path for comparison - // If the file was deleted, canonicalize will fail, so we also - // compare the raw path - if let Ok(canonical) = event_path.canonicalize() - && watched_files.contains(&canonical) - { - return true; - } - // For deleted files, compare using the raw path - watched_files.contains(event_path) - }); - - if !is_watched_file { - continue; - } - - info!(?event.kind, ?event.paths, "Detected file change"); + info!(?event.kind, ?event.paths, "Detected folder change"); callback(); } Err(err) => { @@ -301,55 +274,4 @@ mod tests { "Watcher should have detected file recreation" ); } - - #[test] - fn test_file_watcher_ignores_other_files() { - common_telemetry::init_default_ut_logging(); - - let dir = create_temp_dir("test_file_watcher_other"); - let watched_file = dir.path().join("watched.txt"); - let other_file = dir.path().join("other.txt"); - - // Create both files - std::fs::write(&watched_file, "watched content").unwrap(); - std::fs::write(&other_file, "other content").unwrap(); - - let counter = Arc::new(AtomicUsize::new(0)); - let counter_clone = counter.clone(); - - FileWatcherBuilder::new() - .watch_path(&watched_file) - .unwrap() - .config(FileWatcherConfig::new()) - .spawn(move || { - counter_clone.fetch_add(1, Ordering::SeqCst); - }) - .unwrap(); - - // Give watcher time to start - std::thread::sleep(Duration::from_millis(100)); - - // Modify the other file - should NOT trigger callback - std::fs::write(&other_file, "modified other content").unwrap(); - - // Wait for potential event - std::thread::sleep(Duration::from_millis(500)); - - assert_eq!( - counter.load(Ordering::SeqCst), - 0, - "Watcher should not have detected changes to other files" - ); - - // Now modify the watched file - SHOULD trigger callback - std::fs::write(&watched_file, "modified watched content").unwrap(); - - // Wait for the event to be processed - std::thread::sleep(Duration::from_millis(500)); - - assert!( - counter.load(Ordering::SeqCst) >= 1, - "Watcher should have detected change to watched file" - ); - } } diff --git a/src/servers/src/tls.rs b/src/servers/src/tls.rs index ba4025ab74..9c7815520e 100644 --- a/src/servers/src/tls.rs +++ b/src/servers/src/tls.rs @@ -362,13 +362,13 @@ mod tests { cert_path: "/path/to/cert_path".to_string(), key_path: "/path/to/key_path".to_string(), ca_cert_path: String::new(), - watch: false + watch: false, }, TlsOption::new( Some(Disable), Some("/path/to/cert_path".to_string()), Some("/path/to/key_path".to_string()), - false + false, ) ); }