diff --git a/app/vmselect/promql/rollup.go b/app/vmselect/promql/rollup.go index bf92351f0c..247743ddc5 100644 --- a/app/vmselect/promql/rollup.go +++ b/app/vmselect/promql/rollup.go @@ -326,14 +326,29 @@ func getRollupFunc(funcName string) newRollupFunc { } type rollupFuncArg struct { - prevValue float64 - prevTimestamp int64 - values []float64 - timestamps []int64 + // The value preceeding values if it fits staleness interval. + prevValue float64 + // The timestamp for prevValue. + prevTimestamp int64 + + // Values that fit window ending at currTimestamp. + values []float64 + + // Timestamps for values. + timestamps []int64 + + // Actual value preceeding values without restrictions on staleness interval. + realPrevValue float64 + + // Current timestamp for rollup evaluation. currTimestamp int64 - idx int - window int64 + + // Index for the currently evaluated point relative to time range for query evaluation. + idx int + + // Time window for rollup calculations. + window int64 tsm *timeseriesMap } @@ -538,6 +553,11 @@ func (rc *rollupConfig) doInternal(dstValues []float64, tsm *timeseriesMap, valu rfa.values = nil rfa.timestamps = nil } + if i > 0 { + rfa.realPrevValue = values[i-1] + } else { + rfa.realPrevValue = nan + } rfa.currTimestamp = tEnd value := rc.Func(rfa) rfa.idx++ @@ -1244,6 +1264,12 @@ func rollupDelta(rfa *rollupFuncArg) float64 { if len(values) == 0 { return nan } + if !math.IsNaN(rfa.realPrevValue) { + // Assume that the value didn't change during the current gap. + // This should fix high delta() and increase() values at the end of gaps. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/894 + return values[len(values)-1] - rfa.realPrevValue + } // Assume that the previous non-existing value was 0 // only if the first value doesn't exceed too much the delta with the next value. // diff --git a/app/vmselect/promql/rollup_test.go b/app/vmselect/promql/rollup_test.go index fb7083d809..e0eaacb6cd 100644 --- a/app/vmselect/promql/rollup_test.go +++ b/app/vmselect/promql/rollup_test.go @@ -1103,11 +1103,12 @@ func testRowsEqual(t *testing.T, values []float64, timestamps []int64, valuesExp } func TestRollupDelta(t *testing.T) { - f := func(prevValue float64, values []float64, resultExpected float64) { + f := func(prevValue, realPrevValue float64, values []float64, resultExpected float64) { t.Helper() rfa := &rollupFuncArg{ - prevValue: prevValue, - values: values, + prevValue: prevValue, + values: values, + realPrevValue: realPrevValue, } result := rollupDelta(rfa) if math.IsNaN(result) { @@ -1120,22 +1121,26 @@ func TestRollupDelta(t *testing.T) { t.Fatalf("unexpected result; got %v; want %v", result, resultExpected) } } - f(nan, nil, nan) + f(nan, nan, nil, nan) // Small initial value - f(nan, []float64{1}, 1) - f(nan, []float64{10}, 10) - f(nan, []float64{100}, 100) - f(nan, []float64{1, 2, 3}, 3) - f(1, []float64{1, 2, 3}, 2) - f(nan, []float64{5, 6, 8}, 8) - f(2, []float64{5, 6, 8}, 6) + f(nan, nan, []float64{1}, 1) + f(nan, nan, []float64{10}, 10) + f(nan, nan, []float64{100}, 100) + f(nan, nan, []float64{1, 2, 3}, 3) + f(1, nan, []float64{1, 2, 3}, 2) + f(nan, nan, []float64{5, 6, 8}, 8) + f(2, nan, []float64{5, 6, 8}, 6) // Too big initial value must be skipped. - f(nan, []float64{1000}, 0) - f(nan, []float64{1000, 1001, 1002}, 2) + f(nan, nan, []float64{1000}, 0) + f(nan, nan, []float64{1000, 1001, 1002}, 2) + + // Delta calculations against non-nan realPrevValue + f(nan, 900, []float64{1000}, 100) + f(nan, 900, []float64{1000, 1001, 1002}, 102) // Empty values - f(1, nil, 0) - f(100, nil, 0) + f(1, nan, nil, 0) + f(100, nan, nil, 0) } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3103b83346..064e99a3db 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -15,6 +15,7 @@ * BUGFIX: do not return data points in the end of the selected time range for time series ending in the middle of the selected time range. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/887 and https://github.com/VictoriaMetrics/VictoriaMetrics/issues/845 +* BUGFIX: remove spikes at the end of time series gaps for `increase()` or `delta()` functions. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/894 * BUGFIX: vminsert: properly return HTTP 503 status code when all the vmstorage nodes are unavailable. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/896