From 015cbddc38448b361c4e14d3bdabfab8006160c8 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 21 Oct 2026 14:11:13 -0700 Subject: [PATCH] ec distribution: tolerate zero-value replication configs A ReplicationConfig{} with zeroed minimums made maxShare divide by zero; clamp each level's unit count to at least one. Covers the new behaviors with tests. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../distribution/distribution.go | 15 ++++++----- .../distribution/distribution_test.go | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/weed/storage/erasure_coding/distribution/distribution.go b/weed/storage/erasure_coding/distribution/distribution.go index 74ef3a4e29..3e2376f6f6 100644 --- a/weed/storage/erasure_coding/distribution/distribution.go +++ b/weed/storage/erasure_coding/distribution/distribution.go @@ -31,9 +31,9 @@ type ECDistribution struct { // replication policy describes. func CalculateDistribution(ec ECConfig, rep ReplicationConfig) *ECDistribution { total := ec.TotalShards() - dcUnits := rep.MinDataCenters - rackUnits := rep.MinDataCenters * rep.MinRacksPerDC - nodeUnits := rackUnits * rep.MinNodesPerRack + dcUnits := max(rep.MinDataCenters, 1) + rackUnits := dcUnits * max(rep.MinRacksPerDC, 1) + nodeUnits := rackUnits * max(rep.MinNodesPerRack, 1) return &ECDistribution{ ECConfig: ec, ReplicationConfig: rep, @@ -55,11 +55,14 @@ func evenShare(total, units int) int { // share plus the slack that would pad the total to the next multiple of the // unit count. func maxShare(total, units int) int { - max := ceilDivide(total, units) + units - total%units - if max > total { + if units < 1 { + units = 1 + } + m := ceilDivide(total, units) + units - total%units + if m > total { return total } - return max + return m } func ceilDivide(a, b int) int { diff --git a/weed/storage/erasure_coding/distribution/distribution_test.go b/weed/storage/erasure_coding/distribution/distribution_test.go index 0507be590b..ea5d9ec7a7 100644 --- a/weed/storage/erasure_coding/distribution/distribution_test.go +++ b/weed/storage/erasure_coding/distribution/distribution_test.go @@ -246,3 +246,28 @@ func TestPlanRebalanceNilAnalysis(t *testing.T) { _, err := NewRebalancer(DefaultECConfig(), ReplicationConfig{}).PlanRebalance(nil) assert.Error(t, err) } + +func TestPlanRebalanceNoDuplicateShardOnNode(t *testing.T) { + a := NewTopologyAnalysis() + a.AddNode(makeNode("n1", "dc1", "r1", 10, 0, 1, 2, 3)) + a.AddNode(makeNode("n2", "dc1", "r1", 10, 3)) + a.Finalize() + + plan, err := NewRebalancer(DefaultECConfig(), ReplicationConfig{MinDataCenters: 1, MinRacksPerDC: 1, MinNodesPerRack: 2}).PlanRebalance(a) + require.NoError(t, err) + + // n2 already holds 3, so the balancing move brings a different shard + assert.Equal(t, 3, a.ShardsByNode["n1"]) + assert.Equal(t, 2, a.ShardsByNode["n2"]) + assert.Equal(t, []int{3, 2}, a.AllNodes["n2"].ShardIDs) + for _, move := range plan.Moves { + assert.NotContains(t, move.DestNode.ShardIDs[:len(move.DestNode.ShardIDs)-1], move.ShardID, + "shard %d must not be moved onto a node already holding it", move.ShardID) + } +} + +func TestCalculateDistributionZeroReplication(t *testing.T) { + dist := CalculateDistribution(DefaultECConfig(), ReplicationConfig{}) + assert.Equal(t, 14, dist.TargetShardsPerDC) + assert.NotPanics(t, func() { dist.Summary(); dist.FaultToleranceAnalysis() }) +}