Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvserver: get rid of bespoke relocation logic used by the mergeQueue #62631

Merged
merged 3 commits into from
Mar 29, 2021
Merged

kvserver: get rid of bespoke relocation logic used by the mergeQueue #62631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c08e57
kvserver: remove rotten comment inside `AdminRelocateRange`
Mar 25, 2021
fb51671
kvserver: update `AdminRelocateRange` to leverage explicit swaps of
aayushshah15 Mar 26, 2021
d7a7a49
kvserver: get rid of bespoke relocation logic used by the mergeQueue
aayushshah15 Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion pkg/kv/kvserver/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/base",
"//pkg/build",
"//pkg/clusterversion",
"//pkg/config",
"//pkg/config/zonepb",
Expand Down
33 changes: 31 additions & 2 deletions pkg/kv/kvserver/allocator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,43 @@ const (
nonVoterTarget
)

// AddChangeType returns the roachpb.ReplicaChangeType corresponding to the
// given targetReplicaType.
//
// TODO(aayush): Clean up usages of ADD_{NON_}VOTER. Use
// targetReplicaType.{Add,Remove}ChangeType methods wherever possible.
func (t targetReplicaType) AddChangeType() roachpb.ReplicaChangeType {
switch t {
case voterTarget:
return roachpb.ADD_VOTER
case nonVoterTarget:
return roachpb.ADD_NON_VOTER
default:
panic(fmt.Sprintf("unknown targetReplicaType %d", t))
}
}

// RemoveChangeType returns the roachpb.ReplicaChangeType corresponding to the
// given targetReplicaType.
func (t targetReplicaType) RemoveChangeType() roachpb.ReplicaChangeType {
switch t {
case voterTarget:
return roachpb.REMOVE_VOTER
case nonVoterTarget:
return roachpb.REMOVE_NON_VOTER
default:
panic(fmt.Sprintf("unknown targetReplicaType %d", t))
}
}

func (t targetReplicaType) String() string {
switch typ := t; typ {
switch t {
case voterTarget:
return "voter"
case nonVoterTarget:
return "non-voter"
default:
panic(fmt.Sprintf("unknown targetReplicaType %d", typ))
panic(fmt.Sprintf("unknown targetReplicaType %d", t))
}
}

Expand Down
197 changes: 79 additions & 118 deletions pkg/kv/kvserver/client_merge_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"math/rand"
"reflect"
"regexp"
"sort"
"strings"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -4175,137 +4174,117 @@ func TestMergeQueueSeesNonVoters(t *testing.T) {
type test struct {
name string
leftVoters, rightVoters, leftNonVoters, rightNonVoters []int
expectedRightVoters, expectedRightNonVoters []int
}

// NB: The test setup code places a single voter replica on (n1,s1) for both
// left and right range, which we remove after setting the test up.
tests := []test{
{
name: "collocated-per-type",
leftVoters: []int{2, 3, 4},
rightVoters: []int{2, 3, 4},
leftNonVoters: []int{1},
rightNonVoters: []int{1},
expectedRightVoters: []int{2, 3, 4},
expectedRightNonVoters: []int{1},
name: "collocated-per-type",
leftVoters: []int{2, 3, 4},
rightVoters: []int{2, 3, 4},
leftNonVoters: []int{1},
rightNonVoters: []int{1},
},
{
name: "collocated-overall",
leftVoters: []int{3, 4},
rightVoters: []int{1, 2},
leftNonVoters: []int{1, 2},
rightNonVoters: []int{3, 4},
expectedRightVoters: []int{1, 2},
expectedRightNonVoters: []int{3, 4},
name: "collocated-overall",
leftVoters: []int{3, 4},
rightVoters: []int{1, 2},
leftNonVoters: []int{1, 2},
rightNonVoters: []int{3, 4},
},
{
name: "collocated-voters-only",
leftVoters: []int{3, 4},
rightVoters: []int{3, 4},
leftNonVoters: []int{2},
rightNonVoters: []int{1},
expectedRightVoters: []int{3, 4},
expectedRightNonVoters: []int{2},
name: "collocated-voters-only",
leftVoters: []int{3, 4},
rightVoters: []int{3, 4},
leftNonVoters: []int{2},
rightNonVoters: []int{1},
},
{
name: "collocated-non-voters-only",
leftVoters: []int{3},
rightVoters: []int{4},
leftNonVoters: []int{1, 2},
rightNonVoters: []int{1, 2},
expectedRightVoters: []int{3},
expectedRightNonVoters: []int{1, 2},
name: "collocated-non-voters-only",
leftVoters: []int{3},
rightVoters: []int{4},
leftNonVoters: []int{1, 2},
rightNonVoters: []int{1, 2},
},
{
name: "not-collocated",
leftVoters: []int{3},
rightVoters: []int{4},
leftNonVoters: []int{2},
rightNonVoters: []int{1},
expectedRightVoters: []int{3},
expectedRightNonVoters: []int{2},
name: "not-collocated",
leftVoters: []int{3},
rightVoters: []int{4},
leftNonVoters: []int{2},
rightNonVoters: []int{1},
},
{
name: "partially-collocated-voters-only",
leftVoters: []int{2, 3},
rightVoters: []int{1, 4},
leftNonVoters: []int{1},
rightNonVoters: []int{2},
expectedRightVoters: []int{1, 3},
expectedRightNonVoters: []int{2},
name: "partially-collocated-voters-only",
leftVoters: []int{2, 3},
rightVoters: []int{1, 4},
leftNonVoters: []int{1},
rightNonVoters: []int{2},
},
{
name: "partially-collocated-non-voters-only",
leftVoters: []int{4},
rightVoters: []int{4},
leftNonVoters: []int{1, 3},
rightNonVoters: []int{1, 2},
expectedRightVoters: []int{4},
expectedRightNonVoters: []int{1, 3},
name: "partially-collocated-non-voters-only",
leftVoters: []int{4},
rightVoters: []int{4},
leftNonVoters: []int{1, 3},
rightNonVoters: []int{1, 2},
},
{
name: "partially-collocated",
leftVoters: []int{2},
rightVoters: []int{4},
leftNonVoters: []int{1, 3},
rightNonVoters: []int{1, 2},
expectedRightVoters: []int{3},
expectedRightNonVoters: []int{1, 2},
name: "partially-collocated",
leftVoters: []int{2},
rightVoters: []int{4},
leftNonVoters: []int{1, 3},
rightNonVoters: []int{1, 2},
},
{
name: "collocated-rhs-being-reconfigured-1",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1, 2, 3, 4, 5, 6},
leftNonVoters: []int{4, 5, 6},
rightNonVoters: []int{},
expectedRightVoters: []int{1, 2, 3, 4, 5, 6},
expectedRightNonVoters: []int{},
name: "collocated-rhs-being-reconfigured-1",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1, 2, 3, 4, 5, 6},
leftNonVoters: []int{4, 5, 6},
rightNonVoters: []int{},
},
{
name: "collocated-rhs-being-reconfigured-2",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1, 2, 3, 4},
leftNonVoters: []int{4, 5, 6},
rightNonVoters: []int{},
expectedRightVoters: []int{1, 2, 3, 4},
expectedRightNonVoters: []int{5, 6},
name: "collocated-rhs-being-reconfigured-2",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1, 2, 3, 4},
leftNonVoters: []int{4, 5, 6},
rightNonVoters: []int{},
},
{
name: "collocated-rhs-being-reconfigured-3",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1},
leftNonVoters: []int{4, 5, 6},
rightNonVoters: []int{2, 3, 4, 5, 6},
expectedRightVoters: []int{1},
expectedRightNonVoters: []int{2, 3, 4, 5, 6},
name: "collocated-rhs-being-reconfigured-3",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1},
leftNonVoters: []int{4, 5, 6},
rightNonVoters: []int{2, 3, 4, 5, 6},
},
{
name: "non-collocated-rhs-being-reconfigured",
leftVoters: []int{1, 2, 3},
rightVoters: []int{5},
leftNonVoters: []int{4, 6},
rightNonVoters: []int{},
expectedRightVoters: []int{1, 2, 3},
expectedRightNonVoters: []int{4, 6},
name: "non-collocated-rhs-being-reconfigured",
leftVoters: []int{1, 2, 3},
rightVoters: []int{5},
leftNonVoters: []int{4, 6},
rightNonVoters: []int{},
},
{
name: "partially-collocated-rhs-being-downreplicated",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1, 2, 3, 4, 5, 6},
leftNonVoters: []int{4, 5},
rightNonVoters: []int{},
expectedRightVoters: []int{1, 2, 3, 4, 5},
expectedRightNonVoters: []int{},
name: "partially-collocated-rhs-being-downreplicated",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1, 2, 3, 4, 5, 6},
leftNonVoters: []int{4, 5},
rightNonVoters: []int{},
},
{
name: "partially-collocated-rhs-being-upreplicated",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1},
leftNonVoters: []int{4, 5, 6},
rightNonVoters: []int{},
expectedRightVoters: []int{1, 2, 3},
expectedRightNonVoters: []int{4, 5, 6},
name: "partially-collocated-rhs-being-upreplicated",
leftVoters: []int{1, 2, 3},
rightVoters: []int{1},
leftNonVoters: []int{4, 5, 6},
rightNonVoters: []int{},
},
{
// This is a subtest that should trigger at least 3 voter<->non-voter
// swaps.
name: "lhs-voters-collocated-with-rhs-non-voters",
leftVoters: []int{1, 2, 3},
rightVoters: []int{4},
leftNonVoters: []int{},
rightNonVoters: []int{1, 2, 3},
},
}

Expand Down Expand Up @@ -4374,24 +4353,6 @@ func TestMergeQueueSeesNonVoters(t *testing.T) {
tc.RemoveVotersOrFatal(t, rightDesc.StartKey.AsRawKey(), tc.Target(0))
rightDesc = tc.LookupRangeOrFatal(t, rightDesc.StartKey.AsRawKey())

// Check that we're avoiding superfluous data movement.
voterTargets, nonVoterTargets, err := kvserver.GetTargetsToCollocateRHSForMerge(ctx, leftDesc.Replicas(), rightDesc.Replicas())
require.NoError(t, err)
require.Equal(t, len(subtest.expectedRightVoters), len(voterTargets))
require.Equal(t, len(subtest.expectedRightNonVoters), len(nonVoterTargets))
sort.Slice(voterTargets, func(i, j int) bool {
return voterTargets[i].NodeID < voterTargets[j].NodeID
})
sort.Slice(nonVoterTargets, func(i, j int) bool {
return nonVoterTargets[i].NodeID < nonVoterTargets[j].NodeID
})
for i := range subtest.expectedRightVoters {
require.Equal(t, tc.Target(subtest.expectedRightVoters[i]), voterTargets[i])
}
for i := range subtest.expectedRightNonVoters {
require.Equal(t, tc.Target(subtest.expectedRightNonVoters[i]), nonVoterTargets[i])
}

store.SetMergeQueueActive(true)
store.MustForceMergeScanAndProcess()
verifyMerged(t, store, leftDesc.StartKey, rightDesc.StartKey)
Expand Down
Loading