Hide object pools .have refs

When an object pool has many members, it will have many remotes with
references. That makes the ref advertisement large. This changes
effectively hides all refs of the alternate and thus shrinks the ref
advertisement response.

Part of: https://gitlab.com/gitlab-org/gitaly/issues/1747

6 files changed, 113 insertions, 14 deletions

diff --git a/changelogs/unreleased/zj-hide-object-pool-refs.yml b/changelogs/unreleased/zj-hide-object-pool-refs.yml
new file mode 100644
index 000000000..a3a78262b
--- /dev/null
+++ b/changelogs/unreleased/zj-hide-object-pool-refs.yml
@@ -0,0 +1,5 @@
+---
+title: Hide object pools .have refs
+merge_request: 1323
+author:
+type: performance
diff --git a/internal/service/smarthttp/inforefs.go b/internal/service/smarthttp/inforefs.go
index 3947c1cd9..7b0018679 100644
--- a/internal/service/smarthttp/inforefs.go
+++ b/internal/service/smarthttp/inforefs.go
@@ -42,7 +42,13 @@ func handleInfoRefs(ctx context.Context, service string, req *gitalypb.InfoRefsR
 		return err
 	}
 
-	args := []string{}
+	// In case the repository belongs to an object pool, we want to prevent
+	// Git from including the pool's refs in the ref advertisement. We do
+	// this by rigging core.alternateRefsCommand to produce no output.
+	// Because Git itself will append the pool repository directory, the
+	// command ends with a "#". The end result is that Git runs
+	// `/bin/sh -c 'exit 0 # /path/to/pool.git`.
+	args := []string{"-c", "core.alternateRefsCommand=exit 0 #"}
 	for _, params := range req.GitConfigOptions {
 		args = append(args, "-c", params)
 	}
diff --git a/internal/service/smarthttp/inforefs_test.go b/internal/service/smarthttp/inforefs_test.go
index 6683a6336..d1cc8fb89 100644
--- a/internal/service/smarthttp/inforefs_test.go
+++ b/internal/service/smarthttp/inforefs_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/objectpool"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 	"google.golang.org/grpc/codes"
@@ -139,6 +140,43 @@ func TestSuccessfulInfoRefsReceivePack(t *testing.T) {
 	})
 }
 
+func TestObjectPoolRefAdvertisementHiding(t *testing.T) {
+	server, serverSocketPath := runSmartHTTPServer(t)
+	defer server.Stop()
+
+	client, conn := newSmartHTTPClient(t, serverSocketPath)
+	defer conn.Close()
+
+	repo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	pool, err := objectpool.NewObjectPool(repo.GetStorageName(), testhelper.NewTestObjectPoolName(t))
+	require.NoError(t, err)
+
+	require.NoError(t, pool.Create(ctx, repo))
+	defer pool.Remove(ctx)
+
+	commitID := testhelper.CreateCommit(t, pool.FullPath(), t.Name(), nil)
+
+	require.NoError(t, pool.Link(ctx, repo))
+
+	rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo}
+
+	c, err := client.InfoRefsReceivePack(ctx, rpcRequest)
+	require.NoError(t, err)
+
+	response, err := ioutil.ReadAll(streamio.NewReader(func() ([]byte, error) {
+		resp, err := c.Recv()
+		return resp.GetData(), err
+	}))
+
+	require.NoError(t, err)
+	require.NotContains(t, string(response), commitID+" .have")
+}
+
 func TestFailureRepoNotFoundInfoRefsReceivePack(t *testing.T) {
 	server, serverSocketPath := runSmartHTTPServer(t)
 	defer server.Stop()
diff --git a/internal/service/smarthttp/receive_pack.go b/internal/service/smarthttp/receive_pack.go
index 5ff0d584e..fd214f98c 100644
--- a/internal/service/smarthttp/receive_pack.go
+++ b/internal/service/smarthttp/receive_pack.go
@@ -62,6 +62,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 	env = append(env, command.GitEnv...)
 
 	opts := append([]string{fmt.Sprintf("core.hooksPath=%s", hooks.Path())}, req.GitConfigOptions...)
+
 	gitOptions := git.BuildGitOptions(opts, "receive-pack", "--stateless-rpc", repoPath)
 	cmd, err := git.BareCommand(ctx, stdin, stdout, nil, env, gitOptions...)
 
diff --git a/internal/service/ssh/receive_pack.go b/internal/service/ssh/receive_pack.go
index e952e7777..e8224808c 100644
--- a/internal/service/ssh/receive_pack.go
+++ b/internal/service/ssh/receive_pack.go
@@ -70,6 +70,14 @@ func sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, req *gitaly
 	env = append(env, command.GitEnv...)
 
 	opts := append([]string{fmt.Sprintf("core.hooksPath=%s", hooks.Path())}, req.GitConfigOptions...)
+
+	// In case the repository belongs to an object pool, we want to prevent
+	// Git from including the pool's refs in the ref advertisement. We do
+	// this by rigging core.alternateRefsCommand to produce no output.
+	// Because Git itself will append the pool repository directory, the
+	// command ends with a "#". The end result is that Git runs
+	// `/bin/sh -c 'exit 0 # /path/to/pool.git`.
+	opts = append(opts, "core.alternateRefsCommand=exit 0 #")
 	gitOptions := git.BuildGitOptions(opts, "receive-pack", repoPath)
 	cmd, err := git.BareCommand(ctx, stdin, stdout, stderr, env, gitOptions...)
 
diff --git a/internal/service/ssh/receive_pack_test.go b/internal/service/ssh/receive_pack_test.go
index 44f0ac1c0..a823b56cd 100644
--- a/internal/service/ssh/receive_pack_test.go
+++ b/internal/service/ssh/receive_pack_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -18,7 +19,9 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
+	"gitlab.com/gitlab-org/gitaly/internal/git/objectpool"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/streamio"
 	"google.golang.org/grpc/codes"
 )
 
@@ -60,15 +63,12 @@ func TestFailedReceivePackRequestDueToValidationError(t *testing.T) {
 		t.Run(test.Desc, func(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
+
 			stream, err := client.SSHReceivePack(ctx)
-			if err != nil {
-				t.Fatal(err)
-			}
+			require.NoError(t, err)
 
-			if err = stream.Send(test.Req); err != nil {
-				t.Fatal(err)
-			}
-			stream.CloseSend()
+			require.NoError(t, stream.Send(test.Req))
+			require.NoError(t, stream.CloseSend())
 
 			err = drainPostReceivePackResponse(stream)
 			testhelper.RequireGrpcError(t, err, test.Code)
@@ -130,14 +130,10 @@ func TestReceivePackPushFailure(t *testing.T) {
 	defer server.Stop()
 
 	_, _, err := testCloneAndPush(t, serverSocketPath, pushParams{storageName: "foobar", glID: "1"})
-	if err == nil {
-		t.Errorf("local and remote head equal. push did not fail")
-	}
+	require.Error(t, err, "local and remote head equal. push did not fail")
 
 	_, _, err = testCloneAndPush(t, serverSocketPath, pushParams{storageName: testRepo.GetStorageName(), glID: ""})
-	if err == nil {
-		t.Errorf("local and remote head equal. push did not fail")
-	}
+	require.Error(t, err, "local and remote head equal. push did not fail")
 }
 
 func TestReceivePackPushHookFailure(t *testing.T) {
@@ -161,6 +157,51 @@ func TestReceivePackPushHookFailure(t *testing.T) {
 	require.Contains(t, err.Error(), "(pre-receive hook declined)")
 }
 
+func TestObjectPoolRefAdvertisementHidingSSH(t *testing.T) {
+	server, serverSocketPath := runSSHServer(t)
+	defer server.Stop()
+
+	client, conn := newSSHClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	stream, err := client.SSHReceivePack(ctx)
+	require.NoError(t, err)
+
+	repo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := objectpool.NewObjectPool(repo.GetStorageName(), testhelper.NewTestObjectPoolName(t))
+	require.NoError(t, err)
+
+	require.NoError(t, pool.Create(ctx, repo))
+	defer pool.Remove(ctx)
+
+	require.NoError(t, pool.Link(ctx, repo))
+
+	commitID := testhelper.CreateCommit(t, pool.FullPath(), t.Name(), nil)
+
+	// First request
+	require.NoError(t, stream.Send(&gitalypb.SSHReceivePackRequest{
+		Repository: &gitalypb.Repository{StorageName: "default", RelativePath: repo.GetRelativePath()}, GlId: "user-123",
+	}))
+
+	require.NoError(t, stream.Send(&gitalypb.SSHReceivePackRequest{Stdin: []byte("0000")}))
+	require.NoError(t, stream.CloseSend())
+
+	r := streamio.NewReader(func() ([]byte, error) {
+		msg, err := stream.Recv()
+		return msg.GetStdout(), err
+	})
+
+	var b bytes.Buffer
+	_, err = io.Copy(&b, r)
+	require.NoError(t, err)
+	require.NotContains(t, b.String(), commitID+" .have")
+}
+
 func testCloneAndPush(t *testing.T, serverSocketPath string, params pushParams) (string, string, error) {
 	storagePath := testhelper.GitlabTestStoragePath()
 	tempRepo := "gitlab-test-ssh-receive-pack.git"