Merge branch 'jc-exclude-keep-around-refs-from-repo-size' into 'master'

repository: Exclude merge-requests, keep-around, pipelines from size

See merge request gitlab-org/gitaly!4532

4 files changed, 127 insertions, 11 deletions

diff --git a/internal/git/localrepo/repo.go b/internal/git/localrepo/repo.go
index 7d7f3e409..97525a1fc 100644
--- a/internal/git/localrepo/repo.go
+++ b/internal/git/localrepo/repo.go
@@ -94,18 +94,55 @@ func errorWithStderr(err error, stderr []byte) error {
 	return fmt.Errorf("%w, stderr: %q", err, stderr)
 }
 
+// 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
+	// calculation.
+	Excludes []string
+}
+
+// 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
+// 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 {
+	return func(cfg *repoSizeConfig) {
+		cfg.Excludes = excludes
+	}
+}
+
 // Size calculates the size of all reachable objects in bytes
-func (repo *Repo) Size(ctx context.Context) (int64, error) {
+func (repo *Repo) Size(ctx context.Context, opts ...RepoSizeOption) (int64, error) {
 	var stdout bytes.Buffer
+
+	var cfg repoSizeConfig
+
+	for _, opt := range opts {
+		opt(&cfg)
+	}
+
+	var options []git.Option
+	for _, exclude := range cfg.Excludes {
+		options = append(
+			options,
+			git.Flag{Name: fmt.Sprintf("--exclude=%s", exclude)},
+		)
+	}
+
+	options = append(options,
+		git.Flag{Name: "--all"},
+		git.Flag{Name: "--objects"},
+		git.Flag{Name: "--use-bitmap-index"},
+		git.Flag{Name: "--disk-usage"},
+	)
+
 	if err := repo.ExecAndWait(ctx,
 		git.SubCmd{
-			Name: "rev-list",
-			Flags: []git.Option{
-				git.Flag{Name: "--all"},
-				git.Flag{Name: "--objects"},
-				git.Flag{Name: "--use-bitmap-index"},
-				git.Flag{Name: "--disk-usage"},
-			},
+			Name:  "rev-list",
+			Flags: options,
 		},
 		git.WithStdout(&stdout),
 	); err != nil {
diff --git a/internal/git/localrepo/repo_test.go b/internal/git/localrepo/repo_test.go
index c305519c8..c9f620731 100644
--- a/internal/git/localrepo/repo_test.go
+++ b/internal/git/localrepo/repo_test.go
@@ -171,3 +171,39 @@ func TestSize(t *testing.T) {
 		})
 	}
 }
+
+func TestSize_excludes(t *testing.T) {
+	cfg := testcfg.Build(t)
+	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
+
+	pbRepo, repoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
+	blob := bytes.Repeat([]byte("a"), 1000)
+	blobOID := gittest.WriteBlob(t, cfg, repoPath, blob)
+	treeOID := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
+		{
+			OID:  blobOID,
+			Mode: "100644",
+			Path: "1kbblob",
+		},
+	})
+	commitOID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTree(treeOID))
+
+	repo := New(config.NewLocator(cfg), gitCmdFactory, catfileCache, pbRepo)
+
+	ctx := testhelper.Context(t)
+	sizeBeforeKeepAround, err := repo.Size(ctx)
+	require.NoError(t, err)
+
+	gittest.WriteRef(t, cfg, repoPath, git.ReferenceName("refs/keep-around/keep1"), commitOID)
+
+	sizeWithKeepAround, err := repo.Size(ctx)
+	require.NoError(t, err)
+	assert.Less(t, sizeBeforeKeepAround, sizeWithKeepAround)
+
+	sizeWithoutKeepAround, err := repo.Size(ctx, WithExcludes("refs/keep-around/*"))
+	require.NoError(t, err)
+
+	assert.Equal(t, sizeBeforeKeepAround, sizeWithoutKeepAround)
+}
diff --git a/internal/gitaly/service/repository/size.go b/internal/gitaly/service/repository/size.go
index 4d43dc2b1..a6bb0d33e 100644
--- a/internal/gitaly/service/repository/size.go
+++ b/internal/gitaly/service/repository/size.go
@@ -10,6 +10,8 @@ import (
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/command"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 )
@@ -19,8 +21,16 @@ func (s *server) RepositorySize(ctx context.Context, in *gitalypb.RepositorySize
 	var size int64
 	var err error
 
+	var excludes []string
+	for _, prefix := range git.InternalRefPrefixes {
+		excludes = append(excludes, prefix+"*")
+	}
+
 	if featureflag.RevlistForRepoSize.IsEnabled(ctx) {
-		size, err = repo.Size(ctx)
+		size, err = repo.Size(
+			ctx,
+			localrepo.WithExcludes(excludes...),
+		)
 		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 b9c752723..52252f349 100644
--- a/internal/gitaly/service/repository/size_test.go
+++ b/internal/gitaly/service/repository/size_test.go
@@ -1,10 +1,13 @@
 package repository
 
 import (
+	"bytes"
 	"context"
 	"testing"
 
+	"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/gittest"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/quarantine"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
@@ -26,7 +29,7 @@ func TestRepositorySize_SuccessfulRequest(t *testing.T) {
 }
 
 func testSuccessfulRepositorySizeRequest(t *testing.T, ctx context.Context) {
-	_, repo, _, client := setupRepositoryService(ctx, t)
+	cfg, repo, repoPath, client := setupRepositoryService(ctx, t)
 
 	request := &gitalypb.RepositorySizeRequest{Repository: repo}
 	response, err := client.RepositorySize(ctx, request)
@@ -36,9 +39,39 @@ func testSuccessfulRepositorySizeRequest(t *testing.T, ctx context.Context) {
 		response.Size > testRepoMinSizeKB,
 		"repository size %d should be at least %d", response.Size, testRepoMinSizeKB,
 	)
+
+	blob := bytes.Repeat([]byte("a"), 1000)
+	blobOID := gittest.WriteBlob(t, cfg, repoPath, blob)
+	treeOID := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
+		{
+			OID:  blobOID,
+			Mode: "100644",
+			Path: "1kbblob",
+		},
+	})
+	commitOID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTree(treeOID))
+
+	gittest.WriteRef(t, cfg, repoPath, git.ReferenceName("refs/keep-around/keep1"), commitOID)
+	gittest.WriteRef(t, cfg, repoPath, git.ReferenceName("refs/merge-requests/1123"), commitOID)
+	gittest.WriteRef(t, cfg, repoPath, git.ReferenceName("refs/pipelines/pipeline2"), commitOID)
+	gittest.WriteRef(t, cfg, repoPath, git.ReferenceName("refs/environments/env1"), commitOID)
+
+	responseAfterRefs, err := client.RepositorySize(ctx, request)
+	require.NoError(t, err)
+
+	if featureflag.RevlistForRepoSize.IsEnabled(ctx) {
+		assert.Equal(
+			t,
+			response.Size,
+			responseAfterRefs.Size,
+			"excluded refs do not contribute to the repository size",
+		)
+	} else {
+		assert.Less(t, response.Size, responseAfterRefs.Size)
+	}
 }
 
-func TestRepositorySixe_FailedRequest(t *testing.T) {
+func TestRepositorySize_FailedRequest(t *testing.T) {
 	t.Parallel()
 	testhelper.NewFeatureSets(featureflag.RevlistForRepoSize).
 		Run(t, testFailedRepositorySizeRequest)