-
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) #4435
statistics: fix the problem that the hot cache cannot be emptied when the interval is less than 60 (#4396) #4435
Conversation
Signed-off-by: lhy1024 <admin@liudos.us>
Signed-off-by: lhy1024 <admin@liudos.us>
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. |
/rebuild |
1 similar comment
/rebuild |
Codecov Report
@@ Coverage Diff @@
## release-5.3 #4435 +/- ##
===============================================
+ Coverage 74.87% 74.91% +0.04%
===============================================
Files 264 263 -1
Lines 27285 27329 +44
===============================================
+ Hits 20430 20474 +44
- Misses 5037 5042 +5
+ Partials 1818 1813 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
a7e470f
to
8bc6c00
Compare
/merge |
@JmPotato: 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: 8bc6c00
|
ci failed @lhy1024 |
/rebuild |
/run-unit-test |
/retest |
/run-unit-test |
/merge |
@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. |
/run-unit-test |
2 similar comments
/run-unit-test |
/run-unit-test |
This is an automated cherry-pick of #4396,wait for #4446
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