search

prometheus / prometheus

The Prometheus monitoring system and time series database.
https://prometheus.io/
Apache License 2.0
53.7k stars 8.9k forks source link

SelectHints should contain function parameters in addition to the function name #9979

Open fpetkovski opened 2 years ago

fpetkovski commented 2 years ago

Proposal

Use case. Why is this important?

We would like to optimize the retrieval of SeriesSets by pre-computing the result for certain aggregations. This is currently possible to do for aggregations like max and min, but not for topk and bottomk because the latter two aggregations have an additional parameter which is not provided in the SelectHints: https://github.com/prometheus/prometheus/blob/655e2d28795ff5b1532778d7ce2f014e4a76da6f/promql/engine.go#L777-L783

I am more than happy to make the contribution myself, I just wanted to know whether there are any objections to having the parameters in addition to the function name.

roidelapluie commented 2 years ago

As per previous dev summit, I'd prefer to have a design doc about how we would shard promql queries in the future, instead of changing the protocol right away. I do not know when/who would work on that design doc.

If we do not foresee this in a relativaly close future, we could add this.

bwplotka commented 1 year ago

Ack, we can try combining all we need to add to hints into one proposal to make sure Thanos can use the same APIs, PromQL engine and Prometheus engine can responds to those hints results.