app/vmselect/promql: fix mode_over_time calculations

Previously `mode_over_time` could return garbage due to improper shuffling of input data points.

CHANGELOG.md

@@ -52,6 +52,7 @@
   See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/674
 
 * BUGFIX: vmalert: accept days, weeks and years in `for: ` part of config like Prometheus does. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/817
+* BUGFIX: fix `mode_over_time(m[d])` calculations. Previously the function could return incorrect results.
 
 
 # [v1.43.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.43.0)

app/vmselect/promql/aggr.go

@@ -494,6 +494,8 @@ func aggrFuncZScore(afa *aggrFuncArg) ([]*timeseries, error) {
 //
 // It is expected that a doesn't contain NaNs.
 //
+// The function modifies contents for a, so the caller must prepare it accordingly.
+//
 // See https://en.wikipedia.org/wiki/Mode_(statistics)
 func modeNoNaNs(prevValue float64, a []float64) float64 {
 	if len(a) == 0 {

app/vmselect/promql/rollup.go

@@ -1587,7 +1587,23 @@ func rollupTimestamp(rfa *rollupFuncArg) float64 {
 func rollupModeOverTime(rfa *rollupFuncArg) float64 {
 	// There is no need in handling NaNs here, since they must be cleaned up
 	// before calling rollup funcs.
-	return modeNoNaNs(rfa.prevValue, rfa.values)
+
+	// Copy rfa.values to a.A, since modeNoNaNs modifies a.A contents.
+	a := float64sPool.Get().(*float64s)
+	a.A = append(a.A[:0], rfa.values...)
+	result := modeNoNaNs(rfa.prevValue, a.A)
+	float64sPool.Put(a)
+	return result
+}
+
+var float64sPool = &sync.Pool{
+	New: func() interface{} {
+		return &float64s{}
+	},
+}
+
+type float64s struct {
+	A []float64
 }
 
 func rollupAscentOverTime(rfa *rollupFuncArg) float64 {

app/vmselect/promql/rollup_test.go

@@ -1012,7 +1012,7 @@ func TestRollupFuncsNoWindow(t *testing.T) {
 		}
 		rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
 		values := rc.Do(nil, testValues, testTimestamps)
-		valuesExpected := []float64{nan, nan, 34, 44, 44}
+		valuesExpected := []float64{nan, 21, 34, 34, 34}
 		timestampsExpected := []int64{0, 40, 80, 120, 160}
 		testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
 	})
@@ -1026,7 +1026,7 @@ func TestRollupFuncsNoWindow(t *testing.T) {
 		}
 		rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
 		values := rc.Do(nil, testValues, testTimestamps)
-		valuesExpected := []float64{nan, 1262.5, 3187.5, 4059.523809523809, 6200}
+		valuesExpected := []float64{nan, 2775, 5262.5, 3678.5714285714284, 2880}
 		timestampsExpected := []int64{0, 40, 80, 120, 160}
 		testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
 	})
@@ -1040,7 +1040,7 @@ func TestRollupFuncsNoWindow(t *testing.T) {
 		}
 		rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
 		values := rc.Do(nil, testValues, testTimestamps)
-		valuesExpected := []float64{nan, 0.9397878236968458, 1.1969836716333457, 2.3112921116373175, nan}
+		valuesExpected := []float64{nan, -0.86650328627136, -1.1200838283548589, -0.40035755084856683, nan}
 		timestampsExpected := []int64{0, 40, 80, 120, 160}
 		testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
 	})