-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Conversation
719a6b0
to
0092556
Compare
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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
e5cf2a3
to
c4dee03
Compare
- name: Set up Go | ||
uses: actions/setup-go@v2 | ||
with: | ||
go-version: 1.18 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank You!
Looks like we have some important error to fix: https://app.circleci.com/pipelines/github/thanos-io/thanos/10720/workflows/81ea74ef-96bc-4d53-933b-4ad9c86d9080/jobs/20317 |
Looks like issues related to not skipping new objstore implementations. Will fix this. |
f28e1be
c4dee03
to
f28e1be
Compare
@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. |
f28e1be
to
e194b67
Compare
Signed-off-by: Philip Laine <philip.laine@gmail.com>
e194b67
to
c36905b
Compare
Looks like the e2e test failure was due to flakiness in the test. I have rebased so should be ready to merge. |
There was a problem hiding this 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 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @phillebaba
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.