From a0481c2cb1258cdc5a4e091f3f36ae4199b34075 Mon Sep 17 00:00:00 2001 From: Connor1996 Date: Thu, 4 Apr 2019 11:26:37 +0800 Subject: [PATCH 1/3] fix merge easy to timeout Signed-off-by: Connor1996 --- server/schedule/operator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/schedule/operator.go b/server/schedule/operator.go index b2ad340b1bc..821e7958bcb 100644 --- a/server/schedule/operator.go +++ b/server/schedule/operator.go @@ -507,7 +507,7 @@ func CreateMergeRegionOperator(desc string, cluster Cluster, source *core.Region }) op1 := NewOperator(desc, source.GetID(), source.GetRegionEpoch(), kinds|kind|OpMerge, steps...) - op2 := NewOperator(desc, target.GetID(), target.GetRegionEpoch(), kind|OpMerge, MergeRegion{ + op2 := NewOperator(desc, target.GetID(), target.GetRegionEpoch(), kinds|kind|OpMerge, MergeRegion{ FromRegion: source.GetMeta(), ToRegion: target.GetMeta(), IsPassive: true, From 6ef59bc8dd9bb0d3f82567914a2f8d47a0cf388b Mon Sep 17 00:00:00 2001 From: Connor1996 Date: Thu, 4 Apr 2019 16:47:35 +0800 Subject: [PATCH 2/3] add test Signed-off-by: Connor1996 --- server/schedule/merge_checker_test.go | 249 ++++++++++++++++++++++++++ server/schedulers/balance_test.go | 221 ----------------------- 2 files changed, 249 insertions(+), 221 deletions(-) create mode 100644 server/schedule/merge_checker_test.go diff --git a/server/schedule/merge_checker_test.go b/server/schedule/merge_checker_test.go new file mode 100644 index 00000000000..ae38553997b --- /dev/null +++ b/server/schedule/merge_checker_test.go @@ -0,0 +1,249 @@ +// Copyright 2017 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package schedule + +import ( + "time" + + . "github.com/pingcap/check" + "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/pd/server/core" + "github.com/pingcap/pd/server/namespace" +) + +var _ = Suite(&testMergeCheckerSuite{}) + +type testMergeCheckerSuite struct { + cluster *MockCluster + mc *MergeChecker + regions []*core.RegionInfo +} + +func (s *testMergeCheckerSuite) SetUpTest(c *C) { + cfg := NewMockSchedulerOptions() + cfg.MaxMergeRegionSize = 2 + cfg.MaxMergeRegionKeys = 2 + s.cluster = NewMockCluster(cfg) + s.regions = []*core.RegionInfo{ + core.NewRegionInfo( + &metapb.Region{ + Id: 1, + StartKey: []byte(""), + EndKey: []byte("a"), + Peers: []*metapb.Peer{ + {Id: 101, StoreId: 1}, + {Id: 102, StoreId: 2}, + }, + }, + &metapb.Peer{Id: 101, StoreId: 1}, + core.SetApproximateSize(1), + core.SetApproximateKeys(1), + ), + core.NewRegionInfo( + &metapb.Region{ + Id: 2, + StartKey: []byte("a"), + EndKey: []byte("t"), + Peers: []*metapb.Peer{ + {Id: 103, StoreId: 1}, + {Id: 104, StoreId: 4}, + {Id: 105, StoreId: 5}, + }, + }, + &metapb.Peer{Id: 104, StoreId: 4}, + core.SetApproximateSize(200), + core.SetApproximateKeys(200), + ), + core.NewRegionInfo( + &metapb.Region{ + Id: 3, + StartKey: []byte("t"), + EndKey: []byte("x"), + Peers: []*metapb.Peer{ + {Id: 106, StoreId: 2}, + {Id: 107, StoreId: 5}, + {Id: 108, StoreId: 6}, + }, + }, + &metapb.Peer{Id: 108, StoreId: 6}, + core.SetApproximateSize(1), + core.SetApproximateKeys(1), + ), + core.NewRegionInfo( + &metapb.Region{ + Id: 4, + StartKey: []byte("x"), + EndKey: []byte(""), + Peers: []*metapb.Peer{ + {Id: 109, StoreId: 4}, + }, + }, + &metapb.Peer{Id: 109, StoreId: 4}, + core.SetApproximateSize(10), + core.SetApproximateKeys(10), + ), + } + + for _, region := range s.regions { + s.cluster.PutRegion(region) + } + + s.mc = NewMergeChecker(s.cluster, namespace.DefaultClassifier) +} + +func (s *testMergeCheckerSuite) TestBasic(c *C) { + s.cluster.MockSchedulerOptions.SplitMergeInterval = time.Hour + + // should with same peer count + ops := s.mc.Check(s.regions[0]) + c.Assert(ops, IsNil) + // The size should be small enough. + ops = s.mc.Check(s.regions[1]) + c.Assert(ops, IsNil) + ops = s.mc.Check(s.regions[2]) + c.Assert(ops, NotNil) + for _, op := range ops { + op.createTime = op.createTime.Add(-LeaderOperatorWaitTime - time.Second) + c.Assert(op.IsTimeout(), IsFalse) + op.createTime = op.createTime.Add(-RegionOperatorWaitTime - time.Second) + c.Assert(op.IsTimeout(), IsTrue) + } + // Skip recently split regions. + s.mc.RecordRegionSplit(s.regions[2].GetID()) + ops = s.mc.Check(s.regions[2]) + c.Assert(ops, IsNil) + ops = s.mc.Check(s.regions[3]) + c.Assert(ops, IsNil) +} + +func (s *testMergeCheckerSuite) checkSteps(c *C, op *Operator, steps []OperatorStep) { + c.Assert(op.Kind()&OpMerge, Not(Equals), 0) + c.Assert(steps, NotNil) + c.Assert(op.Len(), Equals, len(steps)) + for i := range steps { + c.Assert(op.Step(i), DeepEquals, steps[i]) + } +} + +func (s *testMergeCheckerSuite) TestMatchPeers(c *C) { + // partial store overlap not including leader + ops := s.mc.Check(s.regions[2]) + s.checkSteps(c, ops[0], []OperatorStep{ + TransferLeader{FromStore: 6, ToStore: 5}, + AddLearner{ToStore: 1, PeerID: 1}, + PromoteLearner{ToStore: 1, PeerID: 1}, + RemovePeer{FromStore: 2}, + AddLearner{ToStore: 4, PeerID: 2}, + PromoteLearner{ToStore: 4, PeerID: 2}, + RemovePeer{FromStore: 6}, + MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: false, + }, + }) + s.checkSteps(c, ops[1], []OperatorStep{ + MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: true, + }, + }) + + // partial store overlap including leader + newRegion := s.regions[2].Clone( + core.SetPeers([]*metapb.Peer{ + {Id: 106, StoreId: 1}, + {Id: 107, StoreId: 5}, + {Id: 108, StoreId: 6}, + }), + core.WithLeader(&metapb.Peer{Id: 106, StoreId: 1}), + ) + s.regions[2] = newRegion + s.cluster.PutRegion(s.regions[2]) + ops = s.mc.Check(s.regions[2]) + s.checkSteps(c, ops[0], []OperatorStep{ + AddLearner{ToStore: 4, PeerID: 3}, + PromoteLearner{ToStore: 4, PeerID: 3}, + RemovePeer{FromStore: 6}, + MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: false, + }, + }) + s.checkSteps(c, ops[1], []OperatorStep{ + MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: true, + }, + }) + + // all stores overlap + s.regions[2] = s.regions[2].Clone(core.SetPeers([]*metapb.Peer{ + {Id: 106, StoreId: 1}, + {Id: 107, StoreId: 5}, + {Id: 108, StoreId: 4}, + })) + s.cluster.PutRegion(s.regions[2]) + ops = s.mc.Check(s.regions[2]) + s.checkSteps(c, ops[0], []OperatorStep{ + MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: false, + }, + }) + s.checkSteps(c, ops[1], []OperatorStep{ + MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: true, + }, + }) + + // all stores not overlap + s.regions[2] = s.regions[2].Clone(core.SetPeers([]*metapb.Peer{ + {Id: 109, StoreId: 2}, + {Id: 110, StoreId: 3}, + {Id: 111, StoreId: 6}, + }), core.WithLeader(&metapb.Peer{Id: 109, StoreId: 2})) + s.cluster.PutRegion(s.regions[2]) + ops = s.mc.Check(s.regions[2]) + s.checkSteps(c, ops[0], []OperatorStep{ + AddLearner{ToStore: 1, PeerID: 4}, + PromoteLearner{ToStore: 1, PeerID: 4}, + RemovePeer{FromStore: 3}, + AddLearner{ToStore: 4, PeerID: 5}, + PromoteLearner{ToStore: 4, PeerID: 5}, + RemovePeer{FromStore: 6}, + AddLearner{ToStore: 5, PeerID: 6}, + PromoteLearner{ToStore: 5, PeerID: 6}, + TransferLeader{FromStore: 2, ToStore: 1}, + RemovePeer{FromStore: 2}, + MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: false, + }, + }) + s.checkSteps(c, ops[1], []OperatorStep{ + MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: true, + }, + }) +} diff --git a/server/schedulers/balance_test.go b/server/schedulers/balance_test.go index 2e02fd8c2d8..d394971e23d 100644 --- a/server/schedulers/balance_test.go +++ b/server/schedulers/balance_test.go @@ -17,7 +17,6 @@ import ( "fmt" "math" "math/rand" - "time" . "github.com/pingcap/check" "github.com/pingcap/kvproto/pkg/metapb" @@ -839,226 +838,6 @@ func (s *testRandomMergeSchedulerSuite) TestMerge(c *C) { c.Assert(mb.IsScheduleAllowed(tc), IsFalse) } -var _ = Suite(&testMergeCheckerSuite{}) - -type testMergeCheckerSuite struct { - cluster *schedule.MockCluster - mc *schedule.MergeChecker - regions []*core.RegionInfo -} - -func (s *testMergeCheckerSuite) SetUpTest(c *C) { - cfg := schedule.NewMockSchedulerOptions() - cfg.MaxMergeRegionSize = 2 - cfg.MaxMergeRegionKeys = 2 - s.cluster = schedule.NewMockCluster(cfg) - s.regions = []*core.RegionInfo{ - core.NewRegionInfo( - &metapb.Region{ - Id: 1, - StartKey: []byte(""), - EndKey: []byte("a"), - Peers: []*metapb.Peer{ - {Id: 101, StoreId: 1}, - {Id: 102, StoreId: 2}, - }, - }, - &metapb.Peer{Id: 101, StoreId: 1}, - core.SetApproximateSize(1), - core.SetApproximateKeys(1), - ), - core.NewRegionInfo( - &metapb.Region{ - Id: 2, - StartKey: []byte("a"), - EndKey: []byte("t"), - Peers: []*metapb.Peer{ - {Id: 103, StoreId: 1}, - {Id: 104, StoreId: 4}, - {Id: 105, StoreId: 5}, - }, - }, - &metapb.Peer{Id: 104, StoreId: 4}, - core.SetApproximateSize(200), - core.SetApproximateKeys(200), - ), - core.NewRegionInfo( - &metapb.Region{ - Id: 3, - StartKey: []byte("t"), - EndKey: []byte("x"), - Peers: []*metapb.Peer{ - {Id: 106, StoreId: 2}, - {Id: 107, StoreId: 5}, - {Id: 108, StoreId: 6}, - }, - }, - &metapb.Peer{Id: 108, StoreId: 6}, - core.SetApproximateSize(1), - core.SetApproximateKeys(1), - ), - core.NewRegionInfo( - &metapb.Region{ - Id: 4, - StartKey: []byte("x"), - EndKey: []byte(""), - Peers: []*metapb.Peer{ - {Id: 109, StoreId: 4}, - }, - }, - &metapb.Peer{Id: 109, StoreId: 4}, - core.SetApproximateSize(10), - core.SetApproximateKeys(10), - ), - } - - for _, region := range s.regions { - s.cluster.PutRegion(region) - } - - s.mc = schedule.NewMergeChecker(s.cluster, namespace.DefaultClassifier) -} - -func (s *testMergeCheckerSuite) TestBasic(c *C) { - s.cluster.MockSchedulerOptions.SplitMergeInterval = time.Hour - - // should with same peer count - ops := s.mc.Check(s.regions[0]) - c.Assert(ops, IsNil) - // The size should be small enough. - ops = s.mc.Check(s.regions[1]) - c.Assert(ops, IsNil) - ops = s.mc.Check(s.regions[2]) - c.Assert(ops, NotNil) - // Skip recently split regions. - s.mc.RecordRegionSplit(s.regions[2].GetID()) - ops = s.mc.Check(s.regions[2]) - c.Assert(ops, IsNil) - ops = s.mc.Check(s.regions[3]) - c.Assert(ops, IsNil) -} - -func (s *testMergeCheckerSuite) checkSteps(c *C, op *schedule.Operator, steps []schedule.OperatorStep) { - c.Assert(op.Kind()&schedule.OpMerge, Not(Equals), 0) - c.Assert(steps, NotNil) - c.Assert(op.Len(), Equals, len(steps)) - for i := range steps { - c.Assert(op.Step(i), DeepEquals, steps[i]) - } -} - -func (s *testMergeCheckerSuite) TestMatchPeers(c *C) { - // partial store overlap not including leader - ops := s.mc.Check(s.regions[2]) - s.checkSteps(c, ops[0], []schedule.OperatorStep{ - schedule.TransferLeader{FromStore: 6, ToStore: 5}, - schedule.AddLearner{ToStore: 1, PeerID: 1}, - schedule.PromoteLearner{ToStore: 1, PeerID: 1}, - schedule.RemovePeer{FromStore: 2}, - schedule.AddLearner{ToStore: 4, PeerID: 2}, - schedule.PromoteLearner{ToStore: 4, PeerID: 2}, - schedule.RemovePeer{FromStore: 6}, - schedule.MergeRegion{ - FromRegion: s.regions[2].GetMeta(), - ToRegion: s.regions[1].GetMeta(), - IsPassive: false, - }, - }) - s.checkSteps(c, ops[1], []schedule.OperatorStep{ - schedule.MergeRegion{ - FromRegion: s.regions[2].GetMeta(), - ToRegion: s.regions[1].GetMeta(), - IsPassive: true, - }, - }) - - // partial store overlap including leader - newRegion := s.regions[2].Clone( - core.SetPeers([]*metapb.Peer{ - {Id: 106, StoreId: 1}, - {Id: 107, StoreId: 5}, - {Id: 108, StoreId: 6}, - }), - core.WithLeader(&metapb.Peer{Id: 106, StoreId: 1}), - ) - s.regions[2] = newRegion - s.cluster.PutRegion(s.regions[2]) - ops = s.mc.Check(s.regions[2]) - s.checkSteps(c, ops[0], []schedule.OperatorStep{ - schedule.AddLearner{ToStore: 4, PeerID: 3}, - schedule.PromoteLearner{ToStore: 4, PeerID: 3}, - schedule.RemovePeer{FromStore: 6}, - schedule.MergeRegion{ - FromRegion: s.regions[2].GetMeta(), - ToRegion: s.regions[1].GetMeta(), - IsPassive: false, - }, - }) - s.checkSteps(c, ops[1], []schedule.OperatorStep{ - schedule.MergeRegion{ - FromRegion: s.regions[2].GetMeta(), - ToRegion: s.regions[1].GetMeta(), - IsPassive: true, - }, - }) - - // all stores overlap - s.regions[2] = s.regions[2].Clone(core.SetPeers([]*metapb.Peer{ - {Id: 106, StoreId: 1}, - {Id: 107, StoreId: 5}, - {Id: 108, StoreId: 4}, - })) - s.cluster.PutRegion(s.regions[2]) - ops = s.mc.Check(s.regions[2]) - s.checkSteps(c, ops[0], []schedule.OperatorStep{ - schedule.MergeRegion{ - FromRegion: s.regions[2].GetMeta(), - ToRegion: s.regions[1].GetMeta(), - IsPassive: false, - }, - }) - s.checkSteps(c, ops[1], []schedule.OperatorStep{ - schedule.MergeRegion{ - FromRegion: s.regions[2].GetMeta(), - ToRegion: s.regions[1].GetMeta(), - IsPassive: true, - }, - }) - - // all stores not overlap - s.regions[2] = s.regions[2].Clone(core.SetPeers([]*metapb.Peer{ - {Id: 109, StoreId: 2}, - {Id: 110, StoreId: 3}, - {Id: 111, StoreId: 6}, - }), core.WithLeader(&metapb.Peer{Id: 109, StoreId: 2})) - s.cluster.PutRegion(s.regions[2]) - ops = s.mc.Check(s.regions[2]) - s.checkSteps(c, ops[0], []schedule.OperatorStep{ - schedule.AddLearner{ToStore: 1, PeerID: 4}, - schedule.PromoteLearner{ToStore: 1, PeerID: 4}, - schedule.RemovePeer{FromStore: 3}, - schedule.AddLearner{ToStore: 4, PeerID: 5}, - schedule.PromoteLearner{ToStore: 4, PeerID: 5}, - schedule.RemovePeer{FromStore: 6}, - schedule.AddLearner{ToStore: 5, PeerID: 6}, - schedule.PromoteLearner{ToStore: 5, PeerID: 6}, - schedule.TransferLeader{FromStore: 2, ToStore: 1}, - schedule.RemovePeer{FromStore: 2}, - schedule.MergeRegion{ - FromRegion: s.regions[2].GetMeta(), - ToRegion: s.regions[1].GetMeta(), - IsPassive: false, - }, - }) - s.checkSteps(c, ops[1], []schedule.OperatorStep{ - schedule.MergeRegion{ - FromRegion: s.regions[2].GetMeta(), - ToRegion: s.regions[1].GetMeta(), - IsPassive: true, - }, - }) -} - var _ = Suite(&testBalanceHotWriteRegionSchedulerSuite{}) type testBalanceHotWriteRegionSchedulerSuite struct{} From 5b40f34ba53d8da1d05d839c7746180bf8a8cd2f Mon Sep 17 00:00:00 2001 From: Connor1996 Date: Thu, 4 Apr 2019 16:53:54 +0800 Subject: [PATCH 3/3] change time Signed-off-by: Connor1996 --- server/schedule/merge_checker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/schedule/merge_checker_test.go b/server/schedule/merge_checker_test.go index ae38553997b..4cf3439e1d8 100644 --- a/server/schedule/merge_checker_test.go +++ b/server/schedule/merge_checker_test.go @@ -1,4 +1,4 @@ -// Copyright 2017 PingCAP, Inc. +// Copyright 2019 PingCAP, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License.