repository: Use long-running filter process for converting LFS pointerspks-lfs-process-filter

Wire up support for running gitaly-lfs-smudge as a long-running filter
process to speed up conversion of LFS pointers into their converted
objects. This should result in a signficant performance boost when
creating archives for large repositories with many LFS pointers and
avoid the "process-spawn storms" we saw in the past whenever such an
archive was requested.

Changelog: performance

3 files changed, 33 insertions, 10 deletions

diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go
index 8cb7947df..fd2497ac2 100644
--- a/internal/gitaly/service/repository/archive.go
+++ b/internal/gitaly/service/repository/archive.go
@@ -18,13 +18,13 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/log"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v15/streamio"
 	"google.golang.org/protobuf/proto"
 )
 
 type archiveParams struct {
-	ctx         context.Context
 	writer      io.Writer
 	in          *gitalypb.GetArchiveRequest
 	compressCmd *exec.Cmd
@@ -83,8 +83,7 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo
 
 	ctxlogrus.Extract(ctx).WithField("request_hash", requestHash(in)).Info("request details")
 
-	return s.handleArchive(archiveParams{
-		ctx:         ctx,
+	return s.handleArchive(ctx, archiveParams{
 		writer:      writer,
 		in:          in,
 		compressCmd: compressCmd,
@@ -173,7 +172,7 @@ func findGetArchivePath(ctx context.Context, f *catfile.TreeEntryFinder, commitI
 	return true, nil
 }
 
-func (s *server) handleArchive(p archiveParams) error {
+func (s *server) handleArchive(ctx context.Context, p archiveParams) error {
 	var args []string
 	pathspecs := make([]string, 0, len(p.exclude)+1)
 	if !p.in.GetElidePath() {
@@ -203,6 +202,10 @@ func (s *server) handleArchive(p archiveParams) error {
 			DriverType:   smudge.DriverTypeFilter,
 		}
 
+		if featureflag.GetArchiveLfsFilterProcess.IsEnabled(ctx) {
+			smudgeCfg.DriverType = smudge.DriverTypeProcess
+		}
+
 		smudgeEnv, err := smudgeCfg.Environment()
 		if err != nil {
 			return fmt.Errorf("setting up smudge environment: %w", err)
@@ -221,7 +224,7 @@ func (s *server) handleArchive(p archiveParams) error {
 		config = append(config, smudgeGitConfig)
 	}
 
-	archiveCommand, err := s.gitCmdFactory.New(p.ctx, p.in.GetRepository(), git.SubCmd{
+	archiveCommand, err := s.gitCmdFactory.New(ctx, p.in.GetRepository(), git.SubCmd{
 		Name:        "archive",
 		Flags:       []git.Option{git.ValueFlag{Name: "--format", Value: p.format}, git.ValueFlag{Name: "--prefix", Value: p.in.GetPrefix() + "/"}},
 		Args:        args,
@@ -232,7 +235,7 @@ func (s *server) handleArchive(p archiveParams) error {
 	}
 
 	if p.compressCmd != nil {
-		command, err := command.New(p.ctx, p.compressCmd, archiveCommand, p.writer, nil)
+		command, err := command.New(ctx, p.compressCmd, archiveCommand, p.writer, nil)
 		if err != nil {
 			return err
 		}
diff --git a/internal/gitaly/service/repository/archive_test.go b/internal/gitaly/service/repository/archive_test.go
index 3f95bcf73..caa64a00b 100644
--- a/internal/gitaly/service/repository/archive_test.go
+++ b/internal/gitaly/service/repository/archive_test.go
@@ -3,6 +3,7 @@ package repository
 import (
 	"archive/zip"
 	"bytes"
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -17,6 +18,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitlab"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
@@ -168,7 +170,12 @@ func TestGetArchiveSuccess(t *testing.T) {
 	}
 }
 
-func TestGetArchiveWithLfsSuccess(t *testing.T) {
+func TestGetArchive_includeLfsBlobs(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.GetArchiveLfsFilterProcess).Run(t, testGetArchiveIncludeLfsBlobs)
+}
+
+func testGetArchiveIncludeLfsBlobs(t *testing.T, ctx context.Context) {
 	t.Parallel()
 	defaultOptions := gitlab.TestServerOptions{
 		SecretToken: secretToken,
@@ -192,7 +199,6 @@ func TestGetArchiveWithLfsSuccess(t *testing.T) {
 	client := newRepositoryClient(t, cfg, serverSocketPath)
 	cfg.SocketPath = serverSocketPath
 
-	ctx := testhelper.Context(t)
 	repo, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
 		Seed: gittest.SeedGitLabTest,
 	})
@@ -470,13 +476,17 @@ func TestGetArchivePathInjection(t *testing.T) {
 	require.Zero(t, authorizedKeysFileStat.Size())
 }
 
-func TestGetArchiveEnv(t *testing.T) {
+func TestGetArchive_environment(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.GetArchiveLfsFilterProcess).Run(t, testGetArchiveEnvironment)
+}
+
+func testGetArchiveEnvironment(t *testing.T, ctx context.Context) {
 	testhelper.SkipWithPraefect(t, "It's not possible to create repositories through the API with the git command overwritten by the script.")
 
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
-	ctx := testhelper.Context(t)
 
 	gitCmdFactory := gittest.NewInterceptingCommandFactory(ctx, t, cfg, func(git.ExecutionEnvironment) string {
 		return `#!/bin/sh
@@ -502,6 +512,11 @@ func TestGetArchiveEnv(t *testing.T) {
 		GlRepository: gittest.GlRepository,
 		Gitlab:       cfg.Gitlab,
 		TLS:          cfg.TLS,
+		DriverType:   smudge.DriverTypeFilter,
+	}
+
+	if featureflag.GetArchiveLfsFilterProcess.IsEnabled(ctx) {
+		smudgeCfg.DriverType = smudge.DriverTypeProcess
 	}
 
 	smudgeEnv, err := smudgeCfg.Environment()
diff --git a/internal/metadata/featureflag/ff_get_archive_lfs_filter_process.go b/internal/metadata/featureflag/ff_get_archive_lfs_filter_process.go
new file mode 100644
index 000000000..489f42f3a
--- /dev/null
+++ b/internal/metadata/featureflag/ff_get_archive_lfs_filter_process.go
@@ -0,0 +1,5 @@
+package featureflag
+
+// GetArchiveLfsFilterProcess enables the use of a long-running filter process to smudge LFS
+// pointers to their contents.
+var GetArchiveLfsFilterProcess = NewFeatureFlag("get_archive_lfs_filter_process", false)