*: reduce get overlap operations #5615

rleungx · 2022-10-21T05:57:35Z

Signed-off-by: Ryan Leung rleungx@gmail.com

What problem does this PR solve?

Issue Number: Ref #5648, #5606.

What is changed and how does it work?

name                                       old time/op    new time/op    delta
LoadRegionsByVolume/input_size_10000-40      0.05ns ±32%    0.04ns ±18%     ~     (p=0.095 n=5+5)
LoadRegionsByVolume/input_size_100000-40     0.58ns ± 2%    0.46ns ± 4%  -21.66%  (p=0.008 n=5+5)
LoadRegionsByVolume/input_size_1000000-40     6.29s ±11%     4.88s ± 4%  -22.34%  (p=0.008 n=5+5)

name                                       old alloc/op   new alloc/op   delta
LoadRegionsByVolume/input_size_10000-40       0.00B          0.00B          ~     (all equal)
LoadRegionsByVolume/input_size_100000-40      0.00B          0.00B          ~     (all equal)
LoadRegionsByVolume/input_size_1000000-40     934MB ± 0%     933MB ± 0%   -0.01%  (p=0.032 n=5+5)

name                                       old allocs/op  new allocs/op  delta
LoadRegionsByVolume/input_size_10000-40        0.00           0.00          ~     (all equal)
LoadRegionsByVolume/input_size_100000-40       0.00           0.00          ~     (all equal)
LoadRegionsByVolume/input_size_1000000-40     15.2M ± 0%     15.2M ± 0%     ~     (p=0.421 n=5+5)

name                                        old time/op    new time/op    delta
LoadRegionsByRandomMerge/merge_ratio_0-40      6.51s ± 4%     4.92s ±10%  -24.48%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_20-40     8.06s ± 4%     4.74s ± 8%  -41.23%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_40-40     9.74s ± 3%     4.93s ± 4%  -49.36%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_60-40     11.1s ± 3%      4.9s ± 4%  -55.85%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_80-40     11.9s ± 3%      5.1s ± 7%  -57.32%  (p=0.008 n=5+5)

name                                        old alloc/op   new alloc/op   delta
LoadRegionsByRandomMerge/merge_ratio_0-40      934MB ± 0%     933MB ± 0%     ~     (p=0.056 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_20-40    1.01GB ± 0%    0.93GB ± 0%   -8.26%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_40-40    1.10GB ± 0%    0.93GB ± 0%  -15.72%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_60-40    1.17GB ± 0%    0.93GB ± 0%  -20.13%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_80-40    1.25GB ± 0%    0.95GB ± 0%  -24.07%  (p=0.008 n=5+5)

name                                        old allocs/op  new allocs/op  delta
LoadRegionsByRandomMerge/merge_ratio_0-40      15.2M ± 0%     15.2M ± 0%     ~     (p=0.881 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_20-40     18.0M ± 0%     15.6M ± 0%  -13.49%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_40-40     20.7M ± 0%     15.9M ± 0%  -22.94%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_60-40     23.3M ± 0%     16.3M ± 0%  -30.12%  (p=0.008 n=5+5)
LoadRegionsByRandomMerge/merge_ratio_80-40     25.8M ± 0%     16.6M ± 0%  -35.63%  (p=0.008 n=5+5)

Check List

Tests

Unit test

Release note

None

ti-chi-bot · 2022-10-21T05:57:37Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

CabinfeverB
lhy1024

To complete the pull request process, please ask the reviewers in the list 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 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.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

codecov · 2022-10-21T06:10:06Z

Codecov Report

Base: 75.53% // Head: 75.62% // Increases project coverage by +0.09% 🎉

Coverage data is based on head (4abba8b) compared to base (268c2b6).
Patch coverage: 91.48% of modified lines in pull request are covered.

❗ Current head 4abba8b differs from pull request most recent head a1cdd25. Consider uploading reports for the commit a1cdd25 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5615      +/-   ##
==========================================
+ Coverage   75.53%   75.62%   +0.09%     
==========================================
  Files         329      329              
  Lines       32960    32963       +3     
==========================================
+ Hits        24897    24929      +32     
+ Misses       5910     5885      -25     
+ Partials     2153     2149       -4

Flag	Coverage Δ
unittests	`75.62% <91.48%> (+0.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
server/core/basic_cluster.go	`92.85% <ø> (+2.04%)`	⬆️
server/core/region.go	`90.16% <90.47%> (-0.12%)`	⬇️
server/cluster/cluster.go	`81.45% <100.00%> (ø)`
server/core/region_tree.go	`95.56% <100.00%> (-0.11%)`	⬇️
server/region_syncer/client.go	`85.82% <100.00%> (-0.75%)`	⬇️
server/schedule/range_cluster.go	`83.63% <100.00%> (ø)`
pkg/errs/errs.go	`75.00% <0.00%> (-25.00%)`	⬇️
pkg/etcdutil/etcdutil.go	`82.55% <0.00%> (-5.82%)`	⬇️
server/storage/kv/etcd_kv.go	`79.71% <0.00%> (-5.80%)`	⬇️
server/region_syncer/server.go	`82.96% <0.00%> (-4.40%)`	⬇️
... and 31 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

CabinfeverB

Pls reslove conflict

CabinfeverB · 2022-11-30T16:29:38Z

server/core/basic_cluster.go

 	bc.Regions.mu.RLock()
 	origin, overlaps := bc.getRelevantRegionsLocked(region)
 	bc.Regions.mu.RUnlock()
 	return bc.check(region, origin, overlaps)
 }

-func (bc *BasicCluster) check(region, origin *RegionInfo, overlaps []*RegionInfo) (*RegionInfo, error) {
+func (bc *BasicCluster) check(region, origin *RegionInfo, overlaps []*regionItem) (*RegionInfo, []*regionItem, error) {


it's not necessary to return the input param

rleungx · 2022-12-01T06:21:23Z

server/core/region.go

+			for i, o := range ol {
+				if o.GetID() == region.GetID() {
+					ol = append(ol[:i], ol[i+1:]...)
+				}
+			}


This part needs to be paid attention to.

nolouch · 2022-12-02T04:02:45Z

server/core/region.go

-// SetRegionWithUpdate sets the RegionInfo to regionTree and regionMap and return the update info of subtree.
-func (r *RegionsInfo) SetRegionWithUpdate(region *RegionInfo) (*RegionInfo, []*RegionInfo, bool) {
+// SetRegion sets the RegionInfo to regionTree and regionMap and return the update info of subtree.
+func (r *RegionsInfo) SetRegion(region *RegionInfo, withOverlaps bool, ol ...*regionItem) (*RegionInfo, []*RegionInfo, bool) {


The function looks hard to use....and how to guarante overlaps is accurate without lock？

rleungx · 2022-12-02T04:17:57Z

/hold

rleungx · 2022-12-02T04:38:49Z

/hold cancel

rleungx · 2022-12-02T06:44:23Z

/hold

rleungx · 2022-12-02T06:44:59Z

This PR won't be released in v6.5.0.

Signed-off-by: Ryan Leung <rleungx@gmail.com>

rleungx · 2022-12-06T04:07:56Z

/hold cancel

nolouch · 2022-12-08T07:17:31Z

/merge

ti-chi-bot · 2022-12-08T07:17:32Z

@nolouch: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-12-08T07:17:34Z

This pull request has been accepted and is ready to merge.

Commit hash: 4abba8b

ti-chi-bot · 2022-12-08T07:17:46Z

@rleungx: Your PR was out of date, I have automatically updated it for you.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ref #5615, ref #5648 Signed-off-by: Ryan Leung <rleungx@gmail.com>

ti-chi-bot added do-not-merge/needs-linked-issue do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Oct 21, 2022

ti-chi-bot requested review from nolouch and Yisaer October 21, 2022 05:57

rleungx force-pushed the overlap branch from 2519bab to 665c8bc Compare October 21, 2022 06:00

rleungx force-pushed the overlap branch from 665c8bc to 089e97e Compare October 24, 2022 10:15

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2022

rleungx force-pushed the overlap branch from 089e97e to 5633494 Compare November 30, 2022 06:33

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 30, 2022

rleungx marked this pull request as ready for review November 30, 2022 06:38

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 30, 2022

rleungx force-pushed the overlap branch from b0b72b9 to dac6b6c Compare November 30, 2022 08:07

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Nov 30, 2022

rleungx force-pushed the overlap branch from dac6b6c to 76bcff8 Compare November 30, 2022 08:14

rleungx mentioned this pull request Nov 30, 2022

Improve the performance of handling region heartbeats #5648

Closed

16 tasks

rleungx requested review from HunDunDM and CabinfeverB and removed request for Yisaer November 30, 2022 08:54

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 30, 2022

CabinfeverB reviewed Nov 30, 2022

View reviewed changes

rleungx force-pushed the overlap branch from 76bcff8 to 3d649d1 Compare December 1, 2022 04:02

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2022

rleungx requested a review from CabinfeverB December 1, 2022 04:02

rleungx force-pushed the overlap branch from 85506f1 to 3582e3a Compare December 1, 2022 04:17

rleungx commented Dec 1, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 2, 2022

nolouch reviewed Dec 2, 2022

View reviewed changes

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2022

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2022

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2022

rleungx added 6 commits December 6, 2022 11:42

reduce get overlap operations

58ca2a5

Signed-off-by: Ryan Leung <rleungx@gmail.com>

resolve conflicts

f16337c

Signed-off-by: Ryan Leung <rleungx@gmail.com>

rename

7d6e7ae

Signed-off-by: Ryan Leung <rleungx@gmail.com>

address the comment

7904ebb

Signed-off-by: Ryan Leung <rleungx@gmail.com>

address the comment

311060c

Signed-off-by: Ryan Leung <rleungx@gmail.com>

address the comment

4abba8b

Signed-off-by: Ryan Leung <rleungx@gmail.com>

rleungx force-pushed the overlap branch from c620745 to 4abba8b Compare December 6, 2022 03:58

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 6, 2022

nolouch approved these changes Dec 8, 2022

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 8, 2022

Merge branch 'master' into overlap

a1cdd25

ti-chi-bot merged commit 1b363d8 into tikv:master Dec 8, 2022

rleungx deleted the overlap branch December 8, 2022 07:29

rleungx mentioned this pull request Dec 8, 2022

*: fix lock in CheckAndPutRegion #5768

Merged

ti-chi-bot pushed a commit that referenced this pull request Dec 8, 2022

*: fix lock in CheckAndPutRegion (#5768)

8c5c666

ref #5615, ref #5648 Signed-off-by: Ryan Leung <rleungx@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: reduce get overlap operations #5615

*: reduce get overlap operations #5615

rleungx commented Oct 21, 2022 •

edited

Loading

ti-chi-bot commented Oct 21, 2022 •

edited

Loading

codecov bot commented Oct 21, 2022 •

edited

Loading

CabinfeverB left a comment

CabinfeverB Nov 30, 2022

rleungx Dec 1, 2022

nolouch Dec 2, 2022 •

edited

Loading

rleungx commented Dec 2, 2022

rleungx commented Dec 2, 2022

rleungx commented Dec 2, 2022

rleungx commented Dec 2, 2022

rleungx commented Dec 6, 2022

nolouch commented Dec 8, 2022

ti-chi-bot commented Dec 8, 2022

ti-chi-bot commented Dec 8, 2022

ti-chi-bot commented Dec 8, 2022

*: reduce get overlap operations #5615

*: reduce get overlap operations #5615

Conversation

rleungx commented Oct 21, 2022 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Oct 21, 2022 • edited Loading

codecov bot commented Oct 21, 2022 • edited Loading

Codecov Report

CabinfeverB left a comment

Choose a reason for hiding this comment

CabinfeverB Nov 30, 2022

Choose a reason for hiding this comment

rleungx Dec 1, 2022

Choose a reason for hiding this comment

nolouch Dec 2, 2022 • edited Loading

Choose a reason for hiding this comment

rleungx commented Dec 2, 2022

rleungx commented Dec 2, 2022

rleungx commented Dec 2, 2022

rleungx commented Dec 2, 2022

rleungx commented Dec 6, 2022

nolouch commented Dec 8, 2022

ti-chi-bot commented Dec 8, 2022

ti-chi-bot commented Dec 8, 2022

ti-chi-bot commented Dec 8, 2022

rleungx commented Oct 21, 2022 •

edited

Loading

ti-chi-bot commented Oct 21, 2022 •

edited

Loading

codecov bot commented Oct 21, 2022 •

edited

Loading

nolouch Dec 2, 2022 •

edited

Loading