Uh oh!
There was an error while loading. Please reload this page.
Fix xrate extended-range iteration and refresh rate-vs-xrate showcase (2.55) - #21
Fix xrate extended-range iteration and refresh rate-vs-xrate showcase (2.55)#21ColinDKelley wants to merge 0 commit into
Conversation
| // replacing it with later points while not yet (strictly) | ||
| // inside the range. | ||
| if t > mint || !appendedPointBeforeMint { | ||
| if t > mintFloats || !appendedPointBeforeMint { |
There was a problem hiding this comment.
@ColinDKelley Could you help me understand the difference between mintFloats and mint?
mint is the minimum timestamp?
There was a problem hiding this comment.
(from Cursor)
Good question — they're related but not the same thing in this function.
mint is the left edge of the current step's matrix-selector window (a timestamp in ms). For each range-query step it's computed as maxt - selRange (with optional lookback adjustments for anchored/smoothed). It's the parameter passed into matrixIterSlice and represents "start of this evaluation window."
mintFloats starts equal to mint, but after overlap handling it becomes the timestamp of the last float already in the buffer from the previous step:
mintFloats, mintHistograms:=mint, mint// ...iflen(floats) >0&&floats[len(floats)-1].T>mint {
// trim/reuse overlapping points from the prior step ...mintFloats=floats[len(floats)-1].T
}So mintFloats means "only pull new float samples strictly after what we already have," which avoids duplicating points when stepping through a range query.
The bug this PR fixes is in the extended-range (extRange) branch, which has special logic to keep one sample at or before mint for xrate/xincrease/xdelta extrapolation. That branch was comparing iterator timestamps against mint instead of mintFloats. On step 2+ of a range query, mint advances with the window, but the buffer already contains points with mint < t ≤ mintFloats. Using mint made those look "new" again (t > mint is true), so they got re-appended and inflated counter-reset math.
Using mintFloats matches the non-extended path (if t > mintFloats at line ~3071) and the histogram path (if t > mintHistograms): compare against what's already buffered, not just the window start.
Happy to walk through a concrete step-by-step example in functions.test if that's helpful.
There was a problem hiding this comment.
No this was really helpful! Thanks for explaining this out further!
There was a problem hiding this comment.
Credit all goes to Cursor! I'm learning this from its response.
| // replacing it with later points while not yet (strictly) | ||
| // inside the range. | ||
| if t > mint || !appendedPointBeforeMint { | ||
| if t > mintFloats || !appendedPointBeforeMint { |
There was a problem hiding this comment.
No this was really helpful! Thanks for explaining this out further!
5d25fc0 to
2e35102CompareColinDKelley
commented
Jul 9, 2026
@ttstarck I had Cursor push this bug fix back to 2.55.1 branch (in case we want to release that) and that caused this PR to become a no-op. |
Branch stack context
This PR is the second layer in the 2.55.1 stack. Review bottom-up:
Summary
Forward-port of #16 onto
invoca-2.55.1-base.Cherry-picked cleanly from
invoca-2.53.1/fix-xrate-extended-range(one commit, auto-merged in bothpromql/engine.goandpromql/promqltest/testdata/functions.test). No code changes from the 2.53.1 version — the 2.53→2.55 upstream delta does not touch the lines involved.mintFloatsvsmintoff-by-one in the extended-range branch ofmatrixIterSlicethat re-appended previous-step samples into the next step's points list (inflated counter-reset corrections forxrate/xincrease/xdeltain range-query mode).rate()showcase expected values to match upstream's 2.53+ extrapolation behaviour (same three values we updated on 2.53.1).Test plan
go test -count=1 ./promql/...passes on Go 1.26.2Related
invoca-2.55.1-base(Merge upstream Prometheus v2.55.1 into invoca-2.53.1-base #27).invoca-2.55.1/add-yratestacks on top of this (Add yrate/yincrease/ydelta (2.55) #22).