Fixes an issue in the index chunks/series intersect code. (#2796)

* Fixes an issue in the index chunks/series intersect code.

This was introduce in #2700, more specifically this line https://github.com/cortexproject/cortex/pull/2700/files#diff-10bca0f4f31a2ca1edc507d0289b143dR537

This causes any query with the first label matcher not matching anything to return all matches of all other labels.
This is a nasty one since, the code was relying on empty slice, and so it would skip nil values instead of returning no matches. I've added a regression test proving this is fixed everywhere. I think in cortex it can probably affect performance (since you have to download all chunk not required) but not read integrity.

I have found this with @slim-bean while deploying Loki, all queriers where OOMing because of this.

Signed-off-by: Cyril Tovena <[email protected]>

* Update changelog.

Signed-off-by: Cyril Tovena <[email protected]>

Author: cyriltovena

CHANGELOG.md

@@ -7,6 +7,7 @@
 * [FEATURE] Introduced `ruler.for-grace-period`, Minimum duration between alert and restored "for" state. This is maintained only for alerts with configured "for" time greater than grace period. #2783
 * [FEATURE] Introduced `ruler.for-resend-delay`, Minimum amount of time to wait before resending an alert to Alertmanager. #2783
 * [ENHANCEMENT] Experimental: Querier can now optionally query secondary store. This is specified by using `-querier.second-store-engine` option, with values `chunks` or `tsdb`. Standard configuration options for this store are used. Additionally, this querying can be configured to happen only for queries that need data older than `-querier.use-second-store-before-time`. Default value of zero will always query secondary store. #2747
+* [BUGFIX] Fixed a bug in the index intersect code causing storage to return more chunks/series than required. #2796
 * [BUGFIX] Fixed the number of reported keys in the background cache queue. #2764
 * [BUGFIX] Fix race in processing of headers in sharded queries. #2762
 

pkg/chunk/chunk_store.go

@@ -424,11 +424,13 @@ func (c *store) lookupChunksByMetricName(ctx context.Context, userID string, fro
 	// Receive chunkSets from all matchers
 	var chunkIDs []string
 	var lastErr error
+	var initialized bool
 	for i := 0; i < len(matchers); i++ {
 		select {
 		case incoming := <-incomingChunkIDs:
-			if chunkIDs == nil {
+			if !initialized {
 				chunkIDs = incoming
+				initialized = true
 			} else {
 				chunkIDs = intersectStrings(chunkIDs, incoming)
 			}

pkg/chunk/chunk_store_test.go

@@ -177,6 +177,10 @@ func TestChunkStore_Get(t *testing.T) {
 			query:  `foo{toms="code", bar="baz"}`,
 			expect: []Chunk{fooChunk1},
 		},
+		{
+			query:  `foo{a="b", bar="baz"}`,
+			expect: nil,
+		},
 		{
 			query: `{__name__=~"foo"}`,
 			err:   "query must contain metric name",

pkg/chunk/series_store.go

@@ -305,12 +305,14 @@ func (c *seriesStore) lookupSeriesByMetricNameMatchers(ctx context.Context, from
 	var lastErr error
 	var cardinalityExceededErrors int
 	var cardinalityExceededError CardinalityExceededError
+	var initialized bool
 	for i := 0; i < len(matchers); i++ {
 		select {
 		case incoming := <-incomingIDs:
 			preIntersectionCount += len(incoming)
-			if ids == nil {
+			if !initialized {
 				ids = incoming
+				initialized = true
 			} else {
 				ids = intersectStrings(ids, incoming)
 			}