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

command:add ctl command to get hot history regions(ref #25281) #4103

Merged
merged 6 commits into from
Dec 21, 2021

Conversation

qidi1
Copy link
Contributor

@qidi1 qidi1 commented Sep 11, 2021

…onfg

Signed-off-by: qidi1 1083369179@qq.com

What problem does this PR solve?
origin pr is here #3989
This PR is oversize and splited into three small one:

  1. config: add hot region history configurations #4019
  2. cluster: add hot_region_storage #4020
  3. api: add interface for hot_region info reqeust #4021
  4. command:add ctl command to get hot history regions(ref #25281) #4103
  5. core:solve bug in hot_region_storage #4121

Proposal: pingcap/tidb#27487
Associated Issue: pingcap/tidb#25281
Associated PR: pingcap/tidb#27224

  1. infoschema: add tidb_hot_regions_history virtual table pingcap/tidb#27373
  2. planner: add extractor for tidb_hot_regions_history pingcap/tidb#27374
  3. planner: add extractor for tidb_hot_regions_history pingcap/tidb#27374

What is changed and how it works?
provide pd-ctl command to pull hot_history_regions information

Check List
Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • No code
  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Code changes

  • Has configuration change
  • Has HTTP API interfaces change (Don't forget to add the declarative for API)
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

Release note

none

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Sep 11, 2021

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • nolouch

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 do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 11, 2021
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 11, 2021
@qidi1 qidi1 changed the title History hot regions command command:add ctl command to get hot history regions Sep 11, 2021
@qidi1 qidi1 force-pushed the history_hot_regions_command branch from 6d3dd5d to 341c1cb Compare September 11, 2021 15:00
@qidi1
Copy link
Contributor Author

qidi1 commented Sep 11, 2021

/run-all-tests

@ti-chi-bot
Copy link
Member

@IcePigZDB: Request changes is only allowed for the reviewers in list.

In response to this:

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.

@qidi1 qidi1 mentioned this pull request Sep 14, 2021
14 tasks
@qidi1
Copy link
Contributor Author

qidi1 commented Sep 27, 2021

/run-all-tests

@qidi1
Copy link
Contributor Author

qidi1 commented Oct 25, 2021

/run-all-tests

@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 15, 2021
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 27, 2021
@qidi1
Copy link
Contributor Author

qidi1 commented Nov 29, 2021

/run-all-tests

@qidi1 qidi1 force-pushed the history_hot_regions_command branch from 03254e6 to e4d712e Compare November 29, 2021 11:52
@ti-chi-bot ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 29, 2021
@qidi1 qidi1 force-pushed the history_hot_regions_command branch from 3edd9f4 to 6ed54d1 Compare November 29, 2021 12:01
@ti-chi-bot ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 29, 2021
@qidi1 qidi1 changed the title command:add ctl command to get hot history regions command:add ctl command to get hot history regions(ref #25281) Nov 29, 2021
@qidi1
Copy link
Contributor Author

qidi1 commented Dec 1, 2021

/run-all-tests

Signed-off-by: qidi1 <1083369179@qq.com>
@qidi1
Copy link
Contributor Author

qidi1 commented Dec 8, 2021

/run-all-tests

@qidi1 qidi1 force-pushed the history_hot_regions_command branch from f46821c to 1240dbc Compare December 10, 2021 03:29
@qidi1
Copy link
Contributor Author

qidi1 commented Dec 10, 2021

/run-all-tests

@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #4103 (ef3ada4) into master (af174e6) will increase coverage by 0.17%.
The diff coverage is 75.19%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4103      +/-   ##
==========================================
+ Coverage   74.86%   75.03%   +0.17%     
==========================================
  Files         264      264              
  Lines       27821    27931     +110     
==========================================
+ Hits        20827    20959     +132     
+ Misses       5134     5113      -21     
+ Partials     1860     1859       -1     
Flag Coverage Δ
unittests 75.03% <75.19%> (+0.17%) ⬆️

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

Impacted Files Coverage Δ
tools/pd-ctl/pdctl/command/global.go 69.56% <72.97%> (+0.49%) ⬆️
tools/pd-ctl/pdctl/command/hot_command.go 71.32% <76.08%> (+8.58%) ⬆️
pkg/errs/errs.go 75.00% <0.00%> (-25.00%) ⬇️
server/id/id.go 76.19% <0.00%> (-4.77%) ⬇️
tools/pd-ctl/pdctl/command/operator.go 65.99% <0.00%> (-1.22%) ⬇️
server/election/leadership.go 77.31% <0.00%> (-1.04%) ⬇️
server/handler.go 49.06% <0.00%> (-0.42%) ⬇️
server/server.go 71.94% <0.00%> (ø)
server/grpc_service.go 51.40% <0.00%> (ø)
server/member/member.go 64.70% <0.00%> (ø)
... and 15 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 af174e6...ef3ada4. Read the comment docs.

@qidi1 qidi1 force-pushed the history_hot_regions_command branch 3 times, most recently from c1accf9 to d9c2277 Compare December 12, 2021 04:26
Signed-off-by: qidi1 <1083369179@qq.com>
@qidi1 qidi1 force-pushed the history_hot_regions_command branch from 6b2edbb to 62725ba Compare December 12, 2021 05:07
@nolouch
Copy link
Contributor

nolouch commented Dec 14, 2021

PTAL @lhy1024

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.

@ti-chi-bot ti-chi-bot added status/LGT1 Indicates that a PR has LGTM 1. release-note Denotes a PR that will be considered when it comes time to generate release notes. release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Dec 21, 2021
@nolouch nolouch added the require-LGT1 Indicates that the PR requires an LGTM. label Dec 21, 2021
@nolouch
Copy link
Contributor

nolouch commented Dec 21, 2021

/merge

@ti-chi-bot
Copy link
Member

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

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

Commit hash: f8b9624

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 21, 2021
@ti-chi-bot
Copy link
Member

[FORMAT CHECKER NOTIFICATION]

Please provide the associated issue number in the commit messages, for example: close #12345, or ref #12345.

@nolouch nolouch merged commit 447fbd4 into tikv:master Dec 21, 2021
CabinfeverB pushed a commit to CabinfeverB/pd that referenced this pull request Dec 28, 2021
…4103)

* command:add hot-history-regions command(ref #pingcap/tidb/issues/25281)

Signed-off-by: qidi1 <1083369179@qq.com>
Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/invalid-commit-message release-note-none Denotes a PR that doesn't merit a release note. require-LGT1 Indicates that the PR requires an LGTM. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants