fix: deduplicate disks in capacity calculation to prevent inflation (#1656)

crates/ecstore/src/data_usage.rs

@@ -28,7 +28,7 @@ use rustfs_common::data_usage::{
 };
 use rustfs_utils::path::SLASH_SEPARATOR;
 use std::{
-    collections::{HashMap, hash_map::Entry},
+    collections::{HashMap, HashSet, hash_map::Entry},
     sync::{Arc, OnceLock},
     time::{Duration, SystemTime},
 };
@@ -223,6 +223,7 @@ pub async fn aggregate_local_snapshots(store: Arc<ECStore>) -> Result<(Vec<DiskU
     let mut aggregated = DataUsageInfo::default();
     let mut latest_update: Option<SystemTime> = None;
     let mut statuses: Vec<DiskUsageStatus> = Vec::new();
+    let mut processed_disks: HashSet<String> = HashSet::new();
 
     for (pool_idx, pool) in store.pools.iter().enumerate() {
         for set_disks in pool.disk_set.iter() {
@@ -246,6 +247,13 @@ pub async fn aggregate_local_snapshots(store: Arc<ECStore>) -> Result<(Vec<DiskU
                 };
 
                 let root = disk.path();
+                let disk_key = format!("{}|{}", disk.endpoint(), root.display());
+
+                // Skip if we've already processed this physical disk
+                if !processed_disks.insert(disk_key.clone()) {
+                    continue;
+                }
+
                 let mut status = DiskUsageStatus {
                     disk_id: disk_id.clone(),
                     pool_index: Some(pool_idx),

crates/ecstore/src/lib.rs

@@ -47,6 +47,10 @@ pub mod store_utils;
 pub mod client;
 pub mod event;
 pub mod event_notification;
+#[cfg(test)]
+mod pools_test;
+#[cfg(test)]
+mod store_test;
 pub mod tier;
 
 pub use global::new_object_layer_fn;

crates/ecstore/src/pools.rs

@@ -41,7 +41,7 @@ use rustfs_rio::{HashReader, WarpReader};
 use rustfs_utils::path::{SLASH_SEPARATOR, encode_dir_object, path_join};
 use rustfs_workers::workers::Workers;
 use serde::{Deserialize, Serialize};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::fmt::Display;
 use std::io::{Cursor, Write};
 use std::path::PathBuf;
@@ -49,7 +49,7 @@ use std::sync::Arc;
 use time::{Duration, OffsetDateTime};
 use tokio::io::{AsyncReadExt, BufReader};
 use tokio_util::sync::CancellationToken;
-use tracing::{error, info, warn};
+use tracing::{debug, error, info, warn};
 
 pub const POOL_META_NAME: &str = "pool.bin";
 pub const POOL_META_FORMAT: u16 = 1;
@@ -1404,30 +1404,26 @@ fn is_disk_online_state(state: &str) -> bool {
     true
 }
 
+#[deprecated(since = "0.1.0", note = "Use fallback_total_capacity_dedup instead")]
+#[allow(dead_code)]
 fn fallback_total_capacity(disks: &[rustfs_madmin::Disk]) -> usize {
-    disks
-        .iter()
-        .filter(|d| is_disk_online_state(&d.state))
-        .map(|d| d.total_space as usize)
-        .sum()
+    fallback_total_capacity_dedup(disks)
 }
 
+#[deprecated(since = "0.1.0", note = "Use fallback_free_capacity_dedup instead")]
+#[allow(dead_code)]
 fn fallback_free_capacity(disks: &[rustfs_madmin::Disk]) -> usize {
-    disks
-        .iter()
-        .filter(|d| is_disk_online_state(&d.state))
-        .map(|d| d.available_space as usize)
-        .sum()
+    fallback_free_capacity_dedup(disks)
 }
 
 pub fn get_total_usable_capacity(disks: &[rustfs_madmin::Disk], info: &rustfs_madmin::StorageInfo) -> usize {
     // If backend info is missing or inconsistent, do a safe fallback to avoid reporting nonsense.
     if info.backend.standard_sc_data.is_empty() {
-        return fallback_total_capacity(disks);
+        return fallback_total_capacity_dedup(disks);
     }
-
     let mut capacity = 0usize;
     let mut matched_any = false;
+    let mut counted_disks: HashSet<String> = HashSet::new();
 
     for disk in disks.iter() {
         if disk.pool_index < 0 {
@@ -1444,8 +1440,27 @@ pub fn get_total_usable_capacity(disks: &[rustfs_madmin::Disk], info: &rustfs_ma
         }
 
         if (disk.disk_index as usize) < usable_disks_per_set {
-            matched_any = true;
-            capacity += disk.total_space as usize;
+            // 🔧 Generate a unique identity using a combination of fields
+            let disk_key = format!(
+                "{}|{}|p{}s{}d{}",
+                disk.endpoint,   // Node address
+                disk.drive_path, // mount path
+                disk.pool_index, // Pool index
+                disk.set_index,  // Collection index
+                disk.disk_index  // Disk index
+            );
+            debug!("get_total_usable_capacity disk_key: {}", disk_key);
+            // 🔧 Only disks that have not been counted are counted towards capacity
+            if counted_disks.insert(disk_key) {
+                matched_any = true;
+                capacity += disk.total_space as usize;
+            } else {
+                // Log duplicate disks: this likely indicates a configuration issue and should always be visible.
+                warn!(
+                    "Duplicate disk detected in capacity calculation: {} at {}",
+                    disk.endpoint, disk.drive_path
+                );
+            }
         }
     }
 
@@ -1454,17 +1469,18 @@ pub fn get_total_usable_capacity(disks: &[rustfs_madmin::Disk], info: &rustfs_ma
     } else {
         // Even if standard_sc_data exists, it might not match disk indexes due to upstream bugs.
         // Fallback to summing all online disks to prevent under-reporting.
-        fallback_total_capacity(disks)
+        fallback_total_capacity_dedup(disks)
     }
 }
 
 pub fn get_total_usable_capacity_free(disks: &[rustfs_madmin::Disk], info: &rustfs_madmin::StorageInfo) -> usize {
     if info.backend.standard_sc_data.is_empty() {
-        return fallback_free_capacity(disks);
+        return fallback_free_capacity_dedup(disks);
     }
 
     let mut capacity = 0usize;
     let mut matched_any = false;
+    let mut counted_disks: HashSet<String> = HashSet::new();
 
     for disk in disks.iter() {
         if disk.pool_index < 0 {
@@ -1481,14 +1497,68 @@ pub fn get_total_usable_capacity_free(disks: &[rustfs_madmin::Disk], info: &rust
         }
 
         if (disk.disk_index as usize) < usable_disks_per_set {
-            matched_any = true;
-            capacity += disk.available_space as usize;
+            let disk_key = format!(
+                "{}|{}|p{}s{}d{}",
+                disk.endpoint, disk.drive_path, disk.pool_index, disk.set_index, disk.disk_index
+            );
+
+            if counted_disks.insert(disk_key) {
+                matched_any = true;
+                capacity += disk.available_space as usize;
+            }
         }
     }
 
     if matched_any {
         capacity
     } else {
-        fallback_free_capacity(disks)
+        fallback_free_capacity_dedup(disks)
     }
 }
+
+/// Total fallback capacity calculation with deweight
+///
+/// Replace original function: fallback_total_capacity()
+pub(crate) fn fallback_total_capacity_dedup(disks: &[rustfs_madmin::Disk]) -> usize {
+    let mut counted_disks: HashSet<String> = HashSet::new();
+    let mut total = 0usize;
+
+    for disk in disks.iter() {
+        // Only online disks are counted
+        if !is_disk_online_state(&disk.state) {
+            continue;
+        }
+
+        // Use endpoint + drive_path as a unique identifier
+        let disk_key = format!("{}|{}", disk.endpoint, disk.drive_path);
+
+        // Capacity is counted only when the disk is encountered for the first time
+        if counted_disks.insert(disk_key) {
+            total += disk.total_space as usize;
+        }
+    }
+
+    total
+}
+
+/// Remove the heavy fallback idle capacity calculation
+///
+/// Replace original function: fallback_free_capacity()
+pub(crate) fn fallback_free_capacity_dedup(disks: &[rustfs_madmin::Disk]) -> usize {
+    let mut counted_disks: HashSet<String> = HashSet::new();
+    let mut total = 0usize;
+
+    for disk in disks.iter() {
+        if !is_disk_online_state(&disk.state) {
+            continue;
+        }
+
+        let disk_key = format!("{}|{}", disk.endpoint, disk.drive_path);
+
+        if counted_disks.insert(disk_key) {
+            total += disk.available_space as usize;
+        }
+    }
+
+    total
+}

crates/ecstore/src/pools_test.rs

@@ -0,0 +1,177 @@
+// Copyright 2024 RustFS Team
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#[cfg(test)]
+mod capacity_dedup_tests {
+    use crate::pools::{
+        fallback_free_capacity_dedup, fallback_total_capacity_dedup, get_total_usable_capacity, get_total_usable_capacity_free,
+    };
+
+    #[test]
+    fn test_single_disk_no_duplication() {
+        let disks = vec![rustfs_madmin::Disk {
+            endpoint: "node1".to_string(),
+            drive_path: "/mnt/disk1".to_string(),
+            pool_index: 0,
+            set_index: 0,
+            disk_index: 0,
+            total_space: 2_000_000_000_000, // 2TB
+            available_space: 500_000_000_000,
+            used_space: 1_500_000_000_000,
+            state: "ok".to_string(),
+            ..Default::default()
+        }];
+
+        let info = rustfs_madmin::StorageInfo {
+            backend: rustfs_madmin::BackendInfo {
+                standard_sc_data: vec![1],
+                ..Default::default()
+            },
+            disks: disks.clone(),
+        };
+
+        let total = get_total_usable_capacity(&disks, &info);
+        let free = get_total_usable_capacity_free(&disks, &info);
+
+        assert_eq!(total, 2_000_000_000_000, "Total capacity should be 2TB");
+        assert_eq!(free, 500_000_000_000, "Free capacity should be 500GB");
+    }
+
+    #[test]
+    fn test_duplicate_disk_entries_deduped() {
+        // Simulate the same disk appearing 232 times
+        let mut disks = Vec::new();
+        for _ in 0..232 {
+            disks.push(rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk1".to_string(),
+                pool_index: 0,
+                set_index: 0,
+                disk_index: 0,
+                total_space: 2_000_000_000_000,
+                available_space: 500_000_000_000,
+                used_space: 1_500_000_000_000,
+                state: "ok".to_string(),
+                ..Default::default()
+            });
+        }
+
+        let info = rustfs_madmin::StorageInfo {
+            backend: rustfs_madmin::BackendInfo {
+                standard_sc_data: vec![1],
+                ..Default::default()
+            },
+            disks: disks.clone(),
+        };
+
+        let total = get_total_usable_capacity(&disks, &info);
+        let free = get_total_usable_capacity_free(&disks, &info);
+
+        // Should only be counted once, not 232 times
+        assert_eq!(total, 2_000_000_000_000, "Duplicate disks should be counted only once");
+        assert_eq!(free, 500_000_000_000, "Free capacity should not be multiplied");
+
+        // If not deduplicated, the result would be:
+        // total = 2TB × 232 = 464TB ❌
+    }
+
+    #[test]
+    fn test_four_disk_erasure_coding() {
+        // 4-disk erasure coding: 2 data disks + 2 parity disks
+        let disks = vec![
+            rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk1".to_string(),
+                pool_index: 0,
+                set_index: 0,
+                disk_index: 0,
+                total_space: 1_000_000_000_000, // 1TB
+                available_space: 250_000_000_000,
+                state: "ok".to_string(),
+                ..Default::default()
+            },
+            rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk2".to_string(),
+                pool_index: 0,
+                set_index: 0,
+                disk_index: 1,
+                total_space: 1_000_000_000_000,
+                available_space: 250_000_000_000,
+                state: "ok".to_string(),
+                ..Default::default()
+            },
+            rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk3".to_string(),
+                pool_index: 0,
+                set_index: 0,
+                disk_index: 2,
+                total_space: 1_000_000_000_000,
+                available_space: 250_000_000_000,
+                state: "ok".to_string(),
+                ..Default::default()
+            },
+            rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk4".to_string(),
+                pool_index: 0,
+                set_index: 0,
+                disk_index: 3,
+                total_space: 1_000_000_000_000,
+                available_space: 250_000_000_000,
+                state: "ok".to_string(),
+                ..Default::default()
+            },
+        ];
+
+        let info = rustfs_madmin::StorageInfo {
+            backend: rustfs_madmin::BackendInfo {
+                standard_sc_data: vec![2],   // 2 data disks
+                standard_sc_parity: Some(2), // 2 parity disks
+                ..Default::default()
+            },
+            disks: disks.clone(),
+        };
+
+        let total = get_total_usable_capacity(&disks, &info);
+
+        // Only count data disks (disk_index < 2)
+        assert_eq!(total, 2_000_000_000_000, "Should count only data disks (2 × 1TB)");
+    }
+
+    #[test]
+    fn test_fallback_dedup() {
+        // Test deduplication capability of fallback functions
+        let mut disks = Vec::new();
+
+        // Add duplicate disks
+        for _ in 0..100 {
+            disks.push(rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk1".to_string(),
+                total_space: 2_000_000_000_000,
+                available_space: 500_000_000_000,
+                state: "ok".to_string(),
+                ..Default::default()
+            });
+        }
+
+        let total = fallback_total_capacity_dedup(&disks);
+        let free = fallback_free_capacity_dedup(&disks);
+
+        assert_eq!(total, 2_000_000_000_000);
+        assert_eq!(free, 500_000_000_000);
+    }
+}

crates/ecstore/src/store.rs

@@ -1010,6 +1010,46 @@ impl ECStore {
         // *self.pool_meta.write().unwrap() = meta;
         Ok(())
     }
+
+    /// Disk information deduplication function
+    ///
+    /// Use multiple field combinations to ensure uniqueness:
+    /// - endpoint (node address)
+    /// - drive_path (mount path)
+    /// - pool_index (pool index)
+    /// - set_index (Collection Index)
+    /// - disk_index (disk index)
+    pub(crate) fn deduplicate_disks(disks: Vec<rustfs_madmin::Disk>) -> Vec<rustfs_madmin::Disk> {
+        use std::collections::HashMap;
+
+        let mut unique_disks: HashMap<String, rustfs_madmin::Disk> = HashMap::new();
+        let mut duplicate_count = 0;
+
+        for disk in disks {
+            // Generate a compound unique key
+            let key = format!(
+                "{}|{}|p{}s{}d{}",
+                disk.endpoint, disk.drive_path, disk.pool_index, disk.set_index, disk.disk_index
+            );
+
+            // Use the entry API to avoid duplicate inserts
+            use std::collections::hash_map::Entry;
+            match unique_disks.entry(key) {
+                Entry::Vacant(e) => {
+                    e.insert(disk);
+                }
+                Entry::Occupied(_) => {
+                    duplicate_count += 1;
+                }
+            }
+        }
+
+        if duplicate_count > 0 {
+            debug!("Deduplicated {} duplicate disk entries", duplicate_count);
+        }
+
+        unique_disks.into_values().collect()
+    }
 }
 
 pub async fn find_local_disk(disk_path: &String) -> Option<DiskStore> {
@@ -1259,7 +1299,23 @@ impl StorageAPI for ECStore {
             return rustfs_madmin::StorageInfo::default();
         };
 
-        notification_sy.storage_info(self).await
+        let mut info = notification_sy.storage_info(self).await;
+
+        // 🔧 Defensive deduplication: This protection mechanism is retained even if the upstream is fixed
+        let original_count = info.disks.len();
+        info.disks = Self::deduplicate_disks(info.disks);
+        let final_count = info.disks.len();
+
+        if original_count != final_count {
+            warn!(
+                "Storage info deduplication: removed {} duplicate disk entries ({} -> {})",
+                original_count - final_count,
+                original_count,
+                final_count
+            );
+        }
+
+        info
     }
     #[instrument(skip(self))]
     async fn local_storage_info(&self) -> rustfs_madmin::StorageInfo {
@@ -1277,6 +1333,16 @@ impl StorageAPI for ECStore {
             disks.extend_from_slice(&res.disks);
         }
 
+        // 🔧 Defensive deduplication: when aggregating disks from all pools, drop duplicate
+        //  entries that may be reported multiple times by backends; this extra layer is kept
+        //  even if the upstream reporting is later fixed.
+        let original_count = disks.len();
+        disks = Self::deduplicate_disks(disks);
+
+        if original_count != disks.len() {
+            warn!("Local storage info deduplication: {} -> {}", original_count, disks.len());
+        }
+
         let backend = self.backend_info().await;
         rustfs_madmin::StorageInfo { backend, disks }
     }

crates/ecstore/src/store_test.rs

@@ -0,0 +1,91 @@
+// Copyright 2024 RustFS Team
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#[cfg(test)]
+mod store_dedup_tests {
+    use crate::store::ECStore;
+
+    #[test]
+    fn test_deduplicate_disks() {
+        let mut disks = Vec::new();
+
+        // Add original disk
+        disks.push(rustfs_madmin::Disk {
+            endpoint: "node1".to_string(),
+            drive_path: "/mnt/disk1".to_string(),
+            pool_index: 0,
+            set_index: 0,
+            disk_index: 0,
+            total_space: 1_000_000_000_000,
+            ..Default::default()
+        });
+
+        // Add 231 duplicates
+        for _ in 0..231 {
+            disks.push(rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk1".to_string(),
+                pool_index: 0,
+                set_index: 0,
+                disk_index: 0,
+                total_space: 1_000_000_000_000,
+                ..Default::default()
+            });
+        }
+
+        assert_eq!(disks.len(), 232);
+
+        let deduped = ECStore::deduplicate_disks(disks);
+
+        assert_eq!(deduped.len(), 1, "Should deduplicate to 1 unique disk");
+    }
+
+    #[test]
+    fn test_deduplicate_multiple_unique_disks() {
+        let disks = vec![
+            rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk1".to_string(),
+                pool_index: 0,
+                set_index: 0,
+                disk_index: 0,
+                total_space: 1_000_000_000_000,
+                ..Default::default()
+            },
+            rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk2".to_string(),
+                pool_index: 0,
+                set_index: 0,
+                disk_index: 1,
+                total_space: 1_000_000_000_000,
+                ..Default::default()
+            },
+            // Duplicate disk1
+            rustfs_madmin::Disk {
+                endpoint: "node1".to_string(),
+                drive_path: "/mnt/disk1".to_string(),
+                pool_index: 0,
+                set_index: 0,
+                disk_index: 0,
+                total_space: 1_000_000_000_000,
+                ..Default::default()
+            },
+        ];
+
+        let deduped = ECStore::deduplicate_disks(disks);
+
+        assert_eq!(deduped.len(), 2, "Should keep 2 unique disks");
+    }
+}