objectpool: Disable transactions for FetchIntoObjectPool

When executing git-fetch(1), git will currently create one referecne
transaction per reference that is to be modified. This is quite
inefficient when used together with the reference-transaction hook, as
it effectively means that we execute the hook once for each reference.
If fetching hundreds of thousands of references, this directly
translates to hundreds of thousands of spawned processes, which means an
extreme performance hit.

Fix the issue for `FetchIntoObjectPool` by disabling the
reference-transaction hook for now. This is only a stop-gap measure
until we land the equivalent of `git fetch --atomic`, which creates a
single transaction only.

2 files changed, 42 insertions, 3 deletions

diff --git a/internal/git/objectpool/fetch.go b/internal/git/objectpool/fetch.go
index d49b60043..ddbfc1e23 100644
--- a/internal/git/objectpool/fetch.go
+++ b/internal/git/objectpool/fetch.go
@@ -18,7 +18,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/housekeeping"
 	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/internal/git/updateref"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
@@ -48,7 +47,7 @@ func (o *ObjectPool) FetchFromOrigin(ctx context.Context, origin *gitalypb.Repos
 	}
 
 	opts := []git.CmdOpt{
-		git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(o), o.cfg),
+		git.WithDisabledHooks(),
 	}
 
 	getRemotes, err := git.SafeCmd(ctx, o, nil, git.SubCmd{Name: "remote"}, opts...)
@@ -153,7 +152,7 @@ func rescueDanglingObjects(ctx context.Context, repo repository.GitRepo) error {
 		return err
 	}
 
-	updater, err := updateref.New(ctx, repo)
+	updater, err := updateref.New(ctx, repo, updateref.WithDisabledTransactions())
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
index 68e605001..aee862e12 100644
--- a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
+++ b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
 	"gitlab.com/gitlab-org/gitaly/internal/git/objectpool"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -84,6 +85,45 @@ func TestFetchIntoObjectPool_Success(t *testing.T) {
 	require.Error(t, err, "Expected refs/heads/broken to be deleted")
 }
 
+func TestFetchIntoObjectPool_hooksDisabled(t *testing.T) {
+	server, serverSocketPath := runObjectPoolServer(t, config.Config, config.NewLocator(config.Config))
+	defer server.Stop()
+
+	client, conn := newObjectPoolClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := objectpool.NewObjectPool(config.Config, config.NewLocator(config.Config), "default", testhelper.NewTestObjectPoolName(t))
+	require.NoError(t, err)
+	defer pool.Remove(ctx)
+
+	hookDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
+
+	defer func(oldValue string) {
+		hooks.Override = oldValue
+	}(hooks.Override)
+	hooks.Override = hookDir
+
+	// Set up a custom reference-transaction hook which simply exits failure. This asserts that
+	// the RPC doesn't invoke any reference-transaction.
+	require.NoError(t, ioutil.WriteFile(filepath.Join(hookDir, "reference-transaction"), []byte("#!/bin/sh\nexit 1\n"), 0777))
+
+	req := &gitalypb.FetchIntoObjectPoolRequest{
+		ObjectPool: pool.ToProto(),
+		Origin:     testRepo,
+		Repack:     true,
+	}
+
+	_, err = client.FetchIntoObjectPool(ctx, req)
+	require.NoError(t, err)
+}
+
 func TestFetchIntoObjectPool_CollectLogStatistics(t *testing.T) {
 	defer func(tl func(tb testing.TB) *logrus.Logger) {
 		testhelper.NewTestLogger = tl