diff options
author | John Cai <jcai@gitlab.com> | 2022-08-08 16:55:13 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2022-08-08 16:55:13 +0300 |
commit | f2a27e4b86af36d0b30f8b47c628c97ddf11d303 (patch) | |
tree | a3b68ab1b3ca50f25e7f820aa056389e707276d4 | |
parent | 6bb5f6969910ce5010f1c894ee671a86e656e6da (diff) |
limithandler: Remove RateLimiting feature flagjc-remove-rate-limit-ff
Now that this feature has been running in production for a few weeks
without issue, we can remove the feature flag.
-rw-r--r-- | internal/metadata/featureflag/ff_rate_limiter.go | 10 | ||||
-rw-r--r-- | internal/middleware/limithandler/middleware_test.go | 27 | ||||
-rw-r--r-- | internal/middleware/limithandler/rate_limiter.go | 36 |
3 files changed, 27 insertions, 46 deletions
diff --git a/internal/metadata/featureflag/ff_rate_limiter.go b/internal/metadata/featureflag/ff_rate_limiter.go deleted file mode 100644 index 7bdbe9afa..000000000 --- a/internal/metadata/featureflag/ff_rate_limiter.go +++ /dev/null @@ -1,10 +0,0 @@ -package featureflag - -// RateLimit will enable the rate limiter to reject requests beyond a configured -// rate. -var RateLimit = NewFeatureFlag( - "rate_limit", - "v14.10.0", - "https://gitlab.com/gitlab-org/gitaly/-/issues/4181", - false, -) diff --git a/internal/middleware/limithandler/middleware_test.go b/internal/middleware/limithandler/middleware_test.go index c929d19ea..9a2fb9870 100644 --- a/internal/middleware/limithandler/middleware_test.go +++ b/internal/middleware/limithandler/middleware_test.go @@ -16,7 +16,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" - "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler" pb "gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler/testdata" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" @@ -482,10 +481,9 @@ gitaly_requests_dropped_total{grpc_method="Unary",grpc_service="test.limithandle func TestRateLimitHandler(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.RateLimit).Run(t, testRateLimitHandler) -} -func testRateLimitHandler(t *testing.T, ctx context.Context) { + ctx := testhelper.Context(t) + methodName := "/test.limithandler.Test/Unary" cfg := config.Cfg{ RateLimiting: []config.RateLimiting{ @@ -519,21 +517,16 @@ func testRateLimitHandler(t *testing.T, ctx context.Context) { for i := 0; i < 10; i++ { _, err := client.Unary(ctx, &pb.UnaryRequest{}) - if featureflag.RateLimit.IsEnabled(ctx) { - s, ok := status.FromError(err) - require.True(t, ok) - details := s.Details() - require.Len(t, details, 1) - - limitErr, ok := details[0].(*gitalypb.LimitError) - require.True(t, ok) + s, ok := status.FromError(err) + require.True(t, ok) + details := s.Details() + require.Len(t, details, 1) - assert.Equal(t, limithandler.ErrRateLimit.Error(), limitErr.ErrorMessage) - assert.Equal(t, durationpb.New(0), limitErr.RetryAfter) + limitErr, ok := details[0].(*gitalypb.LimitError) + require.True(t, ok) - } else { - require.NoError(t, err) - } + assert.Equal(t, limithandler.ErrRateLimit.Error(), limitErr.ErrorMessage) + assert.Equal(t, durationpb.New(0), limitErr.RetryAfter) } expectedMetrics := `# HELP gitaly_requests_dropped_total Number of requests dropped from the queue diff --git a/internal/middleware/limithandler/rate_limiter.go b/internal/middleware/limithandler/rate_limiter.go index 63124b0d7..89f6deeab 100644 --- a/internal/middleware/limithandler/rate_limiter.go +++ b/internal/middleware/limithandler/rate_limiter.go @@ -9,7 +9,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" - "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/labkit/log" "golang.org/x/time/rate" @@ -43,26 +42,25 @@ func (r *RateLimiter) Limit(ctx context.Context, lockKey string, f LimitedFunc) // For now, we are only emitting this metric to get an idea of the shape // of traffic. r.requestsDroppedMetric.Inc() - if featureflag.RateLimit.IsEnabled(ctx) { - err := helper.ErrUnavailable(ErrRateLimit) - - detailedErr, errGeneratingDetailedErr := helper.ErrWithDetails( - err, - &gitalypb.LimitError{ - ErrorMessage: ErrRateLimit.Error(), - RetryAfter: durationpb.New(0), - }, - ) - if errGeneratingDetailedErr != nil { - log.WithField("rate_limit_error", err). - WithError(errGeneratingDetailedErr). - Error("failed to generate detailed error") - - return nil, err - } - return nil, detailedErr + err := helper.ErrUnavailable(ErrRateLimit) + + detailedErr, errGeneratingDetailedErr := helper.ErrWithDetails( + err, + &gitalypb.LimitError{ + ErrorMessage: ErrRateLimit.Error(), + RetryAfter: durationpb.New(0), + }, + ) + if errGeneratingDetailedErr != nil { + log.WithField("rate_limit_error", err). + WithError(errGeneratingDetailedErr). + Error("failed to generate detailed error") + + return nil, err } + + return nil, detailedErr } return f() |