app/vmselect/promql: fix seriesFetched update logic (#7181)

### Describe Your Changes

evalInstantRollup could have overreport the number of fetched series if
`offset` checks will result into retry. This change updates fetched
series only if these checks were successful.

It also adds a comment to another potential place of over-reporting
series fetched. It doesn't fix it, because it would require spending
extra resources on such a check, while discrepancy in seriesFetched
doesn't affect calculations in any way.

Probably related to
https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7170

### Checklist

The following checks are **mandatory**:

- [x] My change adheres [VictoriaMetrics contributing
guidelines](https://docs.victoriametrics.com/contributing/).

Signed-off-by: hagen1778 <roman@victoriametrics.com>
This commit is contained in:
Roman Khavronenko 2024-10-07 14:27:50 +02:00 committed by GitHub
parent 5481fa669c
commit ebd393d8b3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -1092,7 +1092,6 @@ func evalInstantRollup(qt *querytracer.Tracer, ec *EvalConfig, funcName string,
again:
offset := int64(0)
tssCached := rollupResultCacheV.GetInstantValues(qt, expr, window, ec.Step, ec.EnforcedTagFilterss)
ec.QueryStats.addSeriesFetched(len(tssCached))
if len(tssCached) == 0 {
// Cache miss. Re-populate the missing data.
start := int64(fasttime.UnixTimestamp()*1000) - cacheTimestampOffset.Milliseconds()
@ -1136,6 +1135,7 @@ func evalInstantRollup(qt *querytracer.Tracer, ec *EvalConfig, funcName string,
deleteCachedSeries(qt)
goto again
}
ec.QueryStats.addSeriesFetched(len(tssCached))
return tssCached, offset, nil
}
@ -1647,6 +1647,10 @@ func evalRollupFuncWithMetricExpr(qt *querytracer.Tracer, ec *EvalConfig, funcNa
ecNew = copyEvalConfig(ec)
ecNew.Start = start
}
// call to evalWithConfig also updates QueryStats.addSeriesFetched
// without checking whether tss has intersection with tssCached.
// So final number could be bigger than actual number of unique series.
// This discrepancy is acceptable, since seriesFetched stat is used as info only.
tss, err := evalWithConfig(ecNew)
if err != nil {
return nil, err