From 9e1119dab8e52cdbb171649b6aa46caafd8a2a97 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 21 Jun 2019 21:50:44 +0300 Subject: [PATCH] app/vmselect/promql: ajdust data model to the model used in Prometheus Do not take into account data points on the range `[timestamp .. timestamp+step)` when calculating value on the given `timestamp`. Use only data points from the past when performing these calculations like Prometheus does. This should reduce discrepancies between results returned by VictoriaMetrics and results returned by Prometheus. Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/72 Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/71 --- app/vmselect/promql/eval.go | 3 +- app/vmselect/promql/exec_test.go | 38 +++++++++++----------- app/vmselect/promql/rollup.go | 5 ++- app/vmselect/promql/rollup_test.go | 52 +++++++++++++++--------------- 4 files changed, 48 insertions(+), 50 deletions(-) diff --git a/app/vmselect/promql/eval.go b/app/vmselect/promql/eval.go index 570257eb2..625e21bad 100644 --- a/app/vmselect/promql/eval.go +++ b/app/vmselect/promql/eval.go @@ -379,8 +379,7 @@ func evalRollupFuncWithSubquery(ec *EvalConfig, name string, rf rollupFunc, re * } ecSQ := newEvalConfig(ec) - ecSQ.Start -= window + maxSilenceInterval - ecSQ.End += step + ecSQ.Start -= window + maxSilenceInterval + step ecSQ.Step = step if err := ValidateMaxPointsPerTimeseries(ecSQ.Start, ecSQ.End, ecSQ.Step); err != nil { return nil, err diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index 17ec10198..f12ca25d0 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -286,7 +286,7 @@ func TestExecSuccess(t *testing.T) { q := `time()[300s:100s] offset 100s` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{800, 1000, 1200, 1400, 1600, 1800}, + Values: []float64{900, 1100, 1300, 1500, 1700, 1900}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -297,7 +297,7 @@ func TestExecSuccess(t *testing.T) { q := `time()[1.5i:0.5i] offset 0.5i` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{800, 1000, 1200, 1400, 1600, 1800}, + Values: []float64{900, 1100, 1300, 1500, 1700, 1900}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -308,7 +308,7 @@ func TestExecSuccess(t *testing.T) { q := `time()[300s] offset 100s` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{700, 900, 1100, 1300, 1500, 1700}, + Values: []float64{900, 1100, 1300, 1500, 1700, 1900}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -319,7 +319,7 @@ func TestExecSuccess(t *testing.T) { q := `time()[300s]` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{800, 1000, 1200, 1400, 1600, 1800}, + Values: []float64{1000, 1200, 1400, 1600, 1800, 2000}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -2494,7 +2494,7 @@ func TestExecSuccess(t *testing.T) { q := `distinct_over_time((time() < 1700)[500s])` r1 := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{3, 3, 3, 2, 1, nan}, + Values: []float64{3, 3, 3, 3, 2, 1}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r1} @@ -2505,7 +2505,7 @@ func TestExecSuccess(t *testing.T) { q := `distinct_over_time((time() < 1700)[2.5i])` r1 := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{3, 3, 3, 2, 1, nan}, + Values: []float64{3, 3, 3, 3, 2, 1}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r1} @@ -2761,7 +2761,7 @@ func TestExecSuccess(t *testing.T) { q := `integrate(time()*1e-3)` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{200, 240.00000000000003, 280, 320, 360, 400}, + Values: []float64{160, 200, 240.00000000000003, 280, 320, 360}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -2783,7 +2783,7 @@ func TestExecSuccess(t *testing.T) { q := `rate(2000-time())` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{4.5, 3.5, 2.5, 1.5, 0.5, -0.5}, + Values: []float64{5.5, 4.5, 3.5, 2.5, 1.5, 0.5}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -2794,7 +2794,7 @@ func TestExecSuccess(t *testing.T) { q := `rate((2000-time())[100s])` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{4.5, 3.5, 2.5, 1.5, 0.5, -0.5}, + Values: []float64{5.5, 4.5, 3.5, 2.5, 1.5, 0.5}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -2805,7 +2805,7 @@ func TestExecSuccess(t *testing.T) { q := `rate((2000-time())[100s:])` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{4.5, 3.5, 2.5, 1.5, 0.5, -0.5}, + Values: []float64{5.5, 4.5, 3.5, 2.5, 1.5, 0.5}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -2816,7 +2816,7 @@ func TestExecSuccess(t *testing.T) { q := `rate((2000-time())[100s:100s])` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{4, 6.5, 4.5, 2.5, 0.5, -1.5}, + Values: []float64{5.5, 4.5, 6.5, 4.5, 2.5, 0.5}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -2827,7 +2827,7 @@ func TestExecSuccess(t *testing.T) { q := `rate((2000-time())[100s:100s] offset 100s)` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{4.5, 3.5, 5.5, 3.5, 1.5, -0.5}, + Values: []float64{6, 5, 7.5, 5.5, 3.5, 1.5}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -2838,7 +2838,7 @@ func TestExecSuccess(t *testing.T) { q := `rate((2000-time())[100s:100s] offset 100s)[:] offset 100s` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{6, 5, 7.5, 5.5, 3.5, 1.5}, + Values: []float64{7, 6, 5, 7.5, 5.5, 3.5}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -2860,7 +2860,7 @@ func TestExecSuccess(t *testing.T) { q := `increase(2000-time())` r := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{900, 700, 500, 300, 100, -100}, + Values: []float64{1100, 900, 700, 500, 300, 100}, Timestamps: timestampsExpected, } resultExpected := []netstorage.Result{r} @@ -3163,7 +3163,7 @@ func TestExecSuccess(t *testing.T) { q := `sort(rollup(time()[:50s]))` r1 := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{1050, 1250, 1450, 1650, 1850, 2050}, + Values: []float64{850, 1050, 1250, 1450, 1650, 1850}, Timestamps: timestampsExpected, } r1.MetricName.Tags = []storage.Tag{{ @@ -3172,21 +3172,21 @@ func TestExecSuccess(t *testing.T) { }} r2 := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{1200, 1400, 1600, 1800, 2000, 2200}, + Values: []float64{925, 1125, 1325, 1525, 1725, 1925}, Timestamps: timestampsExpected, } r2.MetricName.Tags = []storage.Tag{{ Key: []byte("rollup"), - Value: []byte("max"), + Value: []byte("avg"), }} r3 := netstorage.Result{ MetricName: metricNameExpected, - Values: []float64{1125, 1325, 1525, 1725, 1925, 2125}, + Values: []float64{1000, 1200, 1400, 1600, 1800, 2000}, Timestamps: timestampsExpected, } r3.MetricName.Tags = []storage.Tag{{ Key: []byte("rollup"), - Value: []byte("avg"), + Value: []byte("max"), }} resultExpected := []netstorage.Result{r1, r2, r3} f(q, resultExpected) diff --git a/app/vmselect/promql/rollup.go b/app/vmselect/promql/rollup.go index 7bc099290..d1fe31386 100644 --- a/app/vmselect/promql/rollup.go +++ b/app/vmselect/promql/rollup.go @@ -171,8 +171,7 @@ func (rc *rollupConfig) Do(dstValues []float64, values []float64, timestamps []i i := 0 j := 0 - for _, ts := range rc.Timestamps { - tEnd := ts + rc.Step + for _, tEnd := range rc.Timestamps { tStart := tEnd - window n := sort.Search(len(timestamps)-i, func(n int) bool { return timestamps[i+n] > tStart @@ -655,7 +654,7 @@ func rollupFirst(rfa *rollupFuncArg) float64 { return values[0] } -var rollupDefault = rollupFirst +var rollupDefault = rollupLast func rollupLast(rfa *rollupFuncArg) float64 { // There is no need in handling NaNs here, since they must be cleanup up diff --git a/app/vmselect/promql/rollup_test.go b/app/vmselect/promql/rollup_test.go index 9b6df7927..1f7979a68 100644 --- a/app/vmselect/promql/rollup_test.go +++ b/app/vmselect/promql/rollup_test.go @@ -189,7 +189,7 @@ func TestRollupNewRollupFuncSuccess(t *testing.T) { testRollupFunc(t, funcName, args, &me, vExpected) } - f("default_rollup", 123) + f("default_rollup", 34) f("changes", 10) f("delta", -89) f("deriv", -712) @@ -259,7 +259,7 @@ func TestRollupNoWindowNoPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{nan, nan, nan, nan, 123} + valuesExpected := []float64{nan, nan, nan, nan, nan} timestampsExpected := []int64{0, 1, 2, 3, 4} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -273,7 +273,7 @@ func TestRollupNoWindowNoPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{2, 2, 0, 0, 0, nan, nan} + valuesExpected := []float64{2, 2, 2, 0, 0, 0, nan} timestampsExpected := []int64{120, 124, 128, 132, 136, 140, 144} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -290,7 +290,7 @@ func TestRollupWindowNoPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{nan, nan, nan, nan, 123} + valuesExpected := []float64{nan, nan, nan, nan, nan} timestampsExpected := []int64{0, 1, 2, 3, 4} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -304,7 +304,7 @@ func TestRollupWindowNoPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{34, nan, nan, nan} + valuesExpected := []float64{34, 34, nan, nan} timestampsExpected := []int64{141, 151, 161, 171} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -321,7 +321,7 @@ func TestRollupNoWindowPartialPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{123, 123, 123, 123, 123} + valuesExpected := []float64{nan, 123, 123, 123, 123} timestampsExpected := []int64{0, 5, 10, 15, 20} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -335,7 +335,7 @@ func TestRollupNoWindowPartialPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{44, 34, 34, nan} + valuesExpected := []float64{12, 44, 34, 34} timestampsExpected := []int64{100, 120, 140, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -349,7 +349,7 @@ func TestRollupNoWindowPartialPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{nan, 123, 54, 44, nan} + valuesExpected := []float64{nan, nan, 123, 54, 44} timestampsExpected := []int64{-50, 0, 50, 100, 150} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -366,7 +366,7 @@ func TestRollupWindowPartialPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{123, 123, 34, 34, 44} + valuesExpected := []float64{nan, 123, 123, 34, 34} timestampsExpected := []int64{0, 5, 10, 15, 20} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -380,7 +380,7 @@ func TestRollupWindowPartialPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{34, 34, nan, nan} + valuesExpected := []float64{44, 34, 34, nan} timestampsExpected := []int64{100, 120, 140, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -394,7 +394,7 @@ func TestRollupWindowPartialPoints(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{54, 44, nan, nan} + valuesExpected := []float64{nan, 54, 44, nan} timestampsExpected := []int64{0, 50, 100, 150} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -411,7 +411,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{123, 21, 12, 34, nan} + valuesExpected := []float64{nan, 123, 21, 12, 34} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -425,7 +425,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{4, 4, 3, 1, nan} + valuesExpected := []float64{nan, 4, 4, 3, 1} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -439,7 +439,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{21, 12, 32, 34, nan} + valuesExpected := []float64{nan, 21, 12, 32, 34} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -453,7 +453,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{123, 99, 44, 34, nan} + valuesExpected := []float64{nan, 123, 99, 44, 34} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -467,7 +467,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{222, 199, 110, 34, nan} + valuesExpected := []float64{nan, 222, 199, 110, 34} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -481,7 +481,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{-102, -9, 22, 0, nan} + valuesExpected := []float64{nan, -102, -9, 22, 0} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -495,7 +495,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{33, -87, 0, nan} + valuesExpected := []float64{nan, 33, -87, 0} timestampsExpected := []int64{10, 50, 90, 130} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -509,7 +509,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{3, 4, 3, 0, nan} + valuesExpected := []float64{nan, 3, 4, 3, 0} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -523,7 +523,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{2, 2, 1, 0, nan} + valuesExpected := []float64{nan, 2, 2, 1, 0} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -537,7 +537,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{55.5, 49.75, 36.666666666666664, 34, nan} + valuesExpected := []float64{nan, 55.5, 49.75, 36.666666666666664, 34} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -551,7 +551,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{-3290.3225806451615, -204.54545454545456, 550, 0, nan} + valuesExpected := []float64{nan, -3290.3225806451615, -204.54545454545456, 550, 0} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -565,7 +565,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{-1916.6666666666665, -43500, 400, 0, nan} + valuesExpected := []float64{nan, -1916.6666666666665, -43500, 400, 0} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -579,7 +579,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{39.81519810323691, 32.080952292598795, 5.2493385826745405, 0, nan} + valuesExpected := []float64{nan, 39.81519810323691, 32.080952292598795, 5.2493385826745405, 0} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -593,7 +593,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{4.6035, 4.3934999999999995, 2.166, 0.34, nan} + valuesExpected := []float64{nan, 4.6035, 4.3934999999999995, 2.166, 0.34} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) @@ -607,7 +607,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{4, 4, 3, 1, nan} + valuesExpected := []float64{nan, 4, 4, 3, 1} timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) })