From 5032d300a24f9f2f44c95fb1429f6a510b39bc46 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 8 Mar 2026 20:15:49 -0700 Subject: [PATCH] cleanup: simplify detection logic and remove redundancies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove redundant clusterInfo nil check in detectForDiskType since Detection already guards against nil clusterInfo - Remove adjustments loop for destination servers not in serverVolumeCounts — topology seeding ensures all servers with matching disk type are already present - Merge two-loop min/max calculation into a single loop: min across all servers, max only among non-exhausted servers - Replace magic number 100 with len(metrics) for minC initialization in convergence test --- weed/worker/tasks/balance/detection.go | 27 ++++++--------------- weed/worker/tasks/balance/detection_test.go | 2 +- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/weed/worker/tasks/balance/detection.go b/weed/worker/tasks/balance/detection.go index 76f81fb56..8649310eb 100644 --- a/weed/worker/tasks/balance/detection.go +++ b/weed/worker/tasks/balance/detection.go @@ -81,7 +81,7 @@ func detectForDiskType(diskType string, diskMetrics []*types.VolumeHealthMetrics // Seed from ActiveTopology so servers with matching disk type but zero // volumes are included in the count and imbalance calculation. serverVolumeCounts := make(map[string]int) - if clusterInfo != nil && clusterInfo.ActiveTopology != nil { + if clusterInfo.ActiveTopology != nil { topologyInfo := clusterInfo.ActiveTopology.GetTopologyInfo() if topologyInfo != nil { for _, dc := range topologyInfo.DataCenterInfos { @@ -125,14 +125,6 @@ func detectForDiskType(diskType string, diskMetrics []*types.VolumeHealthMetrics effectiveCounts[server] = effective totalVolumes += effective } - // Include any destination servers not in the original disk metrics - for server, adj := range adjustments { - if _, exists := serverVolumeCounts[server]; !exists && adj > 0 { - effectiveCounts[server] = adj - totalVolumes += adj - } - } - avgVolumesPerServer := float64(totalVolumes) / float64(len(effectiveCounts)) maxVolumes := 0 @@ -141,6 +133,12 @@ func detectForDiskType(diskType string, diskMetrics []*types.VolumeHealthMetrics minServer := "" for server, count := range effectiveCounts { + // Min is calculated across all servers for an accurate imbalance ratio + if count < minVolumes { + minVolumes = count + minServer = server + } + // Max is only among non-exhausted servers since we can only move from them if exhaustedServers[server] { continue } @@ -148,17 +146,6 @@ func detectForDiskType(diskType string, diskMetrics []*types.VolumeHealthMetrics maxVolumes = count maxServer = server } - if count < minVolumes { - minVolumes = count - minServer = server - } - } - // Also consider exhausted servers for minVolumes (they still exist) - for server, count := range effectiveCounts { - if count < minVolumes { - minVolumes = count - minServer = server - } } if maxServer == "" { diff --git a/weed/worker/tasks/balance/detection_test.go b/weed/worker/tasks/balance/detection_test.go index f75d87b31..ac477853f 100644 --- a/weed/worker/tasks/balance/detection_test.go +++ b/weed/worker/tasks/balance/detection_test.go @@ -489,7 +489,7 @@ func TestDetection_ThreeServers_ConvergesToBalance(t *testing.T) { // Verify convergence: effective counts should be within 20% imbalance. effective := computeEffectiveCounts(servers, metrics, tasks) total := 0 - maxC, minC := 0, 100 + maxC, minC := 0, len(metrics) for _, c := range effective { total += c if c > maxC {