Skip to content

Commit

Permalink
Added negative offset check for caching queries
Browse files Browse the repository at this point in the history
Signed-off-by: pawarpranav83 <[email protected]>
  • Loading branch information
pawarpranav83 committed Dec 26, 2023
1 parent a59a3ef commit 16e7e9f
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 0 deletions.
42 changes: 42 additions & 0 deletions internal/cortex/querier/queryrange/results_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@ func (s resultsCache) shouldCacheResponse(ctx context.Context, req Request, r Re
if !s.isAtModifierCachable(req, maxCacheTime) {
return false
}
if !s.isOffsetCachable(req) {
return false
}

if s.cacheGenNumberLoader == nil {
return true
Expand Down Expand Up @@ -357,6 +360,45 @@ func (s resultsCache) isAtModifierCachable(r Request, maxCacheTime int64) bool {
return atModCachable
}

var negativeOffset = errors.New("negative offset")

func (s resultsCache) isOffsetCachable(r Request) bool {
query := r.GetQuery()
if !strings.Contains(query, "offset") {
return true
}
expr, err := parser.ParseExpr(query)
if err != nil {
level.Warn(s.logger).Log("msg", "failed to parse query, considering @ modifier as not cachable", "query", query, "err", err)
return false
}

offsetCachable := true
parser.Inspect(expr, func(n parser.Node, _ []parser.Node) error {
switch e := n.(type) {
case *parser.VectorSelector:
if e.OriginalOffset < 0 {
offsetCachable = false
return negativeOffset
}
case *parser.MatrixSelector:
offset := e.VectorSelector.(*parser.VectorSelector).OriginalOffset
if offset < 0 {
offsetCachable = false
return negativeOffset
}
case *parser.SubqueryExpr:
if e.OriginalOffset < 0 {
offsetCachable = false
return negativeOffset
}
}
return nil
})

return offsetCachable
}

func getHeaderValuesWithName(r Response, headerName string) (headerValues []string) {
for _, hv := range r.GetHeaders() {
if hv.GetName() != headerName {
Expand Down
39 changes: 39 additions & 0 deletions internal/cortex/querier/queryrange/results_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,45 @@ func TestShouldCache(t *testing.T) {
input: Response(&PrometheusResponse{}),
expected: false,
},
// offset on vector selectors.
{
name: "positive offset on vector selector",
request: &PrometheusRequest{Query: "metric offset 10ms", End: 125000},
input: Response(&PrometheusResponse{}),
expected: true,
},
{
name: "negative offset on vector selector",
request: &PrometheusRequest{Query: "metric offset -10ms", End: 125000},
input: Response(&PrometheusResponse{}),
expected: false,
},
// offset on matrix selectors.
{
name: "positive offset on matrix selector",
request: &PrometheusRequest{Query: "rate(metric[5m] offset 10ms)", End: 125000},
input: Response(&PrometheusResponse{}),
expected: true,
},
{
name: "negative offset on matrix selector",
request: &PrometheusRequest{Query: "rate(metric[5m] offset -10ms)", End: 125000},
input: Response(&PrometheusResponse{}),
expected: false,
},
// offset on subqueries.
{
name: "positive offset on subqueries",
request: &PrometheusRequest{Query: "sum_over_time(rate(metric[1m])[10m:1m] offset 10ms)", End: 125000},
input: Response(&PrometheusResponse{}),
expected: true,
},
{
name: "negative offset on subqueries",
request: &PrometheusRequest{Query: "sum_over_time(rate(metric[1m])[10m:1m] offset -10ms)", End: 125000},
input: Response(&PrometheusResponse{}),
expected: false,
},
} {
{
t.Run(tc.name, func(t *testing.T) {
Expand Down

0 comments on commit 16e7e9f

Please sign in to comment.