Skip to content
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.

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

schedule: add more dimension for filter metrics #1746

Merged
merged 8 commits into from
Sep 18, 2019

Conversation

rleungx
Copy link
Member

@rleungx rleungx commented Sep 10, 2019

What problem does this PR solve?

We have added the filter related metrics in grafana, but it's hard to understand how a filter acts on the checkers or the schedulers. It only shows the total filter number which is useless.

What is changed and how it works?

This PR adds a new dimension named ActOn for each filter to indicate that each scheduler or checker is affected by the kind of the filter.
Also this PR removes two unused filters: disconnectFilter and rejectLeaderFilter

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Screen Shot 2019-09-10 at 7 28 33 PM

Related changes

  • Need to update the tidb-ansible repository

@codecov-io
Copy link

codecov-io commented Sep 10, 2019

Codecov Report

Merging #1746 into master will increase coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1746      +/-   ##
==========================================
+ Coverage   76.83%   76.95%   +0.11%     
==========================================
  Files         162      162              
  Lines       15893    15910      +17     
==========================================
+ Hits        12211    12243      +32     
+ Misses       2655     2638      -17     
- Partials     1027     1029       +2
Impacted Files Coverage Δ
server/schedule/filter/metrics.go 100% <ø> (ø) ⬆️
server/schedulers/balance_leader.go 96.69% <100%> (ø) ⬆️
server/schedulers/balance_region.go 84.79% <100%> (ø) ⬆️
server/schedule/filter/filters.go 88.99% <100%> (+11.92%) ⬆️
server/schedulers/shuffle_leader.go 84.84% <100%> (+0.47%) ⬆️
server/checker/namespace_checker.go 75.75% <100%> (+0.37%) ⬆️
server/schedulers/label.go 82.97% <100%> (+0.37%) ⬆️
server/schedulers/adjacent_region.go 72.77% <100%> (+0.15%) ⬆️
server/schedulers/evict_leader.go 83.33% <100%> (+0.4%) ⬆️
server/schedulers/shuffle_hot_region.go 58.97% <100%> (ø) ⬆️
... and 11 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 682b501...c291ec2. Read the comment docs.

server/schedule/filter/filters.go Outdated Show resolved Hide resolved
@@ -22,7 +22,7 @@ var (
Subsystem: "schedule",
Name: "filter",
Help: "Counter of the filter",
}, []string{"action", "address", "store", "type"})
}, []string{"action", "address", "store", "acton", "type"})
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "act-on" is better?

Copy link
Contributor

Choose a reason for hiding this comment

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

"acton" is too similar to "action". Is there any reason for using it instead of "act-on"?

server/schedule/filter/filters.go Outdated Show resolved Hide resolved
server/schedulers/adjacent_region.go Outdated Show resolved Hide resolved
@rleungx rleungx requested a review from nolouch September 17, 2019 07:23
@rleungx rleungx force-pushed the improve-filter-metrics branch from cad5895 to 5312c6f Compare September 17, 2019 07:25
Copy link
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

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

LGTM

@nolouch
Copy link
Contributor

nolouch commented Sep 17, 2019

PTAL @lhy1024

Copy link
Contributor

@lhy1024 lhy1024 left a comment

Choose a reason for hiding this comment

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

the rest LGTM

server/schedule/filter/filters.go Outdated Show resolved Hide resolved
server/schedule/filter/metrics.go Outdated Show resolved Hide resolved
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
@rleungx rleungx force-pushed the improve-filter-metrics branch from 733b0d8 to 0a562cf Compare September 18, 2019 05:25
Signed-off-by: Ryan Leung <[email protected]>
@rleungx rleungx force-pushed the improve-filter-metrics branch from 0a562cf to fe4732c Compare September 18, 2019 05:29
server/checker/namespace_checker.go Show resolved Hide resolved
server/schedule/filter/metrics.go Outdated Show resolved Hide resolved
Signed-off-by: Ryan Leung <[email protected]>
Copy link
Contributor

@lhy1024 lhy1024 left a comment

Choose a reason for hiding this comment

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

LGTM

@rleungx rleungx added the status/can-merge Indicates a PR has been approved by a committer. label Sep 18, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 18, 2019

/run-all-tests

@sre-bot sre-bot merged commit 061eac6 into tikv:master Sep 18, 2019
rleungx added a commit to rleungx/pd that referenced this pull request Nov 7, 2019
rleungx added a commit to rleungx/pd that referenced this pull request Nov 7, 2019
sre-bot pushed a commit that referenced this pull request Nov 8, 2019
nolouch pushed a commit to nolouch/pd that referenced this pull request Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/metrics Metrics. status/can-merge Indicates a PR has been approved by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants