Repository: VictoriaMetrics/VictoriaMetrics
Pull request #606 (2020-07-04 09:27:09)

fix adjust last points avoid influence earlier value
My case just like this test
```
{
	Timestamps: []int64{100, 200, 300, 400, 500},
	Values:     []float64{1, 2, nan, nan, nan},
},
```
Then `queryOffset` is 300, the previous output was
```
{
	Timestamps: []int64{100, 200, 300, 400, 500},
	Values:     []float64{1, 2, 3, 3, nan},
}
```
After this patch, the output was
```
{
	Timestamps: []int64{100, 200, 300, 400, 500},
	Values:     []float64{1, 2, nan, nan, nan},
}
```

I think we need to determine  the time of the NonNaN value exceeds `queryOffset`, rather than simply judging the end time of the metric.


Patch:
diff --git a/app/vmselect/prometheus/prometheus.go b/app/vmselect/prometheus/prometheus.go
index eb106f9b0..19c93c26d 100644
--- a/app/vmselect/prometheus/prometheus.go
+++ b/app/vmselect/prometheus/prometheus.go
@@ -792,7 +792,7 @@ func queryRangeHandler(w http.ResponseWriter, query string, start, end, step int
 	}
 	queryOffset := getLatencyOffsetMilliseconds()
 	if ct-end < queryOffset {
-		result = adjustLastPoints(result)
+		result = adjustLastPoints(result, ct, queryOffset)
 	}
 
 	// Remove NaN values as Prometheus does.
@@ -839,7 +839,7 @@ var queryRangeDuration = metrics.NewSummary(`vm_request_duration_seconds{path="/
 
 // adjustLastPoints substitutes the last point values with the previous
 // point values, since the last points may contain garbage.
-func adjustLastPoints(tss []netstorage.Result) []netstorage.Result {
+func adjustLastPoints(tss []netstorage.Result, ct, queryOffset int64) []netstorage.Result {
 	if len(tss) == 0 {
 		return nil
 	}
@@ -865,12 +865,18 @@ func adjustLastPoints(tss []netstorage.Result) []netstorage.Result {
 	// with the previous values for each timeseries.
 	for i := range tss {
 		values := tss[i].Values
-		for j := 0; j < 2; j++ {
-			idx := lastNonNaNIdx + j
-			if idx <= 0 || idx >= len(values) || math.IsNaN(values[idx-1]) {
-				continue
+		if lastNonNaNIdx > len(values) {
+			continue
+		}
+		end := tss[i].Timestamps[lastNonNaNIdx]
+		if ct-end < queryOffset {
+			for j := 1; j < 3; j++ {
+				idx := lastNonNaNIdx + j
+				if idx <= 0 || idx >= len(values) || math.IsNaN(values[idx-1]) {
+					continue
+				}
+				values[idx] = values[idx-1]
 			}
-			values[idx] = values[idx-1]
 		}
 	}
 	return tss


Test patch:
diff --git a/app/vmselect/prometheus/prometheus_test.go b/app/vmselect/prometheus/prometheus_test.go
index 69c2c1237..e1ce421f7 100644
--- a/app/vmselect/prometheus/prometheus_test.go
+++ b/app/vmselect/prometheus/prometheus_test.go
@@ -113,3 +113,130 @@ func TestGetTimeError(t *testing.T) {
 	f("-292273086-05-16T16:47:07Z")
 	f("292277025-08-18T07:12:54.999999998Z")
 }
+
+func TestAdjustLastPoints(t *testing.T) {
+	f := func(tss []netstorage.Result, ct, queryOffset int64, tssExpected []netstorage.Result) {
+		t.Helper()
+		tss = adjustLastPoints(tss, ct, queryOffset)
+		for i, ts := range tss {
+			for j, value := range ts.Values {
+				expectedValue := tssExpected[i].Values[j]
+				if math.IsNaN(expectedValue) {
+					if !math.IsNaN(value) {
+						t.Fatalf("unexpected result; got %v; want nan", value)
+					}
+				} else if expectedValue != value {
+					t.Fatalf("unexpected result; got %v; want %v", value, expectedValue)
+				}
+			}
+			if !reflect.DeepEqual(ts.Timestamps, tssExpected[i].Timestamps) {
+				t.Fatalf("unexpected result; got %v; want %v", tss, tssExpected)
+			}
+		}
+
+	}
+
+	nan := math.NaN()
+
+	f(nil, 500, 300, nil)
+
+	f([]netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, 4, nan},
+		},
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, nan, nan},
+		},
+	}, 500, 300, []netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, 4, 4},
+		},
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, nan, nan},
+		},
+	})
+
+	f([]netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, nan, nan},
+		},
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, nan, nan, nan},
+		},
+	}, 500, 300, []netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, 3, 3},
+		},
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, nan, nan, nan},
+		},
+	})
+
+	f([]netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, nan, nan, nan},
+		},
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, nan, nan, nan, nan},
+		},
+	}, 500, 300, []netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, nan, nan, nan},
+		},
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, nan, nan, nan, nan},
+		},
+	})
+
+	f([]netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, 4, nan},
+		},
+		{
+			Timestamps: []int64{100, 200, 300, 400},
+			Values:     []float64{1, 2, 3, 4},
+		},
+	}, 500, 300, []netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, 4, 4},
+		},
+		{
+			Timestamps: []int64{100, 200, 300, 400},
+			Values:     []float64{1, 2, 3, 4},
+		},
+	})
+
+	f([]netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, nan, nan},
+		},
+		{
+			Timestamps: []int64{100, 200, 300},
+			Values:     []float64{1, 2, nan},
+		},
+	}, 500, 300, []netstorage.Result{
+		{
+			Timestamps: []int64{100, 200, 300, 400, 500},
+			Values:     []float64{1, 2, 3, 3, 3},
+		},
+		{
+			Timestamps: []int64{100, 200, 300},
+			Values:     []float64{1, 2, nan},
+		},
+	})
+}