-
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
Use prometheus v2.9.2, common v0.4.0 & tsdb v0.8.0 #1133
Conversation
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.
It looks good to me (:
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.
Minor nit, other than that 💯
This needs a rebase. I have no strong opinion on the tsdb_errors vs tsdberrors thing, while tsdb may be doing something wrong here I generally prefer to stay as close as possible to goimports as named imports usually just end up confusing people. In that sense i think i would prefer not renaming and sticking to the upstream and try to continue that consistently. (And fix it in tsdb) In any case once rebased this is good to go. |
81886b5
to
dfa6499
Compare
@povilasv is it okay with you that we leave it like |
@GiedriusS yes @brancz makes a good point |
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, but let's fix this package prefix. Also this is not the most important part - the important part is to grab this and deploy on clusters ): It's always critical change.
Can we walk through the changelog and grab what changes and map to what we use? I was always doing it for last Prom/TSDB upgrades
https://github.com/improbable-eng/thanos/blob/master/CHANGELOG.md
For example as we had in previous one:
Changes that affects Thanos:
query:
[ENHANCEMENT] In histogram_quantile merge buckets with equivalent le values. #5158.
[ENHANCEMENT] Show list of offending labels in the error message in many-to-many scenarios. #5189
[BUGFIX] Fix panic when aggregator param is not a literal. #5290
ruler:
[ENHANCEMENT] Reduce time that Alertmanagers are in flux when reloaded. #5126
[BUGFIX] prometheus_rule_group_last_evaluation_timestamp_seconds is now a unix timestamp. #5186
[BUGFIX] prometheus_rule_group_last_duration_seconds now reports seconds instead of nanoseconds. Fixes our issue #1027
[BUGFIX] Fix sorting of rule groups. #5260
store: [ENHANCEMENT] Fast path for EmptyPostings cases in Merge, Intersect and Without.
tooling: [FEATURE] New dump command to tsdb tool to dump all samples.
compactor:
[ENHANCEMENT] When closing the db any running compaction will be cancelled so it doesn't block.
[CHANGE] Renamed flag --sync-delay to --consistency-delay #1053
Thanks for the work @sylr ! Also please rebase, this failing preview might be worrying (: |
All checks are green now. |
It's conflicting again, will you merge it if I rebase ? |
I think @bwplotka was primarily waiting on something doing:
If we can validate this, I'm happy with moving forward. |
I believe this comment by @bwplotka is still not addressed. It would be nice to have these things in the CHANGELOG.md 😄 Could you do that, @sylr? Really awesome job that you've done here 👍 |
Yea, the changelog is quite manual job ): |
Seems like a rebase is not difficult as well @sylr |
- prometheus/prometheus@v2.9.2+incompatible - prometheus/common@v0.4.0 - prometheus/tsdb@v0.8.0 Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
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 from my side.
However we need to still have a changelog for this @sylr ): This adds a lot, if you can grab what Prometheus and TSDB Changelog covers from our previous versions to the thing we update in this PR offline we can work out what is interesting/impacting Thanos users. What do you think? |
TSDB changelog:
Prometheus changelog:
|
Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
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.
Awesome thanks @sylr for the offline work we did with parsing changelog. Lots of manual work. LGTM
No description provided.