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

Fixed panic when Mimir is running in single binary mode and results cache is enabled #1704

Merged
merged 2 commits into from Apr 14, 2022

Conversation

pracucci
Copy link
Collaborator

What this PR does

We add component label to all memcached clients, except the results cache. This makes memcached client metrics clash when running in single binary mode and results cache is enabled, leading to a panic. This PR fixes it.

Which issue(s) this PR fixes or relates to

Fixes #1698

Checklist

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

…ache is enabled

Signed-off-by: Marco Pracucci <marco@pracucci.com>
@replay
Copy link
Contributor

replay commented Apr 14, 2022

If all other instances of the results cache have the component label in their metrics, why do the query-frontend results cache metrics clash with any of them? Does it also clash if the labelset of a series a subset of another series? if so, then TIL :)

@pracucci
Copy link
Collaborator Author

If all other instances of the results cache have the component label in their metrics, why do the query-frontend results cache metrics clash with any of them? Does it also clash if the labelset of a series a subset of another series? if so, then TIL :)

We register the same metric name multiple times. To make it work, it must have the same exact constant label names but with different values (to not clash). We were already using the component const label for that purpose in all places, except the query-frontend results cache.

Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci
Copy link
Collaborator Author

Since this logic is very difficult to enforce with unit tests, I've added an integration test. It breaks with the version currently in main.

@pracucci pracucci marked this pull request as ready for review April 14, 2022 15:15
@pracucci pracucci merged commit 29020a2 into main Apr 14, 2022
@pracucci pracucci deleted the fix-single-binary-frontend-with-memcached branch April 14, 2022 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mimir crashed with target=all when frontend memcached enabled.
3 participants