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

Bring latest changes from mimir-prometheus #2701

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

jesusvazquez
Copy link
Member

@jesusvazquez jesusvazquez commented Aug 11, 2022

This commit updates the vendored version of mimir-prometheus.

The highlights are:

This commit updates the vendored version of mimir-prometheus.

The highlights are:
- Removed expensive atomic load call from head append.
- Fixed out-of-order query bug that happened due to invalid LabelValues
  responses from the OOOHeadIndexReader that led to wrong query results.
This issue was more common when query sharding was used together with
out-of-order due to postings cache corruption. grafana/mimir-prometheus#314
@jesusvazquez jesusvazquez self-assigned this Aug 11, 2022
@github-actions

This comment has been minimized.

@jesusvazquez jesusvazquez marked this pull request as ready for review August 11, 2022 14:44
@colega
Copy link
Contributor

colega commented Aug 11, 2022

Should we mention this in the changelog? As a bugfix for OOO.

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.

Great job spotting and fixing the bug! I would would be glad to see a CHANGELOG entry before merging.

@pracucci pracucci added this to the 2.3 milestone Aug 12, 2022
@github-actions

This comment has been minimized.

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>
@jesusvazquez jesusvazquez force-pushed the jvp/bring-changes-from-mimir-prometheus branch from d3dc468 to e705912 Compare August 12, 2022 09:28
@colega colega enabled auto-merge (squash) August 12, 2022 09:29
@colega
Copy link
Contributor

colega commented Aug 12, 2022

Changelog entry LGTM! Thanks.

@github-actions
Copy link
Contributor

Helm <> Jsonnet Diff

⚠️ A difference was detected between the Helm chart and the Jsonnet library.

  1. Use make check-helm-jsonnet-diff to reproduce the output locally.
  2. This test is experimental while we gather feedback about its usefulness.
  3. It does not block your PR from being merged, but we would appreciate you trying to keep feature parity between the Helm chart and Jsonnet library if possible.

If you get stuck on this step and the Mimir maintainers aren't able to help, feel free to merge without making this step pass and tag @Logiraptor so the Mimir maintainers can gather feedback later.

Please see the contribution docs here for more info.

Expand to see the output

Output of https://github.com/grafana/mimir/actions/runs/2845807676

Warning: policy/v1beta1 PodDisruptionBudget is deprecated in v1.21+, unavailable in v1.25+; use policy/v1 PodDisruptionBudget
diff -r -u -N scratch/./helm/07-config/ingester-MimirConfig.yml scratch/./jsonnet/08-config/ingester-MimirConfig.yml
--- scratch/./helm/07-config/ingester-MimirConfig.yml	2022-08-12 09:32:54.486536415 +0000
+++ scratch/./jsonnet/08-config/ingester-MimirConfig.yml	2022-08-12 09:33:02.890577989 +0000
@@ -553,7 +553,7 @@
     max_fetched_chunks_per_query: 2000000 (default)
     max_fetched_series_per_query: 0 (default)
     max_global_exemplars_per_user: 0 (default)
-    max_global_series_per_metric: 20000 (default)
+    max_global_series_per_metric: 0
     max_global_series_per_user: 150000 (default)
     max_label_name_length: 1024 (default)
     max_label_names_per_series: 30 (default)
diff -r -u -N scratch/./helm/07-config/overrides-exporter-MimirConfig.yml scratch/./jsonnet/08-config/overrides-exporter-MimirConfig.yml
--- scratch/./helm/07-config/overrides-exporter-MimirConfig.yml	2022-08-12 09:32:54.490536435 +0000
+++ scratch/./jsonnet/08-config/overrides-exporter-MimirConfig.yml	2022-08-12 09:33:02.894578009 +0000
@@ -498,7 +498,7 @@
     max_fetched_chunks_per_query: 2000000 (default)
     max_fetched_series_per_query: 0 (default)
     max_global_exemplars_per_user: 0 (default)
-    max_global_series_per_metric: 20000 (default)
+    max_global_series_per_metric: 0
     max_global_series_per_user: 150000 (default)
     max_label_name_length: 1024 (default)
     max_label_names_per_series: 30 (default)

@colega colega merged commit 64c5aef into main Aug 12, 2022
@colega colega deleted the jvp/bring-changes-from-mimir-prometheus branch August 12, 2022 09:43
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.

3 participants