diff --git a/app/vmselect/promql/binary_op.go b/app/vmselect/promql/binary_op.go index d020825569..3bf8ff5648 100644 --- a/app/vmselect/promql/binary_op.go +++ b/app/vmselect/promql/binary_op.go @@ -404,9 +404,15 @@ func binaryOpDefault(bfa *binaryOpFuncArg) ([]*timeseries, error) { func binaryOpOr(bfa *binaryOpFuncArg) ([]*timeseries, error) { mLeft, mRight := createTimeseriesMapByTagSet(bfa.be, bfa.left, bfa.right) var rvs []*timeseries + for _, tss := range mLeft { rvs = append(rvs, tss...) } + // Sort left-hand-side series by metric name as Prometheus does. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5393 + sortSeriesByMetricName(rvs) + rvsLen := len(rvs) + for k, tssRight := range mRight { tssLeft := mLeft[k] if tssLeft == nil { @@ -415,6 +421,10 @@ func binaryOpOr(bfa *binaryOpFuncArg) ([]*timeseries, error) { } fillLeftNaNsWithRightValues(tssLeft, tssRight) } + // Sort the added right-hand-side series by metric name as Prometheus does. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5393 + sortSeriesByMetricName(rvs[rvsLen:]) + return rvs, nil } diff --git a/app/vmselect/promql/exec.go b/app/vmselect/promql/exec.go index 2317f800ad..6798f85460 100644 --- a/app/vmselect/promql/exec.go +++ b/app/vmselect/promql/exec.go @@ -111,6 +111,7 @@ func maySortResults(e metricsql.Expr) bool { case "sort", "sort_desc", "sort_by_label", "sort_by_label_desc", "sort_by_label_numeric", "sort_by_label_numeric_desc": + // Results already sorted return false } case *metricsql.AggrFuncExpr: @@ -118,6 +119,7 @@ func maySortResults(e metricsql.Expr) bool { case "topk", "bottomk", "outliersk", "topk_max", "topk_min", "topk_avg", "topk_median", "topk_last", "bottomk_max", "bottomk_min", "bottomk_avg", "bottomk_median", "bottomk_last": + // Results already sorted return false } case *metricsql.BinaryOpExpr: @@ -132,6 +134,10 @@ func maySortResults(e metricsql.Expr) bool { func timeseriesToResult(tss []*timeseries, maySort bool) ([]netstorage.Result, error) { tss = removeEmptySeries(tss) + if maySort { + sortSeriesByMetricName(tss) + } + result := make([]netstorage.Result, len(tss)) m := make(map[string]struct{}, len(tss)) bb := bbPool.Get() @@ -152,15 +158,15 @@ func timeseriesToResult(tss []*timeseries, maySort bool) ([]netstorage.Result, e } bbPool.Put(bb) - if maySort { - sort.Slice(result, func(i, j int) bool { - return metricNameLess(&result[i].MetricName, &result[j].MetricName) - }) - } - return result, nil } +func sortSeriesByMetricName(tss []*timeseries) { + sort.Slice(tss, func(i, j int) bool { + return metricNameLess(&tss[i].MetricName, &tss[j].MetricName) + }) +} + func metricNameLess(a, b *storage.MetricName) bool { if string(a.MetricGroup) != string(b.MetricGroup) { return string(a.MetricGroup) < string(b.MetricGroup) diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index b5a674f7fe..3d5cc56d6e 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -3059,6 +3059,51 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r} f(q, resultExpected) }) + t.Run(`series or series`, func(t *testing.T) { + t.Parallel() + q := `( + label_set(time(), "x", "foo"), + label_set(time()+1, "x", "bar"), + ) or ( + label_set(time()+2, "x", "foo"), + label_set(time()+3, "x", "baz"), + )` + r1 := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{1001, 1201, 1401, 1601, 1801, 2001}, + Timestamps: timestampsExpected, + } + r1.MetricName.Tags = []storage.Tag{ + { + Key: []byte("x"), + Value: []byte("bar"), + }, + } + r2 := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{1000, 1200, 1400, 1600, 1800, 2000}, + Timestamps: timestampsExpected, + } + r2.MetricName.Tags = []storage.Tag{ + { + Key: []byte("x"), + Value: []byte("foo"), + }, + } + r3 := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{1003, 1203, 1403, 1603, 1803, 2003}, + Timestamps: timestampsExpected, + } + r3.MetricName.Tags = []storage.Tag{ + { + Key: []byte("x"), + Value: []byte("baz"), + }, + } + resultExpected := []netstorage.Result{r1, r2, r3} + f(q, resultExpected) + }) t.Run(`scalar or scalar`, func(t *testing.T) { t.Parallel() q := `time() > 1400 or 123` diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 9eeca32e0c..a47eef978c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -50,6 +50,7 @@ The sandbox cluster installation is running under the constant load generated by * BUGFIX: `vmstorage`: properly check for `storage/prefetchedMetricIDs` cache expiration deadline. Before, this cache was limited only by size. * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): check `-external.url` schema when starting vmalert, must be `http` or `https`. Before, alertmanager could reject alert notifications if `-external.url` contained no or wrong schema. * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): automatically add `exported_` prefix for original evaluation result label if it's conflicted with external or reserved one, previously it was overridden. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5161). +* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): consistently sort results for `q1 or q2` query, so they do not change colors with each refresh in Grafana. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5393). * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly handle queries, which wrap [rollup functions](https://docs.victoriametrics.com/MetricsQL.html#rollup-functions) with multiple arguments without explicitly specified lookbehind window in square brackets into [aggregate functions](https://docs.victoriametrics.com/MetricsQL.html#aggregate-functions). For example, `sum(quantile_over_time(0.5, process_resident_memory_bytes))` was resulting to `expecting at least 2 args to ...; got 1 args` error. Thanks to @atykhyy for [the pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5414). * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl.html): retry on import errors in `vm-native` mode. Before, retries happened only on writes into a network connection between source and destination. But errors returned by server after all the data was transmitted were logged, but not retried. * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly assume role with [AWS IRSA authorization](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html). Previously role chaining was not supported. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3822) for details.