app/vmselect/promql: move needSilenceIntervalForRollupFunc from eval.go to rollup.go

This should improve maintainability of the code related to rollup functions,
since it is located in rollup.go

While at it, properly return empty results from holt_winters(), rate_over_sum(),
sum2_over_time(), geomean_over_time() and distinct_over_time() when there are no real samples
on the selected lookbehind window. Previously the previous sample value was mistakenly
returned from these functions.
This commit is contained in:
Aliaksandr Valialkin 2024-02-21 18:47:49 +02:00
parent ce3ec3ff2e
commit a6eacfdb11
No known key found for this signature in database
GPG Key ID: 52C003EE2BCDB9EB
2 changed files with 42 additions and 63 deletions

View File

@ -1687,7 +1687,7 @@ func evalRollupFuncNoCache(qt *querytracer.Tracer, ec *EvalConfig, funcName stri
tfss := searchutils.ToTagFilterss(me.LabelFilterss)
tfss = searchutils.JoinTagFilterss(tfss, ec.EnforcedTagFilterss)
minTimestamp := ec.Start
if needSilenceIntervalForRollupFunc(funcName) {
if needSilenceIntervalForRollupFunc[funcName] {
minTimestamp -= maxSilenceInterval()
}
if window > ec.Step {
@ -1788,56 +1788,6 @@ func maxSilenceInterval() int64 {
return d
}
func needSilenceIntervalForRollupFunc(funcName string) bool {
// All the rollup functions, which do not rely on the previous sample
// before the lookbehind window (aka prevValue and realPrevValue), do not need silence interval.
switch strings.ToLower(funcName) {
case "default_rollup":
// The default_rollup implicitly relies on the previous samples in order to fill gaps.
// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5388
return true
case
"absent_over_time",
"avg_over_time",
"count_eq_over_time",
"count_gt_over_time",
"count_le_over_time",
"count_ne_over_time",
"count_over_time",
"first_over_time",
"histogram_over_time",
"hoeffding_bound_lower",
"hoeffding_bound_upper",
"last_over_time",
"mad_over_time",
"max_over_time",
"median_over_time",
"min_over_time",
"predict_linear",
"present_over_time",
"quantile_over_time",
"quantiles_over_time",
"range_over_time",
"share_gt_over_time",
"share_le_over_time",
"share_eq_over_time",
"stale_samples_over_time",
"stddev_over_time",
"stdvar_over_time",
"sum_over_time",
"tfirst_over_time",
"timestamp",
"timestamp_with_name",
"tlast_over_time",
"tmax_over_time",
"tmin_over_time",
"zscore_over_time":
return false
default:
return true
}
}
func evalRollupWithIncrementalAggregate(qt *querytracer.Tracer, funcName string, keepMetricNames bool,
iafc *incrementalAggrFuncContext, rss *netstorage.Results, rcs []*rollupConfig,
preFunc func(values []float64, timestamps []int64), sharedTimestamps []int64) ([]*timeseries, error) {

View File

@ -103,6 +103,42 @@ var rollupFuncs = map[string]newRollupFunc{
"zscore_over_time": newRollupFuncOneArg(rollupZScoreOverTime),
}
// Functions, which need the previous sample before the lookbehind window for proper calculations.
//
// All the rollup functions, which do not rely on the previous sample
// before the lookbehind window (aka prevValue and realPrevValue), do not need silence interval.
var needSilenceIntervalForRollupFunc = map[string]bool{
"ascent_over_time": true,
"changes": true,
"decreases_over_time": true,
// The default_rollup implicitly relies on the previous samples in order to fill gaps.
// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5388
"default_rollup": true,
"delta": true,
"deriv_fast": true,
"descent_over_time": true,
"idelta": true,
"ideriv": true,
"increase": true,
"increase_pure": true,
"increases_over_time": true,
"integrate": true,
"irate": true,
"lag": true,
"lifetime": true,
"rate": true,
"resets": true,
"rollup": true,
"rollup_candlestick": true,
"rollup_delta": true,
"rollup_deriv": true,
"rollup_increase": true,
"rollup_rate": true,
"rollup_scrape_interval": true,
"scrape_interval": true,
"tlast_change_over_time": true,
}
// rollupAggrFuncs are functions that can be passed to `aggr_over_time()`
var rollupAggrFuncs = map[string]rollupFunc{
"absent_over_time": rollupAbsent,
@ -957,7 +993,7 @@ func newRollupHoltWinters(args []interface{}) (rollupFunc, error) {
// before calling rollup funcs.
values := rfa.values
if len(values) == 0 {
return rfa.prevValue
return nan
}
sf := sfs[rfa.idx]
if sf < 0 || sf > 1 {
@ -1586,11 +1622,7 @@ func rollupRateOverSum(rfa *rollupFuncArg) float64 {
// before calling rollup funcs.
timestamps := rfa.timestamps
if len(timestamps) == 0 {
if math.IsNaN(rfa.prevValue) {
return nan
}
// Assume that the value didn't change since rfa.prevValue.
return 0
return nan
}
sum := float64(0)
for _, v := range rfa.values {
@ -1610,7 +1642,7 @@ func rollupSum2(rfa *rollupFuncArg) float64 {
// before calling rollup funcs.
values := rfa.values
if len(values) == 0 {
return rfa.prevValue * rfa.prevValue
return nan
}
var sum2 float64
for _, v := range values {
@ -1624,7 +1656,7 @@ func rollupGeomean(rfa *rollupFuncArg) float64 {
// before calling rollup funcs.
values := rfa.values
if len(values) == 0 {
return rfa.prevValue
return nan
}
p := 1.0
for _, v := range values {
@ -2268,10 +2300,7 @@ func rollupDistinct(rfa *rollupFuncArg) float64 {
// before calling rollup funcs.
values := rfa.values
if len(values) == 0 {
if math.IsNaN(rfa.prevValue) {
return nan
}
return 0
return nan
}
m := make(map[float64]struct{})
for _, v := range values {