region_scatter: fix a bug that PD may panic when scatter region if region is unhealthy (#6128) (#6129)

ti-chi-bot · HunDunDM · web-flow · commit bd807190154f · 2023-03-09T18:55:13.000+08:00
close #6099, ref #6128 Signed-off-by: HunDunDM <hundundm@gmail.com> Co-authored-by: HunDunDM <hundundm@gmail.com>
diff --git a/server/schedule/region_scatterer.go b/server/schedule/region_scatterer.go
@@ -394,12 +394,14 @@ func allowLeader(fit *placement.RegionFit, peer *metapb.Peer) bool {
 	if peer.IsWitness {
 		return false
 	}
-
-	rule := fit.GetRuleFit(peer.GetId()).Rule
-	if rule.IsWitness {
+	peerFit := fit.GetRuleFit(peer.GetId())
+	if peerFit == nil || peerFit.Rule == nil {
+		return false
+	}
+	if peerFit.Rule.IsWitness {
 		return false
 	}
-	switch rule.Role {
+	switch peerFit.Rule.Role {
 	case placement.Voter, placement.Leader:
 		return true
 	}
diff --git a/server/schedule/region_scatterer_test.go b/server/schedule/region_scatterer_test.go
@@ -758,6 +758,11 @@ func TestBalanceRegion(t *testing.T) {
 		re.Equal(uint64(150), scatterer.ordinaryEngine.selectedPeer.Get(i, group))
 		re.Equal(uint64(50), scatterer.ordinaryEngine.selectedLeader.Get(i, group))
 	}
+	// Test for unhealthy region
+	// ref https://github.com/tikv/pd/issues/6099
+	region := tc.AddLeaderRegion(1500, 2, 3, 4, 6)
+	op := scatterer.scatterRegion(region, group)
+	re.False(isPeerCountChanged(op))
 }
 
 func isPeerCountChanged(op *operator.Operator) bool {

Original file line number	Diff line number	Diff line change
`@@ -394,12 +394,14 @@ func allowLeader(fit placement.RegionFit, peer metapb.Peer) bool {`
`394`	`394`	`if peer.IsWitness {`
`395`	`395`	`return false`
`396`	`396`	`}`
`397`		`-`
`398`		`- rule := fit.GetRuleFit(peer.GetId()).Rule`
`399`		`- if rule.IsWitness {`
	`397`	`+ peerFit := fit.GetRuleFit(peer.GetId())`
	`398`	`+ if peerFit == nil \|\| peerFit.Rule == nil {`
	`399`	`+ return false`
	`400`	`+ }`
	`401`	`+ if peerFit.Rule.IsWitness {`
`400`	`402`	`return false`
`401`	`403`	`}`
`402`		`- switch rule.Role {`
	`404`	`+ switch peerFit.Rule.Role {`
`403`	`405`	`case placement.Voter, placement.Leader:`
`404`	`406`	`return true`
`405`	`407`	`}`
Original file line number	Diff line number	Diff line change
`@@ -758,6 +758,11 @@ func TestBalanceRegion(t *testing.T) {`
`758`	`758`	`re.Equal(uint64(150), scatterer.ordinaryEngine.selectedPeer.Get(i, group))`
`759`	`759`	`re.Equal(uint64(50), scatterer.ordinaryEngine.selectedLeader.Get(i, group))`
`760`	`760`	`}`
	`761`	`+ // Test for unhealthy region`
	`762`	`+ // ref https://github.com/tikv/pd/issues/6099`
	`763`	`+ region := tc.AddLeaderRegion(1500, 2, 3, 4, 6)`
	`764`	`+ op := scatterer.scatterRegion(region, group)`
	`765`	`+ re.False(isPeerCountChanged(op))`
`761`	`766`	`}`
`762`	`767`
`763`	`768`	`func isPeerCountChanged(op *operator.Operator) bool {`