Merge branch 'ps-rm-feature-flag-commit_without_batch_check' into 'master'

Skip the batch check for commits and tags

See merge request gitlab-org/gitaly!1812

25 files changed, 200 insertions, 197 deletions

diff --git a/changelogs/unreleased/ps-rm-feature-flag-commit_without_batch_check.yml b/changelogs/unreleased/ps-rm-feature-flag-commit_without_batch_check.yml
new file mode 100644
index 000000000..a2db64791
--- /dev/null
+++ b/changelogs/unreleased/ps-rm-feature-flag-commit_without_batch_check.yml
@@ -0,0 +1,5 @@
+---
+title: 'Skip the batch check for commits and tags'
+merge_request: 1812
+author:
+type: performance
diff --git a/internal/git/catfile/catfile.go b/internal/git/catfile/catfile.go
index a1ce1fbb6..52fb314b2 100644
--- a/internal/git/catfile/catfile.go
+++ b/internal/git/catfile/catfile.go
@@ -2,7 +2,6 @@ package catfile
 
 import (
 	"context"
-	"io"
 	"sync"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -80,7 +79,7 @@ func (c *Batch) Info(revspec string) (*ObjectInfo, error) {
 // point to a tree. To prevent this firstuse Info to resolve the revspec
 // and check the object type. Caller must consume the Reader before
 // making another call on C.
-func (c *Batch) Tree(revspec string) (io.Reader, error) {
+func (c *Batch) Tree(revspec string) (*Object, error) {
 	catfileLookupCounter.WithLabelValues("tree").Inc()
 	return c.batchProcess.reader(revspec, "tree")
 }
@@ -99,14 +98,14 @@ func (c *Batch) Commit(revspec string) (*Object, error) {
 //
 // It is an error if revspec does not point to a blob. To prevent this
 // first use Info to resolve the revspec and check the object type.
-func (c *Batch) Blob(revspec string) (io.Reader, error) {
+func (c *Batch) Blob(revspec string) (*Object, error) {
 	catfileLookupCounter.WithLabelValues("blob").Inc()
 	return c.batchProcess.reader(revspec, "blob")
 }
 
 // Tag returns a raw tag object. Caller must consume the Reader before
 // making another call on C.
-func (c *Batch) Tag(revspec string) (io.Reader, error) {
+func (c *Batch) Tag(revspec string) (*Object, error) {
 	catfileLookupCounter.WithLabelValues("tag").Inc()
 	return c.batchProcess.reader(revspec, "tag")
 }
diff --git a/internal/git/catfile/catfile_test.go b/internal/git/catfile/catfile_test.go
index 0321c9c9e..da1ab8059 100644
--- a/internal/git/catfile/catfile_test.go
+++ b/internal/git/catfile/catfile_test.go
@@ -62,26 +62,55 @@ func TestBlob(t *testing.T) {
 	require.NoError(t, err)
 
 	testCases := []struct {
-		desc   string
-		spec   string
-		output string
+		desc       string
+		spec       string
+		objInfo    ObjectInfo
+		content    []byte
+		requireErr func(*testing.T, error)
 	}{
 		{
-			desc:   "gitignore",
-			spec:   "60ecb67744cb56576c30214ff52294f8ce2def98:.gitignore",
-			output: string(gitignoreBytes),
+			desc: "gitignore",
+			spec: "60ecb67744cb56576c30214ff52294f8ce2def98:.gitignore",
+			objInfo: ObjectInfo{
+				Oid:  "dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82",
+				Type: "blob",
+				Size: int64(len(gitignoreBytes)),
+			},
+			content: gitignoreBytes,
+		},
+		{
+			desc: "not existing ref",
+			spec: "stub",
+			requireErr: func(t *testing.T, err error) {
+				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
+				require.EqualError(t, err, "object not found")
+			},
+		},
+		{
+			desc: "wrong object type",
+			spec: "1e292f8fedd741b75372e19097c76d327140c312", // is commit SHA1
+			requireErr: func(t *testing.T, err error) {
+				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
+				require.EqualError(t, err, "expected 1e292f8fedd741b75372e19097c76d327140c312 to be a blob, got commit")
+			},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			r, err := c.Blob(tc.spec)
-			require.NoError(t, err)
+			blobObj, err := c.Blob(tc.spec)
+
+			if tc.requireErr != nil {
+				tc.requireErr(t, err)
+				return
+			}
 
-			contents, err := ioutil.ReadAll(r)
 			require.NoError(t, err)
+			require.Equal(t, tc.objInfo, blobObj.ObjectInfo)
 
-			require.Equal(t, tc.output, string(contents))
+			contents, err := ioutil.ReadAll(blobObj.Reader)
+			require.NoError(t, err)
+			require.Equal(t, tc.content, contents)
 		})
 	}
 }
@@ -132,26 +161,55 @@ func TestTag(t *testing.T) {
 	require.NoError(t, err)
 
 	testCases := []struct {
-		desc   string
-		spec   string
-		output string
+		desc       string
+		spec       string
+		objInfo    ObjectInfo
+		content    []byte
+		requireErr func(*testing.T, error)
 	}{
 		{
-			desc:   "tag",
-			spec:   "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8",
-			output: string(tagBytes),
+			desc: "tag",
+			spec: "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8",
+			objInfo: ObjectInfo{
+				Oid:  "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8",
+				Type: "tag",
+				Size: int64(len(tagBytes)),
+			},
+			content: tagBytes,
+		},
+		{
+			desc: "not existing ref",
+			spec: "stub",
+			requireErr: func(t *testing.T, err error) {
+				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
+				require.EqualError(t, err, "object not found")
+			},
+		},
+		{
+			desc: "wrong object type",
+			spec: "1e292f8fedd741b75372e19097c76d327140c312", // is commit SHA1
+			requireErr: func(t *testing.T, err error) {
+				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
+				require.EqualError(t, err, "expected 1e292f8fedd741b75372e19097c76d327140c312 to be a tag, got commit")
+			},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			tagReader, err := c.Tag(tc.spec)
-			require.NoError(t, err)
+			tagObj, err := c.Tag(tc.spec)
+
+			if tc.requireErr != nil {
+				tc.requireErr(t, err)
+				return
+			}
 
-			contents, err := ioutil.ReadAll(tagReader)
 			require.NoError(t, err)
+			require.Equal(t, tc.objInfo, tagObj.ObjectInfo)
 
-			require.Equal(t, tc.output, string(contents))
+			contents, err := ioutil.ReadAll(tagObj.Reader)
+			require.NoError(t, err)
+			require.Equal(t, tc.content, contents)
 		})
 	}
 }
@@ -167,26 +225,55 @@ func TestTree(t *testing.T) {
 	require.NoError(t, err)
 
 	testCases := []struct {
-		desc   string
-		spec   string
-		output string
+		desc       string
+		spec       string
+		objInfo    ObjectInfo
+		content    []byte
+		requireErr func(*testing.T, error)
 	}{
 		{
-			desc:   "tree with non-oid spec",
-			spec:   "60ecb67744cb56576c30214ff52294f8ce2def98^{tree}",
-			output: string(treeBytes),
+			desc: "tree with non-oid spec",
+			spec: "60ecb67744cb56576c30214ff52294f8ce2def98^{tree}",
+			objInfo: ObjectInfo{
+				Oid:  "7e2f26d033ee47cd0745649d1a28277c56197921",
+				Type: "tree",
+				Size: int64(len(treeBytes)),
+			},
+			content: treeBytes,
+		},
+		{
+			desc: "not existing ref",
+			spec: "stud",
+			requireErr: func(t *testing.T, err error) {
+				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
+				require.EqualError(t, err, "object not found")
+			},
+		},
+		{
+			desc: "wrong object type",
+			spec: "1e292f8fedd741b75372e19097c76d327140c312", // is commit SHA1
+			requireErr: func(t *testing.T, err error) {
+				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
+				require.EqualError(t, err, "expected 1e292f8fedd741b75372e19097c76d327140c312 to be a tree, got commit")
+			},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			treeReader, err := c.Tree(tc.spec)
-			require.NoError(t, err)
+			treeObj, err := c.Tree(tc.spec)
+
+			if tc.requireErr != nil {
+				tc.requireErr(t, err)
+				return
+			}
 
-			contents, err := ioutil.ReadAll(treeReader)
 			require.NoError(t, err)
+			require.Equal(t, tc.objInfo, treeObj.ObjectInfo)
 
-			require.Equal(t, tc.output, string(contents))
+			contents, err := ioutil.ReadAll(treeObj.Reader)
+			require.NoError(t, err)
+			require.Equal(t, tc.content, contents)
 		})
 	}
 }
@@ -202,10 +289,10 @@ func TestRepeatedCalls(t *testing.T) {
 	treeBytes, err := ioutil.ReadFile("testdata/tree-7e2f26d033ee47cd0745649d1a28277c56197921")
 	require.NoError(t, err)
 
-	tree1Reader, err := c.Tree(treeOid)
+	tree1Obj, err := c.Tree(treeOid)
 	require.NoError(t, err)
 
-	tree1, err := ioutil.ReadAll(tree1Reader)
+	tree1, err := ioutil.ReadAll(tree1Obj.Reader)
 	require.NoError(t, err)
 
 	require.Equal(t, string(treeBytes), string(tree1))
@@ -225,10 +312,10 @@ func TestRepeatedCalls(t *testing.T) {
 	_, err = io.Copy(ioutil.Discard, blobReader)
 	require.NoError(t, err, "blob reading should still work")
 
-	tree2Reader, err := c.Tree(treeOid)
+	tree2Obj, err := c.Tree(treeOid)
 	require.NoError(t, err)
 
-	tree2, err := ioutil.ReadAll(tree2Reader)
+	tree2, err := ioutil.ReadAll(tree2Obj.Reader)
 	require.NoError(t, err, "request should succeed because blob was consumed")
 
 	require.Equal(t, string(treeBytes), string(tree2))
diff --git a/internal/git/log/commit.go b/internal/git/log/commit.go
index 282a54404..81a768be3 100644
--- a/internal/git/log/commit.go
+++ b/internal/git/log/commit.go
@@ -13,7 +13,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
@@ -25,32 +24,11 @@ func GetCommit(ctx context.Context, repo *gitalypb.Repository, revision string)
 		return nil, err
 	}
 
-	return GetCommitCatfile(ctx, c, revision)
+	return GetCommitCatfile(c, revision)
 }
 
 // GetCommitCatfile looks up a commit by revision using an existing *catfile.Batch instance.
-func GetCommitCatfile(ctx context.Context, c *catfile.Batch, revision string) (*gitalypb.GitCommit, error) {
-	if featureflag.IsEnabled(ctx, featureflag.CommitWithoutBatchCheck) {
-		return getCommitCatfileNew(c, revision)
-	}
-	return getCommitCatfileOld(c, revision)
-}
-
-func getCommitCatfileOld(c *catfile.Batch, revision string) (*gitalypb.GitCommit, error) {
-	info, err := c.Info(revision + "^{commit}")
-	if err != nil {
-		return nil, err
-	}
-
-	r, err := c.Commit(info.Oid)
-	if err != nil {
-		return nil, err
-	}
-
-	return parseRawCommit(r, info)
-}
-
-func getCommitCatfileNew(c *catfile.Batch, revision string) (*gitalypb.GitCommit, error) {
+func GetCommitCatfile(c *catfile.Batch, revision string) (*gitalypb.GitCommit, error) {
 	obj, err := c.Commit(revision + "^{commit}")
 	if err != nil {
 		return nil, err
@@ -61,17 +39,12 @@ func getCommitCatfileNew(c *catfile.Batch, revision string) (*gitalypb.GitCommit
 
 // GetCommitMessage looks up a commit message and returns it in its entirety.
 func GetCommitMessage(c *catfile.Batch, repo *gitalypb.Repository, revision string) ([]byte, error) {
-	info, err := c.Info(revision + "^{commit}")
-	if err != nil {
-		return nil, err
-	}
-
-	r, err := c.Commit(info.Oid)
+	obj, err := c.Commit(revision + "^{commit}")
 	if err != nil {
 		return nil, err
 	}
 
-	_, body, err := splitRawCommit(r)
+	_, body, err := splitRawCommit(obj.Reader)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/git/log/commit_test.go b/internal/git/log/commit_test.go
index 5eff5400d..0e3a05c69 100644
--- a/internal/git/log/commit_test.go
+++ b/internal/git/log/commit_test.go
@@ -7,7 +7,6 @@ import (
 	"github.com/golang/protobuf/ptypes/timestamp"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/metadata"
@@ -119,44 +118,23 @@ func TestGetCommitCatfile(t *testing.T) {
 	const blobSha = "c60514b6d3d6bf4bec1030f70026e34dfbd69ad5"
 
 	testCases := []struct {
-		desc      string
-		revision  string
-		featureOn bool
-		errStr    string
+		desc     string
+		revision string
+		errStr   string
 	}{
 		{
-			desc:      "feature off | commit",
-			revision:  commitSha,
-			featureOn: false,
+			desc:     "commit",
+			revision: commitSha,
 		},
 		{
-			desc:      "feature on | commit",
-			revision:  commitSha,
-			featureOn: true,
+			desc:     "not existing commit",
+			revision: "not existing revision",
+			errStr:   "object not found",
 		},
 		{
-			desc:      "feature off | not existing commit",
-			revision:  "not existing revision",
-			featureOn: false,
-			errStr:    "object not found",
-		},
-		{
-			desc:      "feature on | not existing commit",
-			revision:  "not existing revision",
-			featureOn: true,
-			errStr:    "object not found",
-		},
-		{
-			desc:      "feature off | blob sha",
-			revision:  blobSha,
-			featureOn: false,
-			errStr:    "object not found",
-		},
-		{
-			desc:      "feature on | blob sha",
-			revision:  blobSha,
-			featureOn: true,
-			errStr:    "object not found",
+			desc:     "blob sha",
+			revision: blobSha,
+			errStr:   "object not found",
 		},
 	}
 
@@ -164,11 +142,7 @@ func TestGetCommitCatfile(t *testing.T) {
 	require.NoError(t, err)
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			if tc.featureOn {
-				ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.CommitWithoutBatchCheck)
-			}
-
-			c, err := GetCommitCatfile(ctx, c, tc.revision)
+			c, err := GetCommitCatfile(c, tc.revision)
 
 			if tc.errStr == "" {
 				require.NoError(t, err)
diff --git a/internal/git/log/last_commit.go b/internal/git/log/last_commit.go
index 763033f30..76b3c9915 100644
--- a/internal/git/log/last_commit.go
+++ b/internal/git/log/last_commit.go
@@ -27,7 +27,7 @@ func LastCommitForPath(ctx context.Context, batch *catfile.Batch, repo *gitalypb
 		return nil, err
 	}
 
-	return GetCommitCatfile(ctx, batch, text.ChompBytes(commitID))
+	return GetCommitCatfile(batch, text.ChompBytes(commitID))
 }
 
 // GitLogCommand returns a Command that executes git log with the given the arguments
diff --git a/internal/git/log/log.go b/internal/git/log/log.go
index e6de9c37f..c591f36bf 100644
--- a/internal/git/log/log.go
+++ b/internal/git/log/log.go
@@ -19,7 +19,6 @@ type Parser struct {
 	currentCommit *gitalypb.GitCommit
 	err           error
 	c             *catfile.Batch
-	ctx           context.Context
 }
 
 // NewLogParser returns a new Parser
@@ -32,7 +31,6 @@ func NewLogParser(ctx context.Context, repo *gitalypb.Repository, src io.Reader)
 	parser := &Parser{
 		scanner: bufio.NewScanner(src),
 		c:       c,
-		ctx:     ctx,
 	}
 
 	return parser, nil
@@ -48,7 +46,7 @@ func (parser *Parser) Parse() bool {
 
 	commitID := parser.scanner.Text()
 
-	commit, err := GetCommitCatfile(parser.ctx, parser.c, commitID)
+	commit, err := GetCommitCatfile(parser.c, commitID)
 	if err != nil {
 		parser.err = err
 		return false
diff --git a/internal/git/log/tag.go b/internal/git/log/tag.go
index ae82afd74..1f251fa3a 100644
--- a/internal/git/log/tag.go
+++ b/internal/git/log/tag.go
@@ -3,7 +3,6 @@ package log
 import (
 	"bufio"
 	"bytes"
-	"context"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -24,19 +23,19 @@ const (
 // When 'trimRightNewLine' is 'true', the tag message will be trimmed to remove all '\n' characters from right.
 // note: we pass in the tagName because the tag name from refs/tags may be different
 // than the name found in the actual tag object. We want to use the tagName found in refs/tags
-func GetTagCatfile(ctx context.Context, c *catfile.Batch, tagID, tagName string, trimLen, trimRightNewLine bool) (*gitalypb.Tag, error) {
-	r, err := c.Tag(tagID)
+func GetTagCatfile(c *catfile.Batch, tagID, tagName string, trimLen, trimRightNewLine bool) (*gitalypb.Tag, error) {
+	tagObj, err := c.Tag(tagID)
 	if err != nil {
 		return nil, err
 	}
 
-	header, body, err := splitRawTag(r, trimRightNewLine)
+	header, body, err := splitRawTag(tagObj.Reader, trimRightNewLine)
 	if err != nil {
 		return nil, err
 	}
 
 	// the tagID is the oid of the tag object
-	tag, err := buildAnnotatedTag(ctx, c, tagID, tagName, header, body, trimLen, trimRightNewLine)
+	tag, err := buildAnnotatedTag(c, tagID, tagName, header, body, trimLen, trimRightNewLine)
 	if err != nil {
 		return nil, err
 	}
@@ -93,7 +92,7 @@ func splitRawTag(r io.Reader, trimRightNewLine bool) (*tagHeader, []byte, error)
 	return &header, body, nil
 }
 
-func buildAnnotatedTag(ctx context.Context, b *catfile.Batch, tagID, name string, header *tagHeader, body []byte, trimLen, trimRightNewLine bool) (*gitalypb.Tag, error) {
+func buildAnnotatedTag(b *catfile.Batch, tagID, name string, header *tagHeader, body []byte, trimLen, trimRightNewLine bool) (*gitalypb.Tag, error) {
 	tag := &gitalypb.Tag{
 		Id:          tagID,
 		Name:        []byte(name),
@@ -108,13 +107,13 @@ func buildAnnotatedTag(ctx context.Context, b *catfile.Batch, tagID, name string
 	var err error
 	switch header.tagType {
 	case "commit":
-		tag.TargetCommit, err = GetCommitCatfile(ctx, b, header.oid)
+		tag.TargetCommit, err = GetCommitCatfile(b, header.oid)
 		if err != nil {
 			return nil, fmt.Errorf("buildAnnotatedTag error when getting target commit: %v", err)
 		}
 
 	case "tag":
-		tag.TargetCommit, err = dereferenceTag(ctx, b, header.oid)
+		tag.TargetCommit, err = dereferenceTag(b, header.oid)
 		if err != nil {
 			return nil, fmt.Errorf("buildAnnotatedTag error when dereferencing tag: %v", err)
 		}
@@ -138,7 +137,7 @@ func buildAnnotatedTag(ctx context.Context, b *catfile.Batch, tagID, name string
 // we also protect against circular tag references. Even though this is not possible in git,
 // we still want to protect against an infinite looop
 
-func dereferenceTag(ctx context.Context, b *catfile.Batch, Oid string) (*gitalypb.GitCommit, error) {
+func dereferenceTag(b *catfile.Batch, Oid string) (*gitalypb.GitCommit, error) {
 	for depth := 0; depth < MaxTagReferenceDepth; depth++ {
 		i, err := b.Info(Oid)
 		if err != nil {
@@ -147,12 +146,12 @@ func dereferenceTag(ctx context.Context, b *catfile.Batch, Oid string) (*gitalyp
 
 		switch i.Type {
 		case "tag":
-			r, err := b.Tag(Oid)
+			tagObj, err := b.Tag(Oid)
 			if err != nil {
 				return nil, err
 			}
 
-			header, _, err := splitRawTag(r, true)
+			header, _, err := splitRawTag(tagObj.Reader, true)
 			if err != nil {
 				return nil, err
 			}
@@ -160,7 +159,7 @@ func dereferenceTag(ctx context.Context, b *catfile.Batch, Oid string) (*gitalyp
 			Oid = header.oid
 			continue
 		case "commit":
-			return GetCommitCatfile(ctx, b, Oid)
+			return GetCommitCatfile(b, Oid)
 		default: // This current tag points to a tree or a blob
 			return nil, nil
 		}
diff --git a/internal/git/log/tag_test.go b/internal/git/log/tag_test.go
index ab975ee25..2aadb8511 100644
--- a/internal/git/log/tag_test.go
+++ b/internal/git/log/tag_test.go
@@ -52,7 +52,7 @@ func TestGetTag(t *testing.T) {
 		t.Run(testCase.tagName, func(t *testing.T) {
 			tagID := testhelper.CreateTag(t, testRepoPath, testCase.tagName, testCase.rev, &testhelper.CreateTagOpts{Message: testCase.message})
 
-			tag, err := GetTagCatfile(ctx, c, tagID, testCase.tagName, testCase.trim, true)
+			tag, err := GetTagCatfile(c, tagID, testCase.tagName, testCase.trim, true)
 			require.NoError(t, err)
 			if testCase.trim && len(testCase.message) >= helper.MaxCommitOrTagMessageSize {
 				testCase.message = testCase.message[:helper.MaxCommitOrTagMessageSize]
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index c3ad0093a..c4e3540e7 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -12,10 +12,6 @@ const (
 	// annotations (i.e. accessor and mutator RPC's). This enables cache
 	// invalidation by changing state when the repo is modified.
 	CacheInvalidator = "cache_invalidator"
-	// CommitWithoutBatchCheck controls which implementation of the GetCommitCatfile needs to be used.
-	// The old one with query fot Info before fetching info about Commit
-	// or the new one that skips Info call and checks object type in Commit method call.
-	CommitWithoutBatchCheck = "commit_without_batch_check"
 	// UseGitProtocolV2 enables support for git wire protocol v2
 	UseGitProtocolV2 = "use_git_protocol_v2"
 )
diff --git a/internal/service/blob/get_blob.go b/internal/service/blob/get_blob.go
index fac456a5e..53f5c3895 100644
--- a/internal/service/blob/get_blob.go
+++ b/internal/service/blob/get_blob.go
@@ -43,7 +43,7 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic
 		return helper.DecorateError(codes.Unavailable, stream.Send(firstMessage))
 	}
 
-	blobReader, err := c.Blob(objectInfo.Oid)
+	blobObj, err := c.Blob(objectInfo.Oid)
 	if err != nil {
 		return status.Errorf(codes.Internal, "GetBlob: %v", err)
 	}
@@ -58,7 +58,7 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic
 		return stream.Send(msg)
 	})
 
-	_, err = io.CopyN(sw, blobReader, readLimit)
+	_, err = io.CopyN(sw, blobObj.Reader, readLimit)
 	if err != nil {
 		return status.Errorf(codes.Unavailable, "GetBlob: send: %v", err)
 	}
diff --git a/internal/service/blob/get_blobs.go b/internal/service/blob/get_blobs.go
index f2f645316..d676ab721 100644
--- a/internal/service/blob/get_blobs.go
+++ b/internal/service/blob/get_blobs.go
@@ -98,7 +98,7 @@ func sendBlobTreeEntry(response *gitalypb.GetBlobsResponse, stream gitalypb.Blob
 
 	// For correctness it does not matter, but for performance, the order is
 	// important: first check if readlimit == 0, if not, only then create
-	// blobReader.
+	// blobObj.
 	if readLimit == 0 {
 		if err := stream.Send(response); err != nil {
 			return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err)
@@ -106,7 +106,7 @@ func sendBlobTreeEntry(response *gitalypb.GetBlobsResponse, stream gitalypb.Blob
 		return nil
 	}
 
-	blobReader, err := c.Blob(response.Oid)
+	blobObj, err := c.Blob(response.Oid)
 	if err != nil {
 		return status.Errorf(codes.Internal, "GetBlobs: %v", err)
 	}
@@ -123,12 +123,12 @@ func sendBlobTreeEntry(response *gitalypb.GetBlobsResponse, stream gitalypb.Blob
 		return stream.Send(msg)
 	})
 
-	_, err = io.CopyN(sw, blobReader, readLimit)
+	_, err = io.CopyN(sw, blobObj.Reader, readLimit)
 	if err != nil {
 		return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err)
 	}
 
-	if _, err := io.Copy(ioutil.Discard, blobReader); err != nil {
+	if _, err := io.Copy(ioutil.Discard, blobObj.Reader); err != nil {
 		return status.Errorf(codes.Unavailable, "GetBlobs: discarding data: %v", err)
 	}
 
diff --git a/internal/service/commit/commit_signatures.go b/internal/service/commit/commit_signatures.go
index d28f7d27e..283860322 100644
--- a/internal/service/commit/commit_signatures.go
+++ b/internal/service/commit/commit_signatures.go
@@ -35,26 +35,15 @@ func getCommitSignatures(s *server, request *gitalypb.GetCommitSignaturesRequest
 	}
 
 	for _, commitID := range request.CommitIds {
-		info, err := c.Info(commitID)
-
+		commitObj, err := c.Commit(commitID)
 		if err != nil {
 			if catfile.IsNotFound(err) {
 				continue
-			} else {
-				return helper.ErrInternal(err)
 			}
-		}
-
-		if info.Type != "commit" {
-			return helper.ErrInternal(fmt.Errorf("%q is not a commit", commitID))
-		}
-
-		reader, err := c.Commit(commitID)
-		if err != nil {
 			return helper.ErrInternal(err)
 		}
 
-		signatureKey, commitText, err := extractSignature(reader)
+		signatureKey, commitText, err := extractSignature(commitObj.Reader)
 		if err != nil {
 			return helper.ErrInternal(err)
 		}
diff --git a/internal/service/commit/filter_shas_with_signatures.go b/internal/service/commit/filter_shas_with_signatures.go
index 6af3475d0..8cd2f24fc 100644
--- a/internal/service/commit/filter_shas_with_signatures.go
+++ b/internal/service/commit/filter_shas_with_signatures.go
@@ -1,7 +1,6 @@
 package commit
 
 import (
-	"context"
 	"errors"
 	"io"
 
@@ -43,7 +42,7 @@ func (s *server) filterShasWithSignatures(bidi gitalypb.CommitService_FilterShas
 
 	var request = firstRequest
 	for {
-		shas, err := filterCommitShasWithSignatures(ctx, c, request.GetShas())
+		shas, err := filterCommitShasWithSignatures(c, request.GetShas())
 		if err != nil {
 			return err
 		}
@@ -63,10 +62,10 @@ func (s *server) filterShasWithSignatures(bidi gitalypb.CommitService_FilterShas
 	}
 }
 
-func filterCommitShasWithSignatures(ctx context.Context, c *catfile.Batch, shas [][]byte) ([][]byte, error) {
+func filterCommitShasWithSignatures(c *catfile.Batch, shas [][]byte) ([][]byte, error) {
 	var foundShas [][]byte
 	for _, sha := range shas {
-		commit, err := log.GetCommitCatfile(ctx, c, string(sha))
+		commit, err := log.GetCommitCatfile(c, string(sha))
 		if catfile.IsNotFound(err) {
 			continue
 		}
diff --git a/internal/service/commit/find_commits.go b/internal/service/commit/find_commits.go
index e01dad522..8b852734b 100644
--- a/internal/service/commit/find_commits.go
+++ b/internal/service/commit/find_commits.go
@@ -59,7 +59,7 @@ func findCommits(ctx context.Context, req *gitalypb.FindCommitsRequest, stream g
 		return fmt.Errorf("creating catfile: %v", err)
 	}
 
-	getCommits := NewGetCommits(ctx, logCmd, batch)
+	getCommits := NewGetCommits(logCmd, batch)
 
 	if calculateOffsetManually(req) {
 		getCommits.Offset(int(req.GetOffset()))
@@ -79,15 +79,13 @@ func calculateOffsetManually(req *gitalypb.FindCommitsRequest) bool {
 type GetCommits struct {
 	scanner *bufio.Scanner
 	batch   *catfile.Batch
-	ctx     context.Context
 }
 
 // NewGetCommits returns a new GetCommits object
-func NewGetCommits(ctx context.Context, cmd *command.Command, batch *catfile.Batch) *GetCommits {
+func NewGetCommits(cmd *command.Command, batch *catfile.Batch) *GetCommits {
 	return &GetCommits{
 		scanner: bufio.NewScanner(cmd),
 		batch:   batch,
-		ctx:     ctx,
 	}
 }
 
@@ -114,7 +112,7 @@ func (g *GetCommits) Offset(offset int) error {
 // Commit returns the current commit
 func (g *GetCommits) Commit() (*gitalypb.GitCommit, error) {
 	revision := strings.TrimSpace(g.scanner.Text())
-	commit, err := log.GetCommitCatfile(g.ctx, g.batch, revision)
+	commit, err := log.GetCommitCatfile(g.batch, revision)
 	if err != nil {
 		return nil, fmt.Errorf("cat-file get commit %q: %v", revision, err)
 	}
diff --git a/internal/service/commit/list_commits_by_oid.go b/internal/service/commit/list_commits_by_oid.go
index caa08a90a..9d024566d 100644
--- a/internal/service/commit/list_commits_by_oid.go
+++ b/internal/service/commit/list_commits_by_oid.go
@@ -37,7 +37,7 @@ func (s *server) ListCommitsByOid(in *gitalypb.ListCommitsByOidRequest, stream g
 	listCommitsbyOidHistogram.Observe(float64(len(in.Oid)))
 
 	for _, oid := range in.Oid {
-		commit, err := gitlog.GetCommitCatfile(ctx, c, oid)
+		commit, err := gitlog.GetCommitCatfile(c, oid)
 		if catfile.IsNotFound(err) {
 			continue
 		}
diff --git a/internal/service/commit/list_commits_by_ref_name.go b/internal/service/commit/list_commits_by_ref_name.go
index 24379008b..baf281214 100644
--- a/internal/service/commit/list_commits_by_ref_name.go
+++ b/internal/service/commit/list_commits_by_ref_name.go
@@ -19,7 +19,7 @@ func (s *server) ListCommitsByRefName(in *gitalypb.ListCommitsByRefNameRequest,
 	sender := chunk.New(&commitsByRefNameSender{stream: stream})
 
 	for _, refName := range in.RefNames {
-		commit, err := gitlog.GetCommitCatfile(ctx, c, string(refName))
+		commit, err := gitlog.GetCommitCatfile(c, string(refName))
 		if catfile.IsNotFound(err) {
 			continue
 		}
diff --git a/internal/service/commit/tree_entries_helper.go b/internal/service/commit/tree_entries_helper.go
index 5c9a538ed..cb461ca32 100644
--- a/internal/service/commit/tree_entries_helper.go
+++ b/internal/service/commit/tree_entries_helper.go
@@ -58,8 +58,8 @@ const (
 	defaultFlatTreeRecursion = 10
 )
 
-func extractEntryInfoFromTreeData(treeData *bytes.Buffer, commitOid, rootOid, rootPath string, treeInfo *catfile.ObjectInfo) ([]*gitalypb.TreeEntry, error) {
-	if len(treeInfo.Oid) == 0 {
+func extractEntryInfoFromTreeData(treeData *bytes.Buffer, commitOid, rootOid, rootPath, oid string) ([]*gitalypb.TreeEntry, error) {
+	if len(oid) == 0 {
 		return nil, fmt.Errorf("empty tree oid")
 	}
 
@@ -118,30 +118,20 @@ func treeEntries(c *catfile.Batch, revision, path string, rootOid string, recurs
 		rootOid = rootTreeInfo.Oid
 	}
 
-	treeEntryInfo, err := c.Info(fmt.Sprintf("%s:%s", revision, path))
+	treeObj, err := c.Tree(fmt.Sprintf("%s:%s", revision, path))
 	if err != nil {
 		if catfile.IsNotFound(err) {
 			return nil, nil
 		}
-
-		return nil, err
-	}
-
-	if treeEntryInfo.Type != "tree" {
-		return nil, nil
-	}
-
-	treeReader, err := c.Tree(treeEntryInfo.Oid)
-	if err != nil {
 		return nil, err
 	}
 
 	treeBytes := &bytes.Buffer{}
-	if _, err := treeBytes.ReadFrom(treeReader); err != nil {
+	if _, err := treeBytes.ReadFrom(treeObj.Reader); err != nil {
 		return nil, err
 	}
 
-	entries, err := extractEntryInfoFromTreeData(treeBytes, revision, rootOid, path, treeEntryInfo)
+	entries, err := extractEntryInfoFromTreeData(treeBytes, revision, rootOid, path, treeObj.Oid)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/service/commit/tree_entry.go b/internal/service/commit/tree_entry.go
index b7e560d29..36a25c8da 100644
--- a/internal/service/commit/tree_entry.go
+++ b/internal/service/commit/tree_entry.go
@@ -80,7 +80,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c *catfile.Bat
 		return helper.DecorateError(codes.Unavailable, stream.Send(response))
 	}
 
-	blobReader, err := c.Blob(objectInfo.Oid)
+	blobObj, err := c.Blob(objectInfo.Oid)
 	if err != nil {
 		return err
 	}
@@ -98,7 +98,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c *catfile.Bat
 		return nil
 	})
 
-	_, err = io.CopyN(sw, blobReader, dataLength)
+	_, err = io.CopyN(sw, blobObj.Reader, dataLength)
 	return err
 }
 
diff --git a/internal/service/ref/list_new_commits.go b/internal/service/ref/list_new_commits.go
index e5533e42a..d465dca05 100644
--- a/internal/service/ref/list_new_commits.go
+++ b/internal/service/ref/list_new_commits.go
@@ -45,7 +45,7 @@ func listNewCommits(in *gitalypb.ListNewCommitsRequest, stream gitalypb.RefServi
 	for scanner.Scan() {
 		line := scanner.Text()
 
-		commit, err := log.GetCommitCatfile(ctx, batch, line)
+		commit, err := log.GetCommitCatfile(batch, line)
 		if err != nil {
 			return err
 		}
diff --git a/internal/service/ref/refs.go b/internal/service/ref/refs.go
index 0d1ee5de6..f7de0d344 100644
--- a/internal/service/ref/refs.go
+++ b/internal/service/ref/refs.go
@@ -100,7 +100,7 @@ func parseAndReturnTags(ctx context.Context, repo *gitalypb.Repository, stream g
 
 	scanner := bufio.NewScanner(tagsCmd)
 	for scanner.Scan() {
-		tag, err := parseTagLine(ctx, c, scanner.Text())
+		tag, err := parseTagLine(c, scanner.Text())
 		if err != nil {
 			return fmt.Errorf("parsing tag: %v", err)
 		}
@@ -360,7 +360,7 @@ func (s *server) FindTag(ctx context.Context, in *gitalypb.FindTagRequest) (*git
 }
 
 // parseTagLine parses a line of text with the output format %(objectname) %(objecttype) %(refname:lstrip=2)
-func parseTagLine(ctx context.Context, c *catfile.Batch, tagLine string) (*gitalypb.Tag, error) {
+func parseTagLine(c *catfile.Batch, tagLine string) (*gitalypb.Tag, error) {
 	fields := strings.SplitN(tagLine, " ", 3)
 	if len(fields) != 3 {
 		return nil, fmt.Errorf("invalid output from for-each-ref command: %v", tagLine)
@@ -376,13 +376,13 @@ func parseTagLine(ctx context.Context, c *catfile.Batch, tagLine string) (*gital
 	switch refType {
 	// annotated tag
 	case "tag":
-		tag, err := gitlog.GetTagCatfile(ctx, c, tagID, refName, true, true)
+		tag, err := gitlog.GetTagCatfile(c, tagID, refName, true, true)
 		if err != nil {
 			return nil, fmt.Errorf("getting annotated tag: %v", err)
 		}
 		return tag, nil
 	case "commit":
-		commit, err := gitlog.GetCommitCatfile(ctx, c, tagID)
+		commit, err := gitlog.GetCommitCatfile(c, tagID)
 		if err != nil {
 			return nil, fmt.Errorf("getting commit catfile: %v", err)
 		}
@@ -414,7 +414,7 @@ func findTag(ctx context.Context, repository *gitalypb.Repository, tagName []byt
 
 	scanner := bufio.NewScanner(tagCmd)
 	if scanner.Scan() {
-		tag, err = parseTagLine(ctx, c, scanner.Text())
+		tag, err = parseTagLine(c, scanner.Text())
 		if err != nil {
 			return nil, err
 		}
diff --git a/internal/service/ref/refs_test.go b/internal/service/ref/refs_test.go
index 38e63637b..247825ba1 100644
--- a/internal/service/ref/refs_test.go
+++ b/internal/service/ref/refs_test.go
@@ -744,7 +744,7 @@ func TestFindAllTagNestedTags(t *testing.T) {
 
 				// only expect the TargetCommit to be populated if it is a commit and if its less than 10 tags deep
 				if info.Type == "commit" && depth < log.MaxTagReferenceDepth {
-					commit, err := log.GetCommitCatfile(ctx, batch, tc.originalOid)
+					commit, err := log.GetCommitCatfile(batch, tc.originalOid)
 					require.NoError(t, err)
 					expectedTag.TargetCommit = commit
 				}
@@ -1665,7 +1665,7 @@ func TestFindTagNestedTag(t *testing.T) {
 			}
 			// only expect the TargetCommit to be populated if it is a commit and if its less than 10 tags deep
 			if info.Type == "commit" && tc.depth < log.MaxTagReferenceDepth {
-				commit, err := log.GetCommitCatfile(ctx, batch, tc.originalOid)
+				commit, err := log.GetCommitCatfile(batch, tc.originalOid)
 				require.NoError(t, err)
 				expectedTag.TargetCommit = commit
 			}
diff --git a/internal/service/ref/tag_messages.go b/internal/service/ref/tag_messages.go
index d4ec3e07e..e632dd7f8 100644
--- a/internal/service/ref/tag_messages.go
+++ b/internal/service/ref/tag_messages.go
@@ -42,7 +42,7 @@ func (s *server) getAndStreamTagMessages(request *gitalypb.GetTagMessagesRequest
 	}
 
 	for _, tagID := range request.GetTagIds() {
-		tag, err := log.GetTagCatfile(ctx, c, tagID, "", false, false)
+		tag, err := log.GetTagCatfile(c, tagID, "", false, false)
 		if err != nil {
 			return fmt.Errorf("failed to get tag: %v", err)
 		}
diff --git a/internal/service/ref/util.go b/internal/service/ref/util.go
index f648bc5e2..eecc0c49d 100644
--- a/internal/service/ref/util.go
+++ b/internal/service/ref/util.go
@@ -2,7 +2,6 @@ package ref
 
 import (
 	"bytes"
-	"context"
 	"fmt"
 
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
@@ -54,8 +53,8 @@ func buildLocalBranch(name []byte, target *gitalypb.GitCommit) *gitalypb.FindLoc
 	return response
 }
 
-func buildAllBranchesBranch(ctx context.Context, c *catfile.Batch, elements [][]byte) (*gitalypb.FindAllBranchesResponse_Branch, error) {
-	target, err := log.GetCommitCatfile(ctx, c, string(elements[1]))
+func buildAllBranchesBranch(c *catfile.Batch, elements [][]byte) (*gitalypb.FindAllBranchesResponse_Branch, error) {
+	target, err := log.GetCommitCatfile(c, string(elements[1]))
 	if err != nil {
 		return nil, err
 	}
@@ -66,8 +65,8 @@ func buildAllBranchesBranch(ctx context.Context, c *catfile.Batch, elements [][]
 	}, nil
 }
 
-func buildBranch(ctx context.Context, c *catfile.Batch, elements [][]byte) (*gitalypb.Branch, error) {
-	target, err := log.GetCommitCatfile(ctx, c, string(elements[1]))
+func buildBranch(c *catfile.Batch, elements [][]byte) (*gitalypb.Branch, error) {
+	target, err := log.GetCommitCatfile(c, string(elements[1]))
 	if err != nil {
 		return nil, err
 	}
@@ -79,7 +78,6 @@ func buildBranch(ctx context.Context, c *catfile.Batch, elements [][]byte) (*git
 }
 
 func newFindLocalBranchesWriter(stream gitalypb.RefService_FindLocalBranchesServer, c *catfile.Batch) lines.Sender {
-	ctx := stream.Context()
 	return func(refs [][]byte) error {
 		var branches []*gitalypb.FindLocalBranchResponse
 
@@ -89,7 +87,7 @@ func newFindLocalBranchesWriter(stream gitalypb.RefService_FindLocalBranchesServ
 				return err
 			}
 
-			target, err := log.GetCommitCatfile(ctx, c, string(elements[1]))
+			target, err := log.GetCommitCatfile(c, string(elements[1]))
 			if err != nil {
 				return err
 			}
@@ -101,7 +99,6 @@ func newFindLocalBranchesWriter(stream gitalypb.RefService_FindLocalBranchesServ
 }
 
 func newFindAllBranchesWriter(stream gitalypb.RefService_FindAllBranchesServer, c *catfile.Batch) lines.Sender {
-	ctx := stream.Context()
 	return func(refs [][]byte) error {
 		var branches []*gitalypb.FindAllBranchesResponse_Branch
 
@@ -110,7 +107,7 @@ func newFindAllBranchesWriter(stream gitalypb.RefService_FindAllBranchesServer,
 			if err != nil {
 				return err
 			}
-			branch, err := buildAllBranchesBranch(ctx, c, elements)
+			branch, err := buildAllBranchesBranch(c, elements)
 			if err != nil {
 				return err
 			}
@@ -121,7 +118,6 @@ func newFindAllBranchesWriter(stream gitalypb.RefService_FindAllBranchesServer,
 }
 
 func newFindAllRemoteBranchesWriter(stream gitalypb.RefService_FindAllRemoteBranchesServer, c *catfile.Batch) lines.Sender {
-	ctx := stream.Context()
 	return func(refs [][]byte) error {
 		var branches []*gitalypb.Branch
 
@@ -130,7 +126,7 @@ func newFindAllRemoteBranchesWriter(stream gitalypb.RefService_FindAllRemoteBran
 			if err != nil {
 				return err
 			}
-			branch, err := buildBranch(ctx, c, elements)
+			branch, err := buildBranch(c, elements)
 			if err != nil {
 				return err
 			}
diff --git a/internal/service/repository/apply_gitattributes.go b/internal/service/repository/apply_gitattributes.go
index c302ce170..2219b6da9 100644
--- a/internal/service/repository/apply_gitattributes.go
+++ b/internal/service/repository/apply_gitattributes.go
@@ -56,13 +56,13 @@ func applyGitattributes(c *catfile.Batch, repoPath string, revision []byte) erro
 	}
 	defer os.Remove(tempFile.Name())
 
-	blobReader, err := c.Blob(blobInfo.Oid)
+	blobObj, err := c.Blob(blobInfo.Oid)
 	if err != nil {
 		return err
 	}
 
 	// Write attributes to temp file
-	if _, err := io.CopyN(tempFile, blobReader, blobInfo.Size); err != nil {
+	if _, err := io.CopyN(tempFile, blobObj.Reader, blobInfo.Size); err != nil {
 		return err
 	}