From c86286ec1d8b4e285304dfa54455274732ecede1 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Sun, 27 Dec 2020 14:09:22 +0200 Subject: [PATCH] app/vmselect/promql: do not ajdust `offset` value provided in the query Previously it could be modified in order to improve response cache hit ratio. This is unneeded, since cache hit ratio should remain good because the query time range should be already aligned to multiple of `step` values. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/976 --- app/vmselect/promql/eval.go | 10 ++++------ docs/CHANGELOG.md | 1 + 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/vmselect/promql/eval.go b/app/vmselect/promql/eval.go index bd94207a79..c923815790 100644 --- a/app/vmselect/promql/eval.go +++ b/app/vmselect/promql/eval.go @@ -439,12 +439,10 @@ func evalRollupFunc(ec *EvalConfig, name string, rf rollupFunc, expr metricsql.E ecNew = newEvalConfig(ecNew) ecNew.Start -= offset ecNew.End -= offset - if ecNew.MayCache { - start, end := AdjustStartEnd(ecNew.Start, ecNew.End, ecNew.Step) - offset += ecNew.Start - start - ecNew.Start = start - ecNew.End = end - } + // There is no need in calling AdjustStartEnd() on ecNew if ecNew.MayCache is set to true, + // since the time range alignment has been already performed by the caller, + // so cache hit rate should be quite good. + // See also https://github.com/VictoriaMetrics/VictoriaMetrics/issues/976 } if name == "rollup_candlestick" { // Automatically apply `offset -step` to `rollup_candlestick` function diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index df7d27cc90..50ca32ec85 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,6 +9,7 @@ * BUGFIX: vmalert: properly escape multiline queries when passing them to Grafana. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/890 * BUGFIX: vmagent: set missing `__meta_kubernetes_service_*` labels in `kubernetes_sd_config` for `endpoints` and `endpointslices` roles. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/982 +* BUGFIX: do not adjust `offset` value provided in MetricsQL query. Previously it could be modified in order to improve response cache hit ratio. This is unneeded, since cache hit ratio should remain good because the query time range should be already aligned to multiple of `step` values. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/976 # [v1.50.2](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.50.2)