Skip to content

Commit 0646a37

Browse files
ti-chi-botHunDunDMCabinfeverB
authored
region_scatterer: fix the bug that could generate schedule with too many peers (#5920) (#5929)
ref #4570, close #5909, ref #5920 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> Co-authored-by: 混沌DM <hundundm@gmail.com> Co-authored-by: Cabinfever_B <cabinfeveroier@gmail.com>
1 parent a6e77c1 commit 0646a37

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

server/schedule/region_scatterer.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ func (r *RegionScatterer) scatterRegion(region *core.RegionInfo, group string) *
320320
// it is considered that the selected peer select itself.
321321
// This origin peer re-selects.
322322
if _, ok := peers[newPeer.GetStoreId()]; !ok || peer.GetStoreId() == newPeer.GetStoreId() {
323+
selectedStores[peer.GetStoreId()] = struct{}{}
323324
break
324325
}
325326
}

server/schedule/region_scatterer_test.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ func TestRegionHasLearner(t *testing.T) {
607607
checkLeader(scatterer.ordinaryEngine.selectedLeader)
608608
}
609609

610-
// TestSelectedStores tests if the peer count has changed due to the picking strategy.
610+
// TestSelectedStoresTooFewPeers tests if the peer count has changed due to the picking strategy.
611611
// Ref https://github.com/tikv/pd/issues/4565
612612
func (s *testScatterRegionSuite) TestSelectedStores(c *C) {
613613
ctx, cancel := context.WithCancel(context.Background())
@@ -643,6 +643,43 @@ func (s *testScatterRegionSuite) TestSelectedStores(c *C) {
643643
}
644644
}
645645

646+
// TestSelectedStoresTooManyPeers tests if the peer count has changed due to the picking strategy.
647+
// Ref https://github.com/tikv/pd/issues/5909
648+
func TestSelectedStoresTooManyPeers(t *testing.T) {
649+
re := require.New(t)
650+
ctx, cancel := context.WithCancel(context.Background())
651+
defer cancel()
652+
opt := config.NewTestOptions()
653+
tc := mockcluster.NewCluster(ctx, opt)
654+
stream := hbstream.NewTestHeartbeatStreams(ctx, tc.ID, tc, false)
655+
oc := NewOperatorController(ctx, tc, stream)
656+
// Add 4 stores.
657+
for i := uint64(1); i <= 5; i++ {
658+
tc.AddRegionStore(i, 0)
659+
// prevent store from being disconnected
660+
tc.SetStoreLastHeartbeatInterval(i, -10*time.Minute)
661+
}
662+
group := "group"
663+
scatterer := NewRegionScatterer(ctx, tc, oc)
664+
// priority 4 > 1 > 5 > 2 == 3
665+
for i := 0; i < 1200; i++ {
666+
scatterer.ordinaryEngine.selectedPeer.Put(2, group)
667+
scatterer.ordinaryEngine.selectedPeer.Put(3, group)
668+
}
669+
for i := 0; i < 800; i++ {
670+
scatterer.ordinaryEngine.selectedPeer.Put(5, group)
671+
}
672+
for i := 0; i < 400; i++ {
673+
scatterer.ordinaryEngine.selectedPeer.Put(1, group)
674+
}
675+
// test region with peer 1 2 3
676+
for i := uint64(1); i < 20; i++ {
677+
region := tc.AddLeaderRegion(i+200, i%3+1, (i+1)%3+1, (i+2)%3+1)
678+
op := scatterer.scatterRegion(region, group)
679+
re.False(isPeerCountChanged(op))
680+
}
681+
}
682+
646683
func isPeerCountChanged(op *operator.Operator) bool {
647684
if op == nil {
648685
return false

0 commit comments

Comments
 (0)