mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-23 22:40:39 +02:00
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.
This commit is contained in:
1 parent
68b525b6ca
commit
8b16507059
2 files changed
+94
-4
No files matched your search
@@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user