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:
authorPatrick Steinhardt <psteinhardt@gitlab.com>2022-11-08 14:48:13 +0300
committerPatrick Steinhardt <psteinhardt@gitlab.com>2022-11-08 15:49:15 +0300
commit6b32a64bbd4567122643505bdb93a748f2417da4 (patch)
tree1d0a56ea39042bf4b53e9dede05950ab01e6252a
parent3cd2794f55fd6e425aa0ab38d2d043da35e961f7 (diff)
global: Fix use of different cgroups when serving repos
There are several Git commands required for serving repositories to clients that typically don't get spawned in the repository itself, but which get the repository path as argument. These are git-upload-pack(1), git-upload-archive(1) and git-receive-pack(1), all of which we use in our `smarthttp` and `ssh` services. And because they don't require us to set up the repository via the `--git-dir` option, we're currently using `NewWithoutRepo()` to spawn them. This has a major downside in the context of cgroups though. In order to decide the cgroup bucket into which we put spawned commands, we either hash the repository's information or, if not present, the command line. As we're not passing a repo when serving them, it means that we always use the command line for them. This is different from most of the other RPCs though, where we use the repository information instead. Ultimately this means that every repository has two different cgroups it frequently gets assigned to, and that's of course not great. Fix this bug by switching to `New()` instead of `NewWithoutRepo()`. While not really necessary, it also shouldn't harm us either to make the Git directory known to git-upload-pack(1) et al. And given that we now have a repository context available, it means that we will put serving commands into the same cgroup as for all the other commands. Changelog: fixed
-rw-r--r--internal/gitaly/service/smarthttp/inforefs.go2
-rw-r--r--internal/gitaly/service/smarthttp/receive_pack.go2
-rw-r--r--internal/gitaly/service/smarthttp/upload_pack.go2
-rw-r--r--internal/gitaly/service/ssh/monitor_stdin_command.go13
-rw-r--r--internal/gitaly/service/ssh/receive_pack.go2
-rw-r--r--internal/gitaly/service/ssh/upload_archive.go2
-rw-r--r--internal/gitaly/service/ssh/upload_pack.go2
7 files changed, 17 insertions, 8 deletions
diff --git a/internal/gitaly/service/smarthttp/inforefs.go b/internal/gitaly/service/smarthttp/inforefs.go
index 8799f517f..431429eb0 100644
--- a/internal/gitaly/service/smarthttp/inforefs.go
+++ b/internal/gitaly/service/smarthttp/inforefs.go
@@ -70,7 +70,7 @@ func (s *server) handleInfoRefs(ctx context.Context, service, repoPath string, r
}
cmdOpts = append(cmdOpts, git.WithConfig(config...))
- cmd, err := s.gitCmdFactory.NewWithoutRepo(ctx, git.SubCmd{
+ cmd, err := s.gitCmdFactory.New(ctx, req.GetRepository(), git.SubCmd{
Name: service,
Flags: []git.Option{git.Flag{Name: "--stateless-rpc"}, git.Flag{Name: "--advertise-refs"}},
Args: []string{repoPath},
diff --git a/internal/gitaly/service/smarthttp/receive_pack.go b/internal/gitaly/service/smarthttp/receive_pack.go
index f7c1e123e..5fed6f6b0 100644
--- a/internal/gitaly/service/smarthttp/receive_pack.go
+++ b/internal/gitaly/service/smarthttp/receive_pack.go
@@ -50,7 +50,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
return err
}
- cmd, err := s.gitCmdFactory.NewWithoutRepo(ctx,
+ cmd, err := s.gitCmdFactory.New(ctx, req.GetRepository(),
git.SubCmd{
Name: "receive-pack",
Flags: []git.Option{git.Flag{Name: "--stateless-rpc"}},
diff --git a/internal/gitaly/service/smarthttp/upload_pack.go b/internal/gitaly/service/smarthttp/upload_pack.go
index 2fd4956dc..83076ee47 100644
--- a/internal/gitaly/service/smarthttp/upload_pack.go
+++ b/internal/gitaly/service/smarthttp/upload_pack.go
@@ -142,7 +142,7 @@ func (s *server) runUploadPack(ctx context.Context, req basicPostUploadPackReque
git.WithPackObjectsHookEnv(req.GetRepository(), "http"),
}
- cmd, err := s.gitCmdFactory.NewWithoutRepo(ctx, git.SubCmd{
+ cmd, err := s.gitCmdFactory.New(ctx, req.GetRepository(), git.SubCmd{
Name: "upload-pack",
Flags: []git.Option{git.Flag{Name: "--stateless-rpc"}},
Args: []string{repoPath},
diff --git a/internal/gitaly/service/ssh/monitor_stdin_command.go b/internal/gitaly/service/ssh/monitor_stdin_command.go
index dbed02dc3..f3bf43a70 100644
--- a/internal/gitaly/service/ssh/monitor_stdin_command.go
+++ b/internal/gitaly/service/ssh/monitor_stdin_command.go
@@ -8,15 +8,24 @@ import (
"gitlab.com/gitlab-org/gitaly/v15/internal/command"
"gitlab.com/gitlab-org/gitaly/v15/internal/git"
"gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline"
+ "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
)
-func monitorStdinCommand(ctx context.Context, gitCmdFactory git.CommandFactory, stdin io.Reader, stdout, stderr io.Writer, sc git.SubCmd, opts ...git.CmdOpt) (*command.Command, *pktline.ReadMonitor, error) {
+func monitorStdinCommand(
+ ctx context.Context,
+ gitCmdFactory git.CommandFactory,
+ repo *gitalypb.Repository,
+ stdin io.Reader,
+ stdout, stderr io.Writer,
+ sc git.SubCmd,
+ opts ...git.CmdOpt,
+) (*command.Command, *pktline.ReadMonitor, error) {
stdinPipe, monitor, cleanup, err := pktline.NewReadMonitor(ctx, stdin)
if err != nil {
return nil, nil, fmt.Errorf("create monitor: %w", err)
}
- cmd, err := gitCmdFactory.NewWithoutRepo(ctx, sc, append([]git.CmdOpt{
+ cmd, err := gitCmdFactory.New(ctx, repo, sc, append([]git.CmdOpt{
git.WithStdin(stdinPipe),
git.WithStdout(stdout),
git.WithStderr(stderr),
diff --git a/internal/gitaly/service/ssh/receive_pack.go b/internal/gitaly/service/ssh/receive_pack.go
index c8d0657ba..4f2a84988 100644
--- a/internal/gitaly/service/ssh/receive_pack.go
+++ b/internal/gitaly/service/ssh/receive_pack.go
@@ -76,7 +76,7 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer,
return err
}
- cmd, err := s.gitCmdFactory.NewWithoutRepo(ctx,
+ cmd, err := s.gitCmdFactory.New(ctx, req.GetRepository(),
git.SubCmd{
Name: "receive-pack",
Args: []string{repoPath},
diff --git a/internal/gitaly/service/ssh/upload_archive.go b/internal/gitaly/service/ssh/upload_archive.go
index b473a10b9..7ed1c0ac5 100644
--- a/internal/gitaly/service/ssh/upload_archive.go
+++ b/internal/gitaly/service/ssh/upload_archive.go
@@ -53,7 +53,7 @@ func (s *server) sshUploadArchive(stream gitalypb.SSHService_SSHUploadArchiveSer
return stream.Send(&gitalypb.SSHUploadArchiveResponse{Stderr: p})
})
- cmd, monitor, err := monitorStdinCommand(ctx, s.gitCmdFactory, stdin, stdout, stderr, git.SubCmd{
+ cmd, monitor, err := monitorStdinCommand(ctx, s.gitCmdFactory, req.GetRepository(), stdin, stdout, stderr, git.SubCmd{
Name: "upload-archive",
Args: []string{repoPath},
})
diff --git a/internal/gitaly/service/ssh/upload_pack.go b/internal/gitaly/service/ssh/upload_pack.go
index 82318968c..072b8ec9c 100644
--- a/internal/gitaly/service/ssh/upload_pack.go
+++ b/internal/gitaly/service/ssh/upload_pack.go
@@ -128,7 +128,7 @@ func (s *server) sshUploadPack(rpcContext context.Context, req sshUploadPackRequ
var stderrBuilder strings.Builder
stderr = io.MultiWriter(stderr, &stderrBuilder)
- cmd, monitor, err := monitorStdinCommand(ctx, s.gitCmdFactory, stdin, stdout, stderr, git.SubCmd{
+ cmd, monitor, err := monitorStdinCommand(ctx, s.gitCmdFactory, repo, stdin, stdout, stderr, git.SubCmd{
Name: "upload-pack",
Args: []string{repoPath},
}, commandOpts...)