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.

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

operator: allows to skip placement rules checks (#5458) #5462

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ require (
github.com/sirupsen/logrus v1.4.2
github.com/spf13/cobra v1.0.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.7.0
github.com/swaggo/http-swagger v0.0.0-20200308142732-58ac5e232fba
github.com/swaggo/swag v1.6.6-0.20200529100950-7c765ddd0476
github.com/syndtr/goleveldb v1.0.1-0.20190318030020-c3a204f8e965
Expand Down
24 changes: 16 additions & 8 deletions server/schedule/operator/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ import (
)

// Builder is used to create operators. Usage:
// op, err := NewBuilder(desc, cluster, region).
// RemovePeer(store1).
// AddPeer(peer1).
// SetLeader(store2).
// Build(kind)
//
// op, err := NewBuilder(desc, cluster, region).
// RemovePeer(store1).
// AddPeer(peer1).
// SetLeader(store2).
// Build(kind)
//
// The generated Operator will choose the most appropriate execution order
// according to various constraints.
type Builder struct {
Expand All @@ -53,8 +55,9 @@ type Builder struct {
targetLeaderStoreID uint64
err error

// skip origin check flags
// skip check flags
skipOriginJointStateCheck bool
skipPlacementRulesCheck bool

// build flags
useJointConsensus bool
Expand All @@ -80,6 +83,11 @@ func SkipOriginJointStateCheck(b *Builder) {
b.skipOriginJointStateCheck = true
}

// SkipPlacementRulesCheck lets the builder skip the placement rules check for origin and target peers.
func SkipPlacementRulesCheck(b *Builder) {
b.skipPlacementRulesCheck = true
}

// NewBuilder creates a Builder.
func NewBuilder(desc string, cluster opt.Cluster, region *core.RegionInfo, opts ...BuilderOption) *Builder {
b := &Builder{
Expand Down Expand Up @@ -123,7 +131,7 @@ func NewBuilder(desc string, cluster opt.Cluster, region *core.RegionInfo, opts

// placement rules
var rules []*placement.Rule
if err == nil && cluster.GetOpts().IsPlacementRulesEnabled() {
if err == nil && !b.skipPlacementRulesCheck && cluster.GetOpts().IsPlacementRulesEnabled() {
fit := cluster.GetRuleManager().FitRegion(cluster, region)
for _, rf := range fit.RuleFits {
rules = append(rules, rf.Rule)
Expand Down Expand Up @@ -737,7 +745,7 @@ func (b *Builder) allowLeader(peer *metapb.Peer, ignoreClusterLimit bool) bool {
}

// placement rules
if len(b.rules) == 0 {
if b.skipPlacementRulesCheck || len(b.rules) == 0 {
return true
}
for _, r := range b.rules {
Expand Down
4 changes: 2 additions & 2 deletions server/schedule/operator/create_operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func CreateTransferLeaderOperator(desc string, cluster opt.Cluster, region *core

// CreateForceTransferLeaderOperator creates an operator that transfers the leader from a source store to a target store forcible.
func CreateForceTransferLeaderOperator(desc string, cluster opt.Cluster, region *core.RegionInfo, sourceStoreID uint64, targetStoreID uint64, kind OpKind) (*Operator, error) {
return NewBuilder(desc, cluster, region, SkipOriginJointStateCheck).
return NewBuilder(desc, cluster, region, SkipOriginJointStateCheck, SkipPlacementRulesCheck).
SetLeader(targetStoreID).
EnableForceTargetLeader().
Build(kind)
Expand Down Expand Up @@ -219,7 +219,7 @@ func CreateScatterRegionOperator(desc string, cluster opt.Cluster, origin *core.

// CreateLeaveJointStateOperator creates an operator that let region leave joint state.
func CreateLeaveJointStateOperator(desc string, cluster opt.Cluster, origin *core.RegionInfo) (*Operator, error) {
b := NewBuilder(desc, cluster, origin, SkipOriginJointStateCheck)
b := NewBuilder(desc, cluster, origin, SkipOriginJointStateCheck, SkipPlacementRulesCheck)

if b.err == nil && !core.IsInJointState(origin.GetPeers()...) {
b.err = errors.Errorf("cannot build leave joint state operator for region which is not in joint state")
Expand Down
62 changes: 62 additions & 0 deletions server/schedule/operator/create_operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ package operator

import (
"context"
"encoding/hex"
"strings"
"testing"

. "github.com/pingcap/check"
"github.com/pingcap/errors"
Expand All @@ -27,6 +29,8 @@ import (
"github.com/tikv/pd/server/core"
"github.com/tikv/pd/server/schedule/placement"
"github.com/tikv/pd/server/versioninfo"

"github.com/stretchr/testify/require"
)

var _ = Suite(&testCreateOperatorSuite{})
Expand Down Expand Up @@ -1073,3 +1077,61 @@ func (s *testCreateOperatorSuite) TestMoveRegionWithoutJointConsensus(c *C) {
}
}
}

// Ref https://github.com/tikv/pd/issues/5401
func TestCreateLeaveJointStateOperatorWithoutFitRules(t *testing.T) {
re := require.New(t)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

opts := config.NewTestOptions()
cluster := mockcluster.NewCluster(ctx, opts)
re.NoError(cluster.SetRules([]*placement.Rule{
{
GroupID: "pd",
ID: "default",
StartKeyHex: hex.EncodeToString([]byte("")),
EndKeyHex: hex.EncodeToString([]byte("")),
Role: placement.Voter,
Count: 1,
},
{
GroupID: "t1",
ID: "t1",
StartKeyHex: hex.EncodeToString([]byte("a")),
EndKeyHex: hex.EncodeToString([]byte("b")),
Role: placement.Voter,
Count: 1,
},
{
GroupID: "t2",
ID: "t2",
StartKeyHex: hex.EncodeToString([]byte("b")),
EndKeyHex: hex.EncodeToString([]byte("c")),
Role: placement.Voter,
Count: 1,
},
}))
cluster.AddRegionStore(1, 1)
cluster.AddRegionStore(2, 1)
cluster.AddRegionStore(3, 1)
cluster.AddRegionStore(4, 1)
originPeers := []*metapb.Peer{
{Id: 3, StoreId: 3, Role: metapb.PeerRole_DemotingVoter},
{Id: 4, StoreId: 4, Role: metapb.PeerRole_IncomingVoter},
}

region := core.NewRegionInfo(&metapb.Region{Id: 1, Peers: originPeers, StartKey: []byte("a"), EndKey: []byte("c")}, originPeers[0])
op, err := CreateLeaveJointStateOperator("test", cluster, region)
re.NoError(err)
re.Equal(OpLeader, op.Kind())
re.Len(op.steps, 2)
step0 := op.steps[0].(TransferLeader)
re.Equal(uint64(3), step0.FromStore)
re.Equal(uint64(4), step0.ToStore)
step1 := op.steps[1].(ChangePeerV2Leave)
re.Len(step1.PromoteLearners, 1)
re.Len(step1.DemoteVoters, 1)
re.Equal(uint64(4), step1.PromoteLearners[0].ToStore)
re.Equal(uint64(3), step1.DemoteVoters[0].ToStore)
}