From 7ac0559d4f8331c9929d346149fd1465bbf1986f Mon Sep 17 00:00:00 2001
From: Roman Khavronenko <hagen1778@gmail.com>
Date: Fri, 24 Sep 2021 01:03:12 +0300
Subject: [PATCH] app/vmselect: make sorting for query result similar to
 Prometheus (#1647)

* app/vmselect: make sorting for query result similar to Prometheus

Updated sorting allows to get the order of series in result similar or equal
to what Prometheus returns.
The change is needed for compatibility reasons.

* Update app/vmselect/promql/exec_test.go

Co-authored-by: Aliaksandr Valialkin <valyala@victoriametrics.com>
---
 app/vmselect/promql/exec.go       |  2 +-
 app/vmselect/promql/exec_test.go  | 55 ++++++++++++++++++++++++++++++-
 app/vmselect/promql/timeseries.go | 12 +++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/app/vmselect/promql/exec.go b/app/vmselect/promql/exec.go
index e22b4291d9..175389c91f 100644
--- a/app/vmselect/promql/exec.go
+++ b/app/vmselect/promql/exec.go
@@ -95,7 +95,7 @@ func timeseriesToResult(tss []*timeseries, maySort bool) ([]netstorage.Result, e
 	m := make(map[string]struct{}, len(tss))
 	bb := bbPool.Get()
 	for i, ts := range tss {
-		bb.B = marshalMetricNameSorted(bb.B[:0], &ts.MetricName)
+		bb.B = metricNameToBytes(bb.B[:0], &ts.MetricName)
 		if _, ok := m[string(bb.B)]; ok {
 			return nil, fmt.Errorf(`duplicate output timeseries: %s`, stringMetricName(&ts.MetricName))
 		}
diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go
index 6d33f4b49d..e511b56d79 100644
--- a/app/vmselect/promql/exec_test.go
+++ b/app/vmselect/promql/exec_test.go
@@ -1,6 +1,7 @@
 package promql
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -6964,7 +6965,8 @@ func TestExecSuccess(t *testing.T) {
 				Value: []byte("10"),
 			},
 		}
-		resultExpected := []netstorage.Result{r1, r2, r3, r4}
+		// expected sorted output for strings 1, 10, 2, 3
+		resultExpected := []netstorage.Result{r1, r4, r2, r3}
 		f(q, resultExpected)
 	})
 	t.Run(`count_values without (baz)`, func(t *testing.T) {
@@ -7018,6 +7020,44 @@ func TestExecSuccess(t *testing.T) {
 		resultExpected := []netstorage.Result{r1, r2, r3}
 		f(q, resultExpected)
 	})
+	t.Run(`result sorting`, func(t *testing.T) {
+		t.Parallel()
+		q := `label_set(1, "instance", "localhost:1001", "type", "free")
+			or label_set(1, "instance", "localhost:1001", "type", "buffers")
+			or label_set(1, "instance", "localhost:1000", "type", "buffers")
+			or label_set(1, "instance", "localhost:1000", "type", "free")
+`
+		r1 := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{1, 1, 1, 1, 1, 1},
+			Timestamps: timestampsExpected,
+		}
+		testAddLabels(t, &r1.MetricName,
+			"instance", "localhost:1000", "type", "buffers")
+		r2 := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{1, 1, 1, 1, 1, 1},
+			Timestamps: timestampsExpected,
+		}
+		testAddLabels(t, &r2.MetricName,
+			"instance", "localhost:1000", "type", "free")
+		r3 := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{1, 1, 1, 1, 1, 1},
+			Timestamps: timestampsExpected,
+		}
+		testAddLabels(t, &r3.MetricName,
+			"instance", "localhost:1001", "type", "buffers")
+		r4 := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{1, 1, 1, 1, 1, 1},
+			Timestamps: timestampsExpected,
+		}
+		testAddLabels(t, &r4.MetricName,
+			"instance", "localhost:1001", "type", "free")
+		resultExpected := []netstorage.Result{r1, r2, r3, r4}
+		f(q, resultExpected)
+	})
 }
 
 func TestExecError(t *testing.T) {
@@ -7305,3 +7345,16 @@ func testMetricNamesEqual(t *testing.T, mn, mnExpected *storage.MetricName, pos
 		}
 	}
 }
+
+func testAddLabels(t *testing.T, mn *storage.MetricName, labels ...string) {
+	t.Helper()
+	if len(labels)%2 > 0 {
+		t.Fatalf("uneven number of labels passed: %v", labels)
+	}
+	for i := 0; i < len(labels); i += 2 {
+		mn.Tags = append(mn.Tags, storage.Tag{
+			Key:   []byte(labels[i]),
+			Value: []byte(labels[i+1]),
+		})
+	}
+}
diff --git a/app/vmselect/promql/timeseries.go b/app/vmselect/promql/timeseries.go
index 36233fbffe..167686d1ed 100644
--- a/app/vmselect/promql/timeseries.go
+++ b/app/vmselect/promql/timeseries.go
@@ -318,6 +318,18 @@ func marshalMetricTagsFast(dst []byte, tags []storage.Tag) []byte {
 	return dst
 }
 
+func metricNameToBytes(dst []byte, mn *storage.MetricName) []byte {
+	dst = marshalBytesFast(dst, mn.MetricGroup)
+	sortMetricTags(mn.Tags)
+	for i := range mn.Tags {
+		tag := &mn.Tags[i]
+		dst = append(dst, tag.Key...)
+		dst = append(dst, tag.Value...)
+		dst = append(dst, ","...)
+	}
+	return dst
+}
+
 func marshalMetricNameSorted(dst []byte, mn *storage.MetricName) []byte {
 	// Do not marshal AccountID and ProjectID, since they are unused.
 	dst = marshalBytesFast(dst, mn.MetricGroup)