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

Upgraded Prometheus and Cortex dependencies #3382

Merged
merged 9 commits into from
Oct 30, 2020

Conversation

pracucci
Copy link
Contributor

@pracucci pracucci commented Oct 30, 2020

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

We fixed a TSDB WAL corruption which we urgently need to backport to Cortex and deploy (we already got 2 incidents this week because of this).

Prometheus has recently done a refactoring of MultiError, which introduced a circular breaking change in Cortex and Thanos. To solve this, I've interned MultiError into Cortex first (cortexproject/cortex#3425) and in this PR I'm upgrading Cortex and Prometheus. Thanos does an use the MultiError data type assertion in compactor and receiver, but the data type is not exposed anymore, so to unblock this situation I've interned the previous version of MultiError in Thanos, in order to gain some time to discuss how to better address it, while being able to rollout the WAL corruption fix to Cortex.

Thoughts?

Verification

Existing tests.

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks! That looks like ok approach to me 👍

Some CI failures though.

Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
@bwplotka
Copy link
Member

bwplotka commented Oct 30, 2020

Let me think about priv type problem 🤔

Signed-off-by: Marco Pracucci <[email protected]>
@bwplotka
Copy link
Member

Double checked and this looks fair to me.

I would use Prometheus tsdb multi error though everywhere, except where we assert error type. We should probably use local type for this to explicitness. We can fix later this readability suggestion 👍

LGTM

Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
@pracucci pracucci merged commit 47f9a22 into thanos-io:master Oct 30, 2020
@pracucci pracucci deleted the upgrade-prometheus branch October 30, 2020 10:13
Oghenebrume50 pushed a commit to Oghenebrume50/thanos that referenced this pull request Dec 7, 2020
* Upgraded Prometheus and Cortex dependencies

Signed-off-by: Marco Pracucci <[email protected]>

* Updated mod replace

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed linter

Signed-off-by: Marco Pracucci <[email protected]>

* Use errutil.MultiError everywhere

Signed-off-by: Marco Pracucci <[email protected]>

* Small fixes

Signed-off-by: Marco Pracucci <[email protected]>

* Do not use Prometheus MultiError in cmd too

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed linter

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed dot in a comment

Signed-off-by: Marco Pracucci <[email protected]>

* Added copyight

Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Oghenebrume50 <[email protected]>
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.

2 participants