-
Notifications
You must be signed in to change notification settings - Fork 719
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: make hotPeerCache concurrency safe #3460
Conversation
[REVIEW NOTIFICATION] This pull request has not been approved. 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 writing |
It should be met only when handling some heartbeats from the same region at the same time. |
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 rest LGTM
Codecov Report
@@ Coverage Diff @@
## master #3460 +/- ##
==========================================
- Coverage 75.14% 75.00% -0.14%
==========================================
Files 244 244
Lines 23557 23571 +14
==========================================
- Hits 17702 17680 -22
- Misses 4287 4311 +24
- Partials 1568 1580 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
server/statistics/hot_peer_cache.go
Outdated
peersOfStore map[uint64]*TopN // storeID -> hot peers | ||
storesOfRegion map[uint64]map[uint64]struct{} // regionID -> storeIDs | ||
mu struct { | ||
sync.RWMutex |
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.
Will it have a performance problem?
9d205f6
to
f27397b
Compare
/hold |
Signed-off-by: Song Gao disxiaofei@163.com
What problem does this PR solve?
In some cases, pd might update the same region and dimension at the same time which will cause the concurrent problem which may let pd panic.
ref #3461
What is changed and how it works?
This request made
dimStat
concurrency safe.Check List
Tests
Release note