Repository navigation
fix(promql): rate/irate/increase/delta/deriv/resets return no series when step >= range - #981
Merged
rita7lopes merged 3 commits intoSep 17, 2026
Conversation
rate(), increase(), delta(), resets() and changes() accelerated via the ClickHouse pushdown (VectorRange -> CounterPlanner/CounterFlagsPlanner) came back empty whenever the query's step was greater than or equal to the function's own range-vector duration -- e.g. rate(x[300s]) queried with step=300s, or any step above it. This is a very ordinary shape (step == rate window, back-to-back tiling with no gaps or overlap), so it silently broke a standard client pattern: HTTP 200, empty result, no error, while the underlying data was present and healthy the whole time. Root cause: prevValues() (shared by CounterPlanner and CounterFlagsPlanner) bucketed real samples once per ctx.Step via BucketProducer, then looked, per row, at every bucket inside (t-range, t] to find two distinct samples (the first/last for a counter function, or the current/previous for a transition-counting one). Once ctx.Step reached or exceeded the range, every bucket landed exactly on the step grid, so a whole (t-range, t] window could hold at most one of them -- first and last (or current and previous) always collapsed onto the same single row, "two distinct samples" was never satisfied, and the query returned no series for any series. Fix: bucket real samples at min(ctx.Step, range/2) instead of always ctx.Step (bucketResolution in planner/shared.go), so at least two buckets are always in reach inside any (t-range, t] window regardless of how coarse the query's step is. ctx.Step is used unchanged whenever it already leaves room for two buckets, so the common case (step finer than range) is untouched. BucketProducer and FillGapsPlanner now take an explicit Resolution instead of always reading ctx.Step, so the real buckets and the gap-fill grid stay in lockstep with whatever resolution the caller picked; OverTimePlanner picks up the same guard as a byproduct, which only makes its own bucketing more precise in the same step >= range case (a single step-wide bucket previously spanned more than the requested range). Adds TestCounterAcceleratesWhenStepIsNotFinerThanRange, which fails against the old bucketing (asserts it does not fall back to bucketing at the full range/step width) and passes with the fix, across rate, increase, delta, resets and changes at step == range and step > range. Bug report: metrico#980 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…step Verified against a live ClickHouse: pushed a synthetic counter over remote_write and swept irate(x[300s]) across query steps 30s-400s. On the pre-fix binary this failed unpredictably -- not only once step reached the 300s range, but well before that (confirmed empty at step 210, 240, 260, 270, 280, 290, 298, 300 while working at 220, 230, 250 in the same sweep) -- because the affected code path resamples the raw series once per query step, anchored to epoch, and then hands that resampled series to the engine's own irate()/deriv()/etc. as if it were the raw data. Whether a given (t-range, t] window catches enough of those buckets to compute anything depends on an alignment between the epoch-anchored grid and the query's own evaluation timestamps that the caller never controls -- so the same step can work one second and fail the next as "now" ticks forward, which is why the boundary in the original report (step >= range) is necessary but not sufficient: it reproduces reliably there, but the underlying flaw already misfires below it. This is a second, independent instance of the step-vs-range bug fixed for rate/increase/delta/resets/changes in the prior commit: that one covers the ClickHouse-pushdown path (CounterPlanner / CounterFlagsPlanner, used when a MatrixSelector is a direct Call argument); this covers the older per-step resampling path (DownsampleHintsPlanner) that irate, deriv and idelta still go through, since they have no ClickHouse-side acceleration of their own and rely on the engine's own implementation over whatever samples storage hands back. Fix: cap the resample bucket to at most range/2 for every function that measures a change across samples (rate, irate, deriv, delta, idelta, resets; increase and changes included defensively, though they are normally intercepted by the pushdown before reaching this planner) -- the same technique as the prior commit, and for the same reason: two buckets of that width always fit inside any (t-range, t] window regardless of where its boundary falls, which a resample at the query's own step only achieves by accident. Plain reducing aggregates (sum_over_time, last_over_time, ...) need only one point in the window and are left resampling at the query's own step, unchanged. Adds TestDownsampleHintsCapsChangeFunctionBucket (fails against the old bucketing, passes with the fix, across all six affected functions at and past the range boundary) and TestDownsampleHintsLeavesPlainAggregatesAtTheQueryStep (guards the non-regression for functions that don't need the cap). Re-verified end-to-end against ClickHouse after the fix: a dense sweep of steps 30s-400s (5s increments) for rate, irate, deriv, delta, idelta, increase and resets against the same live counter came back with a series every time, with rate/irate/deriv reporting the correct 1/s and delta/increase the correct 300 over the 300s window. Bug report: metrico#980 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The first commit's range/2 bucket cap is a real, if bounded, cost: for any query whose step is coarser than its range, internal row count goes from (span / step) to (span / (range/2)) -- capped at the cost of an equivalent step=range/2 query, never worse, but still strictly more work than the query's own step would otherwise need. That cost buys a correctness floor rate/increase/delta/resets/changes cannot do without: they measure a change between two samples, so fewer than two landing inside (t-range, t] means an empty result, which is the bug being fixed. OverTimePlanner's functions (sum_over_time, avg_over_time, min_over_time, max_over_time, count_over_time, last_over_time) have no such floor -- they reduce over whatever the window holds, so a single bucket is already enough for an answer. The commit applied the same cap to them anyway "as a byproduct", trading that same cost increase for a correctness improvement nobody asked for: at step >= range they were already returning a value, just one computed over the full step-wide bucket rather than strictly the trailing range -- a real but unreported inaccuracy, not the reported empty-result bug. Revert that one line: OverTimePlanner keeps bucketing at ctx.Step however coarse that gets relative to its range, so its cost stays tied to the query's own step, same as before either fix. Adds TestOverTimeKeepsQueryStepEvenWhenCoarserThanRange to hold this scope boundary in place. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rita7lopes
approved these changes
Sep 17, 2026
rita7lopes
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the PR @bzed !
The test TestOverTimeKeepsQueryStepEvenWhenCoarserThanRange asserts a current behaviour that is wrong.
sum_over_time(x[1m]) at step=1h buckets at 1 hour, so it sums a whole hour of samples and calls it a 1-minute sum. Basically it gives wrong numbers and the test asserts it.
This bug is in master so I will not hold this PR which solves a real (different) bug because of something it doesn't introduce.
I will fix the sum bug in a coming PR.
Merging this one
This was referenced Sep 17, 2026
rita7lopes
added a commit
that referenced
this pull request
Sep 21, 2026
A function that measures a change across samples (rate, irate, deriv, delta, idelta, resets, increase, changes) cannot answer from a single bucket, so its bucket has to be sized against its range rather than against the query's step. After #981 that rule existed in three places, with three different lists of function names and two different floors, and they did not agree. deriv is where the disagreement was reachable: it has no ClickHouse pushdown, so it goes through DownsampleHintsPlanner, and it was also in rateFunctions, so adjustHintsForRate had already capped its step by the time it got there. Below a 30s range the request layer floored the cap at 15s -- metrics_15s rows are stamped on a 15s grid, so a finer bucket cannot hold a second row -- and the planner then overrode that back down to range/2, asking ClickHouse for buckets narrower than the table's own resolution. The two also disagreed on the threshold: the request layer capped once step passed range/2, the planner only once step reached the full range, so a step between the two was capped in one place and not the other. Consolidate both decisions into planner.NeedsDistinctSamples and planner.BucketResolution. BucketResolution is idempotent, so the request layer and the planners can both apply it to the same query without fighting over the answer; a test asserts the two layers agree on the width they settle on. Rename rateFunctions to prolongFunctions. That list is consulted by isProlong, which decides whether the raw iterator carries a series forward between steps -- a different question from which functions need a capped bucket, and the two must not be folded back together. Restructure DownsampleHintsPlanner into an explicit three-way switch on what the function needs from its window. The step > range branch was unreachable for change functions and read as if it still applied; it is the shape a reducer takes, reading only the trailing range of each step and snapping it forward, and it is now stated as such. That branch had no test at all, so its output is captured verbatim first. Behavior changes, all of them narrow: - irate and idelta are now capped at the request layer as well. Their SQL is unchanged -- the planner already capped them to the same width -- but hints.Step now reflects it. - The 15s floor is gone. A cap that drops the step under the 15s grid now trips the existing useRawData check, so a range too small for metrics_15s to serve is read from raw samples at real timestamps instead of from a bucket finer than the table's own resolution. Affects change functions with a range under 30s. - The counter path now caps at min(step, range/2) rather than leaving any step below the full range alone, matching what the request layer has always done and what #981 documents itself as doing. A step between range/2 and range previously kept only two grid slots inside (t-range, t], the outer one on its first millisecond, so a series that had not reported into that one bucket collapsed to a single sample and was dropped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #980
Summary
rate(),irate(),increase(),delta(),deriv()(and, byconstruction,
resets()/changes()/idelta()) returned an emptyquery_rangeresult — HTTP 200,"status":"success", no error — forany series whenever the query's
stepreached or exceeded thefunction's own range-vector duration, e.g.
rate(x[300s])queriedwith
step=300s. That's an ordinary, standard PromQL shape (step == range, back-to-back tiling), not an edge case, so any client whosestep happens to match its rate window silently got nothing back. See
#980 for the full reproduction and a binary-searched boundary.
Root cause (two independent code paths, same underlying flaw)
The ClickHouse pushdown (
VectorRangeoptimizer →CounterPlanner/CounterFlagsPlanner, used forrate,increase,delta,resets,changes): real samples were bucketed once perctx.Stepbefore being scanned for two distinct samples inside(t-range, t]. Oncestep >= range, every bucket landed exactly onthe step grid, so that window could hold at most one of them —
"two distinct samples" was never satisfied, and the result was
always empty.
The older per-step resampling path (
DownsampleHintsPlanner,used for
irate,deriv,idelta, which have no ClickHouse-sideacceleration and rely on the Prometheus engine's own function
implementation over whatever the storage layer hands back): same
idea, but worse — the resample bucket grid is anchored to epoch,
not to the query's own evaluation timestamps, so whether a given
(t-range, t]window catches enough buckets to compute anythingdepends on an alignment the caller never controls. Verified live
against ClickHouse:
irate(x[300s])failed unpredictably wellbefore
step >= rangetoo (empty at step 210s, 240s, 260s... whileworking at 220s, 230s, 250s in the same sweep), which is why the
step >= rangeboundary in the report is necessary but notsufficient to describe it.
Fix
Both paths get the same fix: cap the resample/bucket width to at most
range/2for functions that measure a change across samples (rate,irate,deriv,delta,idelta,resets;increase/changesincluded defensively). Two buckets of that width always fit inside any
(t-range, t]window, regardless of where its boundary falls — apigeonhole guarantee, not something that depends on
stepor onalignment.
ctx.Stepis used unchanged whenever it already leavesroom for two buckets, so the common case (step finer than range) is
untouched.
OverTimePlanner(sum_over_time,avg_over_time,min_over_time,max_over_time,count_over_time,last_over_time) deliberately doesnot get this cap: those functions only need one sample in the
window, not two, so there's no correctness floor forcing anything
finer than
ctx.Step, and doing so anyway would raise their query costfor no reported benefit. See the last commit and
TestOverTimeKeepsQueryStepEvenWhenCoarserThanRange.Performance
The cap only changes behavior once
step >= range/2for the affectedfunctions, and even then the cost is bounded by, and identical to, an
already-supported query shape: a query with
step = range/2explicitly(never broken, already accepted). A caller with
step = 1000×rangedoes not pay 1000× anything — they pay exactly what a
step = range/2caller already pays. No new class of expensive query is introduced;
correctness for a previously-silently-empty shape now costs the same
as an already-working neighboring one.
Verification
it):
TestCounterAcceleratesWhenStepIsNotFinerThanRange,TestDownsampleHintsCapsChangeFunctionBucket/TestDownsampleHintsLeavesPlainAggregatesAtTheQueryStep,TestOverTimeKeepsQueryStepEvenWhenCoarserThanRange.synthetic counter via remote_write, swept
stepfrom 30s–500s acrossrate/irate/deriv/delta/idelta/increase/resets, confirmedevery function returns data with correct values at every step
(previously empty at/around
step >= range).rate/delta/increase/resetsmatch Prometheus byte-for-byteacross a 75-step sweep.
irate/derivmatch closely but not alwaysexactly, and
ideltanumerically differs at most resolutions — bothare a pre-existing, separate limitation of
DownsampleHintsPlanner(it hands the engine a resampled proxyseries tagged with the bucket boundary rather than the real sample
timestamp, which these two-point functions are sensitive to),
present identically whether or not this fix's cap is even engaged,
so out of scope for this PR.
🤖 Generated with Claude Code