diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index 8d0896b6eb..c24bb66008 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -3788,6 +3788,27 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r} f(q, resultExpected) }) + t.Run(`histogram_quantile(duplicate-le)`, func(t *testing.T) { + // See https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225 + t.Parallel() + q := `round(sort(histogram_quantile(0.6, + label_set(90, "foo", "bar", "le", "5") + or label_set(100, "foo", "bar", "le", "5.0") + or label_set(200, "foo", "bar", "le", "6.0") + or label_set(300, "foo", "bar", "le", "+Inf") + )), 0.1)` + r1 := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{4.7, 4.7, 4.7, 4.7, 4.7, 4.7}, + Timestamps: timestampsExpected, + } + r1.MetricName.Tags = []storage.Tag{{ + Key: []byte("foo"), + Value: []byte("bar"), + }} + resultExpected := []netstorage.Result{r1} + f(q, resultExpected) + }) t.Run(`histogram_quantile(valid)`, func(t *testing.T) { t.Parallel() q := `sort(histogram_quantile(0.6, diff --git a/app/vmselect/promql/transform.go b/app/vmselect/promql/transform.go index e5181519b7..ea87225aad 100644 --- a/app/vmselect/promql/transform.go +++ b/app/vmselect/promql/transform.go @@ -659,6 +659,7 @@ func transformHistogramShare(tfa *transformFuncArg) ([]*timeseries, error) { sort.Slice(xss, func(i, j int) bool { return xss[i].le < xss[j].le }) + xss = mergeSameLE(xss) dst := xss[0].ts var tsLower, tsUpper *timeseries if len(boundsLabel) > 0 { @@ -936,10 +937,10 @@ func transformHistogramQuantile(tfa *transformFuncArg) ([]*timeseries, error) { } rvs := make([]*timeseries, 0, len(m)) for _, xss := range m { - xss = mergeSameLE(xss) sort.Slice(xss, func(i, j int) bool { return xss[i].le < xss[j].le }) + xss = mergeSameLE(xss) dst := xss[0].ts var tsLower, tsUpper *timeseries if len(boundsLabel) > 0 { @@ -1007,6 +1008,7 @@ func fixBrokenBuckets(i int, xss []leTimeseries) { if len(xss) < 2 { return } + // Fill NaN in upper buckets with the first non-NaN value found in lower buckets. for j := len(xss) - 1; j >= 0; j-- { v := xss[j].ts.Values[i] if !math.IsNaN(v) { @@ -1018,6 +1020,8 @@ func fixBrokenBuckets(i int, xss []leTimeseries) { break } } + // Substitute lower bucket values with upper values if the lower values are NaN + // or are bigger than the upper bucket values. vNext := xss[len(xss)-1].ts.Values[i] for j := len(xss) - 2; j >= 0; j-- { v := xss[j].ts.Values[i] @@ -1030,34 +1034,23 @@ func fixBrokenBuckets(i int, xss []leTimeseries) { } func mergeSameLE(xss []leTimeseries) []leTimeseries { - hit := false - m := make(map[float64]leTimeseries) - for _, xs := range xss { - i, ok := m[xs.le] - if ok { - ts := ×eries{} - ts.CopyFromShallowTimestamps(i.ts) - for k := range ts.Values { - ts.Values[k] += xs.ts.Values[k] - } - m[xs.le] = leTimeseries{ - le: xs.le, - ts: ts, - } - hit = true + // Merge buckets with identical le values. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225 + xsDst := xss[0] + dst := xss[:1] + for j := 1; j < len(xss); j++ { + xs := xss[j] + if xs.le != xsDst.le { + dst = append(dst, xs) + xsDst = xs continue } - m[xs.le] = xs + dstValues := xsDst.ts.Values + for k, v := range xs.ts.Values { + dstValues[k] += v + } } - if (!hit) { - return xss - } - - r := make([]leTimeseries, 0, len(m)) - for _, v := range m { - r = append(r, v) - } - return r + return dst } func transformHour(t time.Time) int { diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 2be56bec06..d78eb90627 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -19,6 +19,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * SECURITY: update Go builder to v1.19.3. This fixes [CVE-2022 security issue](https://github.com/golang/go/issues/56328). See [the changelog](https://github.com/golang/go/issues?q=milestone%3AGo1.19.3+label%3ACherryPickApproved). +* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly merge buckets with identical `le` values, but with different string representation of these values when calculating [histogram_quantile](https://docs.victoriametrics.com/MetricsQL.html#histogram_quantile) and [histogram_share](https://docs.victoriametrics.com/MetricsQL.html#histogram_share). For example, `http_request_duration_seconds_bucket{le="5"}` and `http_requests_duration_seconds_bucket{le="5.0"}`. Such buckets may be returned from distinct targets. Thanks to @647-coder for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225). * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): change severity level for log messages about failed attempts for sending data to remote storage from `error` to `warn`. The message for about all failed send attempts remains at `error` severity level. * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): properly escape string passed to `quotesEscape` [template function](https://docs.victoriametrics.com/vmalert.html#template-functions), so it can be safely embedded into JSON string. This makes obsolete the `crlfEscape` function. See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3139) and [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/890) issue. * BUGFIX: `vmselect`: expose missing metric `vm_cache_size_max_bytes{type="promql/rollupResult"}` . This metric is used for monitoring rollup cache usage with the query `vm_cache_size_bytes{type="promql/rollupResult"} / vm_cache_size_max_bytes{type="promql/rollupResult"}` in the same way as this is done for other cache types.