From 1b5b9ced27baa5418c413a6b82f0351eb2ded302 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Thu, 15 Aug 2019 23:29:59 +0300 Subject: [PATCH] app/vmselect/promql: properly handle corner cases for rollup functions --- app/vmselect/promql/rollup.go | 20 +++++++++++++++----- app/vmselect/promql/rollup_test.go | 17 ++++++++++++++--- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/app/vmselect/promql/rollup.go b/app/vmselect/promql/rollup.go index a627c209ea..16df4755c5 100644 --- a/app/vmselect/promql/rollup.go +++ b/app/vmselect/promql/rollup.go @@ -615,10 +615,15 @@ func rollupDelta(rfa *rollupFuncArg) float64 { if len(values) == 0 { return nan } + if len(values) == 1 { + // Assume that the previous non-existing value was 0. + return values[0] + } prevValue = values[0] values = values[1:] } if len(values) == 0 { + // Assume that the value didn't change on the given interval. return 0 } return values[len(values)-1] - prevValue @@ -632,6 +637,7 @@ func rollupIdelta(rfa *rollupFuncArg) float64 { if math.IsNaN(rfa.prevValue) { return nan } + // Assume that the value didn't change on the given interval. return 0 } lastValue := values[len(values)-1] @@ -639,7 +645,8 @@ func rollupIdelta(rfa *rollupFuncArg) float64 { if len(values) == 0 { prevValue := rfa.prevValue if math.IsNaN(prevValue) { - return 0 + // Assume that the previous non-existing value was 0. + return lastValue } return lastValue - prevValue } @@ -661,7 +668,8 @@ func rollupDerivFast(rfa *rollupFuncArg) float64 { prevValue := rfa.prevValue prevTimestamp := rfa.prevTimestamp if math.IsNaN(prevValue) { - if len(values) == 0 { + if len(values) < 2 { + // It is impossible to calculate derivative on 0 or 1 values. return nan } prevValue = values[0] @@ -670,6 +678,7 @@ func rollupDerivFast(rfa *rollupFuncArg) float64 { timestamps = timestamps[1:] } if len(values) == 0 { + // Assume that the value didn't change on the given interval. return 0 } vEnd := values[len(values)-1] @@ -684,11 +693,12 @@ func rollupIderiv(rfa *rollupFuncArg) float64 { // before calling rollup funcs. values := rfa.values timestamps := rfa.timestamps - if len(values) == 0 { - if math.IsNaN(rfa.prevValue) { + if len(values) < 2 { + if len(values) == 0 || math.IsNaN(rfa.prevValue) { + // It is impossible to calculate derivative on 0 or 1 values. return nan } - return 0 + return (values[0] - rfa.prevValue) / float64(timestamps[0]-rfa.prevTimestamp) } vEnd := values[len(values)-1] tEnd := timestamps[len(timestamps)-1] diff --git a/app/vmselect/promql/rollup_test.go b/app/vmselect/promql/rollup_test.go index fcf598b63a..337d634587 100644 --- a/app/vmselect/promql/rollup_test.go +++ b/app/vmselect/promql/rollup_test.go @@ -45,8 +45,19 @@ func TestRollupIderivDuplicateTimestamps(t *testing.T) { timestamps: []int64{100}, } n = rollupIderiv(rfa) - if n != 0 { - t.Fatalf("unexpected value; got %v; want %v", n, 0) + if !math.IsNaN(n) { + t.Fatalf("unexpected value; got %v; want %v", n, nan) + } + + rfa = &rollupFuncArg{ + prevTimestamp: 90, + prevValue: 10, + values: []float64{15}, + timestamps: []int64{100}, + } + n = rollupIderiv(rfa) + if n != 0.5 { + t.Fatalf("unexpected value; got %v; want %v", n, 0.5) } rfa = &rollupFuncArg{ @@ -569,7 +580,7 @@ func TestRollupFuncsNoWindow(t *testing.T) { } rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) values := rc.Do(nil, testValues, testTimestamps) - valuesExpected := []float64{0, 33, -87, 0} + valuesExpected := []float64{123, 33, -87, 0} timestampsExpected := []int64{10, 50, 90, 130} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) })