Repository: tikv/pd
Pull request #5301 (2022-07-13 07:55:29)

core: fix the wrong statistics of the region tree
Signed-off-by: Ryan Leung <rleungx@gmail.com>

<!--
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
-->

### What problem does this PR solve?

<!--

Please create an issue first to describe the problem.
There MUST be one line starting with "Issue Number:  " and 
linking the relevant issues via the "close" or "ref".
For more info, check https://github.com/tikv/pd/blob/master/CONTRIBUTING.md#linking-issues.

-->
Issue Number: Close https://github.com/tikv/pd/issues/5318

### What is changed and how does it work?

<!--

You could use the "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.

-->

```commit-message
```

### Check List

<!-- Remove the items that are not applicable. -->

Tests

<!-- At least one of these tests 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
Fix the issue that the statistics of the region tree may be not accurate
```


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/5301/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/5301/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**: To remove the `do-not-merge/needs-linked-issue` label, 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-->

rleungx: > Do we need to cherry-pick it to other branches?

I think we need and I will add a unit test later.
codecov[bot]: # [Codecov](https://codecov.io/gh/tikv/pd/pull/5301?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) Report
> Merging [#5301](https://codecov.io/gh/tikv/pd/pull/5301?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) (d49d618) into [master](https://codecov.io/gh/tikv/pd/commit/3a51a52469a35ae7e5b5bc35f95527b2b466e9e9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) (3a51a52) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master    #5301      +/-   ##
==========================================
- Coverage   75.77%   75.75%   -0.02%     
==========================================
  Files         311      311              
  Lines       30781    30781              
==========================================
- Hits        23324    23319       -5     
- Misses       5463     5469       +6     
+ Partials     1994     1993       -1     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `75.75% <100.00%> (-0.02%)` | :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/5301?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) | Coverage Δ | |
|---|---|---|
| [server/core/region\_tree.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL2NvcmUvcmVnaW9uX3RyZWUuZ28=) | `94.16% <100.00%> (ø)` | |
| [pkg/errs/errs.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-cGtnL2VycnMvZXJycy5nbw==) | `75.00% <0.00%> (-25.00%)` | :arrow_down: |
| [pkg/tempurl/tempurl.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-cGtnL3RlbXB1cmwvdGVtcHVybC5nbw==) | `45.00% <0.00%> (-15.00%)` | :arrow_down: |
| [pkg/etcdutil/etcdutil.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-cGtnL2V0Y2R1dGlsL2V0Y2R1dGlsLmdv) | `84.88% <0.00%> (-3.49%)` | :arrow_down: |
| [server/schedulers/random\_merge.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3NjaGVkdWxlcnMvcmFuZG9tX21lcmdlLmdv) | `60.00% <0.00%> (-3.34%)` | :arrow_down: |
| [server/tso/allocator\_manager.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3Rzby9hbGxvY2F0b3JfbWFuYWdlci5nbw==) | `63.22% <0.00%> (-3.00%)` | :arrow_down: |
| [server/tso/local\_allocator.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3Rzby9sb2NhbF9hbGxvY2F0b3IuZ28=) | `75.67% <0.00%> (-2.71%)` | :arrow_down: |
| [server/schedule/labeler/rules.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3NjaGVkdWxlL2xhYmVsZXIvcnVsZXMuZ28=) | `87.50% <0.00%> (-2.28%)` | :arrow_down: |
| [server/region\_syncer/client.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL3JlZ2lvbl9zeW5jZXIvY2xpZW50Lmdv) | `86.56% <0.00%> (-1.50%)` | :arrow_down: |
| [server/election/leadership.go](https://codecov.io/gh/tikv/pd/pull/5301/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c2VydmVyL2VsZWN0aW9uL2xlYWRlcnNoaXAuZ28=) | `77.31% <0.00%> (-1.04%)` | :arrow_down: |
| ... and [10 more](https://codecov.io/gh/tikv/pd/pull/5301/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/5301?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/5301?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv). Last update [3a51a52...d49d618](https://codecov.io/gh/tikv/pd/pull/5301?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**: To remove the `do-not-merge/needs-linked-issue` label, 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-->

CabinfeverB: > Test modified, PTAL @lhy1024 @CabinfeverB

LGTM
nolouch: cc @HunDunDM 
ti-chi-bot: This pull request has been accepted and is ready to merge. <details>Commit hash: 959ad7977fd99062ae4d1aab23340be0638eaec0</details>

Patch:
diff --git a/server/core/region_tree.go b/server/core/region_tree.go
index 4bb1dc9d0..aa32b8a10 100644
--- a/server/core/region_tree.go
+++ b/server/core/region_tree.go
@@ -151,8 +151,8 @@ func (t *regionTree) remove(region *RegionInfo) {
 		return
 	}
 
-	t.totalSize -= region.approximateSize
-	regionWriteBytesRate, regionWriteKeysRate := region.GetWriteRate()
+	t.totalSize -= result.(*regionItem).region.GetApproximateSize()
+	regionWriteBytesRate, regionWriteKeysRate := result.(*regionItem).region.GetWriteRate()
 	t.totalWriteBytesRate -= regionWriteBytesRate
 	t.totalWriteKeysRate -= regionWriteKeysRate
 	t.tree.Remove(result)


Test patch:
diff --git a/server/core/region_tree_test.go b/server/core/region_tree_test.go
index 7538f04dd..a8a8d6a3b 100644
--- a/server/core/region_tree_test.go
+++ b/server/core/region_tree_test.go
@@ -129,7 +129,7 @@ func TestRegionTreeStat(t *testing.T) {
 	re.Equal(int64(4), tree.totalSize)
 	updateNewItem(tree, newRegionWithStat("b", "e", 5, 6))
 	re.Equal(int64(6), tree.totalSize)
-	tree.remove(newRegionWithStat("a", "b", 1, 2))
+	tree.remove(newRegionWithStat("a", "b", 2, 2))
 	re.Equal(int64(5), tree.totalSize)
 	tree.remove(newRegionWithStat("f", "g", 1, 2))
 	re.Equal(int64(5), tree.totalSize)