Mixed rate rollups can corrupt raw maximum values after resets
perfloop/victoriametrics · FAN OUT
https://perfloop.ai/t/oss/case_4e036td369
Verdict
VERIFIED · settled 2026-09-28
What happened: The assertion is violated on the comparison and satisfied with this change.
Hypothesis
A query that requests rate and a raw maximum together can return a maximum of adjusted counter values rather than the stored values. This changes the answer when a counter resets and the adjusted values exceed the raw maximum. Whether users have encountered this is unknown.
At revision 24fd416, `docs/victoriametrics/MetricsQL.md:200-206` says `aggr_over_time` calculates each listed function individually on raw samples and explicitly shows `rate` alongside `max_over_time`. For a nonempty range and `sum(aggr_over_time(("rate","max_over_time"), foo[5m])) by (rollup)`, `evalAggrFunc` selects incremental aggregation. `getRollupConfigs` assigns one `preFunc` to the whole function list when any member needs `removeCounterResets` (`rollup.go:494-510`). `evalRollupWithIncrementalAggregate` calls it on `rs.Values` before looping over the configurations (`eval.go:1935-1949`). The rate member reaches `rollupDerivFast`; the maximum member reads that same, already modified slice. With four non-stale samples `[1,5,0,2]` in the window, reset removal produces `[1,5,5,7]`: a one-series maximum should be 5, but the shared view yields 7. A temporary `TestPerfloopAggrOverTimeProbe` run with `go test -run '^TestPerfloopAggrOverTimeProbe$' -count=1 -v ./app/vmselect/promql` confirmed the parsed incremental binding and produced 7 from the maximum configuration; it did not exercise HTTP or persisted storage.
The Case can seed those counter samples, make an uncached `/api/v1/query_range` request for the mixed expression, and compare its `rollup="max_over_time"` point with standalone `max_over_time(foo[5m])` at the same step. Check the rate member and reverse the function-list order as controls. If the unmodified full-query result already matches the raw maximum of 5 under these premises, that would refute the end-user discrepancy.
Change to test: Preserve each member rollup's required input view instead of passing every member the same reset-adjusted slice. Let raw-value members use the original samples and rate-like members use a safely shared adjusted view.
Where it lives
perfloop/victoriametrics · app/vmselect/promql/eval.go
Evidence
The assertion is violated on the comparison and satisfied with this change: `For one stored vmsingle foo{job="mixed_resets"} series with values [1, 5, 0, 2] at 2024-01-01T00:00:30Z, +60s, +90s, and +120s, uncached /api/v1/query_range requests at +150s with a 5m window, 30s step, and 10s max lookback must succeed for sum(aggr_over_time(("rate", "max_over_time"), foo{job="mixed_resets"}[5m])) by (rollup) and the reversed member order; both must return one max_over_time sample equal to 5 and one rate sample equal to 0.06666666666666667.`
The assertion is violated on the comparison and satisfied with this change: `For TestExecSuccess's deterministic six-point fixture from 1000s through 2000s in 200s steps, sort_by_label(aggr_over_time(("max_over_time", "increase"), (time() % 300)[:10s]), "rollup") must return max_over_time [290, 290, 200, 290, 290, 200] and increase [190, 190, 200, 190, 190, 200], with the fixture's rollup labels and timestamps, values within 1e-13 relative tolerance, and no Exec error.`
The assertion is violated on the comparison and satisfied with this change: `For one cacheable vmsingle query of sum(aggr_over_time(("rate", "max_over_time"), foo{job="mixed_resets"}[5m])) by (rollup), over samples [1, 5, 0, 2] at 2024-01-01T00:00:30Z, +60s, +90s, and +120s, after the comparison-version process persists the result and the selected process reopens the same storage path, the 2024-01-01T00:02:30Z and 00:03:00Z points must return max_over_time 5 and rate 0.06666666666666667 rather than a cached maximum of 7.`
Checks: 8 of 8 passed. Verification: no defect found.