From ff5df0a5aaadb42f4ba54fb613d1cb8b03fb111f Mon Sep 17 00:00:00 2001 From: Hui Wang Date: Sun, 21 Jan 2024 08:53:29 +0800 Subject: [PATCH] =?UTF-8?q?app/vmselect/promql:=20properly=20handle=20poss?= =?UTF-8?q?ible=20negative=20results=20caused=E2=80=A6=20(#5608)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * app/vmselect/promql: properly handle possible negative results caused by float operations precision error in rollup functions like rate() or increase() * fix test --- app/vmselect/promql/rollup.go | 5 +++++ app/vmselect/promql/rollup_test.go | 13 ++++++++++++- docs/CHANGELOG.md | 1 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/vmselect/promql/rollup.go b/app/vmselect/promql/rollup.go index 135c0ec0a0..90189dbe89 100644 --- a/app/vmselect/promql/rollup.go +++ b/app/vmselect/promql/rollup.go @@ -779,6 +779,11 @@ func removeCounterResets(values []float64) { } prevValue = v values[i] = v + correction + // Check again, there could be precision error in float operations, + // see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5571 + if i > 0 && values[i] < values[i-1] { + values[i] = values[i-1] + } } } diff --git a/app/vmselect/promql/rollup_test.go b/app/vmselect/promql/rollup_test.go index 55e7297c74..48f3ddfd16 100644 --- a/app/vmselect/promql/rollup_test.go +++ b/app/vmselect/promql/rollup_test.go @@ -96,7 +96,7 @@ func TestRemoveCounterResets(t *testing.T) { // removeCounterResets doesn't expect negative values, so it doesn't work properly with them. values = []float64{-100, -200, -300, -400} removeCounterResets(values) - valuesExpected = []float64{-100, -300, -600, -1000} + valuesExpected = []float64{-100, -100, -100, -100} timestampsExpected := []int64{0, 1, 2, 3} testRowsEqual(t, values, timestampsExpected, valuesExpected, timestampsExpected) @@ -107,6 +107,17 @@ func TestRemoveCounterResets(t *testing.T) { valuesExpected = []float64{100, 100, 125, 125, 145, 195} timestampsExpected = []int64{0, 1, 2, 3, 4, 5} testRowsEqual(t, values, timestampsExpected, valuesExpected, timestampsExpected) + + // verify results always increase monotonically with possible float operations precision error + values = []float64{34.094223, 2.7518, 2.140669, 0.044878, 1.887095, 2.546569, 2.490149, 0.045, 0.035684, 0.062454, 0.058296} + removeCounterResets(values) + var prev float64 + for i, v := range values { + if v < prev { + t.Fatalf("error: unexpected value keep getting bigger %d; cur %v; pre %v\n", i, v, prev) + } + prev = v + } } func TestDeltaValues(t *testing.T) { diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index b8f197d16a..27239e2b2f 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -17,6 +17,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * BUGFIX: [vmselect](https://docs.victoriametrics.com/vmselect.html): properly determine time range search for instant queries with too big look-behind window like `foo[100y]`. Previously, such queries could return empty responses even if `foo` is present in database. * BUGFIX: properly return errors from [export APIs](https://docs.victoriametrics.com/#how-to-export-time-series). Previously these errors were silently suppressed. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5649). +* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly handle possible negative results caused by float operations precision error in rollup functions like rate() or increase(). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5571). ## [v1.87.13](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.87.13)