filter: replace region.Clone in filter #2794
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Song Gao <disxiaofei@163.com>
Yisaer
added
component/scheduler
Yisaer
force-pushed
the
revise-region-clone
branch
from
ce40df2 to
6bfe1bf
Compare
Signed-off-by: Song Gao <disxiaofei@163.com>
Yisaer
force-pushed
the
revise-region-clone
branch
from
6bfe1bf to
4b9b632
Compare
nolouch
reviewed
Aug 18, 2020
server/schedule/filter/filters.go
Outdated
|
|
||
| // cloneRegion is used to replace region.Clone in filter which call the FitRegion, | ||
| // as FitRegion only needs partial information(startKey, endKey, peers, leader peer) | ||
| func cloneRegion(region *core.RegionInfo, opts ...core.RegionCreateOption) *core.RegionInfo { |
There was a problem hiding this comment.
Do not clone the downPeers, pendingPeers? maybe rename to a suitable name?
There was a problem hiding this comment.
How about createRegionForRuleFit(peers []*metapb.Peer, leader *metapb.Peer) *core.RegionInfo?
Signed-off-by: Song Gao <disxiaofei@163.com>
disksing
reviewed
Aug 19, 2020
| StoreId: leader.StoreId, | ||
| Role: leader.Role, | ||
| } | ||
| copyPeers := make([]*metapb.Peer, 0, len(peers)) |
There was a problem hiding this comment.
Seems we don't have to copy leader. And if the peer list is untouched, we don't need to copy it either.
There was a problem hiding this comment.
I think for both ruleFitFilter and ruleLeaderFitFilter, they will both touch peers and leaders. I think it is necessary to copy peers and leaders either.
disksing
approved these changes
Aug 19, 2020
nolouch
approved these changes
Aug 19, 2020
ti-srebot
approved these changes
Aug 19, 2020
I updated the benchmark result in the description. |
lhy1024
approved these changes
Aug 19, 2020
ti-srebot
added
status/LGT3
|
/merge |
ti-srebot
added
the
status/can-merge
|
/run-all-tests |
ti-srebot
pushed a commit
to ti-srebot/pd
that referenced
this pull request
Aug 19, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
|
cherry pick to release-4.0 in PR #2801 |
howardlau1999
pushed a commit
to howardlau1999/pd
that referenced
this pull request
Aug 20, 2020
Signed-off-by: Song Gao <disxiaofei@163.com>
Yisaer
pushed a commit
that referenced
this pull request
Aug 20, 2020
ZenoTan
pushed a commit
to ZenoTan/pd
that referenced
this pull request
Aug 24, 2020
Signed-off-by: Song Gao <disxiaofei@163.com> Signed-off-by: ZenoTan <zenotan1998@gmail.com>
jyz0309
pushed a commit
to jyz0309/pd
that referenced
this pull request
Aug 24, 2020
Signed-off-by: Song Gao <disxiaofei@163.com>
jyz0309
pushed a commit
to jyz0309/pd
that referenced
this pull request
Aug 24, 2020
Signed-off-by: Song Gao <disxiaofei@163.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Song Gao disxiaofei@163.com
What problem does this PR solve?
ref #2783
region.Clone consumes too much cpu resources.
What is changed and how it works?
replace region.Clone with a self-maintained clone function as FitRegion only need partial information of a region.
BenchmarkCloneRegionTest is using new clone method and BenchmarkCloneRegionTest2 is using region.Clone
$ go test -bench=. -run=none goos: darwin goarch: amd64 pkg: github.com/tikv/pd/server/schedule/filter BenchmarkCloneRegionTest-4 4356267 267 ns/op BenchmarkCloneRegionTest2-4 316604 3292 ns/op PASS ok github.com/tikv/pd/server/schedule/filter 2.555sCheck List
Tests
Related changes
Release note