From 8b16507059e5253d16056fe750a4da26c2640b3b Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 8 Apr 2026 19:02:59 -0700 Subject: [PATCH] fix(master): stop endless volume growth in DCs with more racks than replica count (#8996) fix(master): stop endless volume growth in DCs with more racks than replica count (#8986) ShouldGrowVolumesByDcAndRack checked every DC+rack for a writable volume replica. With "010" replication (different-rack), volumes only span 2 racks. In a DC with 3+ racks, at least one rack always lacked a replica, causing the periodic growth loop to create new volumes endlessly. When DiffRackCount > 0, check at the DC level instead: if any rack in the DC has a non-crowded writable volume, skip growth for uncovered racks. --- weed/topology/volume_layout.go | 19 ++++-- weed/topology/volume_layout_grow_test.go | 79 ++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 weed/topology/volume_layout_grow_test.go diff --git a/weed/topology/volume_layout.go b/weed/topology/volume_layout.go index 6a7ca2c89b..97a3f95584 100644 --- a/weed/topology/volume_layout.go +++ b/weed/topology/volume_layout.go @@ -357,12 +357,23 @@ func (vl *VolumeLayout) ShouldGrowVolumes() bool { } func (vl *VolumeLayout) ShouldGrowVolumesByDcAndRack(writables *[]needle.VolumeId, dcId NodeId, rackId NodeId) bool { + // When replication spans multiple racks (DiffRackCount > 0), a writable + // volume's replicas only cover some racks in a DC. It is wrong to + // require every rack to host a replica — that would create volumes + // endlessly in any DC with more racks than the copy count. + // Instead, check at the DC level: if the DC already has a non-crowded + // writable volume, no growth is needed for uncovered racks. + checkDcOnly := vl.rp.DiffRackCount > 0 for _, v := range *writables { for _, dn := range vl.Lookup(v) { - if dn.GetDataCenter().Id() == dcId && dn.GetRack().Id() == rackId { - if info, err := dn.GetVolumesById(v); err == nil && !vl.isCrowdedVolume(&info) { - return false - } + if dn.GetDataCenter().Id() != dcId { + continue + } + if !checkDcOnly && dn.GetRack().Id() != rackId { + continue + } + if info, err := dn.GetVolumesById(v); err == nil && !vl.isCrowdedVolume(&info) { + return false } } } diff --git a/weed/topology/volume_layout_grow_test.go b/weed/topology/volume_layout_grow_test.go new file mode 100644 index 0000000000..999faeebcf --- /dev/null +++ b/weed/topology/volume_layout_grow_test.go @@ -0,0 +1,79 @@ +package topology + +import ( + "testing" + + "github.com/seaweedfs/seaweedfs/weed/storage/needle" + "github.com/seaweedfs/seaweedfs/weed/storage/super_block" + "github.com/seaweedfs/seaweedfs/weed/storage/types" +) + +// Reproduces https://github.com/seaweedfs/seaweedfs/issues/8986 +// +// Topology: 1 DC with 3 racks, replication "010" (different-rack). +// Each volume has replicas on 2 of the 3 racks. +// The third rack has no replica, so ShouldGrowVolumesByDcAndRack +// incorrectly returns true for that rack, causing endless volume creation +// even though the DC already has plenty of non-crowded writable volumes. +var topologyLayout3Racks = ` +{ + "dc_edge":{ + "rack1":{ + "server1":{ + "ip":"10.0.0.1", + "volumes":[ + {"id":1, "size":12312, "replication":"010"}, + {"id":2, "size":12312, "replication":"010"} + ], + "limit":30 + } + }, + "rack2":{ + "server2":{ + "ip":"10.0.0.2", + "volumes":[ + {"id":1, "size":12312, "replication":"010"}, + {"id":2, "size":12312, "replication":"010"} + ], + "limit":30 + } + }, + "rack3":{ + "server3":{ + "ip":"10.0.0.3", + "volumes":[], + "limit":30 + } + } + } +} +` + +func TestShouldGrowVolumesByDcAndRack_Issue8986(t *testing.T) { + topo := setup(topologyLayout3Racks) + + rp, _ := super_block.NewReplicaPlacementFromString("010") + vl := topo.GetVolumeLayout("", rp, needle.EMPTY_TTL, types.HardDriveType) + + writables := vl.CloneWritableVolumes() + if len(writables) != 2 { + t.Fatalf("expected 2 writable volumes, got %d", len(writables)) + } + + // rack1 and rack2 each have replicas of the writable volumes. + // ShouldGrowVolumesByDcAndRack should return false for them. + if vl.ShouldGrowVolumesByDcAndRack(&writables, "dc_edge", "rack1") { + t.Error("rack1 should NOT need growth — it has writable volumes") + } + if vl.ShouldGrowVolumesByDcAndRack(&writables, "dc_edge", "rack2") { + t.Error("rack2 should NOT need growth — it has writable volumes") + } + + // rack3 has no replicas. With the old logic, this returns true (should grow), + // but since the DC already has non-crowded writable volumes with "010" + // replication, growing a new volume is unnecessary — the existing volumes + // can serve any write request that doesn't pin to a specific rack. + if vl.ShouldGrowVolumesByDcAndRack(&writables, "dc_edge", "rack3") { + t.Error("rack3 should NOT need growth — the DC already has non-crowded writable volumes that can serve writes") + } +}