Skip to content

Update Prometheus vendoring #3934

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

Merged
merged 5 commits into from
Mar 16, 2021

Conversation

gouthamve
Copy link
Contributor

Things needed to be updated to accomodate this change in interface:
prometheus/prometheus#8489

Signed-off-by: Goutham Veeramachaneni gouthamve@gmail.com

Things needed to be updated to accomodate this change in interface:
prometheus/prometheus#8489

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@pstibrany
Copy link
Contributor

Would you mind updating Prometheus in this PR to latest master to get fix for prometheus/prometheus#8558?

@gouthamve
Copy link
Contributor Author

@pstibrany Done

To pull in prometheus/prometheus#8558

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@gouthamve gouthamve force-pushed the update-prom-vendor branch from 75fbd0e to c5cdbe7 Compare March 15, 2021 10:42
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Ingester changes LGTM, but I think we're missing possible case when Head has gc-ed series, and removed reference from memory, before ingester's refcache did. I'm not quite sure if this scenario is real though.

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@gouthamve gouthamve force-pushed the update-prom-vendor branch from 4b3ff23 to bb3041b Compare March 15, 2021 15:49
Copy link
Contributor

@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.

Thanks @gouthamve for working on this! LGTM modulo a comment about the ingester push. Also a couple of things:

  • Could you upgrade Prometheus again? I got a couple of PRs merged which I would love to see backported to Cortex.
  • PromQL engine added EnableNegativeOffset. Should we add a config option for that, similarly to what we did for EnableAtModifier?

Was removed in error

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@gouthamve
Copy link
Contributor Author

Updated Prometheus. Will add option for EnableNegativeOffset in next PR.

OOh, Prometheus is exciting again :D

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants