-
Notifications
You must be signed in to change notification settings - Fork 565
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
[helm] Si bytes calculation always floor fix #10044
[helm] Si bytes calculation always floor fix #10044
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.
thanks for contributing the fix! The two changes in this PR looks orthogonal to each other: fixing siToBytes
and changing the store-gateway GOMEMLIMIT. Can you open a PR for each (perhaps repurpose this PR and open a new one)?
I'm not sure about the 0.9 multiplication of the memory request for GOMEMLIMIT. At the every least we need some comparisons running this side-by-side with the old version for example to prove the need for it.
@@ -41,6 +41,7 @@ Entries should include a reference to the Pull Request that introduced the chang | |||
* [BUGFIX] Fix how `fullnameOverride` is reflected in generated manifests. #9564 | |||
* [BUGFIX] Fix `extraObjects` linting with helm lint by padding with an extra new line. #9863 | |||
* [BUGFIX] Alertmanager: Set -server.http-idle-timeout to avoid EOF errors in ruler, also for zone aware Alertmanager #9851 | |||
* [BUGFIX] Fix calculation of `mimir.siToBytes` and use floating point arithmetics, also multiply the memory request by 0.9 for store-gateway `GOMEMLIMIT` #10044 |
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.
* [BUGFIX] Fix calculation of `mimir.siToBytes` and use floating point arithmetics, also multiply the memory request by 0.9 for store-gateway `GOMEMLIMIT` #10044 | |
* [BUGFIX] Fix calculation of `mimir.siToBytes` and use floating point arithmetics. Also, multiply the memory request by 0.9 for store-gateway `GOMEMLIMIT` #10044 |
@edwintye Thank you for your contribution! Please update the PR following the feedback given in the comments. |
Thanks Dimitar for the suggestion. I didn't realize that the jsonnet variant didn't have a multiplication factor for |
The CHANGELOG has just been cut to prepare for the next release. Please rebase |
5416ec5
to
76670b8
Compare
Thanks for the reminder. Rebased and moved. |
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!
What this PR does
Changes the computation of
mimir.siToBytes
to use floating point arithmetics.The tests in ci -
compare-helm-with-jsonnet/compare-manifests
to be exact - is failing but I can't get it to work and looking at the other PRs it looks like they have been failing for a while.Which issue(s) this PR fixes or relates to
Fixes #9780
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]
.about-versioning.md
updated with experimental features.