Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <psteinhardt@gitlab.com>2022-12-13 16:14:14 +0300
committerPatrick Steinhardt <psteinhardt@gitlab.com>2022-12-30 16:25:03 +0300
commitc73860e8addd42ecca0e9ffa4505d949bb79f182 (patch)
treee3ab22d76fa64684f324ccbd7acf537a377cd5c7
parent92147a9af10c9b22385d952d3d9b6582609428c7 (diff)
git: Enable reading and writing commit-graph generation data
The commit-graph data structure is used to speed up various different queries that would normally require us to parse a Git commit from its object representation. One of the things it speeds up is the question whether a certain commit can in fact be reachable by another commit. This is done by computing a so-called corrected committer date, or generation number. The generation number is calculated as the maximum of the current commit's committer time or any of the parent's generation numbers. As such it is trivial to say if any commit has a generation number greater than the generation of a different commit, that it cannot ever be reached by it. Being able to speed up reachability checks with this generation number is an important optimization especially in the context of packfile negotiation, where we need to check whether a specific Git commit would already be reachable by a set of announced references or not. Right now we explicitly disable both reading and writing generation numbers by setting the unintuitive `commitGraph.generationVersion=1`. This had to be done because a patch series that fixed several bugs in the context of commit-graphs introduced with Git v2.36.0 actually uncovered a different bug that caused corruption when upgrading graphs that contained a v1 generation data. This source of corruption was finally fixed in 9550f6c16a (commit-graph: fix corrupt upgrade from generation v1 to v2, 2022-07-12), which is contained in Git v2.37.2 and newer. Changelog: added
-rw-r--r--internal/git/command_factory.go29
-rw-r--r--internal/git/command_factory_test.go15
-rw-r--r--internal/git/housekeeping/optimization_strategy_test.go64
-rw-r--r--internal/git/stats/commit_graph_test.go26
-rw-r--r--internal/git/stats/repository_info_test.go14
-rw-r--r--internal/gitaly/service/objectpool/reduplicate_test.go5
-rw-r--r--internal/gitaly/service/repository/commit_graph_test.go203
-rw-r--r--internal/gitaly/service/repository/gc_test.go13
-rw-r--r--internal/gitaly/service/repository/optimize_test.go54
-rw-r--r--internal/gitaly/service/repository/repack_test.go24
-rw-r--r--internal/metadata/featureflag/ff_use_commit_graph_generation_data.go10
-rw-r--r--internal/testhelper/testhelper.go3
12 files changed, 353 insertions, 107 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 006f59968..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,15 +599,20 @@ func TestExecCommandFactory_SidecarGitConfiguration(t *testing.T) {
{Key: "custom.key", Value: "injected"},
}
+ 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"},
{Key: "core.fsync", Value: "objects,derived-metadata,reference"},
{Key: "core.fsyncMethod", Value: "fsync"},
+ {Key: "commitGraph.generationVersion", Value: generationVersion},
{Key: "custom.key", Value: "injected"},
}, configPairs)
}
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)