housekeeping: Fix delta islands for pool repositories

When packing objects we make sure to create delta islands. These tell
Git that it may only create deltas against objects that are reachable by
references part of the same island, which can help speed up the creation
of packfiles because it can reuse existing on-disk deltas better.

We use different delta islands for normal and pooled repositories:

    - A normal repository has `refs/heads/` and `refs/tags/` as delta
      islands given that those are the most frequently used references.

    - A pool repository has `refs/remotes/origin/heads/` and
      `refs/remotes/origin/tags` as delta islands given that those
      contain the most frequently used references of the primary pool
      member.

While `FetchIntoObjectPool()` knows to set up the latter, special delta
island, `OptimizeRepository()` doesn't distinguish normal and pooled
repositories at all and thus uses the same delta islands as a normal
repository would. So when our nightly maintenance decides to walk over
any pool repository, it may silently screw up the deltas.

Fix this bug properly distinguishing between pool repositories and
non-pool repositories.

Changelog: fixed

4 files changed, 32 insertions, 7 deletions

diff --git a/internal/git/housekeeping/objects.go b/internal/git/housekeeping/objects.go
index 084e115c1..7ce675c6e 100644
--- a/internal/git/housekeeping/objects.go
+++ b/internal/git/housekeeping/objects.go
@@ -5,6 +5,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/stats"
 )
 
@@ -47,7 +48,7 @@ func RepackObjects(ctx context.Context, repo *localrepo.Repo, cfg RepackObjectsC
 			// details.
 			git.Flag{Name: "-n"},
 		}, options...),
-	}, git.WithConfig(GetRepackGitConfig(ctx, cfg.WriteBitmap)...)); err != nil {
+	}, git.WithConfig(GetRepackGitConfig(ctx, repo, cfg.WriteBitmap)...)); err != nil {
 		return err
 	}
 
@@ -61,14 +62,25 @@ func RepackObjects(ctx context.Context, repo *localrepo.Repo, cfg RepackObjectsC
 }
 
 // GetRepackGitConfig returns configuration suitable for Git commands which write new packfiles.
-func GetRepackGitConfig(ctx context.Context, bitmap bool) []git.ConfigPair {
+func GetRepackGitConfig(ctx context.Context, repo repository.GitRepo, bitmap bool) []git.ConfigPair {
 	config := []git.ConfigPair{
-		{Key: "pack.island", Value: "r(e)fs/heads"},
-		{Key: "pack.island", Value: "r(e)fs/tags"},
-		{Key: "pack.islandCore", Value: "e"},
 		{Key: "repack.useDeltaIslands", Value: "true"},
 	}
 
+	if IsPoolRepository(repo) {
+		config = append(config,
+			git.ConfigPair{Key: "pack.island", Value: git.ObjectPoolRefNamespace + "/he(a)ds"},
+			git.ConfigPair{Key: "pack.island", Value: git.ObjectPoolRefNamespace + "/t(a)gs"},
+			git.ConfigPair{Key: "pack.islandCore", Value: "a"},
+		)
+	} else {
+		config = append(config,
+			git.ConfigPair{Key: "pack.island", Value: "r(e)fs/heads"},
+			git.ConfigPair{Key: "pack.island", Value: "r(e)fs/tags"},
+			git.ConfigPair{Key: "pack.islandCore", Value: "e"},
+		)
+	}
+
 	if bitmap {
 		config = append(config, git.ConfigPair{Key: "repack.writeBitmaps", Value: "true"})
 		config = append(config, git.ConfigPair{Key: "pack.writeBitmapHashCache", Value: "true"})
diff --git a/internal/git/housekeeping/objects_test.go b/internal/git/housekeeping/objects_test.go
index 0d65e60cd..f6b079198 100644
--- a/internal/git/housekeeping/objects_test.go
+++ b/internal/git/housekeeping/objects_test.go
@@ -51,4 +51,17 @@ func TestRepackObjects(t *testing.T) {
 		require.NoFileExists(t, filepath.Join(repoPath, "info", "refs"))
 		require.NoFileExists(t, filepath.Join(repoPath, "objects", "info", "packs"))
 	})
+
+	testRepoAndPool(t, "delta islands", func(t *testing.T, relativePath string) {
+		repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0], gittest.InitRepoOpts{
+			WithRelativePath: relativePath,
+		})
+		repo := localrepo.NewTestRepo(t, cfg, repoProto)
+
+		gittest.TestDeltaIslands(t, cfg, repoPath, IsPoolRepository(repoProto), func() error {
+			return RepackObjects(ctx, repo, RepackObjectsConfig{
+				FullRepack: true,
+			})
+		})
+	})
 }
diff --git a/internal/gitaly/service/repository/gc.go b/internal/gitaly/service/repository/gc.go
index 15fea86c0..8bce85540 100644
--- a/internal/gitaly/service/repository/gc.go
+++ b/internal/gitaly/service/repository/gc.go
@@ -61,7 +61,7 @@ func (s *server) GarbageCollect(ctx context.Context, in *gitalypb.GarbageCollect
 }
 
 func (s *server) gc(ctx context.Context, in *gitalypb.GarbageCollectRequest) error {
-	config := append(housekeeping.GetRepackGitConfig(ctx, in.CreateBitmap), git.ConfigPair{Key: "gc.writeCommitGraph", Value: "false"})
+	config := append(housekeeping.GetRepackGitConfig(ctx, in.GetRepository(), in.CreateBitmap), git.ConfigPair{Key: "gc.writeCommitGraph", Value: "false"})
 
 	var flags []git.Option
 	if in.Prune {
diff --git a/internal/gitaly/service/repository/midx.go b/internal/gitaly/service/repository/midx.go
index 151c6901b..b72cbd3a5 100644
--- a/internal/gitaly/service/repository/midx.go
+++ b/internal/gitaly/service/repository/midx.go
@@ -186,7 +186,7 @@ func (s *server) midxRepack(ctx context.Context, repo repository.GitRepo) error
 				git.ValueFlag{Name: "--batch-size", Value: strconv.FormatInt(batchSize, 10)},
 			},
 		},
-		git.WithConfig(housekeeping.GetRepackGitConfig(ctx, false)...),
+		git.WithConfig(housekeeping.GetRepackGitConfig(ctx, repo, false)...),
 	)
 	if err != nil {
 		return err