From b3ee33eb8e2f529eb10340a0c4f04919a890d0c7 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 16c6cda2b..bcebe46dd 100644 --- a/app/vmselect/promql/transform.go +++ b/app/vmselect/promql/transform.go @@ -548,14 +548,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{ @@ -575,6 +586,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 5c5d7e414..6b9b8c476 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