store: Fixed intersect matching when one matcher filters all series. (#…

…862) Fixes #833 Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
thanos-io · Feb 21, 2019 · 0c730c1 · 0c730c1
1 parent e1ef24d
commit 0c730c1
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,11 +11,14 @@ We use *breaking* word for marking changes that are not backward compatible (rel
 
 ## Unreleased
 
+### Fixed
+- [#833](https://github.com/improbable-eng/thanos/issues/833) Store Gateway matcher regression for intersecting with empty posting.
+
 ## [v0.3.1](https://github.com/improbable-eng/thanos/releases/tag/v0.3.0) - 2019.02.18
 
 ### Fixed
 - [#829](https://github.com/improbable-eng/thanos/issues/829) Store Gateway crashing due to `slice bounds out of range`.
-- [#834](https://github.com/improbable-eng/thanos/issues/834) fixed matcher regression for `<>` `!=`.
+- [#834](https://github.com/improbable-eng/thanos/issues/834) Store Gateway matcher regression for `<>` `!=`.
 
 
 ## [v0.3.0](https://github.com/improbable-eng/thanos/releases/tag/v0.3.0) - 2019.02.08

diff --git a/pkg/store/bucket.go b/pkg/store/bucket.go
@@ -1182,13 +1182,8 @@ func (r *bucketIndexReader) ExpandedPostings(ms []labels.Matcher) ([]uint64, err
 
 	// NOTE: Derived from tsdb.PostingsForMatchers.
 	for _, m := range ms {
-		matchingGroup := toPostingGroup(r.LabelValues, m)
-		if matchingGroup == nil {
-			continue
-		}
-
 		// Each group is separate to tell later what postings are intersecting with what.
-		postingGroups = append(postingGroups, matchingGroup)
+		postingGroups = append(postingGroups, toPostingGroup(r.LabelValues, m))
 	}
 
 	if len(postingGroups) == 0 {
@@ -1240,6 +1235,10 @@ func (p *postingGroup) Fill(i int, posting index.Postings) {
 }
 
 func (p *postingGroup) Postings() index.Postings {
+	if len(p.keys) == 0 {
+		return index.EmptyPostings()
+	}
+
 	return p.aggregate(p.postings)
 }
 
@@ -1289,10 +1288,6 @@ func toPostingGroup(lvalsFn func(name string) []string, m labels.Matcher) *posti
 		}
 	}
 
-	if len(matchingLabels) == 0 {
-		return nil
-	}
-
 	return newPostingGroup(matchingLabels, merge)
 }
 

diff --git a/pkg/store/bucket_e2e_test.go b/pkg/store/bucket_e2e_test.go
@@ -130,6 +130,7 @@ func testBucketStore_e2e(t testing.TB, ctx context.Context, s *storeSuite) {
 	testutil.Ok(t, err)
 	testutil.Equals(t, []string{"1", "2"}, vals.Values)
 
+	// TODO(bwplotka): Add those test cases to TSDB querier_test.go as well, there are no tests for matching.
 	for i, tcase := range []struct {
 		req      *storepb.SeriesRequest
 		expected [][]storepb.Label
@@ -293,6 +294,18 @@ func testBucketStore_e2e(t testing.TB, ctx context.Context, s *storeSuite) {
 				{{Name: "a", Value: "2"}, {Name: "c", Value: "2"}, {Name: "ext2", Value: "value2"}},
 			},
 		},
+		// Regression https://github.com/improbable-eng/thanos/issues/833.
+		// Problem: Matcher that was selecting NO series, was ignored instead of passed as emptyPosting to Intersect.
+		{
+			req: &storepb.SeriesRequest{
+				Matchers: []storepb.LabelMatcher{
+					{Type: storepb.LabelMatcher_EQ, Name: "a", Value: "1"},
+					{Type: storepb.LabelMatcher_RE, Name: "non_existing", Value: "something"},
+				},
+				MinTime: mint,
+				MaxTime: maxt,
+			},
+		},
 	} {
 		t.Log("Run ", i)