Skip to content
This repository was archived by the owner on Apr 2, 2024. It is now read-only.

Commit 3eae912

Browse files
committed
Make findMinTime more exact for MatrixSelector
When a Range is smaller than lookbackDelta the previous version of findMinTime used to offset the minTime by lookbackDelta instead of Range. In reality, a MatrixSelector never needs to use the LookbackDelta and it only did so because the Inspect code cascaded down to the VectorSelector of the Matrix, which is incorrect. Thus prevent the walk from descending to a MatrixSelector's underlying VectorSelector.
1 parent 5812db6 commit 3eae912

File tree

1 file changed

+25
-6
lines changed

1 file changed

+25
-6
lines changed

pkg/promql/engine.go

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,11 @@ func durationMilliseconds(d time.Duration) int64 {
533533
// execEvalStmt evaluates the expression of an evaluation statement for the given time range.
534534
func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *parser.EvalStmt) (parser.Value, storage.Warnings, error) {
535535
prepareSpanTimer, ctxPrepare := query.stats.GetSpanTimer(ctx, stats.QueryPreparationTime, ng.metrics.queryPrepareTime)
536-
mint := ng.findMinTime(s)
536+
mint, err := ng.findMinTime(s)
537+
if err != nil {
538+
prepareSpanTimer.Finish()
539+
return nil, nil, err
540+
}
537541
querier, err := query.queryable.Querier(ctxPrepare, timestamp.FromTime(mint), timestamp.FromTime(s.End))
538542
if err != nil {
539543
prepareSpanTimer.Finish()
@@ -650,9 +654,21 @@ func (ng *Engine) cumulativeSubqueryOffset(path []parser.Node) time.Duration {
650654
return subqOffset
651655
}
652656

653-
func (ng *Engine) findMinTime(s *parser.EvalStmt) time.Time {
657+
type inspector func(parser.Node, []parser.Node) (bool, error)
658+
659+
func (f inspector) Visit(node parser.Node, path []parser.Node) (parser.Visitor, error) {
660+
if v, err := f(node, path); !v || err != nil {
661+
return nil, err
662+
}
663+
664+
return f, nil
665+
}
666+
667+
// This is modified from upstream to avoid descending into a MatrixSelector's VectorSelector
668+
func (ng *Engine) findMinTime(s *parser.EvalStmt) (time.Time, error) {
654669
var maxOffset time.Duration
655-
parser.Inspect(s.Expr, func(node parser.Node, path []parser.Node) error {
670+
671+
err := parser.Walk(inspector(func(node parser.Node, path []parser.Node) (bool, error) {
656672
subqOffset := ng.cumulativeSubqueryOffset(path)
657673
switch n := node.(type) {
658674
case *parser.VectorSelector:
@@ -669,10 +685,13 @@ func (ng *Engine) findMinTime(s *parser.EvalStmt) time.Time {
669685
if m := n.VectorSelector.(*parser.VectorSelector).Offset + n.Range + subqOffset; m > maxOffset {
670686
maxOffset = m
671687
}
688+
//do not process the child VectorSelector since we don't want to add the LookbackDelta etc for the underlying
689+
//VectorSelector.
690+
return false, nil
672691
}
673-
return nil
674-
})
675-
return s.Start.Add(-maxOffset)
692+
return true, nil
693+
}), s.Expr, nil)
694+
return s.Start.Add(-maxOffset), err
676695
}
677696

678697
func (ng *Engine) populateSeries(ctx context.Context, querier Querier, s *parser.EvalStmt) (storage.Warnings, parser.Node, error) {

0 commit comments

Comments
 (0)