-
Notifications
You must be signed in to change notification settings - Fork 476
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
Go: Upgrade to 1.20.3 (#4773) #4791
Conversation
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> (cherry picked from commit 5d9f104)
@@ -87,7 +98,6 @@ | |||
* [ENHANCEMENT] Ingester: improve performance when Active Series Tracker is in use. #4717 | |||
* [ENHANCEMENT] Store-gateway: optionally select `-blocks-storage.bucket-store.series-selection-strategy`, which can limit the impact of large posting lists (when many series share the same label name and value). #4667 #4695 #4698 | |||
* [ENHANCEMENT] Querier: Cache the converted float histogram from chunk iterator, hence there is no need to lookup chunk every time to get the converted float histogram. #4684 | |||
* [ENHANCEMENT] Improved memory limit on the in-memory cache used for regular expression matchers. #4751 |
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.
Question to reviewer: I accidentally pulled this down to the ## 2.8.0-rc.0 list. Should backported changelog before rc tag to be put under here or leave it in main/unreleased section above?
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.
We should remove main / unreleased
section from CHANGELOG in this PR.
PRs that were cherry-picked from main
to release-2.8
should have entries in the 2.8 section, but that should be fixed in a separate PR, not this one.
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.
This specific entry:
* [ENHANCEMENT] Improved memory limit on the in-memory cache used for regular expression matchers. #4751
should be kept in 2.8 section, without modifications.
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.
- [ENHANCEMENT] Improved memory limit on the in-memory cache used for regular expression matchers. Improve memory limit on the in-memory cache used for regular expression matchers #4751
That entry is also coming from backporting. Hence I will remove that entry has it to be added to the changelog together with the rest of backport changelogs in the separate PR.
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.
I think it would make most sense to add a 2.8.0-rc.1
section on top and have this in it:
## 2.8.0-rc.1
### Grafana Mimir
* [ENHANCEMENT] Improved memory limit on the in-memory cache used for regular expression matchers. #4751
* [ENHANCEMENT] Go: update to 1.20.3. #4773
Upon release we merge to a 2.8.0 section and reconcile with main on merge to main. @pstibrany wdyt?
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.
ok, but then removal of line 90 isn't backporting.
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.
We don't rename rc.0, but we can add rc.1.
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.
also you're missing the new changelog entry from the backport
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.
I agree with @krajorama suggestion (about adding rc.1 section).
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.
ok, but then removal of line 90 isn't backporting.
I added it to the first backport I created. I intended to add all backport changelog in the different PR.
also you're missing the new changelog entry from the backport
Yes I intended to add the changelog after rc.0 in a new PR.
All in all, I will just add new rc1. section and get this merged. I will document this process too.
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
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
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
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.
Approved with nit
Signed-off-by: Arve Knudsen arve.knudsen@gmail.com
(cherry picked from commit 5d9f104)
What this PR does
Manual backport of #4773
Which issue(s) this PR fixes or relates to
Fixes #
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]