From 680a661ec04357e9f13fd1f96533a6c1bcc198af Mon Sep 17 00:00:00 2001 From: Ze'ev Klapow Date: Mon, 27 Mar 2023 20:54:19 -0400 Subject: [PATCH] fix le buckets when adjacent vmrange is empty (#4021) There is a bug here where if you have a single bucket like: foo{vmrange="4.084e+02...4.642e+02"} 2 123 The expected output is three le encoded buckets like: foo{le="4.084e+02"} 0 123 foo{le="4.642e+02"} 2 123 foo{le="+Inf"} 2 123 This correctly encodes the start and end of the vmrange. If however, the input contains the previous bucket, and that bucket is empty then you only get the end le and +Inf out currently, i.e: foo{vmrange="7.743e+05...8.799e+05"} 5 123 foo{vmrange="6.813e+05...7.743e+05"} 0 123 results in: foo{le="8.799e+05"} 5 123 foo{le="+Inf"} 5 123 This causes issues when you go to compute a quantile because this means that the assumed lower bound of the buckets is 0 and this we interpolate between 0->end rather than the vmrange start->end as expected. --- app/vmselect/promql/transform.go | 19 ++++++++++++++++++- app/vmselect/promql/transform_test.go | 9 +++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/vmselect/promql/transform.go b/app/vmselect/promql/transform.go index 1197d59aa2..279d2e6c3b 100644 --- a/app/vmselect/promql/transform.go +++ b/app/vmselect/promql/transform.go @@ -552,14 +552,25 @@ func vmrangeBucketsToLE(tss []*timeseries) []*timeseries { sort.Slice(xss, func(i, j int) bool { return xss[i].end < xss[j].end }) xssNew := make([]x, 0, len(xss)+2) var xsPrev x + hasNonEmpty := false uniqTs := make(map[string]*timeseries, len(xss)) for _, xs := range xss { ts := xs.ts if isZeroTS(ts) { - // Skip time series with zeros. They are substituted by xssNew below. xsPrev = xs + + if uniqTs[xs.endStr] == nil { + uniqTs[xs.endStr] = xs.ts + xssNew = append(xssNew, x{ + endStr: xs.endStr, + end: xs.end, + ts: copyTS(ts, xs.endStr), + }) + } continue } + + hasNonEmpty = true if xs.start != xsPrev.end && uniqTs[xs.startStr] == nil { uniqTs[xs.startStr] = xs.ts xssNew = append(xssNew, x{ @@ -579,6 +590,12 @@ func vmrangeBucketsToLE(tss []*timeseries) []*timeseries { } xsPrev = xs } + + if !hasNonEmpty { + xssNew = []x{} + continue + } + if !math.IsInf(xsPrev.end, 1) && !isZeroTS(xsPrev.ts) { xssNew = append(xssNew, x{ endStr: "+Inf", diff --git a/app/vmselect/promql/transform_test.go b/app/vmselect/promql/transform_test.go index 3fe796b3a2..0261353ce3 100644 --- a/app/vmselect/promql/transform_test.go +++ b/app/vmselect/promql/transform_test.go @@ -78,6 +78,15 @@ foo{le="+Inf"} 1.23 456`, foo{le="+Inf"} 5.3 0`, ) + // Adjacent empty vmrange bucket + f( + `foo{vmrange="7.743e+05...8.799e+05"} 5 123 +foo{vmrange="6.813e+05...7.743e+05"} 0 123`, + `foo{le="7.743e+05"} 0 123 +foo{le="8.799e+05"} 5 123 +foo{le="+Inf"} 5 123`, + ) + // Multiple non-empty vmrange buckets f( `foo{vmrange="4.084e+02...4.642e+02"} 2 123