Skip to content
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

store-gateway: limit number of values in LabeValues cache items #5021

Merged

Conversation

dimitarvdimitrov
Copy link
Contributor

What this PR does

encoding/gob has a bug when decoding string slices that are longer than 655360 items. This PR introduces a hard limit on the number of items in a cache item that we store.

Which issue(s) this PR fixes or relates to

Fixes #5014

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

encoding/gob has a bug when decoding string slices that are longer than 655360 items. This PR introduces a hard limit on the number of items in a cache item that we store.

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@pracucci pracucci self-requested a review May 16, 2023 12:28
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Since this check is applied when storing the result to the cache, we'll have to purge the cache if we already cached such long values, which is fine.

@dimitarvdimitrov dimitarvdimitrov merged commit defe3d4 into main May 17, 2023
22 checks passed
@dimitarvdimitrov dimitarvdimitrov deleted the dimitar/st-gw/limit-label-values-item-count-limit branch May 17, 2023 08:44
@colega
Copy link
Contributor

colega commented May 17, 2023

@pracucci @dimitarvdimitrov

we'll have to purge the cache

We should purge the cache by changing the cache key for this.

@dimitarvdimitrov
Copy link
Contributor Author

@pracucci @dimitarvdimitrov

we'll have to purge the cache

We should purge the cache by changing the cache key for this.

I opened #5037 to change the cache key

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

store-gateway: caching large lists of label values causes panics
3 participants