Add query cost estimation and limits proposal#89
Conversation
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
| } | ||
| ``` | ||
|
|
||
| Note: `cost=1` adds a second index lookup on top of executing the query. |
There was a problem hiding this comment.
nit: why cost=1 and not cost=true? Do values of cost other than 0 and 1 have a meaning?
There was a problem hiding this comment.
Maybe we can have cost=2 later that e.g. read chunks metadata ?
| query_max_duration: 0s | ||
| ``` | ||
|
|
||
| These are enforced *during* execution against the query's actual running cost, not against the estimate: a query is rejected as soon as it loads too many series or scans too many samples, and `query_max_duration` surfaces as a query timeout. A client may lower any ceiling for a single request via `max_series`, `max_samples_scanned`, `max_query_duration`; these can only tighten, never loosen, the operator-set value. The estimate is never used to reject a query — enforcement is always on the real cost. |
There was a problem hiding this comment.
If a request tries to override these limits to higher values than what is set on the server, is the request rejected? Or are the requested limits ignored?
There was a problem hiding this comment.
"A client may lower any ceiling". The request is not rejected but you will be capped at the server's limit.
| global: | ||
| query_max_series: 0 # 0 = no limit | ||
| query_max_samples_scanned: 0 | ||
| query_max_duration: 0s |
There was a problem hiding this comment.
Is this different to the existing -query.timeout flag and timeout URL parameter?
There was a problem hiding this comment.
It is not, more a normalization of it.
|
|
||
| Three pieces, all gated by `--enable-feature=query-cost`. | ||
|
|
||
| **1. Estimation (`promql.EstimateCost`).** Parse the query, walk it for every vector and matrix selector, compute the effective time window each selector reads (mirroring the engine's `getTimeRangesForSelector`/`populateSeries`), and ask storage for the series count per selector via a single querier over the union window. `SeriesTouched` is the sum across selectors — an upper bound, because a series shared between selectors is counted once per selector. `SamplesScanned` models the engine's incremental per-step reads (full range window at step 0, then only the samples that advance past the previous cutoff), scaled by a measured average per-point cost so native-histogram points are sized by bucket rather than counted as one float unit. The estimate is index-only apart from decoding at most `histogramSampleLimit` (50) points per selector to size histograms. |
There was a problem hiding this comment.
Can you elaborate on how the sample count estimation would work? Would this require decoding each chunk, or would it use the sample count stored on each chunk to avoid decoding chunks?
There was a problem hiding this comment.
There are 2 options.
In Prometheus we can take the scrape interval for estimationm,
or we can estimate based on the number of chunks and do x120.
Looking at the chunks header would be almost as I/O expensive as running the query.
|
|
||
| Three pieces, all gated by `--enable-feature=query-cost`. | ||
|
|
||
| **1. Estimation (`promql.EstimateCost`).** Parse the query, walk it for every vector and matrix selector, compute the effective time window each selector reads (mirroring the engine's `getTimeRangesForSelector`/`populateSeries`), and ask storage for the series count per selector via a single querier over the union window. `SeriesTouched` is the sum across selectors — an upper bound, because a series shared between selectors is counted once per selector. `SamplesScanned` models the engine's incremental per-step reads (full range window at step 0, then only the samples that advance past the previous cutoff), scaled by a measured average per-point cost so native-histogram points are sized by bucket rather than counted as one float unit. The estimate is index-only apart from decoding at most `histogramSampleLimit` (50) points per selector to size histograms. |
There was a problem hiding this comment.
If I'm understanding correctly, the goal is to roughly estimate the value of the "samples read" statistic introduced in prometheus/prometheus#18081 (rather than the "total samples" statistic that existed before that PR). Is that correct? If so, it might be worth mentioning that here.
| query_max_duration: 0s | ||
| ``` | ||
|
|
||
| These are enforced *during* execution against the query's actual running cost, not against the estimate: a query is rejected as soon as it loads too many series or scans too many samples, and `query_max_duration` surfaces as a query timeout. A client may lower any ceiling for a single request via `max_series`, `max_samples_scanned`, `max_query_duration`; these can only tighten, never loosen, the operator-set value. The estimate is never used to reject a query — enforcement is always on the real cost. |
There was a problem hiding this comment.
I wonder if it might actually make sense to enforce the limit based on the estimation, rather than allowing the query to run up until a limit is hit.
Feels wasteful to let a query that will likely be limited, run and fetch data.
There was a problem hiding this comment.
I'd say it will depend on how we can accurately estimate the limit upfront.
3 participants
No description provided.