Skip to content

Enforce /api/v1/series API query length check at Querier #6018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* [ENHANCEMENT] Store Gateway: Log gRPC requests together with headers configured in `http_request_headers_to_log`. #5958
* [BUGFIX] Configsdb: Fix endline issue in db password. #5920
* [BUGFIX] Ingester: Fix `user` and `type` labels for the `cortex_ingester_tsdb_head_samples_appended_total` TSDB metric. #5952
* [BUGFIX] Querier: Enforce max query length check for `/api/v1/series` API even though `ignoreMaxQueryLength` is set to true. #6018

## 1.17.1 2024-05-20

Expand Down
11 changes: 6 additions & 5 deletions pkg/querier/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,20 +375,21 @@ func (q querier) Select(ctx context.Context, sortSeries bool, sp *storage.Select
// so we make sure changes are reflected back to hints.
sp.Start = startMs
sp.End = endMs
getSeries := sp.Func == "series"

// For series queries without specifying the start time, we prefer to
// only query ingesters and not to query maxQueryLength to avoid OOM kill.
if sp.Func == "series" && startMs == 0 {
if getSeries && startMs == 0 {
return metadataQuerier.Select(ctx, true, sp, matchers...)
}

startTime := model.Time(startMs)
endTime := model.Time(endMs)

// Validate query time range. This validation should be done only for instant / range queries and
// NOT for metadata queries (series, labels) because the query-frontend doesn't support splitting
// of such queries.
if !q.ignoreMaxQueryLength {
// Validate query time range. This validation for instant / range queries can be done either at Query Frontend
// or here at Querier. When the check is done at Query Frontend, we still want to enforce the max query length
// check for /api/v1/series request since there is no specific tripperware for series.
if !q.ignoreMaxQueryLength || getSeries {
if maxQueryLength := q.limits.MaxQueryLength(userID); maxQueryLength > 0 && endTime.Sub(startTime) > maxQueryLength {
limitErr := validation.LimitError(fmt.Sprintf(validation.ErrQueryTooLong, endTime.Sub(startTime), maxQueryLength))
return storage.ErrSeriesSet(limitErr)
Expand Down
36 changes: 36 additions & 0 deletions pkg/querier/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,42 @@ func TestQuerier_ValidateQueryTimeRange_MaxQueryLength(t *testing.T) {
}
}

func TestQuerier_ValidateQueryTimeRange_MaxQueryLength_Series(t *testing.T) {
t.Parallel()
const maxQueryLength = 30 * 24 * time.Hour

//parallel testing causes data race
var cfg Config
flagext.DefaultValues(&cfg)
// Disable active query tracker to avoid mmap error.
cfg.ActiveQueryTrackerDir = ""
// Ignore max query length check at Querier but it still enforces it for Series.
cfg.IgnoreMaxQueryLength = true

limits := DefaultLimitsConfig()
limits.MaxQueryLength = model.Duration(maxQueryLength)
overrides, err := validation.NewOverrides(limits, nil)
require.NoError(t, err)

chunkStore := &emptyChunkStore{}
distributor := &emptyDistributor{}

queryables := []QueryableWithFilter{UseAlwaysQueryable(NewMockStoreQueryable(cfg, chunkStore))}
queryable, _, _ := New(cfg, overrides, distributor, queryables, nil, log.NewNopLogger())

ctx := user.InjectOrgID(context.Background(), "test")
now := time.Now()
end := now.Add(-time.Minute)
start := end.Add(-maxQueryLength - time.Hour)
minT := util.TimeToMillis(start)
maxT := util.TimeToMillis(end)
q, err := queryable.Querier(minT, maxT)
require.NoError(t, err)
ss := q.Select(ctx, false, &storage.SelectHints{Func: "series", Start: minT, End: maxT})
require.False(t, ss.Next())
require.True(t, strings.Contains(ss.Err().Error(), "the query time range exceeds the limit (query length: 721h0m0s, limit: 720h0m0s)"))
}

func TestQuerier_ValidateQueryTimeRange_MaxQueryLookback(t *testing.T) {
t.Parallel()
const (
Expand Down
Loading