Merge branch 'pks-git-grep-limit-threading' into 'master'

git: Globally limit number of threads

Closes #4120

See merge request gitlab-org/gitaly!4443

5 files changed, 37 insertions, 44 deletions

diff --git a/internal/git/command_description.go b/internal/git/command_description.go
index cb26d08d5..d68560554 100644
--- a/internal/git/command_description.go
+++ b/internal/git/command_description.go
@@ -3,6 +3,8 @@ package git
 import (
 	"fmt"
 	"log"
+	"math"
+	"runtime"
 	"strings"
 )
 
@@ -126,6 +128,12 @@ var commandDescriptions = map[string]commandDescription{
 		// git-grep(1) does not support disambiguating options from paths from
 		// revisions.
 		flags: scNoRefUpdates | scNoEndOfOptions,
+		opts: []GlobalOption{
+			// This command by default spawns as many threads as there are CPUs. This
+			// easily impacts concurrently running commands by exhausting cores and
+			// generating excessive I/O load.
+			ConfigPair{Key: "grep.threads", Value: threadsConfigValue(runtime.NumCPU())},
+		},
 	},
 	"hash-object": {
 		flags: scNoRefUpdates,
@@ -434,5 +442,12 @@ func packConfiguration() []GlobalOption {
 	return []GlobalOption{
 		ConfigPair{Key: "pack.windowMemory", Value: "100m"},
 		ConfigPair{Key: "pack.writeReverseIndex", Value: "true"},
+		ConfigPair{Key: "pack.threads", Value: threadsConfigValue(runtime.NumCPU())},
 	}
 }
+
+// threadsConfigValue returns the log-2 number of threads based on the number of provided CPUs. This
+// prevents us from using excessively many threads and thus avoids exhaustion of all available CPUs.
+func threadsConfigValue(numCPUs int) string {
+	return fmt.Sprintf("%d", int(math.Max(1, math.Floor(math.Log2(float64(numCPUs))))))
+}
diff --git a/internal/git/command_description_test.go b/internal/git/command_description_test.go
index 09209df15..52da034c3 100644
--- a/internal/git/command_description_test.go
+++ b/internal/git/command_description_test.go
@@ -45,3 +45,25 @@ func TestCommandDescriptions_revListPositionalArgs(t *testing.T) {
 		})
 	}
 }
+
+func TestThreadsConfigValue(t *testing.T) {
+	t.Parallel()
+	for _, tt := range []struct {
+		cpus    int
+		threads string
+	}{
+		{1, "1"},
+		{2, "1"},
+		{3, "1"},
+		{4, "2"},
+		{8, "3"},
+		{9, "3"},
+		{13, "3"},
+		{16, "4"},
+		{27, "4"},
+		{32, "5"},
+	} {
+		actualThreads := threadsConfigValue(tt.cpus)
+		require.Equal(t, tt.threads, actualThreads)
+	}
+}
diff --git a/internal/git/housekeeping/objects.go b/internal/git/housekeeping/objects.go
index 7fbe33f06..7f5f13e60 100644
--- a/internal/git/housekeeping/objects.go
+++ b/internal/git/housekeeping/objects.go
@@ -2,9 +2,6 @@ package housekeeping
 
 import (
 	"context"
-	"fmt"
-	"math"
-	"runtime"
 
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
@@ -38,7 +35,6 @@ func RepackObjects(ctx context.Context, repo *localrepo.Repo, cfg RepackObjectsC
 			git.Flag{Name: "-A"},
 			git.Flag{Name: "--pack-kept-objects"},
 			git.Flag{Name: "-l"},
-			log2Threads(runtime.NumCPU()),
 		)
 	}
 
@@ -82,11 +78,3 @@ func GetRepackGitConfig(ctx context.Context, bitmap bool) []git.ConfigPair {
 
 	return config
 }
-
-// log2Threads returns the log-2 number of threads based on the number of
-// provided CPUs. This prevents repacking operations from exhausting all
-// available CPUs and increasing request latency
-func log2Threads(numCPUs int) git.ValueFlag {
-	n := math.Max(1, math.Floor(math.Log2(float64(numCPUs))))
-	return git.ValueFlag{Name: "--threads", Value: fmt.Sprint(n)}
-}
diff --git a/internal/gitaly/service/repository/repack.go b/internal/gitaly/service/repository/repack.go
index 18415d4ba..6ba97185d 100644
--- a/internal/gitaly/service/repository/repack.go
+++ b/internal/gitaly/service/repository/repack.go
@@ -3,11 +3,9 @@ package repository
 import (
 	"context"
 	"fmt"
-	"math"
 
 	"github.com/prometheus/client_golang/prometheus"
 	gitalyerrors "gitlab.com/gitlab-org/gitaly/v14/internal/errors"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/housekeeping"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -25,14 +23,6 @@ func init() {
 	prometheus.MustRegister(repackCounter)
 }
 
-// log2Threads returns the log-2 number of threads based on the number of
-// provided CPUs. This prevents repacking operations from exhausting all
-// available CPUs and increasing request latency
-func log2Threads(numCPUs int) git.ValueFlag {
-	n := math.Max(1, math.Floor(math.Log2(float64(numCPUs))))
-	return git.ValueFlag{Name: "--threads", Value: fmt.Sprint(n)}
-}
-
 func (s *server) RepackFull(ctx context.Context, in *gitalypb.RepackFullRequest) (*gitalypb.RepackFullResponse, error) {
 	if in.GetRepository() == nil {
 		return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository)
diff --git a/internal/gitaly/service/repository/repack_test.go b/internal/gitaly/service/repository/repack_test.go
index 7e73290a6..d7efb78dc 100644
--- a/internal/gitaly/service/repository/repack_test.go
+++ b/internal/gitaly/service/repository/repack_test.go
@@ -300,25 +300,3 @@ func TestRepackFullDeltaIslands(t *testing.T) {
 		return err
 	})
 }
-
-func TestLog2Threads(t *testing.T) {
-	t.Parallel()
-	for _, tt := range []struct {
-		cpus    int
-		threads string
-	}{
-		{1, "1"},
-		{2, "1"},
-		{3, "1"},
-		{4, "2"},
-		{8, "3"},
-		{9, "3"},
-		{13, "3"},
-		{16, "4"},
-		{27, "4"},
-		{32, "5"},
-	} {
-		actualThreads := log2Threads(tt.cpus)
-		require.Equal(t, tt.threads, actualThreads.Value)
-	}
-}