Merge branch 'pks-housekeeping-enable-commit-graph-generation-data' into 'master'

git: Enable reading and writing commit-graph generation data

Closes #4691

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5211

Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>

14 files changed, 388 insertions, 143 deletions

diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go
index 41f43948e..5ff1ceb9e 100644
--- a/internal/git/command_factory.go
+++ b/internal/git/command_factory.go
@@ -17,6 +17,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/log"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 )
 
 // CommandFactory is designed to create and run git commands in a protected and fully managed manner.
@@ -536,6 +537,14 @@ func (cf *ExecCommandFactory) GlobalConfiguration(ctx context.Context) ([]Config
 		// disable this mechanism.
 		{Key: "core.useReplaceRefs", Value: "false"},
 
+		// We configure for what data should be fsynced and how that should happen.
+		// Synchronize object files, packed-refs and loose refs to disk to lessen the
+		// likelihood of repository corruption in case the server crashes.
+		{Key: "core.fsync", Value: "objects,derived-metadata,reference"},
+		{Key: "core.fsyncMethod", Value: "fsync"},
+	}
+
+	if featureflag.UseCommitGraphGenerationData.IsDisabled(ctx) {
 		// Commit-graphs are used as an optimization to speed up reading commits and to be
 		// able to perform certain commit-related queries faster. One property that these
 		// graphs are storing is the generation number of a commit, where there are two
@@ -560,15 +569,17 @@ func (cf *ExecCommandFactory) GlobalConfiguration(ctx context.Context) ([]Config
 		// written by Git v2.35.0 or newer with Git v2.36.0 and later with `--changed-paths`
 		// enabled then the resulting commit-graph may be corrupt.
 		//
-		// Let's disable reading and writing corrected committer dates for now until the fix
-		// to this issue is upstream.
-		{Key: "commitGraph.generationVersion", Value: "1"},
-
-		// We configure for what data should be fsynced and how that should happen.
-		// Synchronize object files, packed-refs and loose refs to disk to lessen the
-		// likelihood of repository corruption in case the server crashes.
-		{Key: "core.fsync", Value: "objects,derived-metadata,reference"},
-		{Key: "core.fsyncMethod", Value: "fsync"},
+		// So if the above feature flag is disabled then we disable reading and writing
+		// corrected committer dates. Note that we say to use generation data version 1
+		// here, but due to the bug Git never actually paid attention to generation data in
+		// that case.
+		config = append(config, ConfigPair{Key: "commitGraph.generationVersion", Value: "1"})
+	} else {
+		// The commit-graph corruption was subsequently fixed in 9550f6c16a (commit-graph:
+		// fix corrupt upgrade from generation v1 to v2, 2022-07-12). So if the above
+		// feature flag is enabled then we start generating commit-graph generation data
+		// again.
+		config = append(config, ConfigPair{Key: "commitGraph.generationVersion", Value: "2"})
 	}
 
 	return config, nil
diff --git a/internal/git/command_factory_test.go b/internal/git/command_factory_test.go
index 3356fe467..67883b8c8 100644
--- a/internal/git/command_factory_test.go
+++ b/internal/git/command_factory_test.go
@@ -20,6 +20,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"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"
 )
@@ -559,13 +560,18 @@ func TestExecCommandFactory_config(t *testing.T) {
 	})
 	require.NoError(t, os.Remove(filepath.Join(repoDir, "config")))
 
+	generationVersion := "1"
+	if featureflag.UseCommitGraphGenerationData.IsEnabled(ctx) {
+		generationVersion = "2"
+	}
+
 	expectedEnv := []string{
 		"gc.auto=0",
 		"core.autocrlf=input",
 		"core.usereplacerefs=false",
-		"commitgraph.generationversion=1",
 		"core.fsync=objects,derived-metadata,reference",
 		"core.fsyncmethod=fsync",
+		"commitgraph.generationversion=" + generationVersion,
 	}
 
 	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
@@ -593,44 +599,22 @@ func TestExecCommandFactory_SidecarGitConfiguration(t *testing.T) {
 		{Key: "custom.key", Value: "injected"},
 	}
 
-	commonHead := []git.ConfigPair{
+	generationVersion := "1"
+	if featureflag.UseCommitGraphGenerationData.IsEnabled(ctx) {
+		generationVersion = "2"
+	}
+
+	configPairs, err := gittest.NewCommandFactory(t, cfg).SidecarGitConfiguration(ctx)
+	require.NoError(t, err)
+	require.Equal(t, []git.ConfigPair{
 		{Key: "gc.auto", Value: "0"},
 		{Key: "core.autocrlf", Value: "input"},
 		{Key: "core.useReplaceRefs", Value: "false"},
-		{Key: "commitGraph.generationVersion", Value: "1"},
-	}
-
-	commonTail := []git.ConfigPair{
+		{Key: "core.fsync", Value: "objects,derived-metadata,reference"},
+		{Key: "core.fsyncMethod", Value: "fsync"},
+		{Key: "commitGraph.generationVersion", Value: generationVersion},
 		{Key: "custom.key", Value: "injected"},
-	}
-
-	for _, tc := range []struct {
-		desc           string
-		version        string
-		expectedConfig []git.ConfigPair
-	}{
-		{
-			desc:    "with core.fsync",
-			version: "2.36.0",
-			expectedConfig: append(append(commonHead,
-				git.ConfigPair{Key: "core.fsync", Value: "objects,derived-metadata,reference"},
-				git.ConfigPair{Key: "core.fsyncMethod", Value: "fsync"},
-			), commonTail...),
-		},
-	} {
-		t.Run(tc.desc, func(t *testing.T) {
-			factory := gittest.NewInterceptingCommandFactory(t, ctx, cfg, func(git.ExecutionEnvironment) string {
-				return fmt.Sprintf(
-					`#!/usr/bin/env bash
-					echo "git version %s"
-				`, tc.version)
-			}, gittest.WithInterceptedVersion())
-
-			configPairs, err := factory.SidecarGitConfiguration(ctx)
-			require.NoError(t, err)
-			require.Equal(t, tc.expectedConfig, configPairs)
-		})
-	}
+	}, configPairs)
 }
 
 // TestFsckConfiguration tests the hardcoded configuration of the
diff --git a/internal/git/housekeeping/commit_graph.go b/internal/git/housekeeping/commit_graph.go
index 9e5b406b2..2476083e1 100644
--- a/internal/git/housekeeping/commit_graph.go
+++ b/internal/git/housekeeping/commit_graph.go
@@ -7,6 +7,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/stats"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/structerr"
 )
 
@@ -45,6 +46,9 @@ func WriteCommitGraphConfigForRepository(ctx context.Context, repo *localrepo.Re
 		// ain't got bloom filters enabled. This is because Git will refuse to write any
 		// bloom filters as long as any of the commit-graph slices is missing this info.
 		replaceChain = true
+	} else if !commitGraphInfo.HasGenerationData && featureflag.UseCommitGraphGenerationData.IsEnabled(ctx) {
+		// The same is true for generation data.
+		replaceChain = true
 	}
 
 	return WriteCommitGraphConfig{
diff --git a/internal/git/housekeeping/commit_graph_test.go b/internal/git/housekeeping/commit_graph_test.go
index f3e323702..d95af04d1 100644
--- a/internal/git/housekeeping/commit_graph_test.go
+++ b/internal/git/housekeeping/commit_graph_test.go
@@ -1,19 +1,25 @@
 package housekeeping
 
 import (
+	"context"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
+	"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"
 )
 
 func TestWriteCommitGraphConfigForRepository(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testWriteCommitGraphConfigForRepository)
+}
+
+func testWriteCommitGraphConfigForRepository(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
 	for _, tc := range []struct {
@@ -62,10 +68,26 @@ func TestWriteCommitGraphConfigForRepository(t *testing.T) {
 			},
 		},
 		{
-			desc: "split commit-graph with bloom filter",
+			desc: "split commit-graph with bloom filter without generation data",
+			setup: func(t *testing.T, repoPath string) {
+				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
+				gittest.Exec(t, cfg, "-C", repoPath,
+					"-c", "commitGraph.generationVersion=1",
+					"commit-graph", "write", "--reachable", "--split", "--changed-paths",
+				)
+			},
+			expectedConfig: WriteCommitGraphConfig{
+				ReplaceChain: featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
+			},
+		},
+		{
+			desc: "split commit-graph with bloom filter with generation data",
 			setup: func(t *testing.T, repoPath string) {
 				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable", "--split", "--changed-paths")
+				gittest.Exec(t, cfg, "-C", repoPath,
+					"-c", "commitGraph.generationVersion=2",
+					"commit-graph", "write", "--reachable", "--split", "--changed-paths",
+				)
 			},
 			expectedConfig: WriteCommitGraphConfig{
 				ReplaceChain: false,
diff --git a/internal/git/housekeeping/optimization_strategy_test.go b/internal/git/housekeeping/optimization_strategy_test.go
index 33ae06f2d..f8497eadc 100644
--- a/internal/git/housekeeping/optimization_strategy_test.go
+++ b/internal/git/housekeeping/optimization_strategy_test.go
@@ -22,6 +22,9 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
+	disabledGenerationVersion := "commitGraph.generationVersion=1"
+	enabledGenerationVersion := "commitGraph.generationVersion=2"
+
 	for _, tc := range []struct {
 		desc             string
 		setup            func(t *testing.T, relativePath string) *gitalypb.Repository
@@ -150,7 +153,7 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 			},
 		},
 		{
-			desc: "existing unsplit commit-graph with bloom filters",
+			desc: "existing unsplit commit-graph with bloom filters and no generation data",
 			setup: func(t *testing.T, relativePath string) *gitalypb.Repository {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
 					SkipCreationViaService: true,
@@ -163,7 +166,9 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 				// `--changed-paths` given that it would otherwise cause us to
 				// rewrite the graph regardless of whether the graph is split or not
 				// if they were missing.
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable", "--changed-paths")
+				gittest.Exec(t, cfg, "-C", repoPath, "-c", disabledGenerationVersion,
+					"commit-graph", "write", "--reachable", "--changed-paths",
+				)
 
 				return repoProto
 			},
@@ -177,14 +182,52 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 						LooseReferencesCount: 1,
 					},
 					CommitGraph: stats.CommitGraphInfo{
-						Exists:          true,
-						HasBloomFilters: true,
+						Exists:            true,
+						HasBloomFilters:   true,
+						HasGenerationData: false,
+					},
+				},
+			},
+		},
+		{
+			desc: "existing unsplit commit-graph with bloom filters with generation data",
+			setup: func(t *testing.T, relativePath string) *gitalypb.Repository {
+				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+					SkipCreationViaService: true,
+				})
+				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
+
+				// Write a non-split commit-graph with bloom filters. We should
+				// always rewrite the commit-graphs when we're not using a split
+				// commit-graph. We make sure to add bloom filters via
+				// `--changed-paths` given that it would otherwise cause us to
+				// rewrite the graph regardless of whether the graph is split or not
+				// if they were missing.
+				gittest.Exec(t, cfg, "-C", repoPath, "-c", enabledGenerationVersion,
+					"commit-graph", "write", "--reachable", "--changed-paths",
+				)
+
+				return repoProto
+			},
+			expectedStrategy: HeuristicalOptimizationStrategy{
+				info: stats.RepositoryInfo{
+					LooseObjects: stats.LooseObjectsInfo{
+						Count: 2,
+						Size:  hashDependentObjectSize(142, 158),
+					},
+					References: stats.ReferencesInfo{
+						LooseReferencesCount: 1,
+					},
+					CommitGraph: stats.CommitGraphInfo{
+						Exists:            true,
+						HasBloomFilters:   true,
+						HasGenerationData: true,
 					},
 				},
 			},
 		},
 		{
-			desc: "existing split commit-graph without bloom filters",
+			desc: "existing split commit-graph without bloom filters and generation data",
 			setup: func(t *testing.T, relativePath string) *gitalypb.Repository {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
 					SkipCreationViaService: true,
@@ -194,7 +237,9 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 				// Generate a split commit-graph, but don't enable computation of
 				// changed paths. This should trigger a rewrite so that we can
 				// recompute all graphs and generate the changed paths.
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable", "--split")
+				gittest.Exec(t, cfg, "-C", repoPath, "-c", disabledGenerationVersion,
+					"commit-graph", "write", "--reachable", "--split",
+				)
 
 				return repoProto
 			},
@@ -215,7 +260,7 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 			},
 		},
 		{
-			desc: "existing split commit-graph with bloom filters",
+			desc: "existing split commit-graph with bloom filters and generation data",
 			setup: func(t *testing.T, relativePath string) *gitalypb.Repository {
 				repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
 					SkipCreationViaService: true,
@@ -225,7 +270,9 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 				// Write a split commit-graph with bitmaps. This is the state we
 				// want to be in, so there is no write required if we didn't also
 				// repack objects.
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable", "--split", "--changed-paths")
+				gittest.Exec(t, cfg, "-C", repoPath, "-c", enabledGenerationVersion,
+					"commit-graph", "write", "--reachable", "--split", "--changed-paths",
+				)
 
 				return repoProto
 			},
@@ -242,6 +289,7 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 						Exists:                 true,
 						CommitGraphChainLength: 1,
 						HasBloomFilters:        true,
+						HasGenerationData:      true,
 					},
 				},
 			},
diff --git a/internal/git/stats/commit_graph_test.go b/internal/git/stats/commit_graph_test.go
index 72d8ded1f..c3be4cb8a 100644
--- a/internal/git/stats/commit_graph_test.go
+++ b/internal/git/stats/commit_graph_test.go
@@ -28,9 +28,12 @@ func TestCommitGraphInfoForRepository(t *testing.T) {
 			expectedInfo: CommitGraphInfo{},
 		},
 		{
-			desc: "single commit graph without bloom filter",
+			desc: "single commit graph without bloom filter and generation data",
 			setup: func(t *testing.T, repoPath string) {
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable")
+				gittest.Exec(t, cfg, "-C", repoPath,
+					"-c", "commitGraph.generationVersion=1",
+					"commit-graph", "write", "--reachable",
+				)
 			},
 			expectedInfo: CommitGraphInfo{
 				Exists: true,
@@ -39,7 +42,10 @@ func TestCommitGraphInfoForRepository(t *testing.T) {
 		{
 			desc: "single commit graph with bloom filter",
 			setup: func(t *testing.T, repoPath string) {
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable", "--changed-paths")
+				gittest.Exec(t, cfg, "-C", repoPath,
+					"-c", "commitGraph.generationVersion=1",
+					"commit-graph", "write", "--reachable", "--changed-paths",
+				)
 			},
 			expectedInfo: CommitGraphInfo{
 				Exists:          true,
@@ -61,9 +67,12 @@ func TestCommitGraphInfoForRepository(t *testing.T) {
 			},
 		},
 		{
-			desc: "split commit graph without bloom filter",
+			desc: "split commit graph without bloom filter and generation data",
 			setup: func(t *testing.T, repoPath string) {
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable", "--split")
+				gittest.Exec(t, cfg, "-C", repoPath,
+					"-c", "commitGraph.generationVersion=1",
+					"commit-graph", "write", "--reachable", "--split",
+				)
 			},
 			expectedInfo: CommitGraphInfo{
 				Exists:                 true,
@@ -71,9 +80,12 @@ func TestCommitGraphInfoForRepository(t *testing.T) {
 			},
 		},
 		{
-			desc: "split commit graph with bloom filter",
+			desc: "split commit graph with bloom filter without generation data",
 			setup: func(t *testing.T, repoPath string) {
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable", "--split", "--changed-paths")
+				gittest.Exec(t, cfg, "-C", repoPath,
+					"-c", "commitGraph.generationVersion=1",
+					"commit-graph", "write", "--reachable", "--split", "--changed-paths",
+				)
 			},
 			expectedInfo: CommitGraphInfo{
 				Exists:                 true,
diff --git a/internal/git/stats/repository_info_test.go b/internal/git/stats/repository_info_test.go
index 3bfe14f51..e7fa7582f 100644
--- a/internal/git/stats/repository_info_test.go
+++ b/internal/git/stats/repository_info_test.go
@@ -270,10 +270,13 @@ func TestRepositoryInfoForRepository(t *testing.T) {
 			},
 		},
 		{
-			desc: "non-split commit-graph without bloom filter",
+			desc: "non-split commit-graph without bloom filter and generation data",
 			setup: func(t *testing.T, repoPath string) {
 				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable")
+				gittest.Exec(t, cfg, "-C", repoPath,
+					"-c", "commitGraph.generationVersion=1",
+					"commit-graph", "write", "--reachable",
+				)
 			},
 			expectedInfo: RepositoryInfo{
 				LooseObjects: LooseObjectsInfo{
@@ -289,10 +292,13 @@ func TestRepositoryInfoForRepository(t *testing.T) {
 			},
 		},
 		{
-			desc: "non-split commit-graph with bloom filter",
+			desc: "non-split commit-graph with bloom filter and no generation data",
 			setup: func(t *testing.T, repoPath string) {
 				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable", "--changed-paths")
+				gittest.Exec(t, cfg, "-C", repoPath,
+					"-c", "commitGraph.generationVersion=1",
+					"commit-graph", "write", "--reachable", "--changed-paths",
+				)
 			},
 			expectedInfo: RepositoryInfo{
 				LooseObjects: LooseObjectsInfo{
diff --git a/internal/gitaly/service/objectpool/reduplicate_test.go b/internal/gitaly/service/objectpool/reduplicate_test.go
index b861772f0..621ee0a0a 100644
--- a/internal/gitaly/service/objectpool/reduplicate_test.go
+++ b/internal/gitaly/service/objectpool/reduplicate_test.go
@@ -38,7 +38,7 @@ func testReduplicate(t *testing.T, ctx context.Context) {
 	// Link the repository to the pool and garbage collect it to get rid of the duplicate
 	// objects.
 	require.NoError(t, pool.Link(ctx, repo))
-	gittest.Exec(t, cfg, "-C", repoPath, "gc")
+	gittest.Exec(t, cfg, "-C", repoPath, "-c", "commitGraph.generationVersion=2", "gc")
 	packedRefsStat, err := os.Stat(filepath.Join(repoPath, "packed-refs"))
 	require.NoError(t, err)
 	// Verify that the pool member has no objects on its own anymore.
@@ -49,7 +49,8 @@ func testReduplicate(t *testing.T, ctx context.Context) {
 			PackedReferencesSize: uint64(packedRefsStat.Size()),
 		},
 		CommitGraph: stats.CommitGraphInfo{
-			Exists: true,
+			Exists:            true,
+			HasGenerationData: true,
 		},
 		Alternates: []string{filepath.Join(poolPath, "objects")},
 	}, repoInfo)
diff --git a/internal/gitaly/service/repository/commit_graph_test.go b/internal/gitaly/service/repository/commit_graph_test.go
index 64b0c38af..2691ba16f 100644
--- a/internal/gitaly/service/repository/commit_graph_test.go
+++ b/internal/gitaly/service/repository/commit_graph_test.go
@@ -3,12 +3,14 @@
 package repository
 
 import (
+	"context"
 	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/stats"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -17,84 +19,140 @@ import (
 
 func TestWriteCommitGraph_withExistingCommitGraphCreatedWithDefaults(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testWriteCommitGraphWithExistingCommitGraphCreatedWithDefaults)
+}
 
-	ctx := testhelper.Context(t)
-	cfg, repo, repoPath, client := setupRepositoryService(t, ctx)
-
-	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{})
-
-	// write commit graph using an old approach
-	gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable")
-	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
-		Exists: true,
-	})
-
-	treeEntry := gittest.TreeEntry{Mode: "100644", Path: "file.txt", Content: "something"}
-	gittest.WriteCommit(
-		t,
-		cfg,
-		repoPath,
-		gittest.WithBranch(t.Name()),
-		gittest.WithTreeEntries(treeEntry),
-	)
+func testWriteCommitGraphWithExistingCommitGraphCreatedWithDefaults(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	//nolint:staticcheck
-	res, err := client.WriteCommitGraph(ctx, &gitalypb.WriteCommitGraphRequest{
-		Repository:    repo,
-		SplitStrategy: gitalypb.WriteCommitGraphRequest_SizeMultiple,
-	})
-	require.NoError(t, err)
-	require.NotNil(t, res)
+	for _, tc := range []struct {
+		desc               string
+		commitGraphVersion string
+	}{
+		{
+			desc:               "without preexisting generation data",
+			commitGraphVersion: "1",
+		},
+		{
+			desc:               "with preexisting generation data",
+			commitGraphVersion: "2",
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			cfg, repo, repoPath, client := setupRepositoryService(t, ctx)
+
+			requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{})
+
+			// write commit graph using an old approach
+			gittest.Exec(t, cfg, "-C", repoPath,
+				"-c", "commitGraph.generationVersion="+tc.commitGraphVersion,
+				"commit-graph", "write", "--reachable",
+			)
+			requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
+				Exists:            true,
+				HasGenerationData: tc.commitGraphVersion == "2",
+			})
+
+			treeEntry := gittest.TreeEntry{Mode: "100644", Path: "file.txt", Content: "something"}
+			gittest.WriteCommit(
+				t,
+				cfg,
+				repoPath,
+				gittest.WithBranch(t.Name()),
+				gittest.WithTreeEntries(treeEntry),
+			)
 
-	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
-		Exists: true, HasBloomFilters: true, CommitGraphChainLength: 1,
-	})
+			//nolint:staticcheck
+			res, err := client.WriteCommitGraph(ctx, &gitalypb.WriteCommitGraphRequest{
+				Repository:    repo,
+				SplitStrategy: gitalypb.WriteCommitGraphRequest_SizeMultiple,
+			})
+			require.NoError(t, err)
+			require.NotNil(t, res)
+
+			requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
+				Exists:                 true,
+				HasBloomFilters:        true,
+				HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
+				CommitGraphChainLength: 1,
+			})
+		})
+	}
 }
 
 func TestWriteCommitGraph_withExistingCommitGraphCreatedWithSplit(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testWriteCommitGraphWithExistingCommitGraphCreatedWithSplit)
+}
 
-	ctx := testhelper.Context(t)
-	cfg, repo, repoPath, client := setupRepositoryService(t, ctx)
-
-	// Assert that no commit-graph exists.
-	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{})
-
-	// write commit graph chain
-	gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--reachable", "--split")
-	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
-		Exists:                 true,
-		CommitGraphChainLength: 1,
-	})
-
-	treeEntry := gittest.TreeEntry{Mode: "100644", Path: "file.txt", Content: "something"}
-	gittest.WriteCommit(
-		t,
-		cfg,
-		repoPath,
-		gittest.WithBranch(t.Name()),
-		gittest.WithTreeEntries(treeEntry),
-	)
+func testWriteCommitGraphWithExistingCommitGraphCreatedWithSplit(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	//nolint:staticcheck
-	res, err := client.WriteCommitGraph(ctx, &gitalypb.WriteCommitGraphRequest{
-		Repository:    repo,
-		SplitStrategy: gitalypb.WriteCommitGraphRequest_SizeMultiple,
-	})
-	require.NoError(t, err)
-	require.NotNil(t, res)
+	for _, tc := range []struct {
+		desc               string
+		commitGraphVersion string
+	}{
+		{
+			desc:               "without preexisting generation data",
+			commitGraphVersion: "1",
+		},
+		{
+			desc:               "with preexisting generation data",
+			commitGraphVersion: "2",
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			cfg, repo, repoPath, client := setupRepositoryService(t, ctx)
+
+			// Assert that no commit-graph exists.
+			requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{})
+
+			// write commit graph chain
+			gittest.Exec(t, cfg, "-C", repoPath,
+				"-c", "commitGraph.generationVersion="+tc.commitGraphVersion,
+				"commit-graph", "write", "--reachable", "--split",
+			)
+			requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
+				Exists:                 true,
+				CommitGraphChainLength: 1,
+				HasGenerationData:      tc.commitGraphVersion == "2",
+			})
+
+			treeEntry := gittest.TreeEntry{Mode: "100644", Path: "file.txt", Content: "something"}
+			gittest.WriteCommit(
+				t,
+				cfg,
+				repoPath,
+				gittest.WithBranch(t.Name()),
+				gittest.WithTreeEntries(treeEntry),
+			)
 
-	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
-		Exists:                 true,
-		HasBloomFilters:        true,
-		CommitGraphChainLength: 1,
-	})
+			//nolint:staticcheck
+			res, err := client.WriteCommitGraph(ctx, &gitalypb.WriteCommitGraphRequest{
+				Repository:    repo,
+				SplitStrategy: gitalypb.WriteCommitGraphRequest_SizeMultiple,
+			})
+			require.NoError(t, err)
+			require.NotNil(t, res)
+
+			requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
+				Exists:                 true,
+				CommitGraphChainLength: 1,
+				HasBloomFilters:        true,
+				HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
+			})
+		})
+	}
 }
 
 func TestWriteCommitGraph(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testWriteCommitGraph)
+}
+
+func testWriteCommitGraph(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	_, repo, repoPath, client := setupRepositoryService(t, ctx)
 
 	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{})
@@ -108,14 +166,21 @@ func TestWriteCommitGraph(t *testing.T) {
 	require.NotNil(t, res)
 
 	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
-		Exists: true, HasBloomFilters: true, CommitGraphChainLength: 1,
+		Exists:                 true,
+		CommitGraphChainLength: 1,
+		HasBloomFilters:        true,
+		HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
 	})
 }
 
 func TestWriteCommitGraph_validationChecks(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testWriteCommitGraphValidationChecks)
+}
+
+func testWriteCommitGraphValidationChecks(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	cfg, repo, _, client := setupRepositoryService(t, ctx)
 
 	for _, tc := range []struct {
@@ -166,8 +231,12 @@ func TestWriteCommitGraph_validationChecks(t *testing.T) {
 
 func TestUpdateCommitGraph(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testUpdateCommitGraph)
+}
+
+func testUpdateCommitGraph(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	cfg, repo, repoPath, client := setupRepositoryService(t, ctx)
 
 	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{})
@@ -183,6 +252,7 @@ func TestUpdateCommitGraph(t *testing.T) {
 	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
 		Exists:                 true,
 		HasBloomFilters:        true,
+		HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
 		CommitGraphChainLength: 1,
 	})
 
@@ -206,6 +276,7 @@ func TestUpdateCommitGraph(t *testing.T) {
 	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
 		Exists:                 true,
 		HasBloomFilters:        true,
+		HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
 		CommitGraphChainLength: 2,
 	})
 }
diff --git a/internal/gitaly/service/repository/gc_test.go b/internal/gitaly/service/repository/gc_test.go
index 5c736af2a..372c912a0 100644
--- a/internal/gitaly/service/repository/gc_test.go
+++ b/internal/gitaly/service/repository/gc_test.go
@@ -4,6 +4,7 @@ package repository
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -18,6 +19,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/stats"
 	"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/testserver"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
@@ -32,8 +34,12 @@ var (
 
 func TestGarbageCollectCommitGraph(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testGarbageCollectCommitGraph)
+}
+
+func testGarbageCollectCommitGraph(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	_, repo, repoPath, client := setupRepositoryService(t, ctx)
 
 	//nolint:staticcheck
@@ -42,7 +48,10 @@ func TestGarbageCollectCommitGraph(t *testing.T) {
 	assert.NotNil(t, c)
 
 	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
-		Exists: true, HasBloomFilters: true, CommitGraphChainLength: 1,
+		Exists:                 true,
+		CommitGraphChainLength: 1,
+		HasBloomFilters:        true,
+		HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
 	})
 }
 
diff --git a/internal/gitaly/service/repository/optimize_test.go b/internal/gitaly/service/repository/optimize_test.go
index 35cb5f9e1..88a45eb00 100644
--- a/internal/gitaly/service/repository/optimize_test.go
+++ b/internal/gitaly/service/repository/optimize_test.go
@@ -19,6 +19,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/stats"
 	"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/structerr"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
@@ -27,8 +28,12 @@ import (
 
 func TestOptimizeRepository(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testOptimizeRepository)
+}
+
+func testOptimizeRepository(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	cfg, client := setupRepositoryServiceWithoutRepo(t)
 
 	t.Run("gitconfig credentials get pruned", func(t *testing.T) {
@@ -136,7 +141,45 @@ func TestOptimizeRepository(t *testing.T) {
 		require.NotEmpty(t, bitmaps)
 	})
 
-	t.Run("optimizing repository without commit-graph bloom filters", func(t *testing.T) {
+	t.Run("optimizing repository without commit-graph bloom filters and generation data", func(t *testing.T) {
+		t.Parallel()
+
+		repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
+		gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master"))
+
+		// Prepare the repository so that it has a commit-graph, but that commit-graph is
+		// missing bloom filters.
+		gittest.Exec(t, cfg, "-C", repoPath,
+			"-c", "commitGraph.generationVersion=1",
+			"commit-graph", "write", "--split", "--reachable",
+		)
+		commitGraphInfo, err := stats.CommitGraphInfoForRepository(repoPath)
+		require.NoError(t, err)
+		require.Equal(t, stats.CommitGraphInfo{
+			Exists:                 true,
+			HasBloomFilters:        false,
+			HasGenerationData:      false,
+			CommitGraphChainLength: 1,
+		}, commitGraphInfo)
+
+		// As a result, OptimizeRepository should rewrite the commit-graph.
+		_, err = client.OptimizeRepository(ctx, &gitalypb.OptimizeRepositoryRequest{
+			Repository: repo,
+		})
+		require.NoError(t, err)
+
+		// Which means that we now should see that bloom filters exist.
+		commitGraphInfo, err = stats.CommitGraphInfoForRepository(repoPath)
+		require.NoError(t, err)
+		require.Equal(t, stats.CommitGraphInfo{
+			Exists:                 true,
+			HasBloomFilters:        true,
+			HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
+			CommitGraphChainLength: 1,
+		}, commitGraphInfo)
+	})
+
+	t.Run("optimizing repository without commit-graph bloom filters with generation data", func(t *testing.T) {
 		t.Parallel()
 
 		repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
@@ -144,12 +187,16 @@ func TestOptimizeRepository(t *testing.T) {
 
 		// Prepare the repository so that it has a commit-graph, but that commit-graph is
 		// missing bloom filters.
-		gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--split", "--reachable")
+		gittest.Exec(t, cfg, "-C", repoPath,
+			"-c", "commitGraph.generationVersion=2",
+			"commit-graph", "write", "--split", "--reachable",
+		)
 		commitGraphInfo, err := stats.CommitGraphInfoForRepository(repoPath)
 		require.NoError(t, err)
 		require.Equal(t, stats.CommitGraphInfo{
 			Exists:                 true,
 			HasBloomFilters:        false,
+			HasGenerationData:      true,
 			CommitGraphChainLength: 1,
 		}, commitGraphInfo)
 
@@ -165,6 +212,7 @@ func TestOptimizeRepository(t *testing.T) {
 		require.Equal(t, stats.CommitGraphInfo{
 			Exists:                 true,
 			HasBloomFilters:        true,
+			HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
 			CommitGraphChainLength: 1,
 		}, commitGraphInfo)
 	})
diff --git a/internal/gitaly/service/repository/repack_test.go b/internal/gitaly/service/repository/repack_test.go
index 9bb294f54..713450642 100644
--- a/internal/gitaly/service/repository/repack_test.go
+++ b/internal/gitaly/service/repository/repack_test.go
@@ -3,6 +3,7 @@
 package repository
 
 import (
+	"context"
 	"fmt"
 	"path/filepath"
 	"testing"
@@ -12,6 +13,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/stats"
+	"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/testserver"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
@@ -21,8 +23,12 @@ import (
 
 func TestRepackIncrementalSuccess(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testRepackIncrementalSuccess)
+}
+
+func testRepackIncrementalSuccess(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	cfg, client := setupRepositoryServiceWithoutRepo(t)
 	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
@@ -47,7 +53,10 @@ func TestRepackIncrementalSuccess(t *testing.T) {
 	require.Equal(t, oldPackfileCount+1, newPackfileCount)
 
 	requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
-		Exists: true, HasBloomFilters: true, CommitGraphChainLength: 1,
+		Exists:                 true,
+		HasBloomFilters:        true,
+		HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
+		CommitGraphChainLength: 1,
 	})
 }
 
@@ -163,8 +172,12 @@ func TestRepackIncrementalFailure(t *testing.T) {
 
 func TestRepackFullSuccess(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testRepackFullSuccess)
+}
+
+func testRepackFullSuccess(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	cfg, client := setupRepositoryServiceWithoutRepo(t)
 
 	for _, tc := range []struct {
@@ -223,7 +236,10 @@ func TestRepackFullSuccess(t *testing.T) {
 			// And last but not least the commit-graph must've been written. This is
 			// important because the commit-graph might otherwise be stale.
 			requireCommitGraphInfo(t, repoPath, stats.CommitGraphInfo{
-				Exists: true, HasBloomFilters: true, CommitGraphChainLength: 1,
+				Exists:                 true,
+				HasBloomFilters:        true,
+				HasGenerationData:      featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
+				CommitGraphChainLength: 1,
 			})
 		})
 	}
diff --git a/internal/metadata/featureflag/ff_use_commit_graph_generation_data.go b/internal/metadata/featureflag/ff_use_commit_graph_generation_data.go
new file mode 100644
index 000000000..fba4bf6c3
--- /dev/null
+++ b/internal/metadata/featureflag/ff_use_commit_graph_generation_data.go
@@ -0,0 +1,10 @@
+package featureflag
+
+// UseCommitGraphGenerationData enables reading and writing commit-graph generation data in by
+// setting `commitGraph.generationVersion=2`.
+var UseCommitGraphGenerationData = NewFeatureFlag(
+	"use_commit_graph_generation_data",
+	"v15.8.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/4704",
+	false,
+)
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 689e7b346..07daa58bd 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -203,6 +203,9 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context {
 	// PraefectUseYamuxConfigurationForGitaly gets tested in Praefect when routing RPCs and thus it affects many tests.
 	// Let's randomly select which connection we use so both sets of connections get tested somewhat.
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.PraefectUseYamuxConfigurationForGitaly, rnd.Int()%2 == 0)
+	// We check this feature flag in the Git command factory on every spawned Git command, so
+	// basically in almost all cases.
+	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.UseCommitGraphGenerationData, rnd.Int()%2 == 0)
 
 	// Randomly enable the use of the catfile cache in localrepo.ReadObject.
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.LocalrepoReadObjectCached, rnd.Int()%2 == 0)