Handle the case of large step causing single pt extents (#3818) (#3835)

* Handle the case of large step causing single pt extents

We noticed that we were ignoring samples if the step is 24h and this
fixes it.

Signed-off-by: Goutham Veeramachaneni <[email protected]>

* Handle the case of start == end better

Signed-off-by: Goutham Veeramachaneni <[email protected]>

* Address feedback

Signed-off-by: Goutham Veeramachaneni <[email protected]>
(cherry picked from commit e10ccda)

Co-authored-by: Goutham Veeramachaneni <[email protected]>

Author: khaines

pkg/querier/queryrange/results_cache.go

@@ -424,8 +424,13 @@ func (s resultsCache) partition(req Request, extents []Extent) ([]Request, []Res
 		if extent.GetEnd() < start || extent.Start > req.GetEnd() {
 			continue
 		}
-		// If this extent is tiny, discard it: more efficient to do a few larger queries
-		if extent.End-extent.Start < s.minCacheExtent {
+
+		// If this extent is tiny, discard it: more efficient to do a few larger queries.
+
+		// However if the step is large enough, the split_query_by_interval middleware would generate a query with same start and end.
+		// For example, if the step size is more than 12h and the interval is 24h.
+		// This means the extent's start and end time would be same, even if the timerange covers several hours.
+		if (req.GetStart() != req.GetEnd()) && (extent.End-extent.Start < s.minCacheExtent) {
 			continue
 		}
 
@@ -449,6 +454,12 @@ func (s resultsCache) partition(req Request, extents []Extent) ([]Request, []Res
 		requests = append(requests, r)
 	}
 
+	// If start and end are the same (valid in promql), start == req.GetEnd() and we won't do the query.
+	// But we should only do the request if we don't have a valid cached response for it.
+	if req.GetStart() == req.GetEnd() && len(cachedResponses) == 0 {
+		requests = append(requests, req)
+	}
+
 	return requests, cachedResponses, nil
 }
 

pkg/querier/queryrange/results_cache_test.go

@@ -361,6 +361,36 @@ func TestPartition(t *testing.T) {
 				mkAPIResponse(100, 120, 10),
 			},
 		},
+		// Extent is outside the range and the request has a single step (same start and end).
+		{
+			input: &PrometheusRequest{
+				Start: 100,
+				End:   100,
+			},
+			prevCachedResponse: []Extent{
+				mkExtent(50, 90),
+			},
+			expectedRequests: []Request{
+				&PrometheusRequest{
+					Start: 100,
+					End:   100,
+				},
+			},
+		},
+		// Test when hit has a large step and only a single sample extent.
+		{
+			// If there is a only a single sample in the split interval, start and end will be the same.
+			input: &PrometheusRequest{
+				Start: 100,
+				End:   100,
+			},
+			prevCachedResponse: []Extent{
+				mkExtent(100, 100),
+			},
+			expectedCachedResponse: []Response{
+				mkAPIResponse(100, 105, 10),
+			},
+		},
 	} {
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
 			s := resultsCache{