Skip to content

Report QPS about each type of command in RegionHeartbeat and StoreHeartbeat #3695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 34 commits into from
Jun 15, 2021
Merged

Report QPS about each type of command in RegionHeartbeat and StoreHeartbeat #3695

merged 34 commits into from
Jun 15, 2021

Conversation

lhy1024
Copy link
Contributor

@lhy1024 lhy1024 commented May 20, 2021 •
edited
Loading

Signed-off-by: lhy1024 admin@liudos.us

What problem does this PR solve?

Report QPS about each type of command in RegionHeartbeat and StoreHeartbeat

What is changed and how it works?

Check List

Tests

  • Unit test

Release note

None.

@ti-chi-bot ti-chi-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 20, 2021
@ti-chi-bot ti-chi-bot requested review from HunDunDM and rleungx May 20, 2021 10:11
rleungx
rleungx reviewed May 21, 2021
View reviewed changes
server/statistics/hot_peer_cache.go Outdated
@@ -48,6 +48,8 @@ var minHotThresholds = [RegionStatCount]float64{
RegionWriteKeys: 32,
RegionReadBytes: 8 * 1024,
RegionReadKeys: 128,
RegionReadQuery: 1, // temp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does temp here mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it only is a temp threshold, I will obtain more meanable value by test

rleungx
rleungx reviewed May 24, 2021
View reviewed changes
rleungx
rleungx reviewed May 24, 2021
View reviewed changes
server/schedulers/hot_region_config.go Outdated
@@ -60,6 +61,7 @@ type hotRegionSchedulerConfig struct {
// step = max current * rank step ratio
ByteRateRankStepRatio float64 `json:"byte-rate-rank-step-ratio"`
KeyRateRankStepRatio float64 `json:"key-rate-rank-step-ratio"`
QPSRankStepRatio float64 `json:"qps-rank-step-ratio"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary? It seems we never change these configurations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only unify with the previous configuration

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also have doubt if the previous configurations are necessary.

rleungx
rleungx reviewed May 24, 2021
View reviewed changes
@codecov
Copy link

codecov bot commented May 24, 2021 •
edited
Loading

Codecov Report

Merging #3695 (97a3e3f) into master (7c8f3a7) will decrease coverage by 0.07%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3695      +/-   ##
==========================================
- Coverage   75.12%   75.05%   -0.08%     
==========================================
  Files         245      245              
  Lines       24529    24585      +56     
==========================================
+ Hits        18428    18452      +24     
- Misses       4485     4504      +19     
- Partials     1616     1629      +13
Flag Coverage Δ
unittests 75.05% <90.00%> (-0.08%) ⬇️

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

Impacted Files Coverage Δ
server/statistics/kind.go 0.00% <0.00%> (ø)
server/statistics/metrics.go 100.00% <ø> (ø)
server/statistics/store.go 92.63% <87.50%> (+2.91%) ⬆️
server/cluster/coordinator.go 72.51% <88.88%> (-0.63%) ⬇️
pkg/mock/mockcluster/mockcluster.go 95.71% <100.00%> (ø)
server/api/hot_status.go 100.00% <100.00%> (ø)
server/cluster/cluster.go 83.29% <100.00%> (-0.06%) ⬇️
server/core/region.go 90.71% <100.00%> (+0.29%) ⬆️
server/core/region_option.go 84.44% <100.00%> (+0.35%) ⬆️
server/schedulers/hot_region.go 84.97% <100.00%> (+0.31%) ⬆️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c8f3a7...97a3e3f. Read the comment docs.

@lhy1024
add qps statistic
64bc46c 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
address comment
3977e40 
Signed-off-by: lhy1024 <admin@liudos.us>
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
6.0% 6.0% Duplication

@lhy1024
fix lint
72fd5bf 
Signed-off-by: lhy1024 <admin@liudos.us>
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2021
@lhy1024
add metrics
7baaec4 
Signed-off-by: lhy1024 <admin@liudos.us>
@rleungx
Copy link
Member

rleungx commented May 31, 2021

There are still some comments that need to be resolved.

@lhy1024
address comment
2643279 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
Copy link
Contributor Author

lhy1024 commented May 31, 2021

@rleungx @Yisaer PTAL

lhy1024 and others added 10 commits June 1, 2021 23:32
@lhy1024
replace loads with []float64 in peer info
4445b25 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
statistics: replace loads with []float64 in peer info
64dae82 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
remove unused code
5c3576f 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
address comment
939e506 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
address comment
c7069bd 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
remove unused code
9312d02 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
fix test
3d837f8 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
address comment
fa98344 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
misc
2a0d92f 
Signed-off-by: lhy1024 <admin@liudos.us>
@ti-chi-bot
Merge branch 'master' into reformat-peer
dbcbfa8
lhy1024 added 2 commits June 3, 2021 14:50
@lhy1024
update query output
a0e6f49 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
fix lint
8f783ff 
Signed-off-by: lhy1024 <admin@liudos.us>
@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jun 3, 2021
@lhy1024 lhy1024 removed the status/DNM label Jun 3, 2021
lhy1024 added 2 commits June 3, 2021 15:47
@lhy1024
fix keysort
8780912 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
fix config
edb01c4 
Signed-off-by: lhy1024 <admin@liudos.us>
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 5, 2021
@lhy1024
Merge branch 'master' of http://github.com/tikv/pd into qps-report
620e889 
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024
Copy link
Contributor Author

lhy1024 commented Jun 15, 2021

@rleungx @HunDunDM @nolouch PTAL

@lhy1024
Copy link
Contributor Author

lhy1024 commented Jun 15, 2021

@ti-chi-bot

@lhy1024 lhy1024 removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 15, 2021
@lhy1024
update kvproto
a2bfac6 
Signed-off-by: lhy1024 <admin@liudos.us>
@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • HunDunDM
  • Yisaer

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.

@ti-chi-bot 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 Jun 15, 2021
@lhy1024
Copy link
Contributor Author

lhy1024 commented Jun 15, 2021

/merge

@ti-chi-bot
Copy link
Member

@lhy1024: 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
Copy link
Member

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

Commit hash: a2bfac6

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 15, 2021
@ti-chi-bot ti-chi-bot merged commit cff3201 into tikv:master Jun 15, 2021
bufferflies pushed a commit to bufferflies/pd that referenced this pull request Jul 9, 2021
@lhy1024 @ti-chi-bot @bufferflies
Report QPS about each type of command in RegionHeartbeat and StoreHea…
d96a0e0 
…rtbeat (tikv#3695)

* add qps statistic

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* fix lint

Signed-off-by: lhy1024 <admin@liudos.us>

* add metrics

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* replace loads with []float64 in peer info

Signed-off-by: lhy1024 <admin@liudos.us>

* statistics: replace loads with []float64 in peer info

Signed-off-by: lhy1024 <admin@liudos.us>

* remove unused code

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* remove unused code

Signed-off-by: lhy1024 <admin@liudos.us>

* fix test

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* misc

Signed-off-by: lhy1024 <admin@liudos.us>

* update kvproto

Signed-off-by: lhy1024 <admin@liudos.us>

* fix test

Signed-off-by: lhy1024 <admin@liudos.us>

* fix lint

Signed-off-by: lhy1024 <admin@liudos.us>

* ADD TEST

Signed-off-by: lhy1024 <admin@liudos.us>

* update query output

Signed-off-by: lhy1024 <admin@liudos.us>

* fix lint

Signed-off-by: lhy1024 <admin@liudos.us>

* fix keysort

Signed-off-by: lhy1024 <admin@liudos.us>

* fix config

Signed-off-by: lhy1024 <admin@liudos.us>

* update kvproto

Signed-off-by: lhy1024 <admin@liudos.us>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
bufferflies pushed a commit to bufferflies/pd that referenced this pull request Jul 9, 2021
@lhy1024 @ti-chi-bot @bufferflies
Report QPS about each type of command in RegionHeartbeat and StoreHea…
a3ae742 
…rtbeat (tikv#3695)

* add qps statistic

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* fix lint

Signed-off-by: lhy1024 <admin@liudos.us>

* add metrics

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* replace loads with []float64 in peer info

Signed-off-by: lhy1024 <admin@liudos.us>

* statistics: replace loads with []float64 in peer info

Signed-off-by: lhy1024 <admin@liudos.us>

* remove unused code

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* remove unused code

Signed-off-by: lhy1024 <admin@liudos.us>

* fix test

Signed-off-by: lhy1024 <admin@liudos.us>

* address comment

Signed-off-by: lhy1024 <admin@liudos.us>

* misc

Signed-off-by: lhy1024 <admin@liudos.us>

* update kvproto

Signed-off-by: lhy1024 <admin@liudos.us>

* fix test

Signed-off-by: lhy1024 <admin@liudos.us>

* fix lint

Signed-off-by: lhy1024 <admin@liudos.us>

* ADD TEST

Signed-off-by: lhy1024 <admin@liudos.us>

* update query output

Signed-off-by: lhy1024 <admin@liudos.us>

* fix lint

Signed-off-by: lhy1024 <admin@liudos.us>

* fix keysort

Signed-off-by: lhy1024 <admin@liudos.us>

* fix config

Signed-off-by: lhy1024 <admin@liudos.us>

* update kvproto

Signed-off-by: lhy1024 <admin@liudos.us>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
@nolouch nolouch mentioned this pull request Jul 13, 2021
Closed
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rleungx rleungx rleungx left review comments

@Yisaer Yisaer Yisaer approved these changes

@HunDunDM HunDunDM HunDunDM approved these changes

Assignees
No one assigned
Labels
release-note-none Denotes a PR that doesn't merit a release note. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lhy1024 @rleungx @ti-chi-bot @Yisaer @HunDunDM