app/vmselect/promql: properly handle possible negative results caused… (#5608)

* app/vmselect/promql: properly handle possible negative results caused by float operations precision error in rollup functions like rate() or increase()

* fix test
This commit is contained in:
Hui Wang 2024-01-21 08:53:29 +08:00 committed by Aliaksandr Valialkin
parent d4848f6834
commit 979bec75ff
No known key found for this signature in database
GPG key ID: 52C003EE2BCDB9EB
4 changed files with 26 additions and 9 deletions

View file

@ -7809,10 +7809,10 @@ func TestExecSuccess(t *testing.T) {
})
t.Run(`rollup_rate()`, func(t *testing.T) {
t.Parallel()
q := `rollup_rate((2000-time())[600s])`
q := `rollup_rate((2200-time())[600s])`
r1 := netstorage.Result{
MetricName: metricNameExpected,
Values: []float64{5, 4, 3, 2, 1, 0},
Values: []float64{6, 5, 4, 3, 2, 1},
Timestamps: timestampsExpected,
}
r1.MetricName.Tags = []storage.Tag{{
@ -7821,7 +7821,7 @@ func TestExecSuccess(t *testing.T) {
}}
r2 := netstorage.Result{
MetricName: metricNameExpected,
Values: []float64{6, 5, 4, 3, 2, 1},
Values: []float64{7, 6, 5, 4, 3, 2},
Timestamps: timestampsExpected,
}
r2.MetricName.Tags = []storage.Tag{{
@ -7830,7 +7830,7 @@ func TestExecSuccess(t *testing.T) {
}}
r3 := netstorage.Result{
MetricName: metricNameExpected,
Values: []float64{4, 3, 2, 1, 0, -1},
Values: []float64{5, 4, 3, 2, 1, 0},
Timestamps: timestampsExpected,
}
r3.MetricName.Tags = []storage.Tag{{
@ -7842,10 +7842,10 @@ func TestExecSuccess(t *testing.T) {
})
t.Run(`rollup_rate(q, "max")`, func(t *testing.T) {
t.Parallel()
q := `rollup_rate((2000-time())[600s], "max")`
q := `rollup_rate((2200-time())[600s], "max")`
r := netstorage.Result{
MetricName: metricNameExpected,
Values: []float64{6, 5, 4, 3, 2, 1},
Values: []float64{7, 6, 5, 4, 3, 2},
Timestamps: timestampsExpected,
}
resultExpected := []netstorage.Result{r}
@ -7853,10 +7853,10 @@ func TestExecSuccess(t *testing.T) {
})
t.Run(`rollup_rate(q, "avg")`, func(t *testing.T) {
t.Parallel()
q := `rollup_rate((2000-time())[600s], "avg")`
q := `rollup_rate((2200-time())[600s], "avg")`
r := netstorage.Result{
MetricName: metricNameExpected,
Values: []float64{5, 4, 3, 2, 1, 0},
Values: []float64{6, 5, 4, 3, 2, 1},
Timestamps: timestampsExpected,
}
resultExpected := []netstorage.Result{r}

View file

@ -859,6 +859,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]
}
}
}

View file

@ -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) {

View file

@ -13,6 +13,7 @@ The following `tip` changes can be tested by building VictoriaMetrics components
* 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.93.10](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.93.10)