Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohn Cai <jcai@gitlab.com>2022-08-08 16:55:13 +0300
committerJohn Cai <jcai@gitlab.com>2022-08-08 16:55:13 +0300
commitf2a27e4b86af36d0b30f8b47c628c97ddf11d303 (patch)
treea3b68ab1b3ca50f25e7f820aa056389e707276d4
parent6bb5f6969910ce5010f1c894ee671a86e656e6da (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.go10
-rw-r--r--internal/middleware/limithandler/middleware_test.go27
-rw-r--r--internal/middleware/limithandler/rate_limiter.go36
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()