From 67e4d02c28204c362674df1b72d27767ab9bb355 Mon Sep 17 00:00:00 2001 From: Casie Chen Date: Mon, 12 Aug 2024 17:51:28 -0700 Subject: [PATCH] Fix tests and lint --- pkg/frontend/v2/frontend_test.go | 18 +++++++++--------- pkg/scheduler/queue/queue_test.go | 5 ++--- pkg/scheduler/scheduler.go | 17 +++++++++++------ pkg/util/http.go | 2 +- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/pkg/frontend/v2/frontend_test.go b/pkg/frontend/v2/frontend_test.go index 61ab40b51bf..83fe9f39c49 100644 --- a/pkg/frontend/v2/frontend_test.go +++ b/pkg/frontend/v2/frontend_test.go @@ -68,7 +68,7 @@ func setupFrontendWithConcurrencyAndServerOptions(t *testing.T, reg prometheus.R grpcPort, err := strconv.Atoi(p) require.NoError(t, err) - cfg := Config{AdditionalQueryQueueDimensionsEnabled: true} + cfg := Config{} flagext.DefaultValues(&cfg) cfg.SchedulerAddress = l.Addr().String() cfg.WorkerConcurrency = concurrency @@ -142,7 +142,7 @@ func TestFrontendBasicWorkflow(t *testing.T) { }) req := &httpgrpc.HTTPRequest{ - Url: "/api/v1/query_range?start=946684800&end=946771200&step=60", + Url: "/api/v1/query_range?start=946684800&end=946771200&step=60&query=up{}", } resp, _, err := f.RoundTripGRPC(user.InjectOrgID(context.Background(), userID), req) require.NoError(t, err) @@ -178,7 +178,7 @@ func TestFrontend_ShouldTrackPerRequestMetrics(t *testing.T) { assert.Equal(t, uint64(0), metricsMap["cortex_query_frontend_enqueue_duration_seconds"].GetMetric()[0].GetHistogram().GetSampleCount()) req := &httpgrpc.HTTPRequest{ - Url: "/api/v1/query_range?start=946684800&end=946771200&step=60", + Url: "/api/v1/query_range?start=946684800&end=946771200&step=60&query=up{}", } resp, _, err := f.RoundTripGRPC(user.InjectOrgID(context.Background(), userID), req) require.NoError(t, err) @@ -223,7 +223,7 @@ func TestFrontendRetryEnqueue(t *testing.T) { return &schedulerpb.SchedulerToFrontend{Status: schedulerpb.OK} }) req := &httpgrpc.HTTPRequest{ - Url: "/api/v1/query_range?start=946684800&end=946771200&step=60", + Url: "/api/v1/query_range?start=946684800&end=946771200&step=60&query=up{}", } _, _, err := f.RoundTripGRPC(user.InjectOrgID(context.Background(), userID), req) require.NoError(t, err) @@ -235,7 +235,7 @@ func TestFrontendTooManyRequests(t *testing.T) { }) req := &httpgrpc.HTTPRequest{ - Url: "/api/v1/query_range?start=946684800&end=946771200&step=60", + Url: "/api/v1/query_range?start=946684800&end=946771200&step=60&query=up{}", } resp, _, err := f.RoundTripGRPC(user.InjectOrgID(context.Background(), "test"), req) require.NoError(t, err) @@ -262,7 +262,7 @@ func TestFrontendCancellation(t *testing.T) { defer cancel() req := &httpgrpc.HTTPRequest{ - Url: "/api/v1/query_range?start=946684800&end=946771200&step=60", + Url: "/api/v1/query_range?start=946684800&end=946771200&step=60&query=up{}", } resp, _, err := f.RoundTripGRPC(user.InjectOrgID(ctx, "test"), req) require.EqualError(t, err, context.DeadlineExceeded.Error()) @@ -295,7 +295,7 @@ func TestFrontendWorkerCancellation(t *testing.T) { // send multiple requests > maxconcurrency of scheduler. So that it keeps all the frontend worker busy in serving requests. req := &httpgrpc.HTTPRequest{ - Url: "/api/v1/query_range?start=946684800&end=946771200&step=60", + Url: "/api/v1/query_range?start=946684800&end=946771200&step=60&query=up{}", } reqCount := testFrontendWorkerConcurrency + 5 var wg sync.WaitGroup @@ -366,7 +366,7 @@ func TestFrontendFailedCancellation(t *testing.T) { // send request req := &httpgrpc.HTTPRequest{ - Url: "/api/v1/query_range?start=946684800&end=946771200&step=60", + Url: "/api/v1/query_range?start=946684800&end=946771200&step=60&query=up{}", } resp, _, err := f.RoundTripGRPC(user.InjectOrgID(ctx, "test"), req) require.EqualError(t, err, context.Canceled.Error()) @@ -686,7 +686,7 @@ func TestWithClosingGrpcServer(t *testing.T) { // Connection will be established on the first roundtrip. req := &httpgrpc.HTTPRequest{ - Url: "/api/v1/query_range?start=946684800&end=946771200&step=60", + Url: "/api/v1/query_range?start=946684800&end=946771200&step=60&query=up{}", } resp, _, err := f.RoundTripGRPC(user.InjectOrgID(context.Background(), userID), req) require.NoError(t, err) diff --git a/pkg/scheduler/queue/queue_test.go b/pkg/scheduler/queue/queue_test.go index 4b4c1a0f663..8723fc82b44 100644 --- a/pkg/scheduler/queue/queue_test.go +++ b/pkg/scheduler/queue/queue_test.go @@ -127,13 +127,12 @@ func TestMultiDimensionalQueueFairnessSlowConsumerEffects(t *testing.T) { normalQueueDimension := "normal-request" slowConsumerLatency := 20 * time.Millisecond slowConsumerQueueDimension := "slow-request" - normalQueueDimensionFunc := func(usingMultipleDimensions bool) []string { return []string{"normal-channel"} } + normalQueueDimensionFunc := func(_ bool) []string { return []string{"normal-channel"} } slowQueueDimensionFunc := func(usingMultipleDimensions bool) []string { if usingMultipleDimensions { return []string{"slow-channel"} - } else { - return []string{"normal-channel"} } + return []string{"normal-channel"} } useMultipleDimensions := []bool{false, true} diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 6d64a58f2f2..be191cbcdf9 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -113,11 +113,7 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet, logger log.Logger) { } func (cfg *Config) Validate() error { - err := cfg.ServiceDiscovery.Validate() - if err != nil { - return err - } - return nil + return cfg.ServiceDiscovery.Validate() } // NewScheduler creates a new Scheduler. @@ -162,7 +158,16 @@ func NewScheduler(cfg Config, limits Limits, log log.Logger, registerer promethe []string{"query_component"}, ) - s.requestQueue, err = queue.NewRequestQueue(s.log, cfg.MaxOutstandingPerTenant, cfg.UseMultiAlgorithmQueryQueue, cfg.QuerierForgetDelay, s.queueLength, s.discardedRequests, enqueueDuration, querierInflightRequestsMetric) + s.requestQueue, err = queue.NewRequestQueue( + s.log, + cfg.MaxOutstandingPerTenant, + cfg.UseMultiAlgorithmQueryQueue, + cfg.QuerierForgetDelay, + s.queueLength, + s.discardedRequests, + enqueueDuration, + querierInflightRequestsMetric, + ) if err != nil { return nil, err } diff --git a/pkg/util/http.go b/pkg/util/http.go index 4460a4de4e6..3087b3e37bd 100644 --- a/pkg/util/http.go +++ b/pkg/util/http.go @@ -342,7 +342,7 @@ func SerializeProtoResponse(w http.ResponseWriter, resp proto.Message, compressi } // ParseRequestFormWithoutConsumingBody parsed and returns the request parameters (query string and/or request body) -// from the input http.Request. If the request has a Body, the request's Body is replaces so that it can be consumed again. +// from the input http.Request. If the request has a Body, the request's Body is replaced so that it can be consumed again. // It does not check the req.Body size, so it is the caller's responsibility to ensure that the body is not too large. func ParseRequestFormWithoutConsumingBody(r *http.Request) (url.Values, error) { if r.Body == nil {