diff --git a/CHANGELOG.md b/CHANGELOG.md index 8328ea318ad..57be302df1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index bbb46e88a26..ac01c5e9ce6 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -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) diff --git a/pkg/querier/querier_test.go b/pkg/querier/querier_test.go index 55d5ec02014..aec2c16f8a6 100644 --- a/pkg/querier/querier_test.go +++ b/pkg/querier/querier_test.go @@ -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 (