Repository: VictoriaMetrics/VictoriaMetrics
Pull request #221 (2019-11-03 20:14:15)

add unittests for bytesutil and storage

codecov-io: # [Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221?src=pr&el=h1) Report
> Merging [#221](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221?src=pr&el=desc) into [master](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/9becc26f4b39a5a990cc72998bc124b945bc83b9?src=pr&el=desc) will **increase** coverage by `0.3%`.
> The diff coverage is `n/a`.

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

```diff
@@           Coverage Diff            @@
##           master    #221     +/-   ##
========================================
+ Coverage    69.3%   69.6%   +0.3%     
========================================
  Files          96      96             
  Lines       16188   16188             
========================================
+ Hits        11219   11268     +49     
+ Misses       4018    3966     -52     
- Partials      951     954      +3
```


| [Impacted Files](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [lib/fs/dir\_remover.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221/diff?src=pr&el=tree#diff-bGliL2ZzL2Rpcl9yZW1vdmVyLmdv) | `3.57% <0%> (-25%)` | :arrow_down: |
| [lib/mergeset/table.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221/diff?src=pr&el=tree#diff-bGliL21lcmdlc2V0L3RhYmxlLmdv) | `69.03% <0%> (-0.29%)` | :arrow_down: |
| [app/vmselect/promql/aggr\_incremental.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221/diff?src=pr&el=tree#diff-YXBwL3Ztc2VsZWN0L3Byb21xbC9hZ2dyX2luY3JlbWVudGFsLmdv) | `97.05% <0%> (ø)` | :arrow_up: |
| [lib/storage/index\_db.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221/diff?src=pr&el=tree#diff-bGliL3N0b3JhZ2UvaW5kZXhfZGIuZ28=) | `72.75% <0%> (+0.29%)` | :arrow_up: |
| [lib/storage/metric\_name.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221/diff?src=pr&el=tree#diff-bGliL3N0b3JhZ2UvbWV0cmljX25hbWUuZ28=) | `75.89% <0%> (+14.79%)` | :arrow_up: |
| [lib/bytesutil/bytesutil.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221/diff?src=pr&el=tree#diff-bGliL2J5dGVzdXRpbC9ieXRlc3V0aWwuZ28=) | `84.61% <0%> (+53.84%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221?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/221?src=pr&el=footer). Last update [9becc26...59e56c8](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/221?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/lib/storage/index_db.go b/lib/storage/index_db.go
index 9eefb6e96..3e270fd37 100644
--- a/lib/storage/index_db.go
+++ b/lib/storage/index_db.go
@@ -264,8 +264,7 @@ func (db *indexDB) SetExtDB(extDB *indexDB) {
 	// Add deleted metricIDs from extDB to db.
 	if extDB != nil {
 		dmisExt := extDB.getDeletedMetricIDs()
-		metricIDs := dmisExt.AppendTo(nil)
-		db.updateDeletedMetricIDs(metricIDs)
+		db.updateDeletedMetricIDs(dmisExt)
 	}
 
 	db.extDBLock.Lock()
@@ -885,7 +884,11 @@ func (db *indexDB) DeleteTSIDs(tfss []*TagFilters) (int, error) {
 	deletedCount := len(metricIDs)
 
 	// atomically add deleted metricIDs to an inmemory map.
-	db.updateDeletedMetricIDs(metricIDs)
+	dmis := &uint64set.Set{}
+	for _, metricID := range metricIDs {
+		dmis.Add(metricID)
+	}
+	db.updateDeletedMetricIDs(dmis)
 
 	// Reset TagFilters -> TSIDS cache, since it may contain deleted TSIDs.
 	invalidateTagCache()
@@ -914,13 +917,11 @@ func (db *indexDB) setDeletedMetricIDs(dmis *uint64set.Set) {
 	db.deletedMetricIDs.Store(dmis)
 }
 
-func (db *indexDB) updateDeletedMetricIDs(metricIDs []uint64) {
+func (db *indexDB) updateDeletedMetricIDs(metricIDs *uint64set.Set) {
 	db.deletedMetricIDsUpdateLock.Lock()
 	dmisOld := db.getDeletedMetricIDs()
 	dmisNew := dmisOld.Clone()
-	for _, metricID := range metricIDs {
-		dmisNew.Add(metricID)
-	}
+	dmisNew.Union(metricIDs)
 	db.setDeletedMetricIDs(dmisNew)
 	db.deletedMetricIDsUpdateLock.Unlock()
 }
@@ -1298,10 +1299,10 @@ func (is *indexSearch) adjustMaxMetricsAdaptive(tr TimeRange, maxMetrics int) in
 		return maxMetrics
 	}
 	hourMetrics := hmPrev.m.Len()
-	if hourMetrics >= 256 && maxMetrics > hourMetrics/4 {
+	if maxMetrics > hourMetrics {
 		// It is cheaper to filter on the hour or day metrics if the minimum
-		// number of matching metrics across tfs exceeds hourMetrics / 4.
-		return hourMetrics / 4
+		// number of matching metrics across tfs exceeds hourMetrics.
+		return hourMetrics
 	}
 	return maxMetrics
 }
@@ -1584,9 +1585,7 @@ func (is *indexSearch) updateMetricIDsForTagFilters(metricIDs *uint64set.Set, tf
 		}
 		minMetricIDs = mIDs
 	}
-	for _, metricID := range minMetricIDs.AppendTo(nil) {
-		metricIDs.Add(metricID)
-	}
+	metricIDs.Union(minMetricIDs)
 	return nil
 }
 
@@ -1911,9 +1910,7 @@ func (is *indexSearch) getMetricIDsForRecentHours(tr TimeRange, maxMetrics int)
 			return nil, false
 		}
 		metricIDs := hmCurr.m.Clone()
-		for _, metricID := range hmPrev.m.AppendTo(nil) {
-			metricIDs.Add(metricID)
-		}
+		metricIDs.Union(hmPrev.m)
 		return metricIDs, true
 	}
 	return nil, false
diff --git a/lib/storage/storage.go b/lib/storage/storage.go
index 3339382b0..a446f7f0a 100644
--- a/lib/storage/storage.go
+++ b/lib/storage/storage.go
@@ -938,12 +938,10 @@ func (s *Storage) updateCurrHourMetricIDs() {
 		isFull = true
 	}
 	s.pendingHourMetricIDsLock.Lock()
-	newMetricIDs := s.pendingHourMetricIDs.AppendTo(nil)
+	newMetricIDs := s.pendingHourMetricIDs
 	s.pendingHourMetricIDs = &uint64set.Set{}
 	s.pendingHourMetricIDsLock.Unlock()
-	for _, metricID := range newMetricIDs {
-		m.Add(metricID)
-	}
+	m.Union(newMetricIDs)
 
 	hmNew := &hourMetricIDs{
 		m:      m,
diff --git a/lib/uint64set/uint64set.go b/lib/uint64set/uint64set.go
index 54cc96b87..285f1e527 100644
--- a/lib/uint64set/uint64set.go
+++ b/lib/uint64set/uint64set.go
@@ -126,6 +126,14 @@ func (s *Set) AppendTo(dst []uint64) []uint64 {
 	return dst
 }
 
+// Union adds all the items from a to s.
+func (s *Set) Union(a *Set) {
+	// TODO: optimize it
+	for _, x := range a.AppendTo(nil) {
+		s.Add(x)
+	}
+}
+
 type bucket32 struct {
 	hi      uint32
 	b16his  []uint16


Test patch:
diff --git a/lib/bytesutil/bytesutil_test.go b/lib/bytesutil/bytesutil_test.go
index 29283a341..721b5c2ed 100644
--- a/lib/bytesutil/bytesutil_test.go
+++ b/lib/bytesutil/bytesutil_test.go
@@ -1,6 +1,7 @@
 package bytesutil
 
 import (
+	"bytes"
 	"testing"
 )
 
@@ -20,3 +21,10 @@ func TestResize(t *testing.T) {
 		}
 	}
 }
+
+func TestToUnsafeString(t *testing.T) {
+	s := "str"
+	if !bytes.Equal([]byte("str"), ToUnsafeBytes(s)) {
+		t.Fatalf(`[]bytes(%s) doesnt equal to %s `, s, s)
+	}
+}
diff --git a/lib/storage/metric_name_test.go b/lib/storage/metric_name_test.go
index fb8fde665..fd60a744b 100644
--- a/lib/storage/metric_name_test.go
+++ b/lib/storage/metric_name_test.go
@@ -142,3 +142,82 @@ func TestMetricNameMarshalUnmarshalRaw(t *testing.T) {
 		}
 	}
 }
+
+func TestMetricNameCopyFrom(t *testing.T) {
+	var from MetricName
+	from.MetricGroup = []byte("group")
+	from.AddTag("key", "value")
+
+	var to MetricName
+	to.CopyFrom(&from)
+
+	var expected MetricName
+	expected.MetricGroup = []byte("group")
+	expected.AddTag("key", "value")
+
+	if !reflect.DeepEqual(expected, to) {
+		t.Fatalf("expecting equal metics exp: %s, got %s", expected, to)
+	}
+}
+
+func TestMetricNameRemoveTagsOn(t *testing.T) {
+	var emptyMN MetricName
+	emptyMN.MetricGroup = []byte("name")
+	emptyMN.AddTag("key", "value")
+	emptyMN.RemoveTagsOn(nil)
+	if len(emptyMN.MetricGroup) != 0 || len(emptyMN.Tags) != 0 {
+		t.Fatalf("expecitng empty metric name got %s", emptyMN)
+	}
+
+	var asIsMN MetricName
+	asIsMN.MetricGroup = []byte("name")
+	asIsMN.AddTag("key", "value")
+	asIsMN.RemoveTagsOn([]string{"__name__", "key"})
+	var expAsIsMN MetricName
+	expAsIsMN.MetricGroup = []byte("name")
+	expAsIsMN.AddTag("key", "value")
+	if !reflect.DeepEqual(expAsIsMN, asIsMN) {
+		t.Fatalf("expecitng %s got %s", expAsIsMN, asIsMN)
+	}
+
+	var mn MetricName
+	mn.MetricGroup = []byte("name")
+	mn.AddTag("foo", "bar")
+	mn.AddTag("baz", "qux")
+	mn.RemoveTagsOn([]string{"baz"})
+	var expMN MetricName
+	expMN.AddTag("baz", "qux")
+	if !reflect.DeepEqual(expMN.Tags, mn.Tags) || len(mn.MetricGroup) != len(expMN.MetricGroup) {
+		t.Fatalf("expecitng %s got %s", expMN, mn)
+	}
+}
+
+func TestMetricNameRemoveTag(t *testing.T) {
+	var mn MetricName
+	mn.MetricGroup = []byte("name")
+	mn.AddTag("foo", "bar")
+	mn.AddTag("baz", "qux")
+	mn.RemoveTag("__name__")
+	if len(mn.MetricGroup) != 0 {
+		t.Fatalf("expecting empty metric group got %s", mn)
+	}
+	mn.RemoveTag("foo")
+	var expMN MetricName
+	expMN.AddTag("baz", "qux")
+	if !reflect.DeepEqual(expMN.Tags, mn.Tags) || len(mn.MetricGroup) != len(expMN.MetricGroup) {
+		t.Fatalf("expecitng %s got %s", expMN, mn)
+	}
+}
+
+func TestMetricNameRemoveTagsIgnoring(t *testing.T) {
+	var mn MetricName
+	mn.MetricGroup = []byte("name")
+	mn.AddTag("foo", "bar")
+	mn.AddTag("baz", "qux")
+	mn.RemoveTagsIgnoring([]string{"__name__", "foo"})
+	var expMN MetricName
+	expMN.AddTag("baz", "qux")
+	if !reflect.DeepEqual(expMN.Tags, mn.Tags) || len(mn.MetricGroup) != len(expMN.MetricGroup) {
+		t.Fatalf("expecitng %s got %s", expMN, mn)
+	}
+}
diff --git a/lib/uint64set/uint64set_test.go b/lib/uint64set/uint64set_test.go
index 2b6733b60..65628c2e3 100644
--- a/lib/uint64set/uint64set_test.go
+++ b/lib/uint64set/uint64set_test.go
@@ -88,6 +88,22 @@ func testSetBasicOps(t *testing.T, itemsCount int) {
 		}
 	}
 
+	// Verify union
+	const unionOffset = 12345
+	var s1, s2 Set
+	for i := 0; i < itemsCount; i++ {
+		s1.Add(uint64(i) + offset)
+		s2.Add(uint64(i) + offset + unionOffset)
+	}
+	s1.Union(&s2)
+	expectedLen := 2 * itemsCount
+	if itemsCount > unionOffset {
+		expectedLen = itemsCount + unionOffset
+	}
+	if n := s1.Len(); n != expectedLen {
+		t.Fatalf("unexpected s1.Len() after union; got %d; want %d", n, expectedLen)
+	}
+
 	// Verify Del
 	for i := itemsCount / 2; i < itemsCount-itemsCount/4; i++ {
 		s.Del(uint64(i) + offset)