Merge branch 'jc-exclude-alternates-from-repo-size' into 'master'

repository: Exclude alternate object directories in repository size

See merge request gitlab-org/gitaly!4558

5 files changed, 150 insertions, 12 deletions

diff --git a/internal/git/localrepo/repo.go b/internal/git/localrepo/repo.go
index 97525a1fc..6e37ca8c7 100644
--- a/internal/git/localrepo/repo.go
+++ b/internal/git/localrepo/repo.go
@@ -97,20 +97,30 @@ func errorWithStderr(err error, stderr []byte) error {
 // repoSizeConfig can be used to pass in different options to
 // git rev-list in determining the size of a repository.
 type repoSizeConfig struct {
-	// Excludes is a list of ref glob patterns to exclude from the size
+	// ExcludeRefs is a list of ref glob patterns to exclude from the size
 	// calculation.
-	Excludes []string
+	ExcludeRefs []string
+	// ExcludeAlternates will exclude objects in the alternates directory
+	// from being counted towards the total size of the repository.
+	ExcludeAlternates bool
 }
 
 // RepoSizeOption is an option which can be passed to Size
 type RepoSizeOption func(*repoSizeConfig)
 
-// WithExcludes is an option for Size that excludes certain refs from the size
+// WithExcludeRefs is an option for Size that excludes certain refs from the size
 // calculation. The format must be a glob pattern.
 // see https://git-scm.com/docs/git-rev-list#Documentation/git-rev-list.txt---excludeltglob-patterngt
-func WithExcludes(excludes ...string) RepoSizeOption {
+func WithExcludeRefs(excludeRefs ...string) RepoSizeOption {
 	return func(cfg *repoSizeConfig) {
-		cfg.Excludes = excludes
+		cfg.ExcludeRefs = excludeRefs
+	}
+}
+
+// WithoutAlternates will exclude any objects in the alternate objects directory
+func WithoutAlternates() RepoSizeOption {
+	return func(cfg *repoSizeConfig) {
+		cfg.ExcludeAlternates = true
 	}
 }
 
@@ -125,10 +135,18 @@ func (repo *Repo) Size(ctx context.Context, opts ...RepoSizeOption) (int64, erro
 	}
 
 	var options []git.Option
-	for _, exclude := range cfg.Excludes {
+	for _, refToExclude := range cfg.ExcludeRefs {
 		options = append(
 			options,
-			git.Flag{Name: fmt.Sprintf("--exclude=%s", exclude)},
+			git.Flag{Name: fmt.Sprintf("--exclude=%s", refToExclude)},
+		)
+	}
+
+	if cfg.ExcludeAlternates {
+		options = append(options,
+			git.Flag{Name: "--not"},
+			git.Flag{Name: "--alternate-refs"},
+			git.Flag{Name: "--not"},
 		)
 	}
 
diff --git a/internal/git/localrepo/repo_test.go b/internal/git/localrepo/repo_test.go
index c9f620731..ce84670b5 100644
--- a/internal/git/localrepo/repo_test.go
+++ b/internal/git/localrepo/repo_test.go
@@ -172,7 +172,7 @@ func TestSize(t *testing.T) {
 	}
 }
 
-func TestSize_excludes(t *testing.T) {
+func TestSize_excludeRefs(t *testing.T) {
 	cfg := testcfg.Build(t)
 	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
 	catfileCache := catfile.NewCache(cfg)
@@ -202,8 +202,39 @@ func TestSize_excludes(t *testing.T) {
 	require.NoError(t, err)
 	assert.Less(t, sizeBeforeKeepAround, sizeWithKeepAround)
 
-	sizeWithoutKeepAround, err := repo.Size(ctx, WithExcludes("refs/keep-around/*"))
+	sizeWithoutKeepAround, err := repo.Size(ctx, WithExcludeRefs("refs/keep-around/*"))
 	require.NoError(t, err)
 
 	assert.Equal(t, sizeBeforeKeepAround, sizeWithoutKeepAround)
 }
+
+func TestSize_excludeAlternates(t *testing.T) {
+	cfg := testcfg.Build(t)
+	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
+	locator := config.NewLocator(cfg)
+
+	pbRepo, repoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
+	_, altRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
+
+	require.NoError(t, os.WriteFile(
+		filepath.Join(repoPath, "objects", "info", "alternates"),
+		[]byte(filepath.Join(altRepoPath, "objects")),
+		os.ModePerm,
+	))
+
+	repo := New(locator, gitCmdFactory, catfileCache, pbRepo)
+
+	ctx := testhelper.Context(t)
+
+	gittest.Exec(t, cfg, "-C", repoPath, "gc")
+
+	sizeIncludingAlternates, err := repo.Size(ctx)
+	require.NoError(t, err)
+	assert.Greater(t, sizeIncludingAlternates, int64(0))
+
+	sizeExcludingAlternates, err := repo.Size(ctx, WithoutAlternates())
+	require.NoError(t, err)
+	assert.Equal(t, int64(0), sizeExcludingAlternates)
+}
diff --git a/internal/gitaly/service/repository/size.go b/internal/gitaly/service/repository/size.go
index 3ac49f405..132ad9f86 100644
--- a/internal/gitaly/service/repository/size.go
+++ b/internal/gitaly/service/repository/size.go
@@ -34,7 +34,8 @@ func (s *server) RepositorySize(ctx context.Context, in *gitalypb.RepositorySize
 	if featureflag.RevlistForRepoSize.IsEnabled(ctx) {
 		size, err = repo.Size(
 			ctx,
-			localrepo.WithExcludes(excludes...),
+			localrepo.WithExcludeRefs(excludes...),
+			localrepo.WithoutAlternates(),
 		)
 		if err != nil {
 			return nil, err
diff --git a/internal/gitaly/service/repository/size_test.go b/internal/gitaly/service/repository/size_test.go
index 52252f349..fbfe78413 100644
--- a/internal/gitaly/service/repository/size_test.go
+++ b/internal/gitaly/service/repository/size_test.go
@@ -8,11 +8,14 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/objectpool"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/quarantine"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/protobuf/proto"
@@ -24,8 +27,77 @@ const testRepoMinSizeKB = 10000
 
 func TestRepositorySize_SuccessfulRequest(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.RevlistForRepoSize).
-		Run(t, testSuccessfulRepositorySizeRequest)
+
+	featureSet := testhelper.NewFeatureSets(featureflag.RevlistForRepoSize)
+
+	featureSet.Run(t, testSuccessfulRepositorySizeRequest)
+	featureSet.Run(t, testSuccessfulRepositorySizeRequestPoolMember)
+}
+
+func testSuccessfulRepositorySizeRequestPoolMember(t *testing.T, ctx context.Context) {
+	cfg := testcfg.Build(t)
+
+	serverSocketPath := runRepositoryServerWithConfig(t, cfg, nil)
+	cfg.SocketPath = serverSocketPath
+
+	repoClient := newRepositoryClient(t, cfg, serverSocketPath)
+	objectPoolClient := newObjectPoolClient(t, cfg, serverSocketPath)
+
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
+		Seed: gittest.SeedGitLabTest,
+	})
+
+	sizeRequest := &gitalypb.RepositorySizeRequest{Repository: repo}
+	response, err := repoClient.RepositorySize(ctx, sizeRequest)
+	require.NoError(t, err)
+
+	sizeBeforePool := response.GetSize()
+
+	storage := cfg.Storages[0]
+	relativePath := gittest.NewObjectPoolName(t)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
+
+	// create an object pool
+	gittest.InitRepoDir(t, storage.Path, relativePath)
+	pool, err := objectpool.NewObjectPool(
+		config.NewLocator(cfg),
+		gittest.NewCommandFactory(t, cfg),
+		catfileCache,
+		nil,
+		nil,
+		storage.Name,
+		relativePath,
+	)
+	require.NoError(t, err)
+
+	_, err = objectPoolClient.CreateObjectPool(
+		ctx,
+		&gitalypb.CreateObjectPoolRequest{
+			ObjectPool: pool.ToProto(),
+			Origin:     repo,
+		})
+	require.NoError(t, err)
+
+	_, err = objectPoolClient.LinkRepositoryToObjectPool(
+		ctx,
+		&gitalypb.LinkRepositoryToObjectPoolRequest{
+			Repository: repo,
+			ObjectPool: pool.ToProto(),
+		},
+	)
+	require.NoError(t, err)
+
+	gittest.Exec(t, cfg, "-C", repoPath, "gc")
+
+	response, err = repoClient.RepositorySize(ctx, sizeRequest)
+	require.NoError(t, err)
+
+	if featureflag.RevlistForRepoSize.IsEnabled(ctx) {
+		assert.Equal(t, int64(0), response.GetSize())
+	} else {
+		assert.Less(t, response.GetSize(), sizeBeforePool)
+	}
 }
 
 func testSuccessfulRepositorySizeRequest(t *testing.T, ctx context.Context) {
diff --git a/internal/gitaly/service/repository/testhelper_test.go b/internal/gitaly/service/repository/testhelper_test.go
index bd289de33..576516ad0 100644
--- a/internal/gitaly/service/repository/testhelper_test.go
+++ b/internal/gitaly/service/repository/testhelper_test.go
@@ -20,6 +20,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/commit"
 	hookservice "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/hook"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/objectpool"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/ref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/remote"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/ssh"
@@ -76,6 +77,14 @@ func newRepositoryClient(t testing.TB, cfg config.Cfg, serverSocketPath string)
 	return gitalypb.NewRepositoryServiceClient(conn)
 }
 
+func newObjectPoolClient(t testing.TB, cfg config.Cfg, serverSocketPath string) gitalypb.ObjectPoolServiceClient {
+	conn, err := gclient.Dial(serverSocketPath, nil)
+	require.NoError(t, err)
+	t.Cleanup(func() { require.NoError(t, conn.Close()) })
+
+	return gitalypb.NewObjectPoolServiceClient(conn)
+}
+
 func newMuxedRepositoryClient(t *testing.T, ctx context.Context, cfg config.Cfg, serverSocketPath string, handshaker internalclient.Handshaker) gitalypb.RepositoryServiceClient {
 	conn, err := internalclient.Dial(ctx, serverSocketPath, []grpc.DialOption{
 		grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(cfg.Auth.Token)),
@@ -139,6 +148,13 @@ func runRepositoryServerWithConfig(t testing.TB, cfg config.Cfg, rubySrv *rubyse
 			nil,
 			deps.GetCatfileCache(),
 		))
+		gitalypb.RegisterObjectPoolServiceServer(srv, objectpool.NewServer(
+			deps.GetLocator(),
+			deps.GetGitCmdFactory(),
+			deps.GetCatfileCache(),
+			deps.GetTxManager(),
+			deps.GetHousekeepingManager(),
+		))
 	}, opts...)
 }