diff --git a/pkg/service/repair/plan.go b/pkg/service/repair/plan.go index 50a2cbea3..a615908b8 100644 --- a/pkg/service/repair/plan.go +++ b/pkg/service/repair/plan.go @@ -255,27 +255,18 @@ func ShouldRepairRing(ring scyllaclient.Ring, dcs []string, host string) bool { } } - switch ring.Replication { - case scyllaclient.SimpleStrategy: - // Check range consisting of excluded hosts - excluded := 0 - for _, dc := range ring.HostDC { - if !repairedDCs.Has(dc) { - excluded++ - } - } - return ring.RF > excluded+1 - case scyllaclient.NetworkTopologyStrategy: + for _, rt := range ring.ReplicaTokens { rep := 0 - for dc, rf := range ring.DCrf { - if repairedDCs.Has(dc) { - rep += rf + for _, r := range rt.ReplicaSet { + if repairedDCs.Has(ring.HostDC[r]) { + rep++ } } - return rep > 1 - default: - return false + if rep <= 1 { + return false + } } + return true } // maxHostIntensity sets max_ranges_in_parallel for all repaired host. diff --git a/pkg/service/repair/repair_test.go b/pkg/service/repair/repair_test.go index 6d4d517c2..c93b43468 100644 --- a/pkg/service/repair/repair_test.go +++ b/pkg/service/repair/repair_test.go @@ -6,6 +6,7 @@ import ( "fmt" "testing" + "github.com/scylladb/scylla-manager/v3/pkg/dht" "github.com/scylladb/scylla-manager/v3/pkg/scyllaclient" "github.com/scylladb/scylla-manager/v3/pkg/service/repair" ) @@ -124,6 +125,20 @@ func TestShouldRepairRing(t *testing.T) { "h9": "dc4", } + rs := func(reps ...string) scyllaclient.ReplicaTokenRanges { + return scyllaclient.ReplicaTokenRanges{ + ReplicaSet: reps, + Ranges: []scyllaclient.TokenRange{ + { + StartToken: dht.Murmur3MinToken, + EndToken: dht.Murmur3MaxToken, + }, + }, + } + } + + allDCs := []string{"dc1", "dc2", "dc3", "dc4"} + testCases := []struct { Ring scyllaclient.Ring DCs []string @@ -132,114 +147,118 @@ func TestShouldRepairRing(t *testing.T) { }{ { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.LocalStrategy, - RF: 1, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{rs("h1")}, + HostDC: hostDC, }, - DCs: []string{"dc1", "dc2", "dc3", "dc4"}, + DCs: allDCs, Expected: false, }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.SimpleStrategy, - RF: 1, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1"), rs("h2"), rs("h3"), + rs("h4"), rs("h5"), rs("h6"), + rs("h7"), rs("h8"), rs("h9"), + }, + HostDC: hostDC, }, - DCs: []string{"dc1", "dc2", "dc3", "dc4"}, + DCs: allDCs, Expected: false, }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.SimpleStrategy, - RF: 2, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2"), rs("h2", "h3"), rs("h3", "h4"), + rs("h4", "h5"), rs("h5", "h6"), rs("h6", "h7"), + rs("h7", "h8"), rs("h8", "h9"), rs("h9", "h1"), + }, + HostDC: hostDC, }, - DCs: []string{"dc1", "dc2", "dc3", "dc4"}, + DCs: allDCs, Expected: true, }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.SimpleStrategy, - RF: 3, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2", "h3"), rs("h2", "h3", "h4"), rs("h3", "h4", "h5"), + rs("h4", "h5", "h6"), rs("h5", "h6", "h7"), rs("h6", "h7", "h8"), + rs("h7", "h8", "h9"), rs("h8", "h9", "h1"), rs("h9", "h1", "h2"), + }, + HostDC: hostDC, }, DCs: []string{"dc3", "dc4"}, Expected: false, }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.SimpleStrategy, - RF: 4, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2", "h3", "h4"), rs("h2", "h3", "h4", "h5"), rs("h3", "h4", "h5", "h6"), + rs("h4", "h5", "h6", "h7"), rs("h5", "h6", "h7", "h8"), rs("h6", "h7", "h8", "h9"), + rs("h7", "h8", "h9", "h1"), rs("h8", "h9", "h1", "h2"), rs("h9", "h1", "h2", "h3"), + }, + HostDC: hostDC, }, DCs: []string{"dc3", "dc4"}, Expected: true, }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.SimpleStrategy, - RF: 4, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2", "h3", "h4"), rs("h2", "h3", "h4", "h5"), rs("h3", "h4", "h5", "h6"), + rs("h4", "h5", "h6", "h7"), rs("h5", "h6", "h7", "h8"), rs("h6", "h7", "h8", "h9"), + rs("h7", "h8", "h9", "h1"), rs("h8", "h9", "h1", "h2"), rs("h9", "h1", "h2", "h3"), + }, + HostDC: hostDC, }, DCs: []string{"dc4"}, Expected: false, }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.NetworkTopologyStrategy, - RF: 4, - DCrf: map[string]int{ - "dc1": 1, - "dc2": 1, - "dc3": 1, - "dc4": 1, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2", "h3", "h6"), rs("h1", "h2", "h4", "h6"), rs("h1", "h2", "h5", "h6"), + rs("h1", "h2", "h3", "h7"), rs("h1", "h2", "h4", "h7"), rs("h1", "h2", "h5", "h7"), + rs("h1", "h2", "h3", "h8"), rs("h1", "h2", "h4", "h8"), rs("h1", "h2", "h5", "h8"), + rs("h1", "h2", "h3", "h9"), rs("h1", "h2", "h4", "h9"), rs("h1", "h2", "h5", "h9"), }, + HostDC: hostDC, }, - DCs: []string{"dc1", "dc2", "dc3", "dc4"}, + DCs: allDCs, Expected: true, }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.NetworkTopologyStrategy, - RF: 4, - DCrf: map[string]int{ - "dc1": 1, - "dc2": 1, - "dc3": 1, - "dc4": 1, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2", "h3", "h6"), rs("h1", "h2", "h4", "h6"), rs("h1", "h2", "h5", "h6"), + rs("h1", "h2", "h3", "h7"), rs("h1", "h2", "h4", "h7"), rs("h1", "h2", "h5", "h7"), + rs("h1", "h2", "h3", "h8"), rs("h1", "h2", "h4", "h8"), rs("h1", "h2", "h5", "h8"), + rs("h1", "h2", "h3", "h9"), rs("h1", "h2", "h4", "h9"), rs("h1", "h2", "h5", "h9"), }, + HostDC: hostDC, }, DCs: []string{"dc1"}, Expected: false, }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.NetworkTopologyStrategy, - RF: 8, - DCrf: map[string]int{ - "dc1": 1, - "dc2": 1, - "dc3": 2, - "dc4": 2, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2", "h3", "h4", "h6", "h7"), rs("h1", "h2", "h5", "h4", "h6", "h7"), rs("h1", "h2", "h3", "h5", "h6", "h7"), + rs("h1", "h2", "h3", "h4", "h6", "h8"), rs("h1", "h2", "h5", "h4", "h6", "h8"), rs("h1", "h2", "h3", "h5", "h6", "h8"), + rs("h1", "h2", "h3", "h4", "h6", "h9"), rs("h1", "h2", "h5", "h4", "h6", "h9"), rs("h1", "h2", "h3", "h5", "h6", "h9"), + rs("h1", "h2", "h3", "h4", "h7", "h8"), rs("h1", "h2", "h5", "h4", "h7", "h8"), rs("h1", "h2", "h3", "h5", "h7", "h8"), + rs("h1", "h2", "h3", "h4", "h8", "h9"), rs("h1", "h2", "h5", "h4", "h8", "h9"), rs("h1", "h2", "h3", "h5", "h8", "h9"), }, + HostDC: hostDC, }, DCs: []string{"dc3"}, Expected: true, }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.NetworkTopologyStrategy, - RF: 8, - DCrf: map[string]int{ - "dc1": 1, - "dc2": 1, - "dc3": 3, - "dc4": 4, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2", "h3", "h4", "h5", "h6", "h7", "h8", "h9"), }, + HostDC: hostDC, }, DCs: []string{"dc4"}, Host: "h6", @@ -247,15 +266,10 @@ func TestShouldRepairRing(t *testing.T) { }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.NetworkTopologyStrategy, - RF: 8, - DCrf: map[string]int{ - "dc1": 1, - "dc2": 1, - "dc3": 3, - "dc4": 4, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2", "h3", "h4", "h5", "h6", "h7", "h8", "h9"), }, + HostDC: hostDC, }, DCs: []string{"dc4"}, Host: "h2", @@ -263,17 +277,12 @@ func TestShouldRepairRing(t *testing.T) { }, { Ring: scyllaclient.Ring{ - HostDC: hostDC, - Replication: scyllaclient.NetworkTopologyStrategy, - RF: 8, - DCrf: map[string]int{ - "dc1": 1, - "dc2": 1, - "dc3": 3, - "dc4": 4, + ReplicaTokens: []scyllaclient.ReplicaTokenRanges{ + rs("h1", "h2", "h3", "h4", "h5", "h6", "h7", "h8", "h9"), }, + HostDC: hostDC, }, - DCs: []string{"dc1", "dc2", "dc3", "dc4"}, + DCs: allDCs, Host: "h1", Expected: true, },