Hints: Separating out the range and offsets of PromQL subqueries (#7667)

Fix #7629

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>

promql/engine.go

@@ -576,35 +576,39 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *parser.Eval
 	return mat, warnings, nil
 }
 
-// cumulativeSubqueryOffset returns the sum of range and offset of all subqueries in the path.
-func (ng *Engine) cumulativeSubqueryOffset(path []parser.Node) time.Duration {
-	var subqOffset time.Duration
+// subqueryOffsetRange returns the sum of offsets and ranges of all subqueries in the path.
+func (ng *Engine) subqueryOffsetRange(path []parser.Node) (time.Duration, time.Duration) {
+	var (
+		subqOffset time.Duration
+		subqRange  time.Duration
+	)
 	for _, node := range path {
 		switch n := node.(type) {
 		case *parser.SubqueryExpr:
-			subqOffset += n.Range + n.Offset
+			subqOffset += n.Offset
+			subqRange += n.Range
 		}
 	}
-	return subqOffset
+	return subqOffset, subqRange
 }
 
 func (ng *Engine) findMinTime(s *parser.EvalStmt) time.Time {
 	var maxOffset time.Duration
 	parser.Inspect(s.Expr, func(node parser.Node, path []parser.Node) error {
-		subqOffset := ng.cumulativeSubqueryOffset(path)
+		subqOffset, subqRange := ng.subqueryOffsetRange(path)
 		switch n := node.(type) {
 		case *parser.VectorSelector:
-			if maxOffset < ng.lookbackDelta+subqOffset {
-				maxOffset = ng.lookbackDelta + subqOffset
+			if maxOffset < ng.lookbackDelta+subqOffset+subqRange {
+				maxOffset = ng.lookbackDelta + subqOffset + subqRange
 			}
-			if n.Offset+ng.lookbackDelta+subqOffset > maxOffset {
-				maxOffset = n.Offset + ng.lookbackDelta + subqOffset
+			if n.Offset+ng.lookbackDelta+subqOffset+subqRange > maxOffset {
+				maxOffset = n.Offset + ng.lookbackDelta + subqOffset + subqRange
 			}
 		case *parser.MatrixSelector:
-			if maxOffset < n.Range+subqOffset {
-				maxOffset = n.Range + subqOffset
+			if maxOffset < n.Range+subqOffset+subqRange {
+				maxOffset = n.Range + subqOffset + subqRange
 			}
-			if m := n.VectorSelector.(*parser.VectorSelector).Offset + n.Range + subqOffset; m > maxOffset {
+			if m := n.VectorSelector.(*parser.VectorSelector).Offset + n.Range + subqOffset + subqRange; m > maxOffset {
 				maxOffset = m
 			}
 		}
@@ -629,13 +633,13 @@ func (ng *Engine) populateSeries(querier storage.Querier, s *parser.EvalStmt) {
 			}
 
 			// We need to make sure we select the timerange selected by the subquery.
-			// The cumulativeSubqueryOffset function gives the sum of range and the offset.
-			// TODO(gouthamve): Consider optimising it by separating out the range and offsets, and subtracting the offsets
-			// from end also. See: https://github.com/prometheus/prometheus/issues/7629.
+			// The subqueryOffsetRange function gives the sum of range and the
+			// sum of offset.
 			// TODO(bwplotka): Add support for better hints when subquerying. See: https://github.com/prometheus/prometheus/issues/7630.
-			subqOffset := ng.cumulativeSubqueryOffset(path)
+			subqOffset, subqRange := ng.subqueryOffsetRange(path)
 			offsetMilliseconds := durationMilliseconds(subqOffset)
-			hints.Start = hints.Start - offsetMilliseconds
+			hints.Start = hints.Start - offsetMilliseconds - durationMilliseconds(subqRange)
+			hints.End = hints.End - offsetMilliseconds
 
 			if evalRange == 0 {
 				hints.Start = hints.Start - durationMilliseconds(ng.lookbackDelta)

promql/engine_test.go

@@ -289,12 +289,12 @@ func TestSelectHintsSetCorrectly(t *testing.T) {
 	}, {
 		query: "count_over_time(foo[2m:1s] offset 10s)", start: 300000,
 		expected: []*storage.SelectHints{
-			{Start: 165000, End: 300000, Func: "count_over_time"},
+			{Start: 165000, End: 290000, Func: "count_over_time"},
 		},
 	}, {
 		query: "count_over_time((foo offset 10s)[2m:1s] offset 10s)", start: 300000,
 		expected: []*storage.SelectHints{
-			{Start: 155000, End: 290000, Func: "count_over_time"},
+			{Start: 155000, End: 280000, Func: "count_over_time"},
 		},
 	}, {
 
@@ -325,12 +325,12 @@ func TestSelectHintsSetCorrectly(t *testing.T) {
 	}, {
 		query: "count_over_time(foo[2m:1s] offset 10s)", start: 300000, end: 500000,
 		expected: []*storage.SelectHints{
-			{Start: 165000, End: 500000, Func: "count_over_time", Step: 1000},
+			{Start: 165000, End: 490000, Func: "count_over_time", Step: 1000},
 		},
 	}, {
 		query: "count_over_time((foo offset 10s)[2m:1s] offset 10s)", start: 300000, end: 500000,
 		expected: []*storage.SelectHints{
-			{Start: 155000, End: 490000, Func: "count_over_time", Step: 1000},
+			{Start: 155000, End: 480000, Func: "count_over_time", Step: 1000},
 		},
 	}, {
 		query: "sum by (dim1) (foo)", start: 10000,

promql/testdata/selectors.test

@@ -57,3 +57,12 @@ eval instant at 0s http_requests{foo!~"bar", job="api-server"}
 eval instant at 0s http_requests{foo!~"bar", job="api-server", instance="1", x!="y", z="", group!=""}
 	http_requests{job="api-server", instance="1", group="production"} 0
 	http_requests{job="api-server", instance="1", group="canary"} 0
+
+clear
+load 1m
+    metric1{a="a"} 0+1x100
+    metric2{b="b"} 0+1x50
+
+eval instant at 90m metric1 offset 15m or metric2 offset 45m
+    metric1{a="a"} 75
+    metric2{b="b"} 45