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 Thanos #3661

Merged
merged 4 commits into from
Jan 8, 2021
Merged

Upgraded Thanos #3661

merged 4 commits into from
Jan 8, 2021

Conversation

pracucci
Copy link
Contributor

@pracucci pracucci commented Jan 7, 2021

What this PR does:
In this PR I'm upgrading Thanos to bring in a couple of changes we did (see CHANGELOG). I also took the opportunity to refactor the chunks storage swift client to use the blocks storage config (this brings in a breaking change to container-name default value, but swift is still marked experimental).

Do not merge: I reviewed all Thanos changes and I've found a race condition introduced by thanos-io/thanos#3557. I need to fix it merge merging this PR. (Fixed)

Most Thanos changes are small and safe things. This optimisation PR thanos-io/thanos#3531 is worth noting: diff is not easy to review but I did my best to compare before/after and changes LGTM.

Which issue(s) this PR fixes:
N/A

Checklist

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

@pracucci pracucci requested a review from pstibrany January 7, 2021 16:43
@pracucci pracucci marked this pull request as draft January 7, 2021 16:43
@pracucci
Copy link
Contributor Author

pracucci commented Jan 7, 2021

Do not merge: I reviewed all Thanos changes and I've found a race condition introduced by thanos-io/thanos#3557. I need to fix it merge merging this PR.

This PR introduces a lock to fix it:
thanos-io/thanos#3705

@pstibrany
Copy link
Contributor

Cortex changes LGTM. Amazing that you could spot a new race bug in that huge Thanos diff!

CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: Marco Pracucci <[email protected]>
pracucci and others added 2 commits January 8, 2021 14:54
Signed-off-by: Marco Pracucci <[email protected]>

Co-authored-by: Peter Štibraný <[email protected]>
@pracucci pracucci marked this pull request as ready for review January 8, 2021 13:56
@pracucci
Copy link
Contributor Author

pracucci commented Jan 8, 2021

@pstibrany Thanks for your review. The Thanos fix thanos-io/thanos#3705 has been merged and I've upgraded Thanos again, so should be ready for another review.

@pracucci pracucci merged commit 6c2dab1 into cortexproject:master Jan 8, 2021
@pracucci pracucci deleted the upgrade-thanos branch January 8, 2021 14:42
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.

2 participants