From db2c19c1adc8aa24c2f3cf19d60c3154eeabcdd6 Mon Sep 17 00:00:00 2001 From: Ben Ye Date: Wed, 26 Jul 2023 02:05:49 -0700 Subject: [PATCH 1/2] clean up no step evaluation interval in QFE codec Signed-off-by: Ben Ye --- pkg/cortex/modules.go | 7 +++---- .../tripperware/instantquery/instant_query.go | 7 +------ .../tripperware/instantquery/shard_by_query_test.go | 3 +-- pkg/querier/tripperware/queryrange/query_range.go | 13 ++----------- .../queryrange/query_range_middlewares_test.go | 4 ++-- pkg/querier/tripperware/roundtrip.go | 12 ++++++++---- 6 files changed, 17 insertions(+), 29 deletions(-) diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index f72b182489e..180e5c73904 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -451,11 +451,10 @@ func (t *Cortex) initDeleteRequestsStore() (serv services.Service, err error) { // to optimize Prometheus query requests. func (t *Cortex) initQueryFrontendTripperware() (serv services.Service, err error) { queryAnalyzer := querysharding.NewQueryAnalyzer() - defaultSubQueryInterval := t.Cfg.Querier.DefaultEvaluationInterval // PrometheusCodec is a codec to encode and decode Prometheus query range requests and responses. - prometheusCodec := queryrange.NewPrometheusCodec(false, defaultSubQueryInterval) + prometheusCodec := queryrange.NewPrometheusCodec(false) // ShardedPrometheusCodec is same as PrometheusCodec but to be used on the sharded queries (it sum up the stats) - shardedPrometheusCodec := queryrange.NewPrometheusCodec(true, defaultSubQueryInterval) + shardedPrometheusCodec := queryrange.NewPrometheusCodec(true) queryRangeMiddlewares, cache, err := queryrange.Middlewares( t.Cfg.QueryRange, @@ -486,7 +485,7 @@ func (t *Cortex) initQueryFrontendTripperware() (serv services.Service, err erro instantquery.InstantQueryCodec, t.Overrides, queryAnalyzer, - defaultSubQueryInterval, + t.Cfg.Querier.DefaultEvaluationInterval, ) return services.NewIdleService(nil, func(_ error) error { diff --git a/pkg/querier/tripperware/instantquery/instant_query.go b/pkg/querier/tripperware/instantquery/instant_query.go index a45da6dbbb3..58733ee79eb 100644 --- a/pkg/querier/tripperware/instantquery/instant_query.go +++ b/pkg/querier/tripperware/instantquery/instant_query.go @@ -108,8 +108,7 @@ func (r *PrometheusRequest) WithStats(stats string) tripperware.Request { type instantQueryCodec struct { tripperware.Codec - now func() time.Time - noStepSubQueryInterval time.Duration + now func() time.Time } func newInstantQueryCodec() instantQueryCodec { @@ -139,10 +138,6 @@ func (c instantQueryCodec) DecodeRequest(_ context.Context, r *http.Request, for } result.Query = r.FormValue("query") - if err := tripperware.SubQueryStepSizeCheck(result.Query, c.noStepSubQueryInterval, tripperware.MaxStep); err != nil { - return nil, err - } - result.Stats = r.FormValue("stats") result.Path = r.URL.Path diff --git a/pkg/querier/tripperware/instantquery/shard_by_query_test.go b/pkg/querier/tripperware/instantquery/shard_by_query_test.go index 50c83cb5eeb..0d4dfc41a95 100644 --- a/pkg/querier/tripperware/instantquery/shard_by_query_test.go +++ b/pkg/querier/tripperware/instantquery/shard_by_query_test.go @@ -2,7 +2,6 @@ package instantquery import ( "testing" - "time" "github.com/cortexproject/cortex/pkg/querier/tripperware" "github.com/cortexproject/cortex/pkg/querier/tripperware/queryrange" @@ -10,5 +9,5 @@ import ( func Test_shardQuery(t *testing.T) { t.Parallel() - tripperware.TestQueryShardQuery(t, InstantQueryCodec, queryrange.NewPrometheusCodec(true, time.Minute)) + tripperware.TestQueryShardQuery(t, InstantQueryCodec, queryrange.NewPrometheusCodec(true)) } diff --git a/pkg/querier/tripperware/queryrange/query_range.go b/pkg/querier/tripperware/queryrange/query_range.go index 704049e66a2..016a2e7ef7a 100644 --- a/pkg/querier/tripperware/queryrange/query_range.go +++ b/pkg/querier/tripperware/queryrange/query_range.go @@ -46,15 +46,10 @@ var ( type prometheusCodec struct { sharded bool - - noStepSubQueryInterval time.Duration } -func NewPrometheusCodec(sharded bool, noStepSubQueryInterval time.Duration) *prometheusCodec { //nolint:revive - return &prometheusCodec{ - sharded: sharded, - noStepSubQueryInterval: noStepSubQueryInterval, - } +func NewPrometheusCodec(sharded bool) *prometheusCodec { //nolint:revive + return &prometheusCodec{sharded: sharded} } // WithStartEnd clones the current `PrometheusRequest` with a new `start` and `end` timestamp. @@ -203,10 +198,6 @@ func (c prometheusCodec) DecodeRequest(_ context.Context, r *http.Request, forwa } result.Query = r.FormValue("query") - if err := tripperware.SubQueryStepSizeCheck(result.Query, c.noStepSubQueryInterval, tripperware.MaxStep); err != nil { - return nil, err - } - result.Stats = r.FormValue("stats") result.Path = r.URL.Path diff --git a/pkg/querier/tripperware/queryrange/query_range_middlewares_test.go b/pkg/querier/tripperware/queryrange/query_range_middlewares_test.go index 41821695230..5ade2abd522 100644 --- a/pkg/querier/tripperware/queryrange/query_range_middlewares_test.go +++ b/pkg/querier/tripperware/queryrange/query_range_middlewares_test.go @@ -20,8 +20,8 @@ import ( ) var ( - PrometheusCodec = NewPrometheusCodec(false, time.Minute) - ShardedPrometheusCodec = NewPrometheusCodec(false, time.Minute) + PrometheusCodec = NewPrometheusCodec(false) + ShardedPrometheusCodec = NewPrometheusCodec(false) ) func TestRoundTrip(t *testing.T) { diff --git a/pkg/querier/tripperware/roundtrip.go b/pkg/querier/tripperware/roundtrip.go index 3aa6b1313f1..dc0a5c52712 100644 --- a/pkg/querier/tripperware/roundtrip.go +++ b/pkg/querier/tripperware/roundtrip.go @@ -142,15 +142,19 @@ func NewQueryTripperware( activeUsers.UpdateUserTimestamp(userStr, time.Now()) queriesPerTenant.WithLabelValues(op, userStr).Inc() - if isQueryRange { - return queryrange.RoundTrip(r) - } else if isQuery { - // If the given query is not shardable, use downstream roundtripper. + if isQuery || isQueryRange { query := r.FormValue("query") // Check subquery step size. if err := SubQueryStepSizeCheck(query, defaultSubQueryInterval, MaxStep); err != nil { return nil, err } + } + + if isQueryRange { + return queryrange.RoundTrip(r) + } else if isQuery { + // If the given query is not shardable, use downstream roundtripper. + query := r.FormValue("query") // If vertical sharding is not enabled for the tenant, use downstream roundtripper. numShards := validation.SmallestPositiveIntPerTenant(tenantIDs, limits.QueryVerticalShardSize) From 25481eef785a6cc166f6b561bbdb6aa45a23153d Mon Sep 17 00:00:00 2001 From: Ben Ye Date: Wed, 26 Jul 2023 08:43:11 -0700 Subject: [PATCH 2/2] fix tests Signed-off-by: Ben Ye --- pkg/querier/tripperware/queryrange/query_range_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/querier/tripperware/queryrange/query_range_test.go b/pkg/querier/tripperware/queryrange/query_range_test.go index 8951015f00f..dccbde73962 100644 --- a/pkg/querier/tripperware/queryrange/query_range_test.go +++ b/pkg/querier/tripperware/queryrange/query_range_test.go @@ -61,10 +61,6 @@ func TestRequest(t *testing.T) { url: "api/v1/query_range?start=0&end=11001&step=1", expectedErr: errStepTooSmall, }, - { - url: "/api/v1/query?query=up%5B30d%3A%5D&start=123&end=456&step=10", - expectedErr: httpgrpc.Errorf(http.StatusBadRequest, tripperware.ErrSubQueryStepTooSmall, 11000), - }, } { tc := tc t.Run(tc.url, func(t *testing.T) {