Reduce histogram resolution instead of rejecting #6535
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Use "reduce" instead of "downscale" everywhere. Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
colega
reviewed
Nov 14, 2023
pkg/mimirpb/custom.go
Outdated
| } | ||
|
|
||
| func (h *Histogram) reduceFloatResolution() (int, error) { | ||
| if h.Schema == -4 { |
There was a problem hiding this comment.
Can we have a meaningful constant for this somewhere?
There was a problem hiding this comment.
Added definition in the same file. I looked again, but it's never exported from Prometheus, so I have to make a PR in Prometheus first to solve it...
colega
approved these changes
Nov 14, 2023
There was a problem hiding this comment.
I'm not an expert in all the code that comes from Prometheus, but that already had its review. The new code putting it here looks good to me. Consider my comment about adding a constant for that -4, it seems really strange to someone who never saw anything about native histograms.
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
krajorama
commented
Nov 14, 2023
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does
Introduce new configuration parameter to be able to disable the new function.
Reduce native histogram resolution instead of rejecting it if it is above bucket limit.
Which issue(s) this PR fixes or relates to
Fixes #5367
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]