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

Objstore: Update objstore to latest version #5707

Merged
merged 1 commit into from
Sep 22, 2022

Conversation

phillebaba
Copy link
Contributor

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

Changes

Updates the objstore dependency to the latest version. Additionally the docs have been updated to reflect the requirement of go 1.18 or higher when developing Thanos. This is required because the new Azure SDK dependency introduced in objstore requires go 1.18 due to its use of generics.

Verification

Built a fork from 0.28.0 with the updated objstore version and deployed it, has been running without any issues.

@phillebaba phillebaba changed the title Update objstore to latest version Objstore: Update objstore to latest version Sep 20, 2022
CHANGELOG.md Outdated
@@ -24,6 +24,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re

- [#5255](https://github.com/thanos-io/thanos/pull/5296) Query: Use k-way merging for the proxying logic. The proxying sub-system now uses much less resources (~25-80% less CPU usage, ~30-50% less RAM usage according to our benchmarks). Reduces query duration by a few percent on queries with lots of series.
- [#5690](https://github.com/thanos-io/thanos/pull/5690) Compact: update `--debug.accept-malformed-index` flag to apply to downsampling. Previously the flag only applied to compaction, and fatal errors would still occur when downsampling was attempted.
- [#5707](https://github.com/thanos-io/thanos/pull/5707) Objstore: Update objstore to latest version.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there something specific which the latest version contains that we can mention here?

Copy link
Contributor Author

@phillebaba phillebaba Sep 20, 2022

Choose a reason for hiding this comment

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

Yes the main reason for updating is that Thanos will use the refactored Azure implementation. Will update.

@phillebaba phillebaba force-pushed the update/objstore branch 2 times, most recently from e5cf2a3 to c4dee03 Compare September 20, 2022 09:38
- name: Set up Go
uses: actions/setup-go@v2
with:
go-version: 1.18
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same change was made in objstore. Required for codeql to pass with libraries using generics.

fpetkovski
fpetkovski previously approved these changes Sep 20, 2022
Copy link
Contributor

@fpetkovski fpetkovski left a comment

Choose a reason for hiding this comment

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

Thanks 👍

bwplotka
bwplotka previously approved these changes Sep 20, 2022
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.

LGTM, thank You!

@bwplotka
Copy link
Member

@phillebaba
Copy link
Contributor Author

Looks like issues related to not skipping new objstore implementations. Will fix this.

@phillebaba
Copy link
Contributor Author

@bwplotka @fpetkovski a bit confused about the failing e2e tests. The only changes done to objstore is the introduction of OCI storage and my Azure refactor. None of these things are testsed in the e2e test, so dont really understand how it is failing now.

@phillebaba
Copy link
Contributor Author

Looks like the e2e test failure was due to flakiness in the test. I have rebased so should be ready to merge.

@phillebaba phillebaba requested review from bwplotka and fpetkovski and removed request for bwplotka and fpetkovski September 22, 2022 08:12
Copy link
Contributor

@fpetkovski fpetkovski left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the CI 👍

Copy link
Member

@squat squat left a comment

Choose a reason for hiding this comment

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

Thanks @phillebaba

@squat squat merged commit ff9ee9a into thanos-io:main Sep 22, 2022
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.

4 participants