QFE does not cache request with dedup=false.

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

pkg/queryfrontend/queryrange_codec.go

@@ -192,7 +192,7 @@ func parseDurationMillis(s string) (int64, error) {
 }
 
 func parseEnableDedupParam(s string) (bool, error) {
-	enableDeduplication := true
+	enableDeduplication := true // Deduplication is enabled by default.
 	if s != "" {
 		var err error
 		enableDeduplication, err = strconv.ParseBool(s)

pkg/queryfrontend/request.go

@@ -24,6 +24,11 @@ type RequestHeader struct {
 	Values []string
 }
 
+// ThanosRequestDedup is a an interface for all requests that share setting deduplication.
+type ThanosRequestDedup interface {
+	IsDedupEnabled() bool
+}
+
 type ThanosQueryRangeRequest struct {
 	Path                string
 	Start               int64
@@ -41,6 +46,12 @@ type ThanosQueryRangeRequest struct {
 	Headers             []*RequestHeader
 }
 
+// IsDedupEnabled returns true if deduplication is enabled.
+func (r *ThanosQueryRangeRequest) IsDedupEnabled() bool { return r.Dedup }
+
+// GetStoreMatchers returns store matches.
+func (r *ThanosQueryRangeRequest) GetStoreMatchers() [][]*labels.Matcher { return r.StoreMatchers }
+
 // GetStart returns the start timestamp of the request in milliseconds.
 func (r *ThanosQueryRangeRequest) GetStart() int64 { return r.Start }
 
@@ -102,8 +113,6 @@ func (r *ThanosQueryRangeRequest) String() string { return "" }
 // which is not used in thanos.
 func (r *ThanosQueryRangeRequest) ProtoMessage() {}
 
-func (r *ThanosQueryRangeRequest) GetStoreMatchers() [][]*labels.Matcher { return r.StoreMatchers }
-
 type ThanosLabelsRequest struct {
 	Start           int64
 	End             int64
@@ -116,6 +125,9 @@ type ThanosLabelsRequest struct {
 	Headers         []*RequestHeader
 }
 
+// GetStoreMatchers returns store matches.
+func (r *ThanosLabelsRequest) GetStoreMatchers() [][]*labels.Matcher { return r.StoreMatchers }
+
 // GetStart returns the start timestamp of the request in milliseconds.
 func (r *ThanosLabelsRequest) GetStart() int64 { return r.Start }
 
@@ -173,8 +185,6 @@ func (r *ThanosLabelsRequest) String() string { return "" }
 // which is not used in thanos.
 func (r *ThanosLabelsRequest) ProtoMessage() {}
 
-func (r *ThanosLabelsRequest) GetStoreMatchers() [][]*labels.Matcher { return r.StoreMatchers }
-
 type ThanosSeriesRequest struct {
 	Path            string
 	Start           int64
@@ -188,6 +198,12 @@ type ThanosSeriesRequest struct {
 	Headers         []*RequestHeader
 }
 
+// IsDedupEnabled returns true if deduplication is enabled.
+func (r *ThanosSeriesRequest) IsDedupEnabled() bool { return r.Dedup }
+
+// GetStoreMatchers returns store matches.
+func (r *ThanosSeriesRequest) GetStoreMatchers() [][]*labels.Matcher { return r.StoreMatchers }
+
 // GetStart returns the start timestamp of the request in milliseconds.
 func (r *ThanosSeriesRequest) GetStart() int64 { return r.Start }
 
@@ -243,5 +259,3 @@ func (r *ThanosSeriesRequest) String() string { return "" }
 // ProtoMessage implements proto.Message interface required by queryrange.Request,
 // which is not used in thanos.
 func (r *ThanosSeriesRequest) ProtoMessage() {}
-
-func (r *ThanosSeriesRequest) GetStoreMatchers() [][]*labels.Matcher { return r.StoreMatchers }

pkg/queryfrontend/roundtrip.go

@@ -274,10 +274,20 @@ func newLabelsTripperware(
 	}, nil
 }
 
-// Don't go to response cache if StoreMatchers are set.
+// shouldCache controls what kind of Thanos request should be cached.
+// Don't cache:
+// * if StoreMatchers are set (since it is debug option).
+// * if Dedup is disabled (this is done only for debugging).
+// * Caching option disables cache.
 func shouldCache(r queryrange.Request) bool {
-	if thanosReq, ok := r.(ThanosRequestStoreMatcherGetter); ok {
-		if len(thanosReq.GetStoreMatchers()) > 0 {
+	if thanosReqStoreMatcherGettable, ok := r.(ThanosRequestStoreMatcherGetter); ok {
+		if len(thanosReqStoreMatcherGettable.GetStoreMatchers()) > 0 {
+			return false
+		}
+	}
+
+	if thanosReqDedup, ok := r.(ThanosRequestDedup); ok {
+		if !thanosReqDedup.IsDedupEnabled() {
 			return false
 		}
 	}

pkg/queryfrontend/roundtrip_test.go

@@ -381,15 +381,16 @@ func TestRoundTripQueryRangeCacheMiddleware(t *testing.T) {
 		End:                 2 * hour,
 		Step:                10 * seconds,
 		MaxSourceResolution: 1 * seconds,
+		Dedup:               true, // Deduplication is enabled by default.
 	}
 
-	testRequestWithDedup := &ThanosQueryRangeRequest{
+	testRequestWithoutDedup := &ThanosQueryRangeRequest{
 		Path:                "/api/v1/query_range",
 		Start:               0,
 		End:                 2 * hour,
 		Step:                10 * seconds,
 		MaxSourceResolution: 1 * seconds,
-		Dedup:               true,
+		Dedup:               false,
 	}
 
 	// Non query range request, won't be cached.
@@ -398,6 +399,7 @@ func TestRoundTripQueryRangeCacheMiddleware(t *testing.T) {
 		Start: 0,
 		End:   2 * hour,
 		Step:  10 * seconds,
+		Dedup: true,
 	}
 
 	// Same query params as testRequest, different maxSourceResolution
@@ -408,6 +410,7 @@ func TestRoundTripQueryRangeCacheMiddleware(t *testing.T) {
 		End:                 2 * hour,
 		Step:                10 * seconds,
 		MaxSourceResolution: 10 * seconds,
+		Dedup:               true,
 	}
 
 	// Same query params as testRequest, different maxSourceResolution
@@ -418,6 +421,7 @@ func TestRoundTripQueryRangeCacheMiddleware(t *testing.T) {
 		End:                 2 * hour,
 		Step:                10 * seconds,
 		MaxSourceResolution: 1 * hour,
+		Dedup:               true,
 	}
 
 	// Same query params as testRequest, but with storeMatchers
@@ -428,6 +432,7 @@ func TestRoundTripQueryRangeCacheMiddleware(t *testing.T) {
 		Step:                10 * seconds,
 		MaxSourceResolution: 1 * seconds,
 		StoreMatchers:       [][]*labels.Matcher{{labels.MustNewMatcher(labels.MatchEqual, "foo", "bar")}},
+		Dedup:               true,
 	}
 
 	cacheConf := &queryrange.ResultsCacheConfig{
@@ -466,7 +471,7 @@ func TestRoundTripQueryRangeCacheMiddleware(t *testing.T) {
 	}{
 		{name: "first request", req: testRequest, expected: 1},
 		{name: "same request as the first one, directly use cache", req: testRequest, expected: 1},
-		{name: "same request as the first one but with dedup enabled, should not use cache", req: testRequestWithDedup, expected: 2},
+		{name: "same request as the first one but with dedup disabled, should not use cache", req: testRequestWithoutDedup, expected: 2},
 		{name: "non query range request won't be cached", req: testRequestInstant, expected: 3},
 		{name: "do it again", req: testRequestInstant, expected: 4},
 		{name: "different max source resolution but still same level", req: testRequestSameLevelDownsampling, expected: 4},
@@ -479,8 +484,9 @@ func TestRoundTripQueryRangeCacheMiddleware(t *testing.T) {
 				Start: 0,
 				End:   25 * hour,
 				Step:  10 * seconds,
+				Dedup: true,
 			},
-			expected: 7,
+			expected: 8,
 		},
 		{
 			name: "same query as the previous one",
@@ -489,8 +495,9 @@ func TestRoundTripQueryRangeCacheMiddleware(t *testing.T) {
 				Start: 0,
 				End:   25 * hour,
 				Step:  10 * seconds,
+				Dedup: true,
 			},
-			expected: 7,
+			expected: 8,
 		},
 	} {
 		if !t.Run(tc.name, func(t *testing.T) {
@@ -618,9 +625,7 @@ func TestRoundTripLabelsCacheMiddleware(t *testing.T) {
 			expected: 8,
 		},
 	} {
-
-		t.Run(tc.name, func(t *testing.T) {
-
+		if !t.Run(tc.name, func(t *testing.T) {
 			ctx := user.InjectOrgID(context.Background(), "1")
 			httpReq, err := NewThanosLabelsCodec(true, 24*time.Hour).EncodeRequest(ctx, tc.req)
 			testutil.Ok(t, err)
@@ -629,8 +634,9 @@ func TestRoundTripLabelsCacheMiddleware(t *testing.T) {
 			testutil.Ok(t, err)
 
 			testutil.Equals(t, tc.expected, *res)
-		})
-
+		}) {
+			break
+		}
 	}
 }
 
@@ -641,6 +647,15 @@ func TestRoundTripSeriesCacheMiddleware(t *testing.T) {
 		Start:    0,
 		End:      2 * hour,
 		Matchers: [][]*labels.Matcher{{labels.MustNewMatcher(labels.MatchEqual, "foo", "bar")}},
+		Dedup:    true,
+	}
+
+	testRequestWithoutDedup := &ThanosSeriesRequest{
+		Path:     "/api/v1/series",
+		Start:    0,
+		End:      2 * hour,
+		Matchers: [][]*labels.Matcher{{labels.MustNewMatcher(labels.MatchEqual, "foo", "bar")}},
+		Dedup:    false,
 	}
 
 	// Different matchers set with the first request.
@@ -649,10 +664,11 @@ func TestRoundTripSeriesCacheMiddleware(t *testing.T) {
 		Start:    0,
 		End:      2 * hour,
 		Matchers: [][]*labels.Matcher{{labels.MustNewMatcher(labels.MatchEqual, "foo", "baz")}},
+		Dedup:    true,
 	}
 
 	// Same query params as testRequest, but with storeMatchers
-	testRequestWithStoreMatchers := &ThanosLabelsRequest{
+	testRequestWithStoreMatchers := &ThanosSeriesRequest{
 		Path:          "/api/v1/series",
 		Start:         0,
 		End:           2 * hour,
@@ -695,12 +711,12 @@ func TestRoundTripSeriesCacheMiddleware(t *testing.T) {
 	}{
 		{name: "first request", req: testRequest, expected: 1},
 		{name: "same request as the first one, directly use cache", req: testRequest, expected: 1},
-		{name: "different series request, not use cache", req: testRequest2, expected: 2},
-		{name: "storeMatchers requests won't go to cache", req: testRequestWithStoreMatchers, expected: 3},
+		{name: "same request as the first one but with dedup disabled, should not use cache", req: testRequestWithoutDedup, expected: 2},
+		{name: "different series request, not use cache", req: testRequest2, expected: 3},
+		{name: "storeMatchers requests won't go to cache", req: testRequestWithStoreMatchers, expected: 4},
 	} {
 
-		t.Run(tc.name, func(t *testing.T) {
-
+		if !t.Run(tc.name, func(t *testing.T) {
 			ctx := user.InjectOrgID(context.Background(), "1")
 			httpReq, err := NewThanosLabelsCodec(true, 24*time.Hour).EncodeRequest(ctx, tc.req)
 			testutil.Ok(t, err)
@@ -709,8 +725,9 @@ func TestRoundTripSeriesCacheMiddleware(t *testing.T) {
 			testutil.Ok(t, err)
 
 			testutil.Equals(t, tc.expected, *res)
-		})
-
+		}) {
+			break
+		}
 	}
 }