volume.fix.replication: add test cases for complicated moving

fix https://github.com/chrislusf/seaweedfs/issues/1253

weed/shell/command_volume_fix_replication.go

@@ -145,31 +145,131 @@ func keepDataNodesSorted(dataNodes []location) {
 	})
 }
 
+/*
+  if on an existing data node {
+    return false
+  }
+  if different from existing dcs {
+    if lack on different dcs {
+      return true
+    }else{
+      return false
+    }
+  }
+  if not on primary dc {
+    return false
+  }
+  if different from existing racks {
+    if lack on different racks {
+      return true
+    }else{
+      return false
+    }
+  }
+  if not on primary rack {
+    return false
+  }
+  if lacks on same rack {
+    return true
+  } else {
+    return false
+  }
+*/
 func satisfyReplicaPlacement(replicaPlacement *super_block.ReplicaPlacement, existingLocations []location, possibleLocation location) bool {
 
-	existingDataCenters := make(map[string]bool)
-	existingRacks := make(map[string]bool)
-	existingDataNodes := make(map[string]bool)
+	existingDataNodes := make(map[string]int)
 	for _, loc := range existingLocations {
-		existingDataCenters[loc.DataCenter()] = true
-		existingRacks[loc.Rack()] = true
-		existingDataNodes[loc.String()] = true
+		existingDataNodes[loc.String()] += 1
+	}
+	sameDataNodeCount := existingDataNodes[possibleLocation.String()]
+	// avoid duplicated volume on the same data node
+	if sameDataNodeCount > 0 {
+		return false
+	}
+
+	existingDataCenters := make(map[string]int)
+	for _, loc := range existingLocations {
+		existingDataCenters[loc.DataCenter()] += 1
+	}
+	primaryDataCenters, _ := findTopKeys(existingDataCenters)
+
+	// ensure data center count is within limit
+	if _, found := existingDataCenters[possibleLocation.DataCenter()]; !found {
+		// different from existing dcs
+		if len(existingDataCenters) < replicaPlacement.DiffDataCenterCount+1 {
+			// lack on different dcs
+			return true
+		} else {
+			// adding this would go over the different dcs limit
+			return false
+		}
+	}
+	// now this is same as one of the existing data center
+	if !isAmong(possibleLocation.DataCenter(), primaryDataCenters) {
+		// not on one of the primary dcs
+		return false
 	}
 
-	if replicaPlacement.DiffDataCenterCount >= len(existingDataCenters) {
-		// check dc, good if different from any existing data centers
-		_, found := existingDataCenters[possibleLocation.DataCenter()]
-		return !found
-	} else if replicaPlacement.DiffRackCount >= len(existingRacks) {
-		// check rack, good if different from any existing racks
-		_, found := existingRacks[possibleLocation.Rack()]
-		return !found
-	} else if replicaPlacement.SameRackCount >= len(existingDataNodes) {
-		// check data node, good if different from any existing data nodes
-		_, found := existingDataNodes[possibleLocation.String()]
-		return !found
+	// now this is one of the primary dcs
+	existingRacks := make(map[string]int)
+	for _, loc := range existingLocations {
+		if loc.DataCenter()!=possibleLocation.DataCenter() {
+			continue
+		}
+		existingRacks[loc.Rack()] += 1
+	}
+	primaryRacks, _ := findTopKeys(existingRacks)
+	sameRackCount := existingRacks[possibleLocation.Rack()]
+
+	// ensure rack count is within limit
+	if _, found := existingRacks[possibleLocation.Rack()]; !found {
+		// different from existing racks
+		if len(existingRacks) < replicaPlacement.DiffRackCount+1 {
+			// lack on different racks
+			return true
+		} else {
+			// adding this would go over the different racks limit
+			return false
+		}
+	}
+	// now this is same as one of the existing racks
+	if !isAmong(possibleLocation.Rack(), primaryRacks) {
+		// not on the primary rack
+		return false
 	}
 
+	// now this is on the primary rack
+
+	// different from existing data nodes
+	if sameRackCount < replicaPlacement.SameRackCount+1 {
+		// lack on same rack
+		return true
+	} else {
+		// adding this would go over the same data node limit
+		return false
+	}
+
+}
+
+func findTopKeys(m map[string]int) (topKeys []string, max int) {
+	for k, c := range m {
+		if max < c {
+			topKeys = topKeys[:0]
+			topKeys = append(topKeys, k)
+			max = c
+		} else if max == c {
+			topKeys = append(topKeys, k)
+		}
+	}
+	return
+}
+
+func isAmong(key string, keys []string) bool {
+	for _, k := range keys {
+		if k == key {
+			return true
+		}
+	}
 	return false
 }
 

weed/shell/command_volume_fix_replication_test.go

@@ -0,0 +1,206 @@
+package shell
+
+import (
+	"testing"
+
+	"github.com/chrislusf/seaweedfs/weed/pb/master_pb"
+	"github.com/chrislusf/seaweedfs/weed/storage/super_block"
+)
+
+type testcase struct {
+	name              string
+	replication       string
+	existingLocations []location
+	possibleLocation  location
+	expected          bool
+}
+func TestSatisfyReplicaPlacementComplicated(t *testing.T) {
+
+	var tests = []testcase{
+		{
+			name:        "test 100 negative",
+			replication: "100",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+			},
+			possibleLocation: location{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn2"}},
+			expected:         false,
+		},
+		{
+			name:        "test 100 positive",
+			replication: "100",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+			},
+			possibleLocation: location{"dc2", "r2", &master_pb.DataNodeInfo{Id: "dn2"}},
+			expected:         true,
+		},
+		{
+			name:        "test 022 positive",
+			replication: "022",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn2"}},
+				{"dc1", "r3", &master_pb.DataNodeInfo{Id: "dn3"}},
+			},
+			possibleLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn4"}},
+			expected:         true,
+		},
+		{
+			name:        "test 022 negative",
+			replication: "022",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn2"}},
+				{"dc1", "r3", &master_pb.DataNodeInfo{Id: "dn3"}},
+			},
+			possibleLocation: location{"dc1", "r4", &master_pb.DataNodeInfo{Id: "dn4"}},
+			expected:         false,
+		},
+		{
+			name:        "test 210 moved from 200 positive",
+			replication: "210",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc2", "r2", &master_pb.DataNodeInfo{Id: "dn2"}},
+				{"dc3", "r3", &master_pb.DataNodeInfo{Id: "dn3"}},
+			},
+			possibleLocation: location{"dc1", "r4", &master_pb.DataNodeInfo{Id: "dn4"}},
+			expected:         true,
+		},
+		{
+			name:        "test 210 moved from 200 negative extra dc",
+			replication: "210",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc2", "r2", &master_pb.DataNodeInfo{Id: "dn2"}},
+				{"dc3", "r3", &master_pb.DataNodeInfo{Id: "dn3"}},
+			},
+			possibleLocation: location{"dc4", "r4", &master_pb.DataNodeInfo{Id: "dn4"}},
+			expected:         false,
+		},
+		{
+			name:        "test 210 moved from 200 negative extra data node",
+			replication: "210",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc2", "r2", &master_pb.DataNodeInfo{Id: "dn2"}},
+				{"dc3", "r3", &master_pb.DataNodeInfo{Id: "dn3"}},
+			},
+			possibleLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn4"}},
+			expected:         false,
+		},
+	}
+
+	runTests(tests, t)
+
+}
+
+func TestSatisfyReplicaPlacement01x(t *testing.T) {
+
+	var tests = []testcase{
+		{
+			name:        "test 011 same existing rack",
+			replication: "011",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn2"}},
+			},
+			possibleLocation: location{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn3"}},
+			expected:         true,
+		},
+		{
+			name:        "test 011 negative",
+			replication: "011",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn2"}},
+			},
+			possibleLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn3"}},
+			expected:         false,
+		},
+		{
+			name:        "test 011 different existing racks",
+			replication: "011",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn2"}},
+			},
+			possibleLocation: location{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn3"}},
+			expected:         true,
+		},
+		{
+			name:        "test 011 different existing racks negative",
+			replication: "011",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc1", "r2", &master_pb.DataNodeInfo{Id: "dn2"}},
+			},
+			possibleLocation: location{"dc1", "r3", &master_pb.DataNodeInfo{Id: "dn3"}},
+			expected:         false,
+		},
+	}
+
+	runTests(tests, t)
+
+}
+
+func TestSatisfyReplicaPlacement00x(t *testing.T) {
+
+	var tests = []testcase{
+		{
+			name:        "test 001",
+			replication: "001",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+			},
+			possibleLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn2"}},
+			expected:         true,
+		},
+		{
+			name:        "test 002 positive",
+			replication: "002",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn2"}},
+			},
+			possibleLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn3"}},
+			expected:         true,
+		},
+		{
+			name:        "test 002 negative, repeat the same node",
+			replication: "002",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn2"}},
+			},
+			possibleLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn2"}},
+			expected:         false,
+		},
+		{
+			name:        "test 002 negative, enough node already",
+			replication: "002",
+			existingLocations: []location{
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn1"}},
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn2"}},
+				{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn3"}},
+			},
+			possibleLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn4"}},
+			expected:         false,
+		},
+	}
+
+	runTests(tests, t)
+
+}
+
+func runTests(tests []testcase, t *testing.T) {
+	for _, tt := range tests {
+		replicaPlacement, _ := super_block.NewReplicaPlacementFromString(tt.replication)
+		println("replication:", tt.replication, "expected", tt.expected, "name:", tt.name)
+		if satisfyReplicaPlacement(replicaPlacement, tt.existingLocations, tt.possibleLocation) != tt.expected {
+			t.Errorf("%s: expect %v add %v to %s %+v",
+				tt.name, tt.expected, tt.possibleLocation, tt.replication, tt.existingLocations)
+		}
+	}
+}