Merge branch 'pks-repository-default-enable-tx-create-from-bundle' into 'master'

repository: Remove feature flag for atomic repo creation from bundles

Closes #3686

See merge request gitlab-org/gitaly!3717

3 files changed, 16 insertions, 37 deletions

diff --git a/internal/gitaly/service/repository/create_from_bundle.go b/internal/gitaly/service/repository/create_from_bundle.go
index e5005d711..719a2c0d4 100644
--- a/internal/gitaly/service/repository/create_from_bundle.go
+++ b/internal/gitaly/service/repository/create_from_bundle.go
@@ -10,7 +10,6 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/tempdir"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v14/streamio"
@@ -92,18 +91,16 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
 		return status.Error(codes.Internal, cleanError)
 	}
 
-	fetchFlags := []git.Option{git.Flag{Name: "--quiet"}}
-	if featureflag.CreateRepositoryFromBundleAtomicFetch.IsEnabled(ctx) {
-		fetchFlags = append(fetchFlags, git.Flag{Name: "--atomic"})
-	}
-
 	// We do a fetch to get all refs including keep-around refs
 	stderr.Reset()
 	cmd, err = s.gitCmdFactory.NewWithDir(ctx, repoPath,
 		git.SubCmd{
-			Name:  "fetch",
-			Flags: fetchFlags,
-			Args:  []string{bundlePath, "refs/*:refs/*"},
+			Name: "fetch",
+			Flags: []git.Option{
+				git.Flag{Name: "--quiet"},
+				git.Flag{Name: "--atomic"},
+			},
+			Args: []string{bundlePath, "refs/*:refs/*"},
 		},
 		git.WithStderr(&stderr),
 		git.WithRefTxHook(ctx, repo, s.cfg),
diff --git a/internal/gitaly/service/repository/create_from_bundle_test.go b/internal/gitaly/service/repository/create_from_bundle_test.go
index 40ac54ab0..332f40fb4 100644
--- a/internal/gitaly/service/repository/create_from_bundle_test.go
+++ b/internal/gitaly/service/repository/create_from_bundle_test.go
@@ -19,7 +19,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/tempdir"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testassert"
@@ -95,13 +94,7 @@ func TestServer_CreateRepositoryFromBundle_successful(t *testing.T) {
 	require.NotNil(t, commit)
 }
 
-func TestServer_CreateRepositoryFromBundle_transactional(t *testing.T) {
-	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
-		featureflag.CreateRepositoryFromBundleAtomicFetch,
-	}).Run(t, testServerCreateRepositoryFromBundleTransactional)
-}
-
-func testServerCreateRepositoryFromBundleTransactional(t *testing.T, ctx context.Context) {
+func TestServerCreateRepositoryFromBundleTransactional(t *testing.T) {
 	var votes []voting.Vote
 	txManager := &transaction.MockManager{
 		VoteFn: func(ctx context.Context, tx txinfo.Transaction, vote voting.Vote) error {
@@ -123,6 +116,8 @@ func testServerCreateRepositoryFromBundleTransactional(t *testing.T, ctx context
 		gittest.Exec(t, cfg, "-C", repoPath, "update-ref", keepAroundRef, masterOID)
 	}
 
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 	ctx, err := txinfo.InjectTransaction(ctx, 1, "primary", true)
 	require.NoError(t, err)
 	ctx = helper.IncomingToOutgoing(ctx)
@@ -164,22 +159,13 @@ func testServerCreateRepositoryFromBundleTransactional(t *testing.T, ctx context
 
 	// Keep-around references are not fetched via git-clone(1) because non-mirror clones only
 	// fetch branches and tags. These additional references are thus obtained via git-fetch(1).
-	// If the following feature flag is enabled, then we'll use the `--atomic` flag for
-	// git-fetch(1) and thus bundle all reference updates into a single transaction. Otherwise,
-	// the old behaviour will create one transaction per reference.
-	if featureflag.CreateRepositoryFromBundleAtomicFetch.IsEnabled(ctx) {
-		votingInput = append(votingInput,
-			fmt.Sprintf("%s %s refs/keep-around/2\n%s %s refs/keep-around/1\n", git.ZeroOID, masterOID, git.ZeroOID, masterOID),
-			fmt.Sprintf("%s %s refs/keep-around/2\n%s %s refs/keep-around/1\n", git.ZeroOID, masterOID, git.ZeroOID, masterOID),
-		)
-	} else {
-		votingInput = append(votingInput,
-			fmt.Sprintf("%s %s refs/keep-around/2\n", git.ZeroOID, masterOID),
-			fmt.Sprintf("%s %s refs/keep-around/2\n", git.ZeroOID, masterOID),
-			fmt.Sprintf("%s %s refs/keep-around/1\n", git.ZeroOID, masterOID),
-			fmt.Sprintf("%s %s refs/keep-around/1\n", git.ZeroOID, masterOID),
-		)
-	}
+	// Given that we use the `--atomic` flag for git-fetch(1), all reference updates will be in
+	// a single transaction and we thus expect exactly two votes (once for "prepare", once for
+	// "commit").
+	votingInput = append(votingInput,
+		fmt.Sprintf("%s %s refs/keep-around/2\n%s %s refs/keep-around/1\n", git.ZeroOID, masterOID, git.ZeroOID, masterOID),
+		fmt.Sprintf("%s %s refs/keep-around/2\n%s %s refs/keep-around/1\n", git.ZeroOID, masterOID, git.ZeroOID, masterOID),
+	)
 
 	// And this is the final vote in Create(), which does a git-for-each-ref(1) in the target
 	// repository and then manually invokes the hook. The format is thus different from above
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index c32da572c..c038e4c81 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -6,9 +6,6 @@ package featureflag
 var (
 	// GoSetConfig enables git2go implementation of SetConfig.
 	GoSetConfig = FeatureFlag{Name: "go_set_config", OnByDefault: false}
-	// CreateRepositoryFromBundleAtomicFetch will add the `--atomic` flag to git-fetch(1) in
-	// order to reduce the number of transactional votes.
-	CreateRepositoryFromBundleAtomicFetch = FeatureFlag{Name: "create_repository_from_bundle_atomic_fetch", OnByDefault: true}
 	// ResolveConflictsWithHooks will cause the ResolveConflicts RPC to run Git hooks after committing changes
 	// to the branch.
 	ResolveConflictsWithHooks = FeatureFlag{Name: "resolve_conflicts_with_hooks", OnByDefault: true}
@@ -35,7 +32,6 @@ var (
 // All includes all feature flags.
 var All = []FeatureFlag{
 	GoSetConfig,
-	CreateRepositoryFromBundleAtomicFetch,
 	ResolveConflictsWithHooks,
 	ReplicateRepositoryDirectFetch,
 	FindAllTagsPipeline,