Repository: VictoriaMetrics/VictoriaMetrics
Pull request #1370 (2021-06-11 07:31:35)

vmalert: fix mistake with object reuse while parsing response
During the refactoring, the wrong optimisations was applied in
parse function which caused unintentional metric fields reset. 
The change removes this optimisation.

https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1369

codecov[bot]: # [Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) Report
> Merging [#1370](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) (abbf698) into [master](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/67b17cdd68027d0f96a479664764cdf9e2aa8792?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) (67b17cd) will **decrease** coverage by `0.02%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head abbf698 differs from pull request most recent head 764ee58. Consider uploading reports for the commit 764ee58 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370/graphs/tree.svg?width=650&height=150&src=pr&token=YND8dYVcXn&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics)](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics)

```diff
@@            Coverage Diff             @@
##           master    #1370      +/-   ##
==========================================
- Coverage   60.96%   60.93%   -0.03%     
==========================================
  Files         222      222              
  Lines       26359    26355       -4     
==========================================
- Hits        16069    16059      -10     
- Misses       8801     8806       +5     
- Partials     1489     1490       +1     
```


| [Impacted Files](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) | Coverage Δ | |
|---|---|---|
| [app/vmalert/datasource/vm\_prom\_api.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-YXBwL3ZtYWxlcnQvZGF0YXNvdXJjZS92bV9wcm9tX2FwaS5nbw==) | `86.11% <100.00%> (-0.74%)` | :arrow_down: |
| [app/vmalert/remotewrite/remotewrite.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-YXBwL3ZtYWxlcnQvcmVtb3Rld3JpdGUvcmVtb3Rld3JpdGUuZ28=) | `57.00% <0.00%> (-1.87%)` | :arrow_down: |
| [lib/mergeset/table.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-bGliL21lcmdlc2V0L3RhYmxlLmdv) | `68.82% <0.00%> (-1.33%)` | :arrow_down: |
| [lib/storage/partition.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-bGliL3N0b3JhZ2UvcGFydGl0aW9uLmdv) | `65.40% <0.00%> (-0.24%)` | :arrow_down: |
| [lib/storage/index\_db.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-bGliL3N0b3JhZ2UvaW5kZXhfZGIuZ28=) | `67.01% <0.00%> (+0.15%)` | :arrow_up: |
| [app/vmselect/promql/aggr\_incremental.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-YXBwL3Ztc2VsZWN0L3Byb21xbC9hZ2dyX2luY3JlbWVudGFsLmdv) | `89.53% <0.00%> (+1.55%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics). Last update [67b17cd...764ee58](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/1370?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).


Patch:
diff --git a/app/vmalert/datasource/vm_prom_api.go b/app/vmalert/datasource/vm_prom_api.go
index 6373b4ce4..e1e1b4937 100644
--- a/app/vmalert/datasource/vm_prom_api.go
+++ b/app/vmalert/datasource/vm_prom_api.go
@@ -34,23 +34,18 @@ type promRange struct {
 
 func (r promInstant) metrics() ([]Metric, error) {
 	var result []Metric
-	var m Metric
 	for i, res := range r.Result {
 		f, err := strconv.ParseFloat(res.TV[1].(string), 64)
 		if err != nil {
 			return nil, fmt.Errorf("metric %v, unable to parse float64 from %s: %w", res, res.TV[1], err)
 		}
-		m.Labels = nil
+		var m Metric
 		for k, v := range r.Result[i].Labels {
 			m.AddLabel(k, v)
 		}
 		m.Timestamps = append(m.Timestamps, int64(res.TV[0].(float64)))
 		m.Values = append(m.Values, f)
 		result = append(result, m)
-
-		m.Values = m.Values[:0]
-		m.Labels = m.Labels[:0]
-		m.Timestamps = m.Timestamps[:0]
 	}
 	return result, nil
 }
diff --git a/lib/storage/partition.go b/lib/storage/partition.go
index 22ace7175..3288112f2 100644
--- a/lib/storage/partition.go
+++ b/lib/storage/partition.go
@@ -79,10 +79,10 @@ const finalPartsToMerge = 3
 // Higher number of shards reduces CPU contention and increases the max bandwidth on multi-core systems.
 var rawRowsShardsPerPartition = (cgroup.AvailableCPUs() + 7) / 8
 
-// getMaxRowsPerPartition returns the maximum number of rows that haven't been converted into parts yet.
-func getMaxRawRowsPerPartition() int {
+// getMaxRawRowsPerShard returns the maximum number of rows that haven't been converted into parts yet.
+func getMaxRawRowsPerShard() int {
 	maxRawRowsPerPartitionOnce.Do(func() {
-		n := memory.Allowed() / 256 / int(unsafe.Sizeof(rawRow{}))
+		n := memory.Allowed() / rawRowsShardsPerPartition / 256 / int(unsafe.Sizeof(rawRow{}))
 		if n < 1e4 {
 			n = 1e4
 		}
@@ -515,7 +515,7 @@ type rawRows struct {
 }
 
 func getRawRowsMaxSize() *rawRows {
-	size := getMaxRawRowsPerPartition()
+	size := getMaxRawRowsPerShard()
 	return getRawRowsWithSize(size)
 }
 


Test patch:
diff --git a/app/vmalert/datasource/vm_test.go b/app/vmalert/datasource/vm_test.go
index 622a1e34a..c86deb3cd 100644
--- a/app/vmalert/datasource/vm_test.go
+++ b/app/vmalert/datasource/vm_test.go
@@ -66,7 +66,7 @@ func TestVMInstantQuery(t *testing.T) {
 		case 5:
 			w.Write([]byte(`{"status":"success","data":{"resultType":"matrix"}}`))
 		case 6:
-			w.Write([]byte(`{"status":"success","data":{"resultType":"vector","result":[{"metric":{"__name__":"vm_rows"},"value":[1583786142,"13763"]}]}}`))
+			w.Write([]byte(`{"status":"success","data":{"resultType":"vector","result":[{"metric":{"__name__":"vm_rows"},"value":[1583786142,"13763"]},{"metric":{"__name__":"vm_requests"},"value":[1583786140,"2000"]}]}}`))
 		}
 	})
 
@@ -100,16 +100,23 @@ func TestVMInstantQuery(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected %s", err)
 	}
-	if len(m) != 1 {
-		t.Fatalf("expected 1 metric  got %d in %+v", len(m), m)
+	if len(m) != 2 {
+		t.Fatalf("expected 2 metrics got %d in %+v", len(m), m)
 	}
-	expected := Metric{
-		Labels:     []Label{{Value: "vm_rows", Name: "__name__"}},
-		Timestamps: []int64{1583786142},
-		Values:     []float64{13763},
+	expected := []Metric{
+		{
+			Labels:     []Label{{Value: "vm_rows", Name: "__name__"}},
+			Timestamps: []int64{1583786142},
+			Values:     []float64{13763},
+		},
+		{
+			Labels:     []Label{{Value: "vm_requests", Name: "__name__"}},
+			Timestamps: []int64{1583786140},
+			Values:     []float64{2000},
+		},
 	}
-	if !reflect.DeepEqual(m[0], expected) {
-		t.Fatalf("unexpected metric %+v want %+v", m[0], expected)
+	if !reflect.DeepEqual(m, expected) {
+		t.Fatalf("unexpected metric %+v want %+v", m, expected)
 	}
 
 	g := NewGraphiteType()
@@ -122,12 +129,12 @@ func TestVMInstantQuery(t *testing.T) {
 	if len(m) != 1 {
 		t.Fatalf("expected 1 metric  got %d in %+v", len(m), m)
 	}
-	expected = Metric{
+	exp := Metric{
 		Labels:     []Label{{Value: "constantLine(10)", Name: "name"}},
 		Timestamps: []int64{1611758403},
 		Values:     []float64{10},
 	}
-	if !reflect.DeepEqual(m[0], expected) {
+	if !reflect.DeepEqual(m[0], exp) {
 		t.Fatalf("unexpected metric %+v want %+v", m[0], expected)
 	}
 }
diff --git a/lib/storage/table_search_timing_test.go b/lib/storage/table_search_timing_test.go
index 927b54c40..cbe939d4b 100644
--- a/lib/storage/table_search_timing_test.go
+++ b/lib/storage/table_search_timing_test.go
@@ -110,7 +110,7 @@ func createBenchTable(b *testing.B, path string, startTimestamp int64, rowsPerIn
 
 func benchmarkTableSearch(b *testing.B, rowsCount, tsidsCount, tsidsSearch int, fetchData bool) {
 	startTimestamp := timestampFromTime(time.Now()) - 365*24*3600*1000
-	rowsPerInsert := getMaxRawRowsPerPartition()
+	rowsPerInsert := getMaxRawRowsPerShard()
 
 	tb := openBenchTable(b, startTimestamp, rowsPerInsert, rowsCount, tsidsCount)
 	tr := TimeRange{