From 418de71509876dc45fa9c356c60aa472d82865df Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Thu, 18 Feb 2021 18:32:33 +0200 Subject: [PATCH] lib/storage: properly handle queries containing a filter on metric name plus any number of negative filters and zero non-negative filters Example: `node_cpu_seconds_total{mode!="idle"}` --- docs/CHANGELOG.md | 2 + lib/storage/tag_filters.go | 10 +++- lib/storage/tag_filters_test.go | 84 +++++++++++++++++++++++++++++---- 3 files changed, 84 insertions(+), 12 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3688633f1..b545b8660 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,8 @@ # tip +* BUGFIX: properly handle queries containing a filter on metric name plus any number of negative filters and zero non-negative filters. For example, `node_cpu_seconds_total{mode!="idle"}`. The bug was introduced in [v1.54.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.54.0). + # [v1.54.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.54.0) diff --git a/lib/storage/tag_filters.go b/lib/storage/tag_filters.go index 4ca35b3b4..1377ef99b 100644 --- a/lib/storage/tag_filters.go +++ b/lib/storage/tag_filters.go @@ -36,13 +36,16 @@ func convertToCompositeTagFilters(tfs *TagFilters) *TagFilters { } } if len(name) == 0 { - // There is no metric name filter, so composite filters cannot be created. + // Composite filters cannot be created in the following cases: + // - if there is no filter on metric name + // - if there is no at least a single positive filter. atomic.AddUint64(&compositeFilterMissingConversions, 1) return tfs } tfsNew := make([]tagFilter, 0, len(tfs.tfs)) var compositeKey []byte compositeFilters := 0 + hasPositiveFilter := false for _, tf := range tfs.tfs { if len(tf.key) == 0 { if tf.isNegative || tf.isRegexp || string(tf.value) != string(name) { @@ -59,10 +62,13 @@ func convertToCompositeTagFilters(tfs *TagFilters) *TagFilters { if err := tfNew.Init(tfs.commonPrefix, compositeKey, tf.value, tf.isNegative, tf.isRegexp); err != nil { logger.Panicf("BUG: unexpected error when creating composite tag filter for name=%q and key=%q: %s", name, tf.key, err) } + if !tfNew.isNegative { + hasPositiveFilter = true + } tfsNew = append(tfsNew, tfNew) compositeFilters++ } - if compositeFilters == 0 { + if compositeFilters == 0 || !hasPositiveFilter { atomic.AddUint64(&compositeFilterMissingConversions, 1) return tfs } diff --git a/lib/storage/tag_filters_test.go b/lib/storage/tag_filters_test.go index 80b3f4eee..7b6074071 100644 --- a/lib/storage/tag_filters_test.go +++ b/lib/storage/tag_filters_test.go @@ -156,6 +156,70 @@ func TestConvertToCompositeTagFilters(t *testing.T) { }, }) + // A name filter with a single negative filter + f([]TagFilter{ + { + Key: nil, + Value: []byte("bar"), + IsNegative: false, + IsRegexp: false, + }, + { + Key: []byte("foo"), + Value: []byte("abc"), + IsNegative: true, + IsRegexp: false, + }, + }, []TagFilter{ + { + Key: nil, + Value: []byte("bar"), + IsNegative: false, + IsRegexp: false, + }, + { + Key: []byte("foo"), + Value: []byte("abc"), + IsNegative: true, + IsRegexp: false, + }, + }) + + // A name filter with a negative and a positive filter + f([]TagFilter{ + { + Key: nil, + Value: []byte("bar"), + IsNegative: false, + IsRegexp: false, + }, + { + Key: []byte("foo"), + Value: []byte("abc"), + IsNegative: true, + IsRegexp: false, + }, + { + Key: []byte("a"), + Value: []byte("b.+"), + IsNegative: false, + IsRegexp: true, + }, + }, []TagFilter{ + { + Key: []byte("\xfe\x03barfoo"), + Value: []byte("abc"), + IsNegative: true, + IsRegexp: false, + }, + { + Key: []byte("\xfe\x03bara"), + Value: []byte("b.+"), + IsNegative: false, + IsRegexp: true, + }, + }) + // Two name filters with non-name filter. f([]TagFilter{ { @@ -191,7 +255,7 @@ func TestConvertToCompositeTagFilters(t *testing.T) { }, }) - // A name filter with negative regexp non-name filter, which can be converted to non-regexp. + // A name filter with regexp non-name filter, which can be converted to non-regexp. f([]TagFilter{ { Key: nil, @@ -202,19 +266,19 @@ func TestConvertToCompositeTagFilters(t *testing.T) { { Key: []byte("foo"), Value: []byte("abc"), - IsNegative: true, + IsNegative: false, IsRegexp: true, }, }, []TagFilter{ { Key: []byte("\xfe\x03barfoo"), Value: []byte("abc"), - IsNegative: true, + IsNegative: false, IsRegexp: false, }, }) - // A name filter with negative regexp non-name filter. + // A name filter with regexp non-name filter. f([]TagFilter{ { Key: nil, @@ -225,14 +289,14 @@ func TestConvertToCompositeTagFilters(t *testing.T) { { Key: []byte("foo"), Value: []byte("abc.+"), - IsNegative: true, + IsNegative: false, IsRegexp: true, }, }, []TagFilter{ { Key: []byte("\xfe\x03barfoo"), Value: []byte("abc.+"), - IsNegative: true, + IsNegative: false, IsRegexp: true, }, }) @@ -277,7 +341,7 @@ func TestConvertToCompositeTagFilters(t *testing.T) { { Key: []byte("foo"), Value: []byte("abc"), - IsNegative: true, + IsNegative: false, IsRegexp: true, }, { @@ -290,7 +354,7 @@ func TestConvertToCompositeTagFilters(t *testing.T) { { Key: []byte("\xfe\x03barfoo"), Value: []byte("abc"), - IsNegative: true, + IsNegative: false, IsRegexp: false, }, { @@ -312,14 +376,14 @@ func TestConvertToCompositeTagFilters(t *testing.T) { { Key: []byte("foo"), Value: []byte("abc"), - IsNegative: true, + IsNegative: false, IsRegexp: false, }, }, []TagFilter{ { Key: []byte("\xfe\x03barfoo"), Value: []byte("abc"), - IsNegative: true, + IsNegative: false, IsRegexp: false, }, })