From 717c554fb07ff28f186aff1514c991bcfa1fc019 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Wed, 29 Jul 2020 11:51:37 +0300 Subject: [PATCH] app/vmselect/promql: remove rollupFuncArg.realPrevValue handling, since the corner case in `increase()` is handled in another way now See e00cfc854df22fc64ad41dffcbacb44c32ba2060 for the approach used now. --- app/vmselect/promql/rollup.go | 28 +++------------------------- app/vmselect/promql/rollup_test.go | 9 ++++----- 2 files changed, 7 insertions(+), 30 deletions(-) diff --git a/app/vmselect/promql/rollup.go b/app/vmselect/promql/rollup.go index ba1d0588f3..0cfc4bec26 100644 --- a/app/vmselect/promql/rollup.go +++ b/app/vmselect/promql/rollup.go @@ -28,8 +28,8 @@ var rollupFuncs = map[string]newRollupFunc{ "deriv_fast": newRollupFuncOneArg(rollupDerivFast), "holt_winters": newRollupHoltWinters, "idelta": newRollupFuncOneArg(rollupIdelta), - "increase": newRollupFuncOneArg(rollupIncrease), // + rollupFuncsRemoveCounterResets - "irate": newRollupFuncOneArg(rollupIderiv), // + rollupFuncsRemoveCounterResets + "increase": newRollupFuncOneArg(rollupDelta), // + rollupFuncsRemoveCounterResets + "irate": newRollupFuncOneArg(rollupIderiv), // + rollupFuncsRemoveCounterResets "predict_linear": newRollupPredictLinear, "rate": newRollupFuncOneArg(rollupDerivFast), // + rollupFuncsRemoveCounterResets "resets": newRollupFuncOneArg(rollupResets), @@ -94,7 +94,7 @@ var rollupAggrFuncs = map[string]rollupFunc{ "deriv": rollupDerivSlow, "deriv_fast": rollupDerivFast, "idelta": rollupIdelta, - "increase": rollupIncrease, // + rollupFuncsRemoveCounterResets + "increase": rollupDelta, // + rollupFuncsRemoveCounterResets "irate": rollupIderiv, // + rollupFuncsRemoveCounterResets "rate": rollupDerivFast, // + rollupFuncsRemoveCounterResets "resets": rollupResets, @@ -327,10 +327,6 @@ type rollupFuncArg struct { idx int step int64 - // Real previous value even if it is located too far from the current window. - // It matches prevValue if prevValue is not nan. - realPrevValue float64 - tsm *timeseriesMap } @@ -342,7 +338,6 @@ func (rfa *rollupFuncArg) reset() { rfa.currTimestamp = 0 rfa.idx = 0 rfa.step = 0 - rfa.realPrevValue = nan rfa.tsm = nil } @@ -487,7 +482,6 @@ func (rc *rollupConfig) doInternal(dstValues []float64, tsm *timeseriesMap, valu rfa := getRollupFuncArg() rfa.idx = 0 rfa.step = rc.Step - rfa.realPrevValue = nan rfa.tsm = tsm i := 0 @@ -514,11 +508,6 @@ func (rc *rollupConfig) doInternal(dstValues []float64, tsm *timeseriesMap, valu rfa.values = values[i:j] rfa.timestamps = timestamps[i:j] rfa.currTimestamp = tEnd - if i > 0 { - rfa.realPrevValue = values[i-1] - } else { - rfa.realPrevValue = nan - } value := rc.Func(rfa) rfa.idx++ dstValues = append(dstValues, value) @@ -1205,14 +1194,6 @@ func rollupStdvar(rfa *rollupFuncArg) float64 { } func rollupDelta(rfa *rollupFuncArg) float64 { - return rollupDeltaInternal(rfa, false) -} - -func rollupIncrease(rfa *rollupFuncArg) float64 { - return rollupDeltaInternal(rfa, true) -} - -func rollupDeltaInternal(rfa *rollupFuncArg, canUseRealPrevValue bool) float64 { // There is no need in handling NaNs here, since they must be cleaned up // before calling rollup funcs. values := rfa.values @@ -1236,9 +1217,6 @@ func rollupDeltaInternal(rfa *rollupFuncArg, canUseRealPrevValue bool) float64 { } if math.Abs(values[0]) < 10*(math.Abs(d)+1) { prevValue = 0 - if canUseRealPrevValue && !math.IsNaN(rfa.realPrevValue) { - prevValue = rfa.realPrevValue - } } else { prevValue = values[0] values = values[1:] diff --git a/app/vmselect/promql/rollup_test.go b/app/vmselect/promql/rollup_test.go index 9b63706154..7a467413b9 100644 --- a/app/vmselect/promql/rollup_test.go +++ b/app/vmselect/promql/rollup_test.go @@ -1040,15 +1040,14 @@ func testRowsEqual(t *testing.T, values []float64, timestamps []int64, valuesExp } } -func TestRollupIncrease(t *testing.T) { +func TestRollupDelta(t *testing.T) { f := func(prevValue float64, values []float64, resultExpected float64) { t.Helper() rfa := &rollupFuncArg{ - prevValue: prevValue, - realPrevValue: prevValue, - values: values, + prevValue: prevValue, + values: values, } - result := rollupIncrease(rfa) + result := rollupDelta(rfa) if math.IsNaN(result) { if !math.IsNaN(resultExpected) { t.Fatalf("unexpected result; got %v; want %v", result, resultExpected)