fix(targets): make target removal and reload transactional; prevent reappearing entries (#662)

* feat: improve code for notify * upgrade starshard version * upgrade version * Fix ETag format to comply with HTTP standards by wrapping with quotes (#592) * Initial plan * Fix ETag format to comply with HTTP standards by wrapping with quotes Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com> * bufigx --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com> Co-authored-by: overtrue <anzhengchao@gmail.com> * Improve lock (#596) * improve lock Signed-off-by: Mu junxiang <1948535941@qq.com> * feat(tests): add wait_for_object_absence helper and improve lifecycle test reliability Signed-off-by: Mu junxiang <1948535941@qq.com> * chore: remove dirty docs Signed-off-by: Mu junxiang <1948535941@qq.com> --------- Signed-off-by: Mu junxiang <1948535941@qq.com> * feat(append): implement object append operations with state tracking (#599) * feat(append): implement object append operations with state tracking Signed-off-by: junxiang Mu <1948535941@qq.com> * chore: rebase Signed-off-by: junxiang Mu <1948535941@qq.com> --------- Signed-off-by: junxiang Mu <1948535941@qq.com> * build(deps): upgrade s3s (#595) Co-authored-by: loverustfs <155562731+loverustfs@users.noreply.github.com> * fix: validate mqtt broker * improve code for `import` * fix * improve * remove logger from `rustfs-obs` crate * remove code for config Observability * fix * improve code * fix comment * up * up * upgrade version * fix * fmt * upgrade tokio version to 1.48.0 * upgrade `datafusion` and `reed-solomon-simd` version * fix * fmt * improve code for notify webhook example * improve code * fix * fix * fmt --------- Signed-off-by: Mu junxiang <1948535941@qq.com> Signed-off-by: junxiang Mu <1948535941@qq.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com> Co-authored-by: overtrue <anzhengchao@gmail.com> Co-authored-by: guojidan <63799833+guojidan@users.noreply.github.com> Co-authored-by: Nugine <nugine@foxmail.com> Co-authored-by: loverustfs <155562731+loverustfs@users.noreply.github.com>
2026-08-17 10:17:55 +00:00 · 2025-10-17 15:34:53 +08:00
parent 30e7f00b02
commit 42d3645d6f
42 changed files with 514 additions and 3219 deletions
@@ -296,8 +296,8 @@ impl NotificationSystem {
        info!("Removing config for target {} of type {}", target_name, target_type);
        self.update_config_and_reload(|config| {
            let mut changed = false;
-            if let Some(targets) = config.0.get_mut(target_type) {
-                if targets.remove(target_name).is_some() {
+            if let Some(targets) = config.0.get_mut(&target_type.to_lowercase()) {
+                if targets.remove(&target_name.to_lowercase()).is_some() {
                    changed = true;
                }
                if targets.is_empty() {
@@ -307,6 +307,7 @@ impl NotificationSystem {
            if !changed {
                info!("Target {} of type {} not found, no changes made.", target_name, target_type);
            }
+            debug!("Config after remove: {:?}", config);
            changed
        })
        .await
@@ -16,12 +16,9 @@ use crate::Event;
 use crate::factory::{MQTTTargetFactory, TargetFactory, WebhookTargetFactory};
 use futures::stream::{FuturesUnordered, StreamExt};
 use hashbrown::{HashMap, HashSet};
-use rustfs_config::notify::NOTIFY_ROUTE_PREFIX;
-use rustfs_config::{DEFAULT_DELIMITER, ENABLE_KEY, ENV_PREFIX};
+use rustfs_config::{DEFAULT_DELIMITER, ENABLE_KEY, ENV_PREFIX, notify::NOTIFY_ROUTE_PREFIX};
 use rustfs_ecstore::config::{Config, KVS};
-use rustfs_targets::Target;
-use rustfs_targets::TargetError;
-use rustfs_targets::target::ChannelTargetType;
+use rustfs_targets::{Target, TargetError, target::ChannelTargetType};
 use tracing::{debug, error, info, warn};

 /// Registry for managing target factories
@@ -90,7 +87,9 @@ impl TargetRegistry {
        let all_env: Vec<(String, String)> = std::env::vars().filter(|(key, _)| key.starts_with(ENV_PREFIX)).collect();
        // A collection of asynchronous tasks for concurrently executing target creation
        let mut tasks = FuturesUnordered::new();
-        let mut final_config = config.clone(); // Clone a configuration for aggregating the final result
+        // let final_config = config.clone(); // Clone a configuration for aggregating the final result
+        // Record the defaults for each segment so that the segment can eventually be rebuilt
+        let mut section_defaults: HashMap<String, KVS> = HashMap::new();
        // 1. Traverse all registered plants and process them by target type
        for (target_type, factory) in &self.factories {
            tracing::Span::current().record("target_type", target_type.as_str());
@@ -98,12 +97,15 @@ impl TargetRegistry {

            // 2. Prepare the configuration source
            // 2.1. Get the configuration segment in the file, e.g. 'notify_webhook'
-            let section_name = format!("{NOTIFY_ROUTE_PREFIX}{target_type}");
+            let section_name = format!("{NOTIFY_ROUTE_PREFIX}{target_type}").to_lowercase();
            let file_configs = config.0.get(&section_name).cloned().unwrap_or_default();
            // 2.2. Get the default configuration for that type
            let default_cfg = file_configs.get(DEFAULT_DELIMITER).cloned().unwrap_or_default();
            debug!(?default_cfg, "Get the default configuration");

+            // Save defaults for eventual write back
+            section_defaults.insert(section_name.clone(), default_cfg.clone());
+
            // *** Optimization point 1: Get all legitimate fields of the current target type ***
            let valid_fields = factory.get_valid_fields();
            debug!(?valid_fields, "Get the legitimate configuration fields");
@@ -111,7 +113,9 @@ impl TargetRegistry {
            // 3. Resolve instance IDs and configuration overrides from environment variables
            let mut instance_ids_from_env = HashSet::new();
            // 3.1. Instance discovery: Based on the '..._ENABLE_INSTANCEID' format
-            let enable_prefix = format!("{ENV_PREFIX}{NOTIFY_ROUTE_PREFIX}{target_type}_{ENABLE_KEY}_").to_uppercase();
+            let enable_prefix =
+                format!("{ENV_PREFIX}{NOTIFY_ROUTE_PREFIX}{target_type}{DEFAULT_DELIMITER}{ENABLE_KEY}{DEFAULT_DELIMITER}")
+                    .to_uppercase();
            for (key, value) in &all_env {
                if value.eq_ignore_ascii_case(rustfs_config::EnableState::One.as_str())
                    || value.eq_ignore_ascii_case(rustfs_config::EnableState::On.as_str())
@@ -128,14 +132,14 @@ impl TargetRegistry {

            // 3.2. Parse all relevant environment variable configurations
            // 3.2.1. Build environment variable prefixes such as 'RUSTFS_NOTIFY_WEBHOOK_'
-            let env_prefix = format!("{ENV_PREFIX}{NOTIFY_ROUTE_PREFIX}{target_type}_").to_uppercase();
+            let env_prefix = format!("{ENV_PREFIX}{NOTIFY_ROUTE_PREFIX}{target_type}{DEFAULT_DELIMITER}").to_uppercase();
            // 3.2.2. 'env_overrides' is used to store configurations parsed from environment variables in the format: {instance id -> {field -> value}}
            let mut env_overrides: HashMap<String, HashMap<String, String>> = HashMap::new();
            for (key, value) in &all_env {
                if let Some(rest) = key.strip_prefix(&env_prefix) {
                    // Use rsplitn to split from the right side to properly extract the INSTANCE_ID at the end
                    // Format: <FIELD_NAME>_<INSTANCE_ID> or <FIELD_NAME>
-                    let mut parts = rest.rsplitn(2, '_');
+                    let mut parts = rest.rsplitn(2, DEFAULT_DELIMITER);

                    // The first part from the right is INSTANCE_ID
                    let instance_id_part = parts.next().unwrap_or(DEFAULT_DELIMITER);
@@ -224,7 +228,7 @@ impl TargetRegistry {
                } else {
                    info!(instance_id = %id, "Skip the disabled target and will be removed from the final configuration");
                    // Remove disabled target from final configuration
-                    final_config.0.entry(section_name.clone()).or_default().remove(&id);
+                    // final_config.0.entry(section_name.clone()).or_default().remove(&id);
                }
            }
        }
@@ -246,15 +250,50 @@ impl TargetRegistry {
        }

        // 7. Aggregate new configuration and write back to system configuration
-        if !successful_configs.is_empty() {
+        if !successful_configs.is_empty() || !section_defaults.is_empty() {
            info!(
                "Prepare to update {} successfully created target configurations to the system configuration...",
                successful_configs.len()
            );
-            let mut new_config = config.clone();
+
+            let mut successes_by_section: HashMap<String, HashMap<String, KVS>> = HashMap::new();
+
            for (target_type, id, kvs) in successful_configs {
                let section_name = format!("{NOTIFY_ROUTE_PREFIX}{target_type}").to_lowercase();
-                new_config.0.entry(section_name).or_default().insert(id, (*kvs).clone());
+                successes_by_section
+                    .entry(section_name)
+                    .or_default()
+                    .insert(id.to_lowercase(), (*kvs).clone());
+            }
+
+            let mut new_config = config.clone();
+            // Collection of segments that need to be processed: Collect all segments where default items exist or where successful instances exist
+            let mut sections: HashSet<String> = HashSet::new();
+            sections.extend(section_defaults.keys().cloned());
+            sections.extend(successes_by_section.keys().cloned());
+
+            for section in sections {
+                let mut section_map: std::collections::HashMap<String, KVS> = std::collections::HashMap::new();
+                // Add default item
+                if let Some(default_kvs) = section_defaults.get(&section) {
+                    if !default_kvs.is_empty() {
+                        section_map.insert(DEFAULT_DELIMITER.to_string(), default_kvs.clone());
+                    }
+                }
+
+                // Add successful instance item
+                if let Some(instances) = successes_by_section.get(&section) {
+                    for (id, kvs) in instances {
+                        section_map.insert(id.clone(), kvs.clone());
+                    }
+                }
+
+                // Empty breaks are removed and non-empty breaks are replaced entirely.
+                if section_map.is_empty() {
+                    new_config.0.remove(&section);
+                } else {
+                    new_config.0.insert(section, section_map);
+                }
            }

            let Some(store) = rustfs_ecstore::global::new_object_layer_fn() else {