-
Notifications
You must be signed in to change notification settings - Fork 720
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
statistics: fix the problem that the hot cache cannot be emptied when the interval is less than 60 #4396
Conversation
Signed-off-by: lhy1024 <admin@liudos.us>
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue and PR comments cannot let me understand the problem.
Could you provide more information on how it works? |
Signed-off-by: lhy1024 <admin@liudos.us>
server/statistics/kind.go
Outdated
switch k { | ||
case Origin: | ||
return "origin" | ||
case Inherit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is Inherit
used for? It looks we only need Adopt
right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, only distinct different source
Signed-off-by: lhy1024 <admin@liudos.us>
Signed-off-by: lhy1024 <admin@liudos.us>
Codecov Report
@@ Coverage Diff @@
## master #4396 +/- ##
==========================================
+ Coverage 74.71% 74.89% +0.18%
==========================================
Files 263 263
Lines 27629 27678 +49
==========================================
+ Hits 20642 20730 +88
+ Misses 5130 5110 -20
+ Partials 1857 1838 -19
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Signed-off-by: lhy1024 <admin@liudos.us>
/merge |
@disksing: 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. |
This pull request has been accepted and is ready to merge. Commit hash: f771b4e
|
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
In response to a cherrypick label: new pull request created: #4431. |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
In response to a cherrypick label: new pull request created: #4432. |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
In response to a cherrypick label: new pull request created: #4433. |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
In response to a cherrypick label: new pull request created: #4434. |
In response to a cherrypick label: new pull request created: #4435. |
… the interval is less than 60 (#4396) (#4432) * This is an automated cherry-pick of #4396 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> * fix conflict without sync test Signed-off-by: lhy1024 <admin@liudos.us> * fix lint Signed-off-by: lhy1024 <admin@liudos.us> * ref #4390 Signed-off-by: lhy1024 <admin@liudos.us> * statistics: fix hot peer cache (#4446) * fix hot peer cache Signed-off-by: lhy1024 <admin@liudos.us> * fix Signed-off-by: lhy1024 <admin@liudos.us> * fix Signed-off-by: lhy1024 <admin@liudos.us> * add more test Signed-off-by: lhy1024 <admin@liudos.us> * fix ci Signed-off-by: lhy1024 <admin@liudos.us> * address comment Signed-off-by: lhy1024 <admin@liudos.us> * ref #4390 Signed-off-by: lhy1024 <admin@liudos.us> * add comment and test Signed-off-by: lhy1024 <admin@liudos.us> * address comments Signed-off-by: lhy1024 <admin@liudos.us> * fix Signed-off-by: lhy1024 <admin@liudos.us> * add more test Signed-off-by: lhy1024 <admin@liudos.us> * add comment Signed-off-by: lhy1024 <admin@liudos.us> Co-authored-by: ShuNing <nolouch@gmail.com> Signed-off-by: lhy1024 <admin@liudos.us> * fix test Signed-off-by: lhy1024 <admin@liudos.us> * revert log Signed-off-by: lhy1024 <admin@liudos.us> * remove todo Signed-off-by: lhy1024 <admin@liudos.us> Co-authored-by: lhy1024 <admin@liudos.us> Co-authored-by: ShuNing <nolouch@gmail.com>
… the interval is less than 60 (#4396) (#4433) * This is an automated cherry-pick of #4396 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> * fix test Signed-off-by: lhy1024 <admin@liudos.us> * ref #4390 Signed-off-by: lhy1024 <admin@liudos.us> * fix ci Signed-off-by: lhy1024 <admin@liudos.us> * pick for #4446, #4512 Signed-off-by: lhy1024 <admin@liudos.us> * fix test Signed-off-by: lhy1024 <admin@liudos.us> * update Signed-off-by: lhy1024 <admin@liudos.us> Co-authored-by: lhy1024 <admin@liudos.us>
What problem does this PR solve?
close #4390
What is changed and how it works?
Our current heartbeat process looks like this.
If the interval is less than the default interval for heartbeats (60 seconds), then we will put it into the cache temporarily and wait until it has made up 60 seconds before considering whether it is hot enough.
For a peer that has just been reported, if the region is also in the hot cache, then there are three cases.
The problem occurs in the third branch, if the old peer is used directly without clone, then the old item and the new item will be written at the same time. And when the new peer interval is less than 60 seconds, it means that it will be temporarily put into the cache.
If the old peer will be cooled down at this time, it will keep the peer in the hot cache for a long time and cannot be exited.
This pr does a clone for the third branch to avoid this problem, and changes the store and region heartbeats to configurable in another pr.
Check List
Tests
Release note