feat(nhcb): add support for NHCB over RW1/RW2 #11143
Conversation
|
💻 Deploy preview deleted. |
e51eeed to
ebebdf0
Compare
Add support Native Histograms with Custom Bucket over Remote Write. Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
ebebdf0 to
925eca9
Compare
| @@ -52,6 +52,8 @@ var softErrProcessor = mimir_storage.NewSoftAppendErrorProcessor( | |||
| func([]mimirpb.LabelAdapter) {}, func([]mimirpb.LabelAdapter) {}, func(error, int64, []mimirpb.LabelAdapter) {}, | |||
| func(error, int64, []mimirpb.LabelAdapter) {}, func(error, int64, []mimirpb.LabelAdapter) {}, | |||
| func(error, int64, []mimirpb.LabelAdapter) {}, func(error, int64, []mimirpb.LabelAdapter) {}, | |||
| func(error, int64, []mimirpb.LabelAdapter) {}, func(error, int64, []mimirpb.LabelAdapter) {}, | |||
There was a problem hiding this comment.
Question to reviewers: Is this having any performance impact? We're passing more pointers on the heap I assume?
Co-authored-by: Taylor C <41653732+tacole02@users.noreply.github.com>
Co-authored-by: Taylor C <41653732+tacole02@users.noreply.github.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
There was a problem hiding this comment.
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
integration/distributor_test.go:546
- [nitpick] The introduction of a negative value in PositiveDeltas may be non-intuitive given that bucket counts are typically non-negative; please verify that this test data intentionally represents the desired custom bucket behavior.
PositiveDeltas: []int64{150, -100},
| @@ -287,6 +288,12 @@ func validateSampleHistogram(m *sampleValidationMetrics, now model.Time, cfg sam | |||
| } else { | |||
| bucketCount = len(s.GetNegativeDeltas()) + len(s.GetPositiveDeltas()) | |||
| } | |||
| if s.Schema == mimirpb.NativeHistogramsWithCustomBucketsSchema { | |||
There was a problem hiding this comment.
The error returned for custom buckets uses the generic 'notReducibleNativeHistogramMsgFormat'. Consider defining a dedicated error message for NHCB to clearly indicate that custom bucket histograms cannot be scaled down.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
I agree, I think the error message could be more clear for this scenario, and specifically mention that the error is thrown because custom bucket native histograms cannot undergo a reduction in resolution.
There was a problem hiding this comment.
Looks really good, a couple of really small nits. Thanks so much for fixing all of these notes, @krajorama !
Co-authored-by: Taylor C <41653732+tacole02@users.noreply.github.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
What this PR does
Add support Native Histograms with Custom Bucket over Remote Write.
Which issue(s) this PR fixes or relates to
Fixes #7817
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.