diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index 0679fbe2a4..bf56fbcbfc 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -3799,6 +3799,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 65d45390a7..e48b028a20 100644 --- a/app/vmselect/promql/transform.go +++ b/app/vmselect/promql/transform.go @@ -665,6 +665,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 { @@ -942,10 +943,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 { @@ -1013,6 +1014,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) { @@ -1024,6 +1026,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] @@ -1036,34 +1040,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 955db2d888..feadb8d266 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -44,6 +44,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): fix integration with Grafana via `-vmalert.proxyURL`, which has been broken in [v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820). See [this issue](https://github.com/VictoriaMetrics/helm-charts/issues/391). * BUGFIX: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): set default region to `us-east-1` if `AWS_REGION` environment variable isn't set. The issue was introduced in [vmbackup v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820). See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3224). * BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/vmbackupmanager.html): fix deletion of old backups at [Azure blob storage](https://azure.microsoft.com/en-us/products/storage/blobs/). +* 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). ## [v1.82.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.82.0)