store-gateway: detect cache key collisions for ExpandedPostings cache #4770
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
This reverts commit 2e691a7 Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
2 tasks
pracucci
approved these changes
Apr 19, 2023
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
dimitarvdimitrov
changed the title
* Add test for postings cache collision Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> * Implement cache collision for postings Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> * Update CHANGELOG.md Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> * Change cache key for postings Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> * Remove practically unreachable code * diff-varint-snappy postings are now only used for Series calls * this commit moves diffVarintSnappyDecode to testing code * removes being able to handle diff-varint-snappy postings without matchers * since now caching log expects to have some anti-collision material in the cache item, we can also remove the handling of raw stored postings. I checked the metric for compression failures (cortex_bucket_store_cached_postings_compression_errors_total) in the last 30 days across all grafana labs clusters, and it is always 0 Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> * Encode as string Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> * Avoid sprintf Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> * use strings builder Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> * Update CHANGELOG.md Co-authored-by: Marco Pracucci <marco@pracucci.com> --------- Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> Co-authored-by: Marco Pracucci <marco@pracucci.com>
|
LGTM! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does
When storing an ExpandedPostings cache item the key is the hashed matchers from the query. However, we do not check the stored values for cache collisions.
This PR adds cache collisions for ExpandedPostings. It stores the string of the request matchers with the item value and then rechecks it upon retrieval.
To ensure we don't misinterpret some cache item, this PR also changes cache key for expanded postings.
Which issue(s) this PR fixes or relates to
closes #4523
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]