From 47872ada7e52e1139ce7dc61f8088bb11f4ac840 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 b0abb1f7e..64efb07eb 100644 --- a/app/vmselect/promql/eval.go +++ b/app/vmselect/promql/eval.go @@ -455,12 +455,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 df7d27cc9..50ca32ec8 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)