-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
doc: add design doc for stats lru cache #36804
Conversation
Signed-off-by: yisaer <[email protected]>
[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. |
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/ed0aed4c6679833a87b0327e252a0b2b17f3e0dd |
### Sync Stats Compatibility | ||
|
||
Previously tidb server has Sync Stats and asynchronously Loading Histograms in order to load column stats into memory during query. | ||
As Stats LRU Cache Policy may evict index stats in memory, we also need Sync Stats and asynchronously Loading Histograms to support loading index stats according to its loading status to keep compatible. |
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.
Do we need more details to tell when the load will be triggered after eviction?
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.
updated.
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.
LGTM for this part, while Sync Stats doesn't have design doc in this repo yet...
### Goal | ||
|
||
- Use LRU to maintain the stats cache in memory | ||
- Keep the total memory usage of stats cache under the quota |
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 third one, provide a way to efficiently load evicted stats back into cache once needed?
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.
updated.
### Stats Cache Interface | ||
|
||
We will provide a Stats Cache Interface which is implemented by LRU Cache and Map Cache. | ||
If the tidb server didn't enable Stats LRU Cache, it will use Map Cache by default. |
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 about introducing the variables to turn on LRU and to set the capacity?
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.
updated.
Signed-off-by: yisaer <[email protected]>
/merge |
This pull request has been accepted and is ready to merge. Commit hash: a3c4cf0
|
TiDB MergeCI notify
|
Signed-off-by: yisaer [email protected]
What problem does this PR solve?
Issue Number: ref #34052
Problem Summary:
What is changed and how it works?
add design doc for stats lru cache
Check List
Tests
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.