go-graphite / graphite-clickhouse

Graphite cluster backend with ClickHouse support
MIT License
213 stars 52 forks source link

Improve from/until calculation #251

Closed ikosenn closed 10 months ago

ikosenn commented 10 months ago

closes #248

Felixoid commented 10 months ago

This place is very hard tuned. And step is nothing provided with requests, it only can be calculated from the data retention. So it won't work

ikosenn commented 10 months ago

That's correct. I am still using step value calculated from data retention. The only thing I am preventing it from doing is using step calculated from maxDataPoints here https://github.com/go-graphite/graphite-clickhouse/blob/1db4b3d740c0f3e72ed8a3449603f4a016b5dda8/render/data/query.go#L344 to compute the from and to values. If you have a small maxDataPoint the value of step becomes huge. Performing a ceil or floor calculation leads to huge diff.

Here is an example using the retention schema provided in the ticket.

step = dry.Max(rStep, dry.Ceil(c.Until-c.From, c.MaxDataPoints))
# step = dry.Max(30, dry.Ceil(1697429820-1694837820, 1))
# step = 2592000
c.step = dry.CeilToMultiplier(step, rStep)
# c.step = dry.CeilToMultiplier(2592000, 30)
# c.step = 2592000

If you use c.step above to calculate c.from you'll end up with 1695168000 which is 4 days after the initial From value

In the PR c.step and c.rStep will be the same in cases where maxDataPoint is not provided. When we have maxDataPoint we use the step calculated from that on the Resample function and use the step from data retention to calculate the time ranges.

Felixoid commented 10 months ago

But nothing supports a query parameter step. Basically, you want to override the supported parameter by unsupported. And what you need is to increase the maxDataPoints instead, right? Having it like 30+