Repository: VictoriaMetrics/VictoriaMetrics
Pull request #607 (2020-07-06 20:06:30)

lib/protoparser: fix metric name of unmarshal errors in promremotewrite
The change fixes the typo in metric name `vm_protoparser_unmarshal_errors` to
respect the naming standard.

codecov[bot]: # [Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607?src=pr&el=h1) Report
> Merging [#607](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607?src=pr&el=desc) into [master](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/a23806f486d72cbc43623356537dc09487446281&el=desc) will **decrease** coverage by `0.17%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607/graphs/tree.svg?width=650&height=150&src=pr&token=YND8dYVcXn)](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #607      +/-   ##
==========================================
- Coverage   68.10%   67.92%   -0.18%     
==========================================
  Files         156      156              
  Lines       21431    21431              
==========================================
- Hits        14595    14558      -37     
- Misses       5658     5697      +39     
+ Partials     1178     1176       -2     
```


| [Impacted Files](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [lib/storage/storage.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607/diff?src=pr&el=tree#diff-bGliL3N0b3JhZ2Uvc3RvcmFnZS5nbw==) | `74.21% <100.00%> (ø)` | |
| [lib/mergeset/block\_stream\_reader.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607/diff?src=pr&el=tree#diff-bGliL21lcmdlc2V0L2Jsb2NrX3N0cmVhbV9yZWFkZXIuZ28=) | `50.96% <0.00%> (-11.62%)` | :arrow_down: |
| [app/vmselect/promql/aggr\_incremental.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607/diff?src=pr&el=tree#diff-YXBwL3Ztc2VsZWN0L3Byb21xbC9hZ2dyX2luY3JlbWVudGFsLmdv) | `90.72% <0.00%> (-2.75%)` | :arrow_down: |
| [app/vmalert/remotewrite/remotewrite.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607/diff?src=pr&el=tree#diff-YXBwL3ZtYWxlcnQvcmVtb3Rld3JpdGUvcmVtb3Rld3JpdGUuZ28=) | `61.01% <0.00%> (-1.70%)` | :arrow_down: |
| [lib/mergeset/table.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607/diff?src=pr&el=tree#diff-bGliL21lcmdlc2V0L3RhYmxlLmdv) | `66.75% <0.00%> (-1.26%)` | :arrow_down: |
| [lib/storage/index\_db.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607/diff?src=pr&el=tree#diff-bGliL3N0b3JhZ2UvaW5kZXhfZGIuZ28=) | `75.67% <0.00%> (-0.11%)` | :arrow_down: |
| [lib/storage/part.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607/diff?src=pr&el=tree#diff-bGliL3N0b3JhZ2UvcGFydC5nbw==) | `74.48% <0.00%> (+1.37%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607?src=pr&el=footer). Last update [12b1607...07b20d7](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/607?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/lib/protoparser/graphite/streamparser.go b/lib/protoparser/graphite/streamparser.go
index 825ffdcb7..cdca2a3ed 100644
--- a/lib/protoparser/graphite/streamparser.go
+++ b/lib/protoparser/graphite/streamparser.go
@@ -75,7 +75,7 @@ func (ctx *streamContext) Read(r io.Reader) bool {
 	currentTimestamp := int64(fasttime.UnixTimestamp())
 	for i := range rows {
 		r := &rows[i]
-		if r.Timestamp == 0 {
+		if r.Timestamp == 0 || r.Timestamp == -1 {
 			r.Timestamp = currentTimestamp
 		}
 	}
diff --git a/lib/protoparser/promremotewrite/streamparser.go b/lib/protoparser/promremotewrite/streamparser.go
index 5f7268180..19e75d2e2 100644
--- a/lib/protoparser/promremotewrite/streamparser.go
+++ b/lib/protoparser/promremotewrite/streamparser.go
@@ -65,7 +65,7 @@ var (
 	readCalls       = metrics.NewCounter(`vm_protoparser_read_calls_total{type="promremotewrite"}`)
 	readErrors      = metrics.NewCounter(`vm_protoparser_read_errors_total{type="promremotewrite"}`)
 	rowsRead        = metrics.NewCounter(`vm_protoparser_rows_read_total{type="promremotewrite"}`)
-	unmarshalErrors = metrics.NewCounter(`vm_protoparser_unmarshal_errors{type="promremotewrite"}`)
+	unmarshalErrors = metrics.NewCounter(`vm_protoparser_unmarshal_errors_total{type="promremotewrite"}`)
 )
 
 func getPushCtx() *pushCtx {
diff --git a/lib/storage/storage.go b/lib/storage/storage.go
index 8d26ab862..04e306b92 100644
--- a/lib/storage/storage.go
+++ b/lib/storage/storage.go
@@ -1082,7 +1082,8 @@ func (s *Storage) add(rows []rawRow, mrs []MetricRow, precisionBits uint8) ([]ra
 		if mr.Timestamp < minTimestamp {
 			// Skip rows with too small timestamps outside the retention.
 			if firstWarn == nil {
-				firstWarn = fmt.Errorf("cannot insert row with too small timestamp %d outside the retention; minimum allowed timestamp is %d",
+				firstWarn = fmt.Errorf("cannot insert row with too small timestamp %d outside the retention; minimum allowed timestamp is %d; "+
+					"probably you need updating -retentionPeriod command-line flag",
 					mr.Timestamp, minTimestamp)
 			}
 			atomic.AddUint64(&s.tooSmallTimestampRows, 1)
@@ -1091,7 +1092,8 @@ func (s *Storage) add(rows []rawRow, mrs []MetricRow, precisionBits uint8) ([]ra
 		if mr.Timestamp > maxTimestamp {
 			// Skip rows with too big timestamps significantly exceeding the current time.
 			if firstWarn == nil {
-				firstWarn = fmt.Errorf("cannot insert row with too big timestamp %d exceeding the current time; maximum allowd timestamp is %d",
+				firstWarn = fmt.Errorf("cannot insert row with too big timestamp %d exceeding the current time; maximum allowd timestamp is %d; "+
+					"propbably you need updating -retentionPeriod command-line flag",
 					mr.Timestamp, maxTimestamp)
 			}
 			atomic.AddUint64(&s.tooBigTimestampRows, 1)


Test patch:
diff --git a/lib/protoparser/graphite/parser_test.go b/lib/protoparser/graphite/parser_test.go
index ca8bcccb1..6c299d0f7 100644
--- a/lib/protoparser/graphite/parser_test.go
+++ b/lib/protoparser/graphite/parser_test.go
@@ -1,7 +1,9 @@
 package graphite
 
 import (
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime"
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -84,6 +86,13 @@ func TestRowsUnmarshalSuccess(t *testing.T) {
 			Value:  1123,
 		}},
 	})
+	f("aaa 1123 -1", &Rows{
+		Rows: []Row{{
+			Metric:    "aaa",
+			Value:     1123,
+			Timestamp: -1,
+		}},
+	})
 
 	// Timestamp bigger than 1<<31
 	f("aaa 1123 429496729600", &Rows{
@@ -161,3 +170,56 @@ func TestRowsUnmarshalSuccess(t *testing.T) {
 		},
 	})
 }
+
+func Test_streamContext_Read(t *testing.T) {
+	f := func(s string, rowsExpected *Rows) {
+		t.Helper()
+		ctx := &streamContext{}
+		ctx.Read(strings.NewReader(s))
+		if len(ctx.Rows.Rows) != len(rowsExpected.Rows) {
+			t.Fatalf("different len of expected rows;\ngot\n%+v;\nwant\n%+v", ctx.Rows, rowsExpected.Rows)
+		}
+		if !reflect.DeepEqual(ctx.Rows.Rows, rowsExpected.Rows) {
+			t.Fatalf("unexpected rows;\ngot\n%+v;\nwant\n%+v", ctx.Rows.Rows, rowsExpected.Rows)
+		}
+	}
+
+	// Full line without tags
+	f("aaa 1123 345", &Rows{
+		Rows: []Row{{
+			Metric:    "aaa",
+			Value:     1123,
+			Timestamp: 345 * 1000,
+		}},
+	})
+	// Full line with tags
+	f("aaa;x=y 1123 345", &Rows{
+		Rows: []Row{{
+			Metric: "aaa",
+			Tags: []Tag{{
+				Key:   "x",
+				Value: "y",
+			}},
+			Value:     1123,
+			Timestamp: 345 * 1000,
+		}},
+	})
+	// missing timestamp.
+	// Note that this test may be flaky due to timing issues. TODO: fix it
+	f("aaa 1123", &Rows{
+		Rows: []Row{{
+			Metric:    "aaa",
+			Value:     1123,
+			Timestamp: int64(fasttime.UnixTimestamp()) * 1000,
+		}},
+	})
+	// -1 timestamp. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/610
+	// Note that this test may be flaky due to timing issues. TODO: fix it.
+	f("aaa 1123 -1", &Rows{
+		Rows: []Row{{
+			Metric:    "aaa",
+			Value:     1123,
+			Timestamp: int64(fasttime.UnixTimestamp()) * 1000,
+		}},
+	})
+}