Merge branch 'pks-optimize-repository-fix-commit-graph-rewriting' into 'master'

housekeeping: Fix rewriting of commit-graphs without generation data

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

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

5 files changed, 123 insertions, 57 deletions

diff --git a/internal/git/housekeeping/commit_graph.go b/internal/git/housekeeping/commit_graph.go
index 2476083e1..b2e71a235 100644
--- a/internal/git/housekeeping/commit_graph.go
+++ b/internal/git/housekeeping/commit_graph.go
@@ -29,31 +29,36 @@ func WriteCommitGraphConfigForRepository(ctx context.Context, repo *localrepo.Re
 		return WriteCommitGraphConfig{}, err
 	}
 
-	var replaceChain bool
-
 	commitGraphInfo, err := stats.CommitGraphInfoForRepository(repoPath)
 	if err != nil {
 		return WriteCommitGraphConfig{}, structerr.NewInternal("getting commit-graph info: %w", err)
 	}
 
+	return WriteCommitGraphConfig{
+		ReplaceChain: commitGraphNeedsRewrite(ctx, commitGraphInfo),
+	}, nil
+}
+
+// commitGraphNeedsRewrite determines whether the commit-graph needs to be rewritten. This can be
+// the case when it is either a monolithic commit-graph or when it is missing some extensions that
+// only get written on a full rewrite.
+func commitGraphNeedsRewrite(ctx context.Context, commitGraphInfo stats.CommitGraphInfo) bool {
 	if commitGraphInfo.CommitGraphChainLength == 0 {
 		// The repository does not have a commit-graph chain. This either indicates we ain't
 		// got no commit-graph at all, or that it's monolithic. In both cases we want to
 		// replace the commit-graph chain.
-		replaceChain = true
+		return true
 	} else if !commitGraphInfo.HasBloomFilters {
 		// If the commit-graph-chain exists, we want to rewrite it in case we see that it
 		// 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
+		return true
 	} else if !commitGraphInfo.HasGenerationData && featureflag.UseCommitGraphGenerationData.IsEnabled(ctx) {
 		// The same is true for generation data.
-		replaceChain = true
+		return true
 	}
 
-	return WriteCommitGraphConfig{
-		ReplaceChain: replaceChain,
-	}, nil
+	return false
 }
 
 // WriteCommitGraph updates the commit-graph in the given repository. The commit-graph is updated
diff --git a/internal/git/housekeeping/optimization_strategy.go b/internal/git/housekeeping/optimization_strategy.go
index 04235eabe..b0c62d35c 100644
--- a/internal/git/housekeeping/optimization_strategy.go
+++ b/internal/git/housekeeping/optimization_strategy.go
@@ -15,16 +15,16 @@ import (
 type OptimizationStrategy interface {
 	// ShouldRepackObjects determines whether the repository needs to be repacked and, if so,
 	// how it should be done.
-	ShouldRepackObjects() (bool, RepackObjectsConfig)
+	ShouldRepackObjects(context.Context) (bool, RepackObjectsConfig)
 	// ShouldPruneObjects determines whether the repository has stale objects that should be
 	// pruned.
-	ShouldPruneObjects() bool
+	ShouldPruneObjects(context.Context) bool
 	// ShouldRepackReferences determines whether the repository's references need to be
 	// repacked.
-	ShouldRepackReferences() bool
+	ShouldRepackReferences(context.Context) bool
 	// ShouldWriteCommitGraph determines whether we need to write the commit-graph and how it
 	// should be written.
-	ShouldWriteCommitGraph() (bool, WriteCommitGraphConfig)
+	ShouldWriteCommitGraph(context.Context) (bool, WriteCommitGraphConfig)
 }
 
 // HeuristicalOptimizationStrategy is an optimization strategy that is based on a set of
@@ -54,7 +54,7 @@ func NewHeuristicalOptimizationStrategy(ctx context.Context, repo *localrepo.Rep
 // ShouldRepackObjects checks whether the repository's objects need to be repacked. This uses a
 // set of heuristics that scales with the size of the object database: the larger the repository,
 // the less frequent does it get a full repack.
-func (s HeuristicalOptimizationStrategy) ShouldRepackObjects() (bool, RepackObjectsConfig) {
+func (s HeuristicalOptimizationStrategy) ShouldRepackObjects(context.Context) (bool, RepackObjectsConfig) {
 	// If there are neither packfiles nor loose objects in this repository then there is no need
 	// to repack anything.
 	if s.info.Packfiles.Count == 0 && s.info.LooseObjects.Count == 0 {
@@ -149,7 +149,7 @@ func (s HeuristicalOptimizationStrategy) ShouldRepackObjects() (bool, RepackObje
 
 // ShouldWriteCommitGraph determines whether we need to write the commit-graph and how it should be
 // written.
-func (s HeuristicalOptimizationStrategy) ShouldWriteCommitGraph() (bool, WriteCommitGraphConfig) {
+func (s HeuristicalOptimizationStrategy) ShouldWriteCommitGraph(ctx context.Context) (bool, WriteCommitGraphConfig) {
 	// If the repository doesn't have any references at all then there is no point in writing
 	// commit-graphs given that it would only contain reachable objects, of which there are
 	// none.
@@ -165,17 +165,13 @@ func (s HeuristicalOptimizationStrategy) ShouldWriteCommitGraph() (bool, WriteCo
 	//
 	// To fix this case we will replace the complete commit-chain when we have pruned objects
 	// from the repository.
-	if s.ShouldPruneObjects() {
+	if s.ShouldPruneObjects(ctx) {
 		return true, WriteCommitGraphConfig{
 			ReplaceChain: true,
 		}
 	}
 
-	// Bloom filters are part of the commit-graph and allow us to efficiently determine which
-	// paths have been modified in a given commit without having to look into the object
-	// database. In the past we didn't compute bloom filters at all, so we want to rewrite the
-	// whole commit-graph to generate them.
-	if s.info.CommitGraph.CommitGraphChainLength == 0 || !s.info.CommitGraph.HasBloomFilters {
+	if commitGraphNeedsRewrite(ctx, s.info.CommitGraph) {
 		return true, WriteCommitGraphConfig{
 			ReplaceChain: true,
 		}
@@ -183,7 +179,7 @@ func (s HeuristicalOptimizationStrategy) ShouldWriteCommitGraph() (bool, WriteCo
 
 	// When we repacked the repository then chances are high that we have accumulated quite some
 	// objects since the last time we wrote a commit-graph.
-	if needsRepacking, _ := s.ShouldRepackObjects(); needsRepacking {
+	if needsRepacking, _ := s.ShouldRepackObjects(ctx); needsRepacking {
 		return true, WriteCommitGraphConfig{}
 	}
 
@@ -193,7 +189,7 @@ func (s HeuristicalOptimizationStrategy) ShouldWriteCommitGraph() (bool, WriteCo
 // ShouldPruneObjects determines whether the repository has stale objects that should be pruned.
 // Object pools are never pruned to not lose data in them, but otherwise we prune when we've found
 // enough stale objects that might in fact get pruned.
-func (s HeuristicalOptimizationStrategy) ShouldPruneObjects() bool {
+func (s HeuristicalOptimizationStrategy) ShouldPruneObjects(context.Context) bool {
 	// Pool repositories must never prune any objects, or otherwise we may corrupt members of
 	// that pool if they still refer to that object.
 	if s.isObjectPool {
@@ -212,7 +208,7 @@ func (s HeuristicalOptimizationStrategy) ShouldPruneObjects() bool {
 // ShouldRepackReferences determines whether the repository's references need to be repacked based
 // on heuristics. The more references there are, the more loose referencos may exist until they are
 // packed again.
-func (s HeuristicalOptimizationStrategy) ShouldRepackReferences() bool {
+func (s HeuristicalOptimizationStrategy) ShouldRepackReferences(context.Context) bool {
 	// If there aren't any loose refs then there is nothing we need to do.
 	if s.info.References.LooseReferencesCount == 0 {
 		return false
@@ -274,7 +270,7 @@ func NewEagerOptimizationStrategy(ctx context.Context, repo *localrepo.Repo) (Ea
 // ShouldRepackObjects always instructs the caller to repack objects. The strategy will always be to
 // repack all objects into a single packfile. The bitmap will be written in case the repository does
 // not have any alterantes.
-func (s EagerOptimizationStrategy) ShouldRepackObjects() (bool, RepackObjectsConfig) {
+func (s EagerOptimizationStrategy) ShouldRepackObjects(context.Context) (bool, RepackObjectsConfig) {
 	return true, RepackObjectsConfig{
 		FullRepack:  true,
 		WriteBitmap: !s.hasAlternate,
@@ -283,7 +279,7 @@ func (s EagerOptimizationStrategy) ShouldRepackObjects() (bool, RepackObjectsCon
 
 // ShouldWriteCommitGraph always instructs the caller to write the commit-graph. The strategy will
 // always be to completely rewrite the commit-graph chain.
-func (s EagerOptimizationStrategy) ShouldWriteCommitGraph() (bool, WriteCommitGraphConfig) {
+func (s EagerOptimizationStrategy) ShouldWriteCommitGraph(context.Context) (bool, WriteCommitGraphConfig) {
 	return true, WriteCommitGraphConfig{
 		ReplaceChain: true,
 	}
@@ -291,11 +287,11 @@ func (s EagerOptimizationStrategy) ShouldWriteCommitGraph() (bool, WriteCommitGr
 
 // ShouldPruneObjects always instructs the caller to prune objects, unless the repository is an
 // object pool.
-func (s EagerOptimizationStrategy) ShouldPruneObjects() bool {
+func (s EagerOptimizationStrategy) ShouldPruneObjects(context.Context) bool {
 	return !s.isObjectPool
 }
 
 // ShouldRepackReferences always instructs the caller to repack references.
-func (s EagerOptimizationStrategy) ShouldRepackReferences() bool {
+func (s EagerOptimizationStrategy) ShouldRepackReferences(context.Context) bool {
 	return true
 }
diff --git a/internal/git/housekeeping/optimization_strategy_test.go b/internal/git/housekeeping/optimization_strategy_test.go
index f8497eadc..8f257064a 100644
--- a/internal/git/housekeeping/optimization_strategy_test.go
+++ b/internal/git/housekeeping/optimization_strategy_test.go
@@ -1,6 +1,7 @@
 package housekeeping
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -11,6 +12,7 @@ import (
 	"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/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/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
@@ -317,6 +319,8 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 func TestHeuristicalOptimizationStrategy_ShouldRepackObjects(t *testing.T) {
 	t.Parallel()
 
+	ctx := testhelper.Context(t)
+
 	for _, tc := range []struct {
 		desc           string
 		strategy       HeuristicalOptimizationStrategy
@@ -374,7 +378,7 @@ func TestHeuristicalOptimizationStrategy_ShouldRepackObjects(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			repackNeeded, repackCfg := tc.strategy.ShouldRepackObjects()
+			repackNeeded, repackCfg := tc.strategy.ShouldRepackObjects(ctx)
 			require.Equal(t, tc.expectedNeeded, repackNeeded)
 			require.Equal(t, tc.expectedConfig, repackCfg)
 		})
@@ -458,14 +462,14 @@ func TestHeuristicalOptimizationStrategy_ShouldRepackObjects(t *testing.T) {
 						isObjectPool: tc.isPool,
 					}
 
-					repackNeeded, _ := strategy.ShouldRepackObjects()
+					repackNeeded, _ := strategy.ShouldRepackObjects(ctx)
 					require.False(t, repackNeeded)
 
 					// Now we add the last packfile that should bring us across
 					// the boundary of having to repack.
 					strategy.info.Packfiles.Count++
 
-					repackNeeded, repackCfg := strategy.ShouldRepackObjects()
+					repackNeeded, repackCfg := strategy.ShouldRepackObjects(ctx)
 					require.True(t, repackNeeded)
 					require.Equal(t, RepackObjectsConfig{
 						FullRepack:  true,
@@ -530,7 +534,7 @@ func TestHeuristicalOptimizationStrategy_ShouldRepackObjects(t *testing.T) {
 					isObjectPool: tc.isPool,
 				}
 
-				repackNeeded, repackCfg := strategy.ShouldRepackObjects()
+				repackNeeded, repackCfg := strategy.ShouldRepackObjects(ctx)
 				require.Equal(t, outerTC.expectedRepack, repackNeeded)
 				require.Equal(t, RepackObjectsConfig{
 					FullRepack:  false,
@@ -544,6 +548,8 @@ func TestHeuristicalOptimizationStrategy_ShouldRepackObjects(t *testing.T) {
 func TestHeuristicalOptimizationStrategy_ShouldPruneObjects(t *testing.T) {
 	t.Parallel()
 
+	ctx := testhelper.Context(t)
+
 	for _, tc := range []struct {
 		desc                       string
 		strategy                   HeuristicalOptimizationStrategy
@@ -590,13 +596,13 @@ func TestHeuristicalOptimizationStrategy_ShouldPruneObjects(t *testing.T) {
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			t.Run("normal repository", func(t *testing.T) {
-				require.Equal(t, tc.expectedShouldPruneObjects, tc.strategy.ShouldPruneObjects())
+				require.Equal(t, tc.expectedShouldPruneObjects, tc.strategy.ShouldPruneObjects(ctx))
 			})
 
 			t.Run("object pool", func(t *testing.T) {
 				strategy := tc.strategy
 				strategy.isObjectPool = true
-				require.False(t, strategy.ShouldPruneObjects())
+				require.False(t, strategy.ShouldPruneObjects(ctx))
 			})
 		})
 	}
@@ -605,6 +611,8 @@ func TestHeuristicalOptimizationStrategy_ShouldPruneObjects(t *testing.T) {
 func TestHeuristicalOptimizationStrategy_ShouldRepackReferences(t *testing.T) {
 	t.Parallel()
 
+	ctx := testhelper.Context(t)
+
 	const kiloByte = 1024
 
 	for _, tc := range []struct {
@@ -650,16 +658,20 @@ func TestHeuristicalOptimizationStrategy_ShouldRepackReferences(t *testing.T) {
 				},
 			}
 
-			require.False(t, strategy.ShouldRepackReferences())
+			require.False(t, strategy.ShouldRepackReferences(ctx))
 
 			strategy.info.References.LooseReferencesCount++
 
-			require.True(t, strategy.ShouldRepackReferences())
+			require.True(t, strategy.ShouldRepackReferences(ctx))
 		})
 	}
 }
 
 func TestHeuristicalOptimizationStrategy_NeedsWriteCommitGraph(t *testing.T) {
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testHeuristicalOptimizationStrategyNeedsWriteCommitGraph)
+}
+
+func testHeuristicalOptimizationStrategyNeedsWriteCommitGraph(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
 	for _, tc := range []struct {
@@ -730,6 +742,27 @@ func TestHeuristicalOptimizationStrategy_NeedsWriteCommitGraph(t *testing.T) {
 					},
 				},
 			},
+			// If we have no generation data then we want to rewrite the commit-graph,
+			// but only if the feature flag is enabled.
+			expectedNeeded: featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
+			expectedCfg: WriteCommitGraphConfig{
+				ReplaceChain: featureflag.UseCommitGraphGenerationData.IsEnabled(ctx),
+			},
+		},
+		{
+			desc: "repository with split commit-graph and generation data with bitmap without repack",
+			strategy: HeuristicalOptimizationStrategy{
+				info: stats.RepositoryInfo{
+					References: stats.ReferencesInfo{
+						LooseReferencesCount: 1,
+					},
+					CommitGraph: stats.CommitGraphInfo{
+						CommitGraphChainLength: 1,
+						HasBloomFilters:        true,
+						HasGenerationData:      true,
+					},
+				},
+			},
 			// We use the information about whether we repacked objects as an indicator
 			// whether something has changed in the repository. If it didn't, then we
 			// assume no new objects exist and thus we don't rewrite the commit-graph.
@@ -783,7 +816,7 @@ func TestHeuristicalOptimizationStrategy_NeedsWriteCommitGraph(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			needed, writeCommitGraphCfg := tc.strategy.ShouldWriteCommitGraph()
+			needed, writeCommitGraphCfg := tc.strategy.ShouldWriteCommitGraph(ctx)
 			require.Equal(t, tc.expectedNeeded, needed)
 			require.Equal(t, tc.expectedCfg, writeCommitGraphCfg)
 		})
@@ -851,6 +884,8 @@ func TestNewEagerOptimizationStrategy(t *testing.T) {
 func TestEagerOptimizationStrategy(t *testing.T) {
 	t.Parallel()
 
+	ctx := testhelper.Context(t)
+
 	for _, tc := range []struct {
 		desc                     string
 		strategy                 EagerOptimizationStrategy
@@ -885,21 +920,21 @@ func TestEagerOptimizationStrategy(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			shouldRepackObjects, repackObjectsCfg := tc.strategy.ShouldRepackObjects()
+			shouldRepackObjects, repackObjectsCfg := tc.strategy.ShouldRepackObjects(ctx)
 			require.True(t, shouldRepackObjects)
 			require.Equal(t, RepackObjectsConfig{
 				FullRepack:  true,
 				WriteBitmap: tc.expectWriteBitmap,
 			}, repackObjectsCfg)
 
-			shouldWriteCommitGraph, writeCommitGraphCfg := tc.strategy.ShouldWriteCommitGraph()
+			shouldWriteCommitGraph, writeCommitGraphCfg := tc.strategy.ShouldWriteCommitGraph(ctx)
 			require.True(t, shouldWriteCommitGraph)
 			require.Equal(t, WriteCommitGraphConfig{
 				ReplaceChain: true,
 			}, writeCommitGraphCfg)
 
-			require.Equal(t, tc.expectShouldPruneObjects, tc.strategy.ShouldPruneObjects())
-			require.True(t, tc.strategy.ShouldRepackReferences())
+			require.Equal(t, tc.expectShouldPruneObjects, tc.strategy.ShouldPruneObjects(ctx))
+			require.True(t, tc.strategy.ShouldRepackReferences(ctx))
 		})
 	}
 }
@@ -914,19 +949,19 @@ type mockOptimizationStrategy struct {
 	writeCommitGraphCfg    WriteCommitGraphConfig
 }
 
-func (m mockOptimizationStrategy) ShouldRepackObjects() (bool, RepackObjectsConfig) {
+func (m mockOptimizationStrategy) ShouldRepackObjects(context.Context) (bool, RepackObjectsConfig) {
 	return m.shouldRepackObjects, m.repackObjectsCfg
 }
 
-func (m mockOptimizationStrategy) ShouldPruneObjects() bool {
+func (m mockOptimizationStrategy) ShouldPruneObjects(context.Context) bool {
 	return m.shouldPruneObjects
 }
 
-func (m mockOptimizationStrategy) ShouldRepackReferences() bool {
+func (m mockOptimizationStrategy) ShouldRepackReferences(context.Context) bool {
 	return m.shouldRepackReferences
 }
 
-func (m mockOptimizationStrategy) ShouldWriteCommitGraph() (bool, WriteCommitGraphConfig) {
+func (m mockOptimizationStrategy) ShouldWriteCommitGraph(context.Context) (bool, WriteCommitGraphConfig) {
 	return m.shouldWriteCommitGraph, m.writeCommitGraphCfg
 }
 
diff --git a/internal/git/housekeeping/optimize_repository.go b/internal/git/housekeeping/optimize_repository.go
index 52fe8b156..b4827259c 100644
--- a/internal/git/housekeeping/optimize_repository.go
+++ b/internal/git/housekeeping/optimize_repository.go
@@ -159,7 +159,7 @@ func optimizeRepository(
 
 // repackIfNeeded repacks the repository according to the strategy.
 func repackIfNeeded(ctx context.Context, repo *localrepo.Repo, strategy OptimizationStrategy) (bool, RepackObjectsConfig, error) {
-	repackNeeded, cfg := strategy.ShouldRepackObjects()
+	repackNeeded, cfg := strategy.ShouldRepackObjects(ctx)
 	if !repackNeeded {
 		return false, RepackObjectsConfig{}, nil
 	}
@@ -173,7 +173,7 @@ func repackIfNeeded(ctx context.Context, repo *localrepo.Repo, strategy Optimiza
 
 // writeCommitGraphIfNeeded writes the commit-graph if required.
 func writeCommitGraphIfNeeded(ctx context.Context, repo *localrepo.Repo, strategy OptimizationStrategy) (bool, WriteCommitGraphConfig, error) {
-	needed, cfg := strategy.ShouldWriteCommitGraph()
+	needed, cfg := strategy.ShouldWriteCommitGraph(ctx)
 	if !needed {
 		return false, WriteCommitGraphConfig{}, nil
 	}
@@ -188,7 +188,7 @@ func writeCommitGraphIfNeeded(ctx context.Context, repo *localrepo.Repo, strateg
 // pruneIfNeeded removes objects from the repository which are either unreachable or which are
 // already part of a packfile. We use a grace period of two weeks.
 func pruneIfNeeded(ctx context.Context, repo *localrepo.Repo, strategy OptimizationStrategy) (bool, error) {
-	if !strategy.ShouldPruneObjects() {
+	if !strategy.ShouldPruneObjects(ctx) {
 		return false, nil
 	}
 
@@ -210,7 +210,7 @@ func pruneIfNeeded(ctx context.Context, repo *localrepo.Repo, strategy Optimizat
 }
 
 func packRefsIfNeeded(ctx context.Context, repo *localrepo.Repo, strategy OptimizationStrategy) (bool, error) {
-	if !strategy.ShouldRepackReferences() {
+	if !strategy.ShouldRepackReferences(ctx) {
 		return false, nil
 	}
 
diff --git a/internal/git/housekeeping/optimize_repository_test.go b/internal/git/housekeeping/optimize_repository_test.go
index 565c2e267..746ee9ad4 100644
--- a/internal/git/housekeeping/optimize_repository_test.go
+++ b/internal/git/housekeeping/optimize_repository_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/gitaly/transaction"
+	"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/proto/go/gitalypb"
@@ -151,9 +152,13 @@ func TestPackRefsIfNeeded(t *testing.T) {
 
 func TestOptimizeRepository(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
-	cfg := testcfg.Build(t)
+	testhelper.NewFeatureSets(featureflag.UseCommitGraphGenerationData).Run(t, testOptimizeRepository)
+}
 
+func testOptimizeRepository(t *testing.T, ctx context.Context) {
+	t.Parallel()
+
+	cfg := testcfg.Build(t)
 	txManager := transaction.NewManager(cfg, backchannel.NewRegistry())
 
 	for _, tc := range []struct {
@@ -213,6 +218,33 @@ gitaly_housekeeping_tasks_total{housekeeping_task="total", status="success"} 1
 `,
 		},
 		{
+			desc: "repository with commit-graph without generation data writes commit-graph",
+			setup: func(t *testing.T, relativePath string) *gitalypb.Repository {
+				repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+					SkipCreationViaService: true,
+					RelativePath:           relativePath,
+				})
+				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
+				gittest.Exec(t, cfg, "-C", repoPath, "repack", "-A", "-d", "--write-bitmap-index")
+				gittest.Exec(t, cfg, "-c", "commitGraph.generationVersion=1", "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
+				return repo
+			},
+			expectedMetrics: func() string {
+				if featureflag.UseCommitGraphGenerationData.IsEnabled(ctx) {
+					return `# HELP gitaly_housekeeping_tasks_total Total number of housekeeping tasks performed in the repository
+# TYPE gitaly_housekeeping_tasks_total counter
+gitaly_housekeeping_tasks_total{housekeeping_task="written_commit_graph_full", status="success"} 1
+gitaly_housekeeping_tasks_total{housekeeping_task="total", status="success"} 1
+`
+				}
+
+				return `# HELP gitaly_housekeeping_tasks_total Total number of housekeeping tasks performed in the repository
+# TYPE gitaly_housekeeping_tasks_total counter
+gitaly_housekeeping_tasks_total{housekeeping_task="total", status="success"} 1
+`
+			}(),
+		},
+		{
 			desc: "repository with multiple packfiles packs only for object pool",
 			setup: func(t *testing.T, relativePath string) *gitalypb.Repository {
 				repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
@@ -228,7 +260,7 @@ gitaly_housekeeping_tasks_total{housekeeping_task="total", status="success"} 1
 				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("second"), gittest.WithMessage("second"))
 				gittest.Exec(t, cfg, "-C", repoPath, "repack")
 
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
+				gittest.Exec(t, cfg, "-c", "commitGraph.generationVersion=2", "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
 
 				return repo
 			},
@@ -253,7 +285,7 @@ gitaly_housekeeping_tasks_total{housekeeping_task="total", status="success"} 1
 				})
 				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
 				gittest.Exec(t, cfg, "-C", repoPath, "repack", "-A", "-d", "--write-bitmap-index")
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
+				gittest.Exec(t, cfg, "-c", "commitGraph.generationVersion=2", "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
 				return repo
 			},
 			expectedMetrics: `# HELP gitaly_housekeeping_tasks_total Total number of housekeeping tasks performed in the repository
@@ -270,7 +302,7 @@ gitaly_housekeeping_tasks_total{housekeeping_task="total", status="success"} 1
 				})
 				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
 				gittest.Exec(t, cfg, "-C", repoPath, "repack", "-A", "-d", "--write-bitmap-index")
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
+				gittest.Exec(t, cfg, "-c", "commitGraph.generationVersion=2", "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
 
 				// The repack won't repack the following objects because they're
 				// broken, and thus we'll retry to prune them afterwards.
@@ -305,7 +337,7 @@ gitaly_housekeeping_tasks_total{housekeeping_task="total", status="success"} 1
 				})
 				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
 				gittest.Exec(t, cfg, "-C", repoPath, "repack", "-A", "-d", "--write-bitmap-index")
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
+				gittest.Exec(t, cfg, "-c", "commitGraph.generationVersion=2", "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
 
 				// The repack won't repack the following objects because they're
 				// broken, and thus we'll retry to prune them afterwards.
@@ -349,7 +381,7 @@ gitaly_housekeeping_tasks_total{housekeeping_task="total", status="success"} 1
 				}
 
 				gittest.Exec(t, cfg, "-C", repoPath, "repack", "-A", "--write-bitmap-index")
-				gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
+				gittest.Exec(t, cfg, "-c", "commitGraph.generationVersion=2", "-C", repoPath, "commit-graph", "write", "--split", "--changed-paths")
 
 				return repo
 			},
@@ -365,8 +397,6 @@ gitaly_housekeeping_tasks_total{housekeeping_task="total", status="success"} 1
 		testRepoAndPool(t, tc.desc, func(t *testing.T, relativePath string) {
 			t.Parallel()
 
-			ctx := testhelper.Context(t)
-
 			repoProto := tc.setup(t, relativePath)
 			repo := localrepo.NewTestRepo(t, cfg, repoProto)