From f1e700ce2f37a702eb33c2294cb08e638a74c4ea Mon Sep 17 00:00:00 2001 From: Max Denushev Date: Thu, 26 Sep 2024 18:34:13 +0300 Subject: [PATCH] Fix/copy before delete replication (#6064) * fix(shell): volume.fix.replication misplaced volumes unsatisfying replication factor * fix(shell): simplify replication check * fix(shell): add test for satisfyReplicaCurrentLocation --- weed/shell/command_volume_fix_replication.go | 23 ++++- .../command_volume_fix_replication_test.go | 87 +++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/weed/shell/command_volume_fix_replication.go b/weed/shell/command_volume_fix_replication.go index 074931f40..97b06adb5 100644 --- a/weed/shell/command_volume_fix_replication.go +++ b/weed/shell/command_volume_fix_replication.go @@ -99,7 +99,7 @@ func (c *commandVolumeFixReplication) Do(args []string, commandEnv *CommandEnv, replica := replicas[0] replicaPlacement, _ := super_block.NewReplicaPlacementFromByte(byte(replica.info.ReplicaPlacement)) switch { - case replicaPlacement.GetCopyCount() > len(replicas): + case replicaPlacement.GetCopyCount() > len(replicas) || !satisfyReplicaCurrentLocation(replicaPlacement, replicas): underReplicatedVolumeIds = append(underReplicatedVolumeIds, vid) case isMisplaced(replicas, replicaPlacement): misplacedVolumeIds = append(misplacedVolumeIds, vid) @@ -377,6 +377,27 @@ func keepDataNodesSorted(dataNodes []location, diskType types.DiskType) { }) } +func satisfyReplicaCurrentLocation(replicaPlacement *super_block.ReplicaPlacement, replicas []*VolumeReplica) bool { + existingDataCenters, existingRacks, _ := countReplicas(replicas) + + if replicaPlacement.DiffDataCenterCount+1 > len(existingDataCenters) { + return false + } + if replicaPlacement.DiffRackCount+1 > len(existingRacks) { + return false + } + if replicaPlacement.SameRackCount > 0 { + foundSatisfyRack := false + for _, rackCount := range existingRacks { + if rackCount >= replicaPlacement.SameRackCount+1 { + foundSatisfyRack = true + } + } + return foundSatisfyRack + } + return true +} + /* if on an existing data node { return false diff --git a/weed/shell/command_volume_fix_replication_test.go b/weed/shell/command_volume_fix_replication_test.go index 97f270306..5f2318c32 100644 --- a/weed/shell/command_volume_fix_replication_test.go +++ b/weed/shell/command_volume_fix_replication_test.go @@ -438,3 +438,90 @@ func TestPickingMisplacedVolumeToDelete(t *testing.T) { } } + +func TestSatisfyReplicaCurrentLocation(t *testing.T) { + + var tests = []testcase{ + { + name: "test 001", + replication: "001", + replicas: []*VolumeReplica{ + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}}, + }, + { + location: &location{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn2"}}, + }, + }, + expected: false, + }, + { + name: "test 010", + replication: "010", + replicas: []*VolumeReplica{ + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}}, + }, + { + location: &location{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn2"}}, + }, + }, + expected: true, + }, + { + name: "test 011", + replication: "011", + replicas: []*VolumeReplica{ + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}}, + }, + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn2"}}, + }, + { + location: &location{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn3"}}, + }, + }, + expected: true, + }, + { + name: "test 110", + replication: "110", + replicas: []*VolumeReplica{ + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}}, + }, + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn2"}}, + }, + { + location: &location{"dc2", "r2", &master_pb.DataNodeInfo{Id: "dn3"}}, + }, + }, + expected: true, + }, + { + name: "test 100", + replication: "100", + replicas: []*VolumeReplica{ + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}}, + }, + { + location: &location{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn2"}}, + }, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + replicaPlacement, _ := super_block.NewReplicaPlacementFromString(tt.replication) + if satisfyReplicaCurrentLocation(replicaPlacement, tt.replicas) != tt.expected { + t.Errorf("%s: expect %v %v %+v", + tt.name, tt.expected, tt.replication, tt.replicas) + } + }) + } +}