Repository: tikv/pd
Pull request #4626 (2022-02-08 13:08:08)

server/core/region.go: tiny refactor set_region
<!--
Thank you for working on PD! Please read PD's [CONTRIBUTING](https://github.com/tikv/pd/blob/master/CONTRIBUTING.md) document **BEFORE** filing this PR.
PR Title Format:
1. pkg [, pkg2, pkg3]: what's changed
2. *: what's changed


If you want to open the **Challenge Program** pull request, please use the following template:
https://raw.githubusercontent.com/tikv/.github/master/.github/PULL_REQUEST_TEMPLATE/challenge-program.md
You can use it with query parameters: https://github.com/tikv/pd/compare/master...${you branch}?template=challenge-program.md
-->

### What problem does this PR solve?
The function [SetRegion](https://github.com/tikv/pd/blob/master/server/core/region.go#L645) is not very friendly to our reviewers, exp: too many `{}` in `{}`,  the max degree of `{}` is 4. 

### What is changed and how it works?
<!--

You could use "commit message" code block to add more description to the final commit message.
For more info, check https://github.com/tikv/pd/blob/master/CONTRIBUTING.md#format-of-the-commit-message.

-->
 This PR tidy refactor the function [SetRegion](https://github.com/tikv/pd/blob/master/server/core/region.go#L645) to make it more friendly to our reviewers and maintainers, exp: we decrease the max degree of `{}` from 4 to 2 and try to return as early as possible. The function is still very long and it is not the perfect refactor, however, we are trying to make it better. Feel free to refactor it ～～

And we  use closure to avoid the performance loss, now the benchmark function ```BenchmarkRegionTreeUpdate-8` `BenchmarkRegionTreeUpdateUnordered-8 `  works well :

```
core git:(server/core/set_region) go test -bench BenchmarkRegion
OK: 23 passed
goos: darwin
goarch: amd64
pkg: github.com/tikv/pd/server/core
cpu: VirtualApple @ 2.50GHz
BenchmarkRegionTreeUpdate-8              1000000              1227 ns/op
BenchmarkRegionTreeUpdateUnordered-8     2931008               432.0 ns/op
PASS
ok      github.com/tikv/pd/server/core  28.238s

➜  core git:(master) go test -bench BenchmarkRegion
OK: 23 passed
goos: darwin
goarch: amd64
pkg: github.com/tikv/pd/server/core
cpu: VirtualApple @ 2.50GHz
BenchmarkRegionTreeUpdate-8              1000000              1262 ns/op
BenchmarkRegionTreeUpdateUnordered-8     2857729               502.2 ns/op
PASS
ok      github.com/tikv/pd/server/core  27.978s
```

### Check List

<!-- Remove the items that are not applicable. -->

Tests

<!-- At least one of them must be included. -->

- Unit test


### Release note

<!-- A bugfix or a new feature needs a release note. If there is no need release note, just uncomment the below line. -->
```release-note
None.
```


ti-chi-bot: [REVIEW NOTIFICATION]

This pull request has not been approved.


To complete the [pull request process](https://book.prow.tidb.io/#/en/workflows/pr), please ask the reviewers in the [list](https://prow.tidb.io/tichi/repos/tikv/pd/pulls/4626/owners) to review by filling `/cc @reviewer` in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the [list](https://prow.tidb.io/tichi/repos/tikv/pd/pulls/4626/owners) by filling  `/assign @committer` in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found [here](https://prow.tidb.io/command-help?repo=tikv%2Fpd).

<details>

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.
</details>

<!--Review Notification Identifier-->
ti-chi-bot: 
[FORMAT CHECKER NOTIFICATION]

**Notice**: Please provide the linked issue number on one line in the PR body, for example: `Issue Number: close #123` or `Issue Number: ref #456`, multiple issues should use full syntax for each issue and be separated by a comma, like: `Issue Number: close #123, ref #456`.

<sub>:open_book: For more info, you can check the ["Linking issues"](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md#linking-issues) section in the [CONTRIBUTING.md](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md).</sub>


<!--Checker Notification Identifier-->

ti-chi-bot: 
[FORMAT CHECKER NOTIFICATION]

**Notice**: Please provide the linked issue number on one line in the PR body, for example: `Issue Number: close #123` or `Issue Number: ref #456`, multiple issues should use full syntax for each issue and be separated by a comma, like: `Issue Number: close #123, ref #456`.

<sub>:open_book: For more info, you can check the ["Linking issues"](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md#linking-issues) section in the [CONTRIBUTING.md](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md).</sub>


<!--Checker Notification Identifier-->

ti-chi-bot: 
[FORMAT CHECKER NOTIFICATION]

**Notice**: Please provide the linked issue number on one line in the PR body, for example: `Issue Number: close #123` or `Issue Number: ref #456`, multiple issues should use full syntax for each issue and be separated by a comma, like: `Issue Number: close #123, ref #456`.

<sub>:open_book: For more info, you can check the ["Linking issues"](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md#linking-issues) section in the [CONTRIBUTING.md](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md).</sub>


<!--Checker Notification Identifier-->

codecov[bot]: # [Codecov](https://codecov.io/gh/tikv/pd/pull/4626?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) Report
> Merging [#4626](https://codecov.io/gh/tikv/pd/pull/4626?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) (6accf63) into [master](https://codecov.io/gh/tikv/pd/commit/88409b75418f0de46b62b7121627c23359b571b3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) (88409b7) will **decrease** coverage by `0.07%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/tikv/pd/pull/4626/graphs/tree.svg?width=650&height=150&src=pr&token=J1yRL471OI&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv)](https://codecov.io/gh/tikv/pd/pull/4626?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv)

```diff
@@            Coverage Diff             @@
##           master    #4626      +/-   ##
==========================================
- Coverage   74.86%   74.79%   -0.08%     
==========================================
  Files         281      281              
  Lines       27731    27714      -17     
==========================================
- Hits        20761    20728      -33     
- Misses       5117     5125       +8     
- Partials     1853     1861       +8     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `74.79% <100.00%> (-0.08%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/tikv/pd/pull/4626?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) | Coverage Δ | |
|---|---|---|
| [server/core/region.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL2NvcmUvcmVnaW9uLmdv) | `91.58% <100.00%> (-0.28%)` | :arrow_down: |
| [server/storage/kv/etcd\_kv.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3N0b3JhZ2Uva3YvZXRjZF9rdi5nbw==) | `69.56% <0.00%> (-10.15%)` | :arrow_down: |
| [server/tso/tso.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3Rzby90c28uZ28=) | `63.06% <0.00%> (-3.41%)` | :arrow_down: |
| [pkg/dashboard/adapter/manager.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-cGtnL2Rhc2hib2FyZC9hZGFwdGVyL21hbmFnZXIuZ28=) | `79.78% <0.00%> (-3.20%)` | :arrow_down: |
| [server/election/leadership.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL2VsZWN0aW9uL2xlYWRlcnNoaXAuZ28=) | `73.19% <0.00%> (-2.07%)` | :arrow_down: |
| [server/member/member.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL21lbWJlci9tZW1iZXIuZ28=) | `63.10% <0.00%> (-1.61%)` | :arrow_down: |
| [server/storage/hot\_region\_storage.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3N0b3JhZ2UvaG90X3JlZ2lvbl9zdG9yYWdlLmdv) | `78.73% <0.00%> (-1.15%)` | :arrow_down: |
| [server/encryptionkm/key\_manager.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL2VuY3J5cHRpb25rbS9rZXlfbWFuYWdlci5nbw==) | `72.90% <0.00%> (-0.80%)` | :arrow_down: |
| [server/tso/allocator\_manager.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3Rzby9hbGxvY2F0b3JfbWFuYWdlci5nbw==) | `65.77% <0.00%> (-0.72%)` | :arrow_down: |
| [server/schedulers/hot\_region.go](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3NjaGVkdWxlcnMvaG90X3JlZ2lvbi5nbw==) | `82.58% <0.00%> (-0.30%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/tikv/pd/pull/4626/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/tikv/pd/pull/4626?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/tikv/pd/pull/4626?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv). Last update [88409b7...6accf63](https://codecov.io/gh/tikv/pd/pull/4626?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv).

ti-chi-bot: 
[FORMAT CHECKER NOTIFICATION]

**Notice**: Please provide the linked issue number on one line in the PR body, for example: `Issue Number: close #123` or `Issue Number: ref #456`, multiple issues should use full syntax for each issue and be separated by a comma, like: `Issue Number: close #123, ref #456`.

<sub>:open_book: For more info, you can check the ["Linking issues"](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md#linking-issues) section in the [CONTRIBUTING.md](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md).</sub>


<!--Checker Notification Identifier-->

ti-chi-bot: 
[FORMAT CHECKER NOTIFICATION]

**Notice**: Please provide the linked issue number on one line in the PR body, for example: `Issue Number: close #123` or `Issue Number: ref #456`, multiple issues should use full syntax for each issue and be separated by a comma, like: `Issue Number: close #123, ref #456`.

<sub>:open_book: For more info, you can check the ["Linking issues"](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md#linking-issues) section in the [CONTRIBUTING.md](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md).</sub>


<!--Checker Notification Identifier-->

ti-chi-bot: 
[FORMAT CHECKER NOTIFICATION]

**Notice**: Please provide the linked issue number on one line in the PR body, for example: `Issue Number: close #123` or `Issue Number: ref #456`, multiple issues should use full syntax for each issue and be separated by a comma, like: `Issue Number: close #123, ref #456`.

<sub>:open_book: For more info, you can check the ["Linking issues"](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md#linking-issues) section in the [CONTRIBUTING.md](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md).</sub>


<!--Checker Notification Identifier-->

disksing: /merge
ti-chi-bot: This pull request has been accepted and is ready to merge. <details>Commit hash: 606c9eb65745b7bf819a99243a46b447193d7e2f</details>

Patch:
diff --git a/server/core/region.go b/server/core/region.go
index 889c33eeb..eb5b67b1a 100644
--- a/server/core/region.go
+++ b/server/core/region.go
@@ -90,6 +90,20 @@ func classifyVoterAndLearner(region *RegionInfo) {
 	region.voters = voters
 }
 
+// peersEqualTo returns true when the peers are not changed, which may caused by: the region leader not changed,
+// peer transferred, new peer was created, learners changed, pendingPeers changed.
+func (r *RegionInfo) peersEqualTo(region *RegionInfo) bool {
+	return r.leader.GetId() == region.leader.GetId() &&
+		SortedPeersEqual(r.GetVoters(), region.GetVoters()) &&
+		SortedPeersEqual(r.GetLearners(), region.GetLearners()) &&
+		SortedPeersEqual(r.GetPendingPeers(), region.GetPendingPeers())
+}
+
+// rangeEqualsTo returns true when the start_key and end_key are the same.
+func (r *RegionInfo) rangeEqualsTo(region *RegionInfo) bool {
+	return bytes.Equal(r.GetStartKey(), region.GetStartKey()) && bytes.Equal(r.GetEndKey(), region.GetEndKey())
+}
+
 const (
 	// EmptyRegionApproximateSize is the region approximate size of an empty region
 	// (heartbeat size <= 1MB).
@@ -643,43 +657,39 @@ func (r *RegionsInfo) GetRegion(regionID uint64) *RegionInfo {
 // SetRegion sets the RegionInfo to regionTree and regionMap, also update leaders and followers by region peers
 // overlaps: Other regions that overlap with the specified region, excluding itself.
 func (r *RegionsInfo) SetRegion(region *RegionInfo) (overlaps []*RegionInfo) {
-	var item *regionItem   // Pointer to the *RegionInfo of this ID.
-	var origin *RegionInfo // This is the original region information of this ID.
-	var rangeChanged bool  // This Region is new, or its range has changed.
-	var peersChanged bool  // This Region is new, or its peers have changed, including leader-change/pending/down.
-
+	var item *regionItem // Pointer to the *RegionInfo of this ID.
+	rangeChanged := true // This Region is new, or its range has changed.
 	if item = r.regions.Get(region.GetID()); item != nil {
 		// If this ID already exists, use the existing regionItem and pick out the origin.
-		origin = item.region
-		rangeChanged = !bytes.Equal(origin.GetStartKey(), region.GetStartKey()) ||
-			!bytes.Equal(origin.GetEndKey(), region.GetEndKey())
+		origin := item.region
+		rangeChanged = !origin.rangeEqualsTo(region)
 		if rangeChanged {
 			// Delete itself in regionTree so that overlaps will not contain itself.
 			// Because the regionItem is reused, there is no need to delete it in the regionMap.
 			r.tree.remove(origin)
-			// A change in the range is equivalent to a change in all peers.
-			peersChanged = true
 		} else {
-			peersChanged = r.shouldRemoveFromSubTree(region, origin)
+			// If the range is not changed, only the statistical on the regionTree needs to be updated.
+			r.tree.updateStat(origin, region)
 		}
-		// If the peers have changed, the sub regionTree needs to be cleaned up.
-		if peersChanged {
-			// TODO: Improve performance by deleting only the different peers.
-			r.removeRegionFromSubTree(origin)
+
+		if !rangeChanged && origin.peersEqualTo(region) {
+			// If the peers are not changed, only the statistical on the sub regionTree needs to be updated.
+			r.updateSubTreeStat(origin, region)
+			// Update the RegionInfo in the regionItem.
+			item.region = region
+			return
 		}
+		// If the range or peers have changed, the sub regionTree needs to be cleaned up.
+		// TODO: Improve performance by deleting only the different peers.
+		r.removeRegionFromSubTree(origin)
 		// Update the RegionInfo in the regionItem.
 		item.region = region
 	} else {
 		// If this ID does not exist, generate a new regionItem and save it in the regionMap.
-		rangeChanged = true
-		peersChanged = true
 		item = r.regions.AddNew(region)
 	}
 
-	if !rangeChanged {
-		// If the range is not changed, only the statistical on the regionTree needs to be updated.
-		r.tree.updateStat(origin, region)
-	} else {
+	if rangeChanged {
 		// It has been removed and all information needs to be updated again.
 		overlaps = r.tree.update(item)
 		for _, old := range overlaps {
@@ -687,54 +697,40 @@ func (r *RegionsInfo) SetRegion(region *RegionInfo) (overlaps []*RegionInfo) {
 		}
 	}
 
-	if !peersChanged {
-		// If the peers are not changed, only the statistical on the sub regionTree needs to be updated.
-		r.updateSubTreeStat(origin, region)
-	} else {
-		// It has been removed and all information needs to be updated again.
+	// It has been removed and all information needs to be updated again.
+	// Set peers then.
 
-		// Add to leaders and followers.
-		for _, peer := range region.GetVoters() {
-			storeID := peer.GetStoreId()
-			if peer.GetId() == region.leader.GetId() {
-				// Add leader peer to leaders.
-				store, ok := r.leaders[storeID]
-				if !ok {
-					store = newRegionTree()
-					r.leaders[storeID] = store
-				}
-				store.update(item)
-			} else {
-				// Add follower peer to followers.
-				store, ok := r.followers[storeID]
-				if !ok {
-					store = newRegionTree()
-					r.followers[storeID] = store
-				}
-				store.update(item)
-			}
+	setPeer := func(peersMap map[uint64]*regionTree, storeID uint64, item *regionItem) {
+		store, ok := peersMap[storeID]
+		if !ok {
+			store = newRegionTree()
+			peersMap[storeID] = store
 		}
-		// Add to learners.
-		for _, peer := range region.GetLearners() {
-			storeID := peer.GetStoreId()
-			store, ok := r.learners[storeID]
-			if !ok {
-				store = newRegionTree()
-				r.learners[storeID] = store
-			}
-			store.update(item)
+		store.update(item)
+	}
+
+	// Add to leaders and followers.
+	for _, peer := range region.GetVoters() {
+		storeID := peer.GetStoreId()
+		if peer.GetId() == region.leader.GetId() {
+			// Add leader peer to leaders.
+			setPeer(r.leaders, storeID, item)
+		} else {
+			// Add follower peer to followers.
+			setPeer(r.followers, storeID, item)
 		}
-		// Add to PendingPeers
-		for _, peer := range region.GetPendingPeers() {
+	}
+
+	setPeers := func(peersMap map[uint64]*regionTree, peers []*metapb.Peer) {
+		for _, peer := range peers {
 			storeID := peer.GetStoreId()
-			store, ok := r.pendingPeers[storeID]
-			if !ok {
-				store = newRegionTree()
-				r.pendingPeers[storeID] = store
-			}
-			store.update(item)
+			setPeer(peersMap, storeID, item)
 		}
 	}
+	// Add to learners.
+	setPeers(r.learners, region.GetLearners())
+	// Add to PendingPeers
+	setPeers(r.pendingPeers, region.GetPendingPeers())
 
 	return
 }
@@ -750,28 +746,27 @@ func (r *RegionsInfo) TreeLen() int {
 }
 
 func (r *RegionsInfo) updateSubTreeStat(origin *RegionInfo, region *RegionInfo) {
+	updatePeerStat := func(peersMap map[uint64]*regionTree, storeID uint64) {
+		if tree, ok := peersMap[storeID]; ok {
+			tree.updateStat(origin, region)
+		}
+	}
 	for _, peer := range region.GetVoters() {
 		storeID := peer.GetStoreId()
 		if peer.GetId() == region.leader.GetId() {
-			if tree, ok := r.leaders[storeID]; ok {
-				tree.updateStat(origin, region)
-			}
+			updatePeerStat(r.leaders, storeID)
 		} else {
-			if tree, ok := r.followers[storeID]; ok {
-				tree.updateStat(origin, region)
-			}
-		}
-	}
-	for _, peer := range region.GetLearners() {
-		if tree, ok := r.learners[peer.GetStoreId()]; ok {
-			tree.updateStat(origin, region)
+			updatePeerStat(r.followers, storeID)
 		}
 	}
-	for _, peer := range region.GetPendingPeers() {
-		if tree, ok := r.pendingPeers[peer.GetStoreId()]; ok {
-			tree.updateStat(origin, region)
+
+	updatePeersStat := func(peersMap map[uint64]*regionTree, peers []*metapb.Peer) {
+		for _, peer := range peers {
+			updatePeerStat(peersMap, peer.GetStoreId())
 		}
 	}
+	updatePeersStat(r.learners, region.GetLearners())
+	updatePeersStat(r.pendingPeers, region.GetPendingPeers())
 }
 
 // GetOverlaps returns the regions which are overlapped with the specified region range.
@@ -853,15 +848,6 @@ func SortedPeersStatsEqual(peersA, peersB []*pdpb.PeerStats) bool {
 	return true
 }
 
-// shouldRemoveFromSubTree return true when the region leader changed, peer transferred,
-// new peer was created, learners changed, pendingPeers changed, and so on.
-func (r *RegionsInfo) shouldRemoveFromSubTree(region *RegionInfo, origin *RegionInfo) bool {
-	return origin.leader.GetId() != region.leader.GetId() ||
-		!SortedPeersEqual(origin.GetVoters(), region.GetVoters()) ||
-		!SortedPeersEqual(origin.GetLearners(), region.GetLearners()) ||
-		!SortedPeersEqual(origin.GetPendingPeers(), region.GetPendingPeers())
-}
-
 // GetRegionByKey searches RegionInfo from regionTree
 func (r *RegionsInfo) GetRegionByKey(regionKey []byte) *RegionInfo {
 	region := r.tree.search(regionKey)


Test patch:
diff --git a/server/core/region_test.go b/server/core/region_test.go
index 1f0dc59d2..be1c174a5 100644
--- a/server/core/region_test.go
+++ b/server/core/region_test.go
@@ -487,7 +487,6 @@ func (*testRegionKey) TestSetRegion(c *C) {
 }
 
 func (*testRegionKey) TestShouldRemoveFromSubTree(c *C) {
-	regions := NewRegionsInfo()
 	peer1 := &metapb.Peer{StoreId: uint64(1), Id: uint64(1)}
 	peer2 := &metapb.Peer{StoreId: uint64(2), Id: uint64(2)}
 	peer3 := &metapb.Peer{StoreId: uint64(3), Id: uint64(3)}
@@ -505,25 +504,25 @@ func (*testRegionKey) TestShouldRemoveFromSubTree(c *C) {
 		StartKey: []byte(fmt.Sprintf("%20d", 10)),
 		EndKey:   []byte(fmt.Sprintf("%20d", 20)),
 	}, peer1)
-	c.Assert(regions.shouldRemoveFromSubTree(region, origin), IsFalse)
+	c.Assert(region.peersEqualTo(origin), IsTrue)
 
 	region.leader = peer2
-	c.Assert(regions.shouldRemoveFromSubTree(region, origin), IsTrue)
+	c.Assert(region.peersEqualTo(origin), IsFalse)
 
 	region.leader = peer1
 	region.pendingPeers = append(region.pendingPeers, peer4)
-	c.Assert(regions.shouldRemoveFromSubTree(region, origin), IsTrue)
+	c.Assert(region.peersEqualTo(origin), IsFalse)
 
 	region.pendingPeers = nil
 	region.learners = append(region.learners, peer2)
-	c.Assert(regions.shouldRemoveFromSubTree(region, origin), IsTrue)
+	c.Assert(region.peersEqualTo(origin), IsFalse)
 
 	origin.learners = append(origin.learners, peer2, peer3)
 	region.learners = append(region.learners, peer4)
-	c.Assert(regions.shouldRemoveFromSubTree(region, origin), IsFalse)
+	c.Assert(region.peersEqualTo(origin), IsTrue)
 
 	region.voters[2].StoreId = 4
-	c.Assert(regions.shouldRemoveFromSubTree(region, origin), IsTrue)
+	c.Assert(region.peersEqualTo(origin), IsFalse)
 }
 
 func checkRegions(c *C, regions *RegionsInfo) {