-
Notifications
You must be signed in to change notification settings - Fork 849
HATracker: Add a local cache warmup on startup #7213
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
base: master
Are you sure you want to change the base?
HATracker: Add a local cache warmup on startup #7213
Conversation
3567f6f to
7786687
Compare
|
Do you see distributors in clusters with large number of pairs using more memory after this change? Do we need a flag for this? |
|
@friedrichg |
friedrichg
left a comment
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.
@SungJin1212 Thanks, this is great work.
Signed-off-by: SungJin1212 <[email protected]>
Signed-off-by: SungJin1212 <[email protected]>
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.
I am a bit doubt of how much this cache warmup would help.
I think at least we never see this issue in production. If we think it is good to have I prefer to put this behind a feature flag and disable by default.
I am a bit worried that during a big fleet scale up distributors ended up sending too many requests to KV stores just to warmup cache and causing much bigger impact.
Signed-off-by: SungJin1212 <[email protected]>
2644e14 to
0850259
Compare
|
@yeya24 |
friedrichg
left a comment
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.
I think this flag might also prove useful for #7220.
At least bring us closer
|
@SungJin1212 maybe also mark it as experimental and include the flag in https://github.com/cortexproject/cortex/blob/master/docs/configuration/v1-guarantees.md |
Signed-off-by: SungJin1212 <[email protected]>
|
@friedrichg |
This PR introduces local cache warmup logic on HA tracker startup that fetches all keys from the KV store and warms the local cache.
Previously, whenever the Distributor started, it suffered from initial cold cache misses. Since the local map was empty, the HATracker treated every incoming request as a new entry. This caused unnecessary CAS operations to the KV store even for existing valid keys.
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]