app/vmselect/promql: assume that time series value doesnt change during gaps when calculating increase() and delta()

This should remove unexpected spikes at the end of gaps.
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/894
This commit is contained in:
Aliaksandr Valialkin 2020-11-13 14:55:58 +02:00
parent d21b1606a1
commit da6d82a8dd
3 changed files with 53 additions and 21 deletions

View File

@ -326,14 +326,29 @@ func getRollupFunc(funcName string) newRollupFunc {
} }
type rollupFuncArg struct { type rollupFuncArg struct {
prevValue float64 // The value preceeding values if it fits staleness interval.
prevTimestamp int64 prevValue float64
values []float64
timestamps []int64
// 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 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 tsm *timeseriesMap
} }
@ -538,6 +553,11 @@ func (rc *rollupConfig) doInternal(dstValues []float64, tsm *timeseriesMap, valu
rfa.values = nil rfa.values = nil
rfa.timestamps = nil rfa.timestamps = nil
} }
if i > 0 {
rfa.realPrevValue = values[i-1]
} else {
rfa.realPrevValue = nan
}
rfa.currTimestamp = tEnd rfa.currTimestamp = tEnd
value := rc.Func(rfa) value := rc.Func(rfa)
rfa.idx++ rfa.idx++
@ -1244,6 +1264,12 @@ func rollupDelta(rfa *rollupFuncArg) float64 {
if len(values) == 0 { if len(values) == 0 {
return nan 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 // 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. // only if the first value doesn't exceed too much the delta with the next value.
// //

View File

@ -1103,11 +1103,12 @@ func testRowsEqual(t *testing.T, values []float64, timestamps []int64, valuesExp
} }
func TestRollupDelta(t *testing.T) { func TestRollupDelta(t *testing.T) {
f := func(prevValue float64, values []float64, resultExpected float64) { f := func(prevValue, realPrevValue float64, values []float64, resultExpected float64) {
t.Helper() t.Helper()
rfa := &rollupFuncArg{ rfa := &rollupFuncArg{
prevValue: prevValue, prevValue: prevValue,
values: values, values: values,
realPrevValue: realPrevValue,
} }
result := rollupDelta(rfa) result := rollupDelta(rfa)
if math.IsNaN(result) { if math.IsNaN(result) {
@ -1120,22 +1121,26 @@ func TestRollupDelta(t *testing.T) {
t.Fatalf("unexpected result; got %v; want %v", result, resultExpected) t.Fatalf("unexpected result; got %v; want %v", result, resultExpected)
} }
} }
f(nan, nil, nan) f(nan, nan, nil, nan)
// Small initial value // Small initial value
f(nan, []float64{1}, 1) f(nan, nan, []float64{1}, 1)
f(nan, []float64{10}, 10) f(nan, nan, []float64{10}, 10)
f(nan, []float64{100}, 100) f(nan, nan, []float64{100}, 100)
f(nan, []float64{1, 2, 3}, 3) f(nan, nan, []float64{1, 2, 3}, 3)
f(1, []float64{1, 2, 3}, 2) f(1, nan, []float64{1, 2, 3}, 2)
f(nan, []float64{5, 6, 8}, 8) f(nan, nan, []float64{5, 6, 8}, 8)
f(2, []float64{5, 6, 8}, 6) f(2, nan, []float64{5, 6, 8}, 6)
// Too big initial value must be skipped. // Too big initial value must be skipped.
f(nan, []float64{1000}, 0) f(nan, nan, []float64{1000}, 0)
f(nan, []float64{1000, 1001, 1002}, 2) 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 // Empty values
f(1, nil, 0) f(1, nan, nil, 0)
f(100, nil, 0) f(100, nan, nil, 0)
} }

View File

@ -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. * 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 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 * BUGFIX: vminsert: properly return HTTP 503 status code when all the vmstorage nodes are unavailable. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/896