Remove ruby concurrency limiter

2 files changed, 1 insertions, 69 deletions

diff --git a/internal/rubyserver/concurrency_test.go b/internal/rubyserver/concurrency_test.go
index 98b86f90a..e82311513 100644
--- a/internal/rubyserver/concurrency_test.go
+++ b/internal/rubyserver/concurrency_test.go
@@ -7,7 +7,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"google.golang.org/grpc/codes"
@@ -15,42 +14,6 @@ import (
 	"google.golang.org/grpc/status"
 )
 
-func TestConcurrency(t *testing.T) {
-	testhelper.ConfigureRuby()
-	config.Config.Ruby.Concurrency = 1
-
-	s, err := Start()
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer s.Stop()
-
-	require.NoError(t, waitPing(s), "wait for gitaly-ruby to boot")
-
-	start := make(chan struct{})
-	wg := &sync.WaitGroup{}
-
-	for i := 0; i < 100; i++ {
-		wg.Add(1)
-
-		go func() {
-			defer wg.Done()
-
-			<-start
-			for j := 0; j < 10; j++ {
-				// Before we had gitaly-ruby rate limiting, this could result in
-				// ResourceExhausted errors. This test is meant to guard against that
-				// problem coming back.
-				require.NoError(t, makeRequest(s))
-			}
-		}()
-	}
-
-	close(start) // increase chances that goroutines all run at once
-
-	wg.Wait()
-}
-
 func waitPing(s *Server) error {
 	var err error
 	for start := time.Now(); time.Since(start) < ConnectTimeout; time.Sleep(100 * time.Millisecond) {
diff --git a/internal/rubyserver/rubyserver.go b/internal/rubyserver/rubyserver.go
index 0c07e9875..8b9f159dc 100644
--- a/internal/rubyserver/rubyserver.go
+++ b/internal/rubyserver/rubyserver.go
@@ -14,7 +14,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/internal/middleware/limithandler"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver/balancer"
 	"gitlab.com/gitlab-org/gitaly/internal/supervisor"
 	"gitlab.com/gitlab-org/gitaly/internal/version"
@@ -70,7 +69,6 @@ type Server struct {
 	workers      []*worker
 	clientConnMu sync.Mutex
 	clientConn   *grpc.ClientConn
-	limiter      *limithandler.ConcurrencyLimiter
 }
 
 // Stop shuts down the gitaly-ruby helper process and cleans up resources.
@@ -103,19 +101,6 @@ func Start() (*Server, error) {
 	}
 
 	cfg := config.Config
-
-	// We need a wide margin between the concurrency limit enforced on the
-	// client side and the thread pool size in the gitaly-ruby grpc server.
-	// If the client side limit is too close to the server thread pool size,
-	// or if the client does not limit at all, we can get ResourceExhausted
-	// errors from gitaly-ruby when it runs out of threads in its pool.
-	//
-	// Our choice of 2 * cfg.Ruby.Concurrency is probably not the optimal
-	// formula but it is good enough. Ruby threads are not that expensive,
-	// and it is pointless to set up large thread pools (e.g. 100 threads) in
-	// a single Ruby process anyway because of its Global Interpreter Lock.
-	rubyThreadPoolSize := 2 * cfg.Ruby.Concurrency
-
 	env := append(
 		os.Environ(),
 		"GITALY_RUBY_GIT_BIN_PATH="+command.GitPath(),
@@ -124,7 +109,6 @@ func Start() (*Server, error) {
 		"GITALY_RUBY_GITLAB_SHELL_PATH="+cfg.GitlabShell.Dir,
 		"GITALY_RUBY_GITALY_BIN_DIR="+cfg.BinDir,
 		"GITALY_VERSION="+version.GetVersion(),
-		fmt.Sprintf("GITALY_RUBY_THREAD_POOL_SIZE=%d", rubyThreadPoolSize),
 	)
 	if dsn := cfg.Logging.RubySentryDSN; dsn != "" {
 		env = append(env, "SENTRY_DSN="+dsn)
@@ -132,9 +116,7 @@ func Start() (*Server, error) {
 
 	gitalyRuby := path.Join(cfg.Ruby.Dir, "bin/gitaly-ruby")
 
-	s := &Server{
-		limiter: limithandler.NewLimiter(cfg.Ruby.Concurrency, limithandler.NewPromMonitor("gitaly-ruby", "")),
-	}
+	s := &Server{}
 	for i := 0; i < cfg.Ruby.NumWorkers; i++ {
 		name := fmt.Sprintf("gitaly-ruby.%d", i)
 		socketPath := socketPath(i)
@@ -230,19 +212,6 @@ func (s *Server) BlobServiceClient(ctx context.Context) (pb.BlobServiceClient, e
 }
 
 func (s *Server) getConnection(ctx context.Context) (*grpc.ClientConn, error) {
-	ourTurn := make(chan struct{})
-	go s.limiter.Limit(ctx, "gitaly-ruby", func() (interface{}, error) {
-		close(ourTurn)
-		<-ctx.Done()
-		return nil, nil
-	})
-
-	select {
-	case <-ctx.Done():
-		return nil, ctx.Err()
-	case <-ourTurn:
-	}
-
 	s.clientConnMu.Lock()
 	conn := s.clientConn
 	s.clientConnMu.Unlock()