lib/storage: follow up after 00cbb099b6

docs/CHANGELOG.md

@@ -19,6 +19,7 @@ sort: 15
 * FEATURE: vmagent: reduce CPU usage when applying `series_limit` to scrape targets with constant set of metrics. See more information about `series_limit` [here](https://docs.victoriametrics.com/vmagent.html#cardinality-limiter).
 * FEATURE: vminsert: disable rerouting by default when a few of `vmstorage` nodes start accepting data at lower speed than the rest of `vmstorage` nodes. This should improve VictoriaMetrics cluster stability during rolling restarts and during spikes in [time series churn rate](https://docs.victoriametrics.com/FAQ.html#what-is-high-churn-rate). The rerouting can be enabled by passing `-disableRerouting=false` command-line flag to `vminsert`.
 * FEATURE: vmauth: do not put invalid auth tokens into log by default due to security reasons. The logging can be returned back by passing `-logInvalidAuthTokens` command-line flag to `vmauth`. Requests with invalid auth tokens are counted at `vmagent_http_request_errors_total{reason="invalid_auth_token"}` metric exposed by `vmauth` at `/metrics` page.
+* FEATURE: optimize performance for queries with regexp filters on metric name like `{__name__=~"metric1|...|metricN"}`. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/1610) from @faceair.
 
 * BUGFIX: properly handle queries with multiple filters matching empty labels such as `metric{label1=~"foo|",label2="bar|"}`. This filter must match the following series: `metric`, `metric{label1="foo"}`, `metric{label2="bar"}` and `metric{label1="foo",label2="bar"}`. Previously it was matching only `metric{label1="foo",label2="bar"}`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1601).
 * BUGFIX: vmselect: reset connection timeouts after each request to `vmstorage`. This should prevent from `cannot read data in 0.000 seconds: unexpected EOF` warning in logs. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1562). Thanks to @mxlxm .

lib/storage/tag_filters.go

@@ -21,65 +21,80 @@ import (
 func convertToCompositeTagFilterss(tfss []*TagFilters) []*TagFilters {
 	tfssNew := make([]*TagFilters, 0, len(tfss))
 	for _, tfs := range tfss {
-		tfssNew = append(tfss, convertToCompositeTagFilters(tfs)...)
+		tfssNew = append(tfssNew, convertToCompositeTagFilters(tfs)...)
 	}
 	return tfssNew
 }
 
 func convertToCompositeTagFilters(tfs *TagFilters) []*TagFilters {
-	tfssCompiled := make([]*TagFilters, 0)
-
+	var tfssCompiled []*TagFilters
 	// Search for filters on metric name, which will be used for creating composite filters.
 	var names [][]byte
 	hasPositiveFilter := false
 	for _, tf := range tfs.tfs {
-		if len(tf.key) == 0 && !tf.isNegative && !tf.isRegexp {
-			names = [][]byte{tf.value}
-		} else if len(tf.key) == 0 && !tf.isNegative && tf.isRegexp && len(tf.orSuffixes) > 0 {
-			// Split the filter {__name__=~"name1|...|nameN", other_filters}
-			// into `name1{other_filters}`, ..., `nameN{other_filters}`
-			// and generate composite filters for each of them
-			names = names[:0]
-			for _, orSuffix := range tf.orSuffixes {
-				names = append(names, []byte(orSuffix))
+		if len(tf.key) == 0 {
+			if !tf.isNegative && !tf.isRegexp {
+				names = [][]byte{tf.value}
+			} else if !tf.isNegative && tf.isRegexp && len(tf.orSuffixes) > 0 {
+				// Split the filter {__name__=~"name1|...|nameN", other_filters}
+				// into name1{other_filters}, ..., nameN{other_filters}
+				// and generate composite filters for each of them.
+				names = names[:0] // override the previous filters on metric name
+				for _, orSuffix := range tf.orSuffixes {
+					names = append(names, []byte(orSuffix))
+				}
 			}
 		} else if !tf.isNegative && !tf.isEmptyMatch {
 			hasPositiveFilter = true
 		}
 	}
 	if len(names) == 0 {
-		tfssCompiled = append(tfssCompiled, tfs)
 		atomic.AddUint64(&compositeFilterMissingConversions, 1)
-		return tfssCompiled
+		return []*TagFilters{tfs}
 	}
 
+	// Create composite filters for the found names.
 	var compositeKey []byte
-	compositeFilters := 0
 	for _, name := range names {
+		compositeFilters := 0
 		tfsNew := make([]tagFilter, 0, len(tfs.tfs))
 		for _, tf := range tfs.tfs {
 			if len(tf.key) == 0 {
-				sameOrSuffixes := true
-				if len(names) != len(tf.orSuffixes) {
-					sameOrSuffixes = false
-				} else {
-					for i, orSuffix := range tf.orSuffixes {
-						if string(names[i]) != orSuffix {
-							sameOrSuffixes = false
+				if !hasPositiveFilter || tf.isNegative {
+					// Negative filters on metric name cannot be used for building composite filter, so leave them as is.
+					tfsNew = append(tfsNew, tf)
+					continue
+				}
+				if tf.isRegexp {
+					matchName := false
+					for _, orSuffix := range tf.orSuffixes {
+						if orSuffix == string(name) {
+							matchName = true
 							break
 						}
 					}
+					if !matchName {
+						// Leave as is the regexp filter on metric name if it doesn't match the current name.
+						tfsNew = append(tfsNew, tf)
+						continue
+					}
+					// Skip the tf, since its part (name) is used as a prefix in composite filter.
+					continue
 				}
-				if !hasPositiveFilter || tf.isNegative || tf.isRegexp && !sameOrSuffixes || !tf.isRegexp && string(tf.value) != string(name) {
+				if string(tf.value) != string(name) {
+					// Leave as is the filter on another metric name.
 					tfsNew = append(tfsNew, tf)
+					continue
 				}
+				// Skip the tf, since it is used as a prefix in composite filter.
 				continue
 			}
 			if string(tf.key) == "__graphite__" || bytes.Equal(tf.key, graphiteReverseTagKey) {
+				// Leave as is __graphite__ filters, since they cannot be used for building composite filter.
 				tfsNew = append(tfsNew, tf)
 				continue
 			}
-
+			// Create composite filter on (name, tf)
 			compositeKey = marshalCompositeTagKey(compositeKey[:0], name, tf.key)
 			var tfNew tagFilter
 			if err := tfNew.Init(tfs.commonPrefix, compositeKey, tf.value, tf.isNegative, tf.isRegexp); err != nil {
@@ -88,15 +103,16 @@ func convertToCompositeTagFilters(tfs *TagFilters) []*TagFilters {
 			tfsNew = append(tfsNew, tfNew)
 			compositeFilters++
 		}
+		if compositeFilters == 0 {
+			// Cannot use tfsNew, since it doesn't contain composite filters, e.g. it may match broader set of series.
+			// Fall back to the original tfs.
+			atomic.AddUint64(&compositeFilterMissingConversions, 1)
+			return []*TagFilters{tfs}
+		}
 		tfsCompiled := NewTagFilters(tfs.accountID, tfs.projectID)
 		tfsCompiled.tfs = tfsNew
 		tfssCompiled = append(tfssCompiled, tfsCompiled)
 	}
-	if compositeFilters == 0 {
-		tfssCompiled = append(tfssCompiled[:0], tfs)
-		atomic.AddUint64(&compositeFilterMissingConversions, 1)
-		return tfssCompiled
-	}
 	atomic.AddUint64(&compositeFilterSuccessConversions, 1)
 	return tfssCompiled
 }

lib/storage/tag_filters_test.go

@@ -17,9 +17,9 @@ func TestConvertToCompositeTagFilters(t *testing.T) {
 				t.Fatalf("cannot add tf=%s: %s", tf.String(), err)
 			}
 		}
-		resultCompileds := convertToCompositeTagFilters(tfsCompiled)
-		result := make([][]TagFilter, len(resultCompileds))
-		for i, resultCompiled := range resultCompileds {
+		resultsCompiled := convertToCompositeTagFilterss([]*TagFilters{tfsCompiled})
+		result := make([][]TagFilter, len(resultsCompiled))
+		for i, resultCompiled := range resultsCompiled {
 			if resultCompiled.accountID != accountID {
 				t.Fatalf("unexpected accountID; got %d; want %d", resultCompiled.accountID, accountID)
 			}