Merge branch 'pks-go-port-get-lfs-pointers' into 'master'

blob: Port GetAllLFSPointers and GetLFSPointers to Go

See merge request gitlab-org/gitaly!3173

10 files changed, 731 insertions, 157 deletions

diff --git a/changelogs/unreleased/pks-go-port-get-lfs-pointers.yml b/changelogs/unreleased/pks-go-port-get-lfs-pointers.yml
new file mode 100644
index 000000000..609ac13fb
--- /dev/null
+++ b/changelogs/unreleased/pks-go-port-get-lfs-pointers.yml
@@ -0,0 +1,5 @@
+---
+title: 'blob: Port GetAllLFSPointers and GetLFSPointers to Go'
+merge_request: 3173
+author:
+type: performance
diff --git a/internal/gitaly/service/blob/lfs_pointers.go b/internal/gitaly/service/blob/lfs_pointers.go
index 12b502106..cbd0e0c6f 100644
--- a/internal/gitaly/service/blob/lfs_pointers.go
+++ b/internal/gitaly/service/blob/lfs_pointers.go
@@ -1,26 +1,38 @@
 package blob
 
 import (
+	"bufio"
+	"bytes"
+	"context"
+	"errors"
 	"fmt"
+	"io"
+	"io/ioutil"
+	"strings"
 
-	"gitlab.com/gitlab-org/gitaly/internal/errors"
+	gitaly_errors "gitlab.com/gitlab-org/gitaly/internal/errors"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+	"gitlab.com/gitlab-org/gitaly/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
 const (
-	// These limits are used as a heuristic to ignore files which can't be LFS
-	// pointers. The format of these is described in
-	// https://github.com/git-lfs/git-lfs/blob/master/docs/spec.md#the-pointer
-
-	// LfsPointerMinSize is the minimum size for an lfs pointer text blob
-	LfsPointerMinSize = 120
-	// LfsPointerMaxSize is the maximum size for an lfs pointer text blob
-	LfsPointerMaxSize = 200
+	// lfsPointerMaxSize is the maximum size for an lfs pointer text blob. This limit is used
+	// as a heuristic to filter blobs which can't be LFS pointers. The format of these pointers
+	// is described in https://github.com/git-lfs/git-lfs/blob/master/docs/spec.md#the-pointer.
+	lfsPointerMaxSize = 200
+
+	// lfsPointerSliceSize is the maximum number of LFSPointers to send at once.
+	lfsPointerSliceSize = 100
+)
+
+var (
+	errInvalidRevision = errors.New("invalid revision")
 )
 
 type getLFSPointerByRevisionRequest interface {
@@ -28,6 +40,9 @@ type getLFSPointerByRevisionRequest interface {
 	GetRevision() []byte
 }
 
+// GetLFSPointers takes the list of requested blob IDs and filters them down to blobs which are
+// valid LFS pointers. It is fine to pass blob IDs which do not point to a valid LFS pointer, but
+// passing blob IDs which do not exist results in an error.
 func (s *server) GetLFSPointers(req *gitalypb.GetLFSPointersRequest, stream gitalypb.BlobService_GetLFSPointersServer) error {
 	ctx := stream.Context()
 
@@ -35,6 +50,33 @@ func (s *server) GetLFSPointers(req *gitalypb.GetLFSPointersRequest, stream gita
 		return status.Errorf(codes.InvalidArgument, "GetLFSPointers: %v", err)
 	}
 
+	if featureflag.IsDisabled(ctx, featureflag.GoGetLFSPointers) {
+		return s.rubyGetLFSPointers(req, stream)
+	}
+
+	repo := localrepo.New(s.gitCmdFactory, req.Repository, s.cfg)
+	objectIDs := strings.Join(req.BlobIds, "\n")
+
+	lfsPointers, err := readLFSPointers(ctx, repo, s.gitCmdFactory, strings.NewReader(objectIDs), false)
+	if err != nil {
+		return err
+	}
+
+	err = sliceLFSPointers(lfsPointers, func(slice []*gitalypb.LFSPointer) error {
+		return stream.Send(&gitalypb.GetLFSPointersResponse{
+			LfsPointers: slice,
+		})
+	})
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (s *server) rubyGetLFSPointers(req *gitalypb.GetLFSPointersRequest, stream gitalypb.BlobService_GetLFSPointersServer) error {
+	ctx := stream.Context()
+
 	client, err := s.ruby.BlobServiceClient(ctx)
 	if err != nil {
 		return err
@@ -63,7 +105,7 @@ func (s *server) GetLFSPointers(req *gitalypb.GetLFSPointersRequest, stream gita
 
 func validateGetLFSPointersRequest(req *gitalypb.GetLFSPointersRequest) error {
 	if req.GetRepository() == nil {
-		return errors.ErrEmptyRepository
+		return gitaly_errors.ErrEmptyRepository
 	}
 
 	if len(req.GetBlobIds()) == 0 {
@@ -106,13 +148,54 @@ func (s *server) GetNewLFSPointers(in *gitalypb.GetNewLFSPointersRequest, stream
 	})
 }
 
+func validateGetLfsPointersByRevisionRequest(in getLFSPointerByRevisionRequest) error {
+	if in.GetRepository() == nil {
+		return fmt.Errorf("empty Repository")
+	}
+
+	return git.ValidateRevision(in.GetRevision())
+}
+
+// GetAllLFSPointers returns all LFS pointers of the git repository which are reachable by any git
+// reference. LFS pointers are streamed back in batches of lfsPointerSliceSize.
 func (s *server) GetAllLFSPointers(in *gitalypb.GetAllLFSPointersRequest, stream gitalypb.BlobService_GetAllLFSPointersServer) error {
 	ctx := stream.Context()
 
-	if in.GetRepository() == nil {
-		return helper.ErrInvalidArgument(fmt.Errorf("empty Repository"))
+	if err := validateGetAllLFSPointersRequest(in); err != nil {
+		return status.Errorf(codes.InvalidArgument, "GetAllLFSPointers: %v", err)
+	}
+
+	if featureflag.IsDisabled(ctx, featureflag.GoGetAllLFSPointers) {
+		return s.rubyGetAllLFSPointers(in, stream)
+	}
+
+	repo := localrepo.New(s.gitCmdFactory, in.Repository, s.cfg)
+
+	lfsPointers, err := findLFSPointersByRevisions(ctx, repo, s.gitCmdFactory, []git.Option{
+		git.Flag{Name: "--all"},
+	})
+	if err != nil {
+		if errors.Is(err, errInvalidRevision) {
+			return status.Errorf(codes.InvalidArgument, err.Error())
+		}
+		return err
+	}
+
+	err = sliceLFSPointers(lfsPointers, func(slice []*gitalypb.LFSPointer) error {
+		return stream.Send(&gitalypb.GetAllLFSPointersResponse{
+			LfsPointers: slice,
+		})
+	})
+	if err != nil {
+		return err
 	}
 
+	return nil
+}
+
+func (s *server) rubyGetAllLFSPointers(in *gitalypb.GetAllLFSPointersRequest, stream gitalypb.BlobService_GetAllLFSPointersServer) error {
+	ctx := stream.Context()
+
 	client, err := s.ruby.BlobServiceClient(ctx)
 	if err != nil {
 		return err
@@ -139,10 +222,162 @@ func (s *server) GetAllLFSPointers(in *gitalypb.GetAllLFSPointersRequest, stream
 	})
 }
 
-func validateGetLfsPointersByRevisionRequest(in getLFSPointerByRevisionRequest) error {
+func validateGetAllLFSPointersRequest(in *gitalypb.GetAllLFSPointersRequest) error {
 	if in.GetRepository() == nil {
 		return fmt.Errorf("empty Repository")
 	}
+	return nil
+}
 
-	return git.ValidateRevision(in.GetRevision())
+// findLFSPointersByRevisions will return all LFS objects reachable via the given set of revisions.
+// Revisions accept all syntax supported by git-rev-list(1). This function also accepts a set of
+// options accepted by git-rev-list(1). Note that because git.Commands do not accept dashed
+// positional arguments, it is currently not possible to mix options and revisions (e.g. "git
+// rev-list master --not feature").
+func findLFSPointersByRevisions(
+	ctx context.Context,
+	repo *localrepo.Repo,
+	gitCmdFactory git.CommandFactory,
+	opts []git.Option,
+	revisions ...string,
+) (_ []*gitalypb.LFSPointer, returnErr error) {
+	for _, revision := range revisions {
+		if strings.HasPrefix(revision, "-") && revision != "--all" && revision != "--not" {
+			return nil, fmt.Errorf("%w: %q", errInvalidRevision, revision)
+		}
+	}
+
+	// git-rev-list(1) currently does not have any way to list all reachable objects of a
+	// certain type. As an optimization, we thus call it with `--object-names`, which will
+	// print an associated name for a given revision, if there is any. This allows us to skip
+	// at least some objects, namely all commits.
+	//
+	// It is questionable whether this optimization helps at all: by including object names,
+	// git cannot make use of bitmap indices. We thus pessimize git-rev-list(1) and optimize
+	// git-cat-file(1). And given that there's going to be a lot more blobs and trees (which
+	// both _do_ have an object name) than commits, it's probably an optimization which even
+	// slows down execution. Still, we keep this to stay compatible with the Ruby
+	// implementation.
+	var revListStderr bytes.Buffer
+	revlist, err := repo.Exec(ctx, nil, git.SubCmd{
+		Name: "rev-list",
+		Flags: append([]git.Option{
+			git.Flag{Name: "--in-commit-order"},
+			git.Flag{Name: "--objects"},
+			git.Flag{Name: "--object-names"},
+			git.Flag{Name: fmt.Sprintf("--filter=blob:limit=%d", lfsPointerMaxSize)},
+		}, opts...),
+		Args: revisions,
+	}, git.WithStderr(&revListStderr))
+	if err != nil {
+		return nil, fmt.Errorf("could not execute rev-list: %w", err)
+	}
+	defer func() {
+		if err := revlist.Wait(); err != nil && returnErr == nil {
+			returnErr = fmt.Errorf("rev-list failed: %w, stderr: %q",
+				err, revListStderr.String())
+		}
+	}()
+
+	return readLFSPointers(ctx, repo, gitCmdFactory, revlist, true)
+}
+
+// readLFSPointers reads object IDs of potential LFS pointers from the given reader and for each of
+// them, it will determine whether the referenced object is an LFS pointer. Objects which are not a
+// valid LFS pointer will be ignored. Objects which do not exist result in an error.
+//
+// If filterByObjectName is set to true, only IDs which have an associated object name will be
+// read. This is helpful to pass output of git-rev-list(1) with `--object-names` directly to this
+// function.
+func readLFSPointers(
+	ctx context.Context,
+	repo *localrepo.Repo,
+	gitCmdFactory git.CommandFactory,
+	objectIDReader io.Reader,
+	filterByObjectName bool,
+) ([]*gitalypb.LFSPointer, error) {
+	catfileBatch, err := catfile.New(ctx, gitCmdFactory, repo)
+	if err != nil {
+		return nil, fmt.Errorf("could not execute cat-file: %w", err)
+	}
+
+	var lfsPointers []*gitalypb.LFSPointer
+	scanner := bufio.NewScanner(objectIDReader)
+
+	for scanner.Scan() {
+		revision := git.Revision(scanner.Bytes())
+		if filterByObjectName {
+			revAndPath := strings.SplitN(revision.String(), " ", 2)
+			if len(revAndPath) != 2 {
+				continue
+			}
+			revision = git.Revision(revAndPath[0])
+		}
+
+		objectInfo, err := catfileBatch.Info(ctx, revision)
+		if err != nil {
+			return nil, fmt.Errorf("could not get LFS pointer info: %w", err)
+		}
+
+		if objectInfo.Type != "blob" {
+			continue
+		}
+
+		blob, err := catfileBatch.Blob(ctx, revision)
+		if err != nil {
+			return nil, fmt.Errorf("could not retrieve LFS pointer: %w", err)
+		}
+
+		data, err := ioutil.ReadAll(blob.Reader)
+		if err != nil {
+			return nil, fmt.Errorf("could not read LFS pointer: %w", err)
+		}
+
+		if !isLFSPointer(data) {
+			continue
+		}
+
+		lfsPointers = append(lfsPointers, &gitalypb.LFSPointer{
+			Data: data,
+			Size: int64(len(data)),
+			Oid:  revision.String(),
+		})
+	}
+
+	if err := scanner.Err(); err != nil {
+		return nil, fmt.Errorf("scanning LFS pointers failed: %w", err)
+	}
+
+	return lfsPointers, nil
+}
+
+// isLFSPointer determines whether the given blob contents are an LFS pointer or not.
+func isLFSPointer(data []byte) bool {
+	// TODO: this is incomplete as it does not recognize pre-release version of LFS blobs with
+	// the "https://hawser.github.com/spec/v1" version. For compatibility with the Ruby RPC, we
+	// leave this as-is for now though.
+	return bytes.HasPrefix(data, []byte("version https://git-lfs.github.com/spec"))
+}
+
+// sliceLFSPointers slices the given pointers into subsets of slices with at most
+// lfsPointerSliceSize many pointers and executes the given fallback function. If the callback
+// returns an error, slicing is aborted and the error is returned verbosely.
+func sliceLFSPointers(pointers []*gitalypb.LFSPointer, fn func([]*gitalypb.LFSPointer) error) error {
+	chunkSize := lfsPointerSliceSize
+
+	for {
+		if len(pointers) == 0 {
+			return nil
+		}
+
+		if len(pointers) < chunkSize {
+			chunkSize = len(pointers)
+		}
+
+		if err := fn(pointers[:chunkSize]); err != nil {
+			return err
+		}
+
+		pointers = pointers[chunkSize:]
+	}
 }
diff --git a/internal/gitaly/service/blob/lfs_pointers_test.go b/internal/gitaly/service/blob/lfs_pointers_test.go
index ddceb9cc8..612153121 100644
--- a/internal/gitaly/service/blob/lfs_pointers_test.go
+++ b/internal/gitaly/service/blob/lfs_pointers_test.go
@@ -1,19 +1,75 @@
 package blob
 
 import (
+	"context"
+	"errors"
+	"fmt"
 	"io"
 	"os/exec"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	"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/codes"
 )
 
+const (
+	lfsPointer1 = "0c304a93cb8430108629bbbcaa27db3343299bc0"
+	lfsPointer2 = "f78df813119a79bfbe0442ab92540a61d3ab7ff3"
+	lfsPointer3 = "bab31d249f78fba464d1b75799aad496cc07fa3b"
+	lfsPointer4 = "125fcc9f6e33175cb278b9b2809154d2535fe19f"
+	lfsPointer5 = "0360724a0d64498331888f1eaef2d24243809230"
+	lfsPointer6 = "ff0ab3afd1616ff78d0331865d922df103b64cf0"
+)
+
+var (
+	lfsPointers = map[string]*gitalypb.LFSPointer{
+		lfsPointer1: &gitalypb.LFSPointer{
+			Size: 133,
+			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897\nsize 1575078\n\n"),
+			Oid:  lfsPointer1,
+		},
+		lfsPointer2: &gitalypb.LFSPointer{
+			Size: 127,
+			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:f2b0a1e7550e9b718dafc9b525a04879a766de62e4fbdfc46593d47f7ab74636\nsize 20\n"),
+			Oid:  lfsPointer2,
+		},
+		lfsPointer3: &gitalypb.LFSPointer{
+			Size: 127,
+			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:bad71f905b60729f502ca339f7c9f001281a3d12c68a5da7f15de8009f4bd63d\nsize 18\n"),
+			Oid:  lfsPointer3,
+		},
+		lfsPointer4: &gitalypb.LFSPointer{
+			Size: 129,
+			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:47997ea7ecff33be61e3ca1cc287ee72a2125161518f1a169f2893a5a82e9d95\nsize 7501\n"),
+			Oid:  lfsPointer4,
+		},
+		lfsPointer5: &gitalypb.LFSPointer{
+			Size: 129,
+			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:8c1e8de917525f83104736f6c64d32f0e2a02f5bf2ee57843a54f222cba8c813\nsize 2797\n"),
+			Oid:  lfsPointer5,
+		},
+		lfsPointer6: &gitalypb.LFSPointer{
+			Size: 132,
+			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:96f74c6fe7a2979eefb9ec74a5dfc6888fb25543cf99b77586b79afea1da6f97\nsize 1219696\n"),
+			Oid:  lfsPointer6,
+		},
+	}
+)
+
 func TestSuccessfulGetLFSPointersRequest(t *testing.T) {
+	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
+		featureflag.GoGetLFSPointers,
+	}).Run(t, testSuccessfulGetLFSPointersRequest)
+}
+
+func testSuccessfulGetLFSPointersRequest(t *testing.T, ctx context.Context) {
 	stop, serverSocketPath := runBlobServer(t, testhelper.DefaultLocator())
 	defer stop()
 
@@ -23,13 +79,10 @@ func TestSuccessfulGetLFSPointersRequest(t *testing.T) {
 	client, conn := newBlobClient(t, serverSocketPath)
 	defer conn.Close()
 
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
 	lfsPointerIds := []string{
-		"0c304a93cb8430108629bbbcaa27db3343299bc0",
-		"f78df813119a79bfbe0442ab92540a61d3ab7ff3",
-		"bab31d249f78fba464d1b75799aad496cc07fa3b",
+		lfsPointer1,
+		lfsPointer2,
+		lfsPointer3,
 	}
 	otherObjectIds := []string{
 		"d5b560e9c17384cf8257347db63167b54e0c97ff", // tree
@@ -37,21 +90,9 @@ func TestSuccessfulGetLFSPointersRequest(t *testing.T) {
 	}
 
 	expectedLFSPointers := []*gitalypb.LFSPointer{
-		{
-			Size: 133,
-			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897\nsize 1575078\n\n"),
-			Oid:  "0c304a93cb8430108629bbbcaa27db3343299bc0",
-		},
-		{
-			Size: 127,
-			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:f2b0a1e7550e9b718dafc9b525a04879a766de62e4fbdfc46593d47f7ab74636\nsize 20\n"),
-			Oid:  "f78df813119a79bfbe0442ab92540a61d3ab7ff3",
-		},
-		{
-			Size: 127,
-			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:bad71f905b60729f502ca339f7c9f001281a3d12c68a5da7f15de8009f4bd63d\nsize 18\n"),
-			Oid:  "bab31d249f78fba464d1b75799aad496cc07fa3b",
-		},
+		lfsPointers[lfsPointer1],
+		lfsPointers[lfsPointer2],
+		lfsPointers[lfsPointer3],
 	}
 
 	request := &gitalypb.GetLFSPointersRequest{
@@ -78,6 +119,12 @@ func TestSuccessfulGetLFSPointersRequest(t *testing.T) {
 }
 
 func TestFailedGetLFSPointersRequestDueToValidations(t *testing.T) {
+	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
+		featureflag.GoGetLFSPointers,
+	}).Run(t, testFailedGetLFSPointersRequestDueToValidations)
+}
+
+func testFailedGetLFSPointersRequestDueToValidations(t *testing.T, ctx context.Context) {
 	stop, serverSocketPath := runBlobServer(t, testhelper.DefaultLocator())
 	defer stop()
 
@@ -112,9 +159,6 @@ func TestFailedGetLFSPointersRequestDueToValidations(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.desc, func(t *testing.T) {
-			ctx, cancel := testhelper.Context()
-			defer cancel()
-
 			stream, err := client.GetLFSPointers(ctx, testCase.request)
 			require.NoError(t, err)
 
@@ -161,21 +205,9 @@ func TestSuccessfulGetNewLFSPointersRequest(t *testing.T) {
 				Revision:   revision,
 			},
 			expectedLFSPointers: []*gitalypb.LFSPointer{
-				{
-					Size: 133,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897\nsize 1575078\n\n"),
-					Oid:  "0c304a93cb8430108629bbbcaa27db3343299bc0",
-				},
-				{
-					Size: 127,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:f2b0a1e7550e9b718dafc9b525a04879a766de62e4fbdfc46593d47f7ab74636\nsize 20\n"),
-					Oid:  "f78df813119a79bfbe0442ab92540a61d3ab7ff3",
-				},
-				{
-					Size: 127,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:bad71f905b60729f502ca339f7c9f001281a3d12c68a5da7f15de8009f4bd63d\nsize 18\n"),
-					Oid:  "bab31d249f78fba464d1b75799aad496cc07fa3b",
-				},
+				lfsPointers[lfsPointer1],
+				lfsPointers[lfsPointer2],
+				lfsPointers[lfsPointer3],
 			},
 		},
 		{
@@ -185,21 +217,9 @@ func TestSuccessfulGetNewLFSPointersRequest(t *testing.T) {
 				Revision:   altDirsCommit,
 			},
 			expectedLFSPointers: []*gitalypb.LFSPointer{
-				{
-					Size: 133,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897\nsize 1575078\n\n"),
-					Oid:  "0c304a93cb8430108629bbbcaa27db3343299bc0",
-				},
-				{
-					Size: 127,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:f2b0a1e7550e9b718dafc9b525a04879a766de62e4fbdfc46593d47f7ab74636\nsize 20\n"),
-					Oid:  "f78df813119a79bfbe0442ab92540a61d3ab7ff3",
-				},
-				{
-					Size: 127,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:bad71f905b60729f502ca339f7c9f001281a3d12c68a5da7f15de8009f4bd63d\nsize 18\n"),
-					Oid:  "bab31d249f78fba464d1b75799aad496cc07fa3b",
-				},
+				lfsPointers[lfsPointer1],
+				lfsPointers[lfsPointer2],
+				lfsPointers[lfsPointer3],
 			},
 		},
 		{
@@ -235,16 +255,8 @@ func TestSuccessfulGetNewLFSPointersRequest(t *testing.T) {
 				Limit:      2,
 			},
 			expectedLFSPointers: []*gitalypb.LFSPointer{
-				{
-					Size: 127,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:bad71f905b60729f502ca339f7c9f001281a3d12c68a5da7f15de8009f4bd63d\nsize 18\n"),
-					Oid:  "bab31d249f78fba464d1b75799aad496cc07fa3b",
-				},
-				{
-					Size: 127,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:f2b0a1e7550e9b718dafc9b525a04879a766de62e4fbdfc46593d47f7ab74636\nsize 20\n"),
-					Oid:  "f78df813119a79bfbe0442ab92540a61d3ab7ff3",
-				},
+				lfsPointers[lfsPointer3],
+				lfsPointers[lfsPointer2],
 			},
 		},
 		{
@@ -255,11 +267,7 @@ func TestSuccessfulGetNewLFSPointersRequest(t *testing.T) {
 				NotInAll:   true,
 			},
 			expectedLFSPointers: []*gitalypb.LFSPointer{
-				{
-					Size: 133,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897\nsize 1575078\n\n"),
-					Oid:  "0c304a93cb8430108629bbbcaa27db3343299bc0",
-				},
+				lfsPointers[lfsPointer1],
 			},
 		},
 		{
@@ -270,16 +278,8 @@ func TestSuccessfulGetNewLFSPointersRequest(t *testing.T) {
 				NotInRefs:  [][]byte{[]byte("048721d90c449b244b7b4c53a9186b04330174ec")},
 			},
 			expectedLFSPointers: []*gitalypb.LFSPointer{
-				{
-					Size: 127,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:bad71f905b60729f502ca339f7c9f001281a3d12c68a5da7f15de8009f4bd63d\nsize 18\n"),
-					Oid:  "bab31d249f78fba464d1b75799aad496cc07fa3b",
-				},
-				{
-					Size: 127,
-					Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:f2b0a1e7550e9b718dafc9b525a04879a766de62e4fbdfc46593d47f7ab74636\nsize 20\n"),
-					Oid:  "f78df813119a79bfbe0442ab92540a61d3ab7ff3",
-				},
+				lfsPointers[lfsPointer3],
+				lfsPointers[lfsPointer2],
 			},
 		},
 	}
@@ -372,6 +372,12 @@ func drainNewPointers(c gitalypb.BlobService_GetNewLFSPointersClient) error {
 }
 
 func TestSuccessfulGetAllLFSPointersRequest(t *testing.T) {
+	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
+		featureflag.GoGetAllLFSPointers,
+	}).Run(t, testSuccessfulGetAllLFSPointersRequest)
+}
+
+func testSuccessfulGetAllLFSPointersRequest(t *testing.T, ctx context.Context) {
 	stop, serverSocketPath := runBlobServer(t, testhelper.DefaultLocator())
 	defer stop()
 
@@ -381,44 +387,17 @@ func TestSuccessfulGetAllLFSPointersRequest(t *testing.T) {
 	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
 	defer cleanupFn()
 
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
 	request := &gitalypb.GetAllLFSPointersRequest{
 		Repository: testRepo,
 	}
 
 	expectedLFSPointers := []*gitalypb.LFSPointer{
-		{
-			Size: 133,
-			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897\nsize 1575078\n\n"),
-			Oid:  "0c304a93cb8430108629bbbcaa27db3343299bc0",
-		},
-		{
-			Size: 127,
-			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:f2b0a1e7550e9b718dafc9b525a04879a766de62e4fbdfc46593d47f7ab74636\nsize 20\n"),
-			Oid:  "f78df813119a79bfbe0442ab92540a61d3ab7ff3",
-		},
-		{
-			Size: 127,
-			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:bad71f905b60729f502ca339f7c9f001281a3d12c68a5da7f15de8009f4bd63d\nsize 18\n"),
-			Oid:  "bab31d249f78fba464d1b75799aad496cc07fa3b",
-		},
-		{
-			Size: 132,
-			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:96f74c6fe7a2979eefb9ec74a5dfc6888fb25543cf99b77586b79afea1da6f97\nsize 1219696\n"),
-			Oid:  "ff0ab3afd1616ff78d0331865d922df103b64cf0",
-		},
-		{
-			Size: 129,
-			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:8c1e8de917525f83104736f6c64d32f0e2a02f5bf2ee57843a54f222cba8c813\nsize 2797\n"),
-			Oid:  "0360724a0d64498331888f1eaef2d24243809230",
-		},
-		{
-			Size: 129,
-			Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:47997ea7ecff33be61e3ca1cc287ee72a2125161518f1a169f2893a5a82e9d95\nsize 7501\n"),
-			Oid:  "125fcc9f6e33175cb278b9b2809154d2535fe19f",
-		},
+		lfsPointers[lfsPointer1],
+		lfsPointers[lfsPointer2],
+		lfsPointers[lfsPointer3],
+		lfsPointers[lfsPointer4],
+		lfsPointers[lfsPointer5],
+		lfsPointers[lfsPointer6],
 	}
 
 	c, err := client.GetAllLFSPointers(ctx, request)
@@ -443,15 +422,18 @@ func getAllPointers(t *testing.T, c gitalypb.BlobService_GetAllLFSPointersClient
 }
 
 func TestFailedGetAllLFSPointersRequestDueToValidations(t *testing.T) {
+	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
+		featureflag.GoGetAllLFSPointers,
+	}).Run(t, testFailedGetAllLFSPointersRequestDueToValidations)
+}
+
+func testFailedGetAllLFSPointersRequestDueToValidations(t *testing.T, ctx context.Context) {
 	stop, serverSocketPath := runBlobServer(t, testhelper.DefaultLocator())
 	defer stop()
 
 	client, conn := newBlobClient(t, serverSocketPath)
 	defer conn.Close()
 
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
 	testCases := []struct {
 		desc       string
 		repository *gitalypb.Repository
@@ -487,9 +469,15 @@ func drainAllPointers(c gitalypb.BlobService_GetAllLFSPointersClient) error {
 	}
 }
 
+func TestGetAllLFSPointersVerifyScope(t *testing.T) {
+	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
+		featureflag.GoGetAllLFSPointers,
+	}).Run(t, testGetAllLFSPointersVerifyScope)
+}
+
 // TestGetAllLFSPointersVerifyScope verifies that this RPC returns all LFS
 // pointers in a repository, not only ones reachable from the default branch
-func TestGetAllLFSPointersVerifyScope(t *testing.T) {
+func testGetAllLFSPointersVerifyScope(t *testing.T, ctx context.Context) {
 	stop, serverSocketPath := runBlobServer(t, testhelper.DefaultLocator())
 	defer stop()
 
@@ -499,9 +487,6 @@ func TestGetAllLFSPointersVerifyScope(t *testing.T) {
 	testRepo, repoPath, cleanupFn := testhelper.NewTestRepo(t)
 	defer cleanupFn()
 
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
 	request := &gitalypb.GetAllLFSPointersRequest{
 		Repository: testRepo,
 	}
@@ -509,11 +494,7 @@ func TestGetAllLFSPointersVerifyScope(t *testing.T) {
 	c, err := client.GetAllLFSPointers(ctx, request)
 	require.NoError(t, err)
 
-	lfsPtr := &gitalypb.LFSPointer{
-		Size: 127,
-		Data: []byte("version https://git-lfs.github.com/spec/v1\noid sha256:f2b0a1e7550e9b718dafc9b525a04879a766de62e4fbdfc46593d47f7ab74636\nsize 20\n"),
-		Oid:  "f78df813119a79bfbe0442ab92540a61d3ab7ff3",
-	}
+	lfsPtr := lfsPointers[lfsPointer2]
 
 	// the LFS pointer is reachable from a non-default branch:
 	require.True(t, refHasPtr(t, repoPath, "moar-lfs-ptrs", lfsPtr))
@@ -532,3 +513,254 @@ func refHasPtr(t *testing.T, repoPath, ref string, lfsPtr *gitalypb.LFSPointer)
 
 	return strings.Contains(objects, lfsPtr.Oid)
 }
+
+func TestFindLFSPointersByRevisions(t *testing.T) {
+	gitCmdFactory := git.NewExecCommandFactory(config.Config)
+
+	repoProto, _, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+	repo := localrepo.New(gitCmdFactory, repoProto, config.Config)
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	for _, tc := range []struct {
+		desc                string
+		opts                []git.Option
+		revs                []string
+		expectedErr         error
+		expectedLFSPointers []*gitalypb.LFSPointer
+	}{
+		{
+			desc: "--all",
+			opts: []git.Option{
+				git.Flag{Name: "--all"},
+			},
+			expectedLFSPointers: []*gitalypb.LFSPointer{
+				lfsPointers[lfsPointer1],
+				lfsPointers[lfsPointer2],
+				lfsPointers[lfsPointer3],
+				lfsPointers[lfsPointer4],
+				lfsPointers[lfsPointer5],
+				lfsPointers[lfsPointer6],
+			},
+		},
+		{
+			desc: "--not --all",
+			opts: []git.Option{
+				git.Flag{Name: "--not"},
+				git.Flag{Name: "--all"},
+			},
+		},
+		{
+			desc: "initial commit",
+			revs: []string{"1a0b36b3cdad1d2ee32457c102a8c0b7056fa863"},
+		},
+		{
+			desc: "master",
+			revs: []string{"master"},
+			expectedLFSPointers: []*gitalypb.LFSPointer{
+				lfsPointers[lfsPointer1],
+			},
+		},
+		{
+			desc: "multiple revisions",
+			revs: []string{"master", "moar-lfs-ptrs"},
+			expectedLFSPointers: []*gitalypb.LFSPointer{
+				lfsPointers[lfsPointer1],
+				lfsPointers[lfsPointer2],
+				lfsPointers[lfsPointer3],
+			},
+		},
+		{
+			desc:        "invalid dashed option",
+			revs:        []string{"master", "--foobar"},
+			expectedErr: fmt.Errorf("invalid revision: \"--foobar\""),
+		},
+		{
+			desc:        "invalid revision",
+			revs:        []string{"does-not-exist"},
+			expectedErr: fmt.Errorf("fatal: ambiguous argument 'does-not-exist'"),
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			actualLFSPointers, err := findLFSPointersByRevisions(
+				ctx, repo, gitCmdFactory, tc.opts, tc.revs...)
+			if tc.expectedErr == nil {
+				require.NoError(t, err)
+			} else {
+				require.Contains(t, err.Error(), tc.expectedErr.Error())
+			}
+			require.ElementsMatch(t, tc.expectedLFSPointers, actualLFSPointers)
+		})
+	}
+}
+
+func TestReadLFSPointers(t *testing.T) {
+	gitCmdFactory := git.NewExecCommandFactory(config.Config)
+
+	repoProto, _, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+	repo := localrepo.New(gitCmdFactory, repoProto, config.Config)
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	for _, tc := range []struct {
+		desc                string
+		input               string
+		filterByObjectName  bool
+		expectedErr         error
+		expectedLFSPointers []*gitalypb.LFSPointer
+	}{
+		{
+			desc:  "single object ID",
+			input: strings.Join([]string{lfsPointer1}, "\n"),
+			expectedLFSPointers: []*gitalypb.LFSPointer{
+				lfsPointers[lfsPointer1],
+			},
+		},
+		{
+			desc: "multiple object IDs",
+			input: strings.Join([]string{
+				lfsPointer1,
+				lfsPointer2,
+				lfsPointer3,
+				lfsPointer4,
+				lfsPointer5,
+				lfsPointer6,
+			}, "\n"),
+			expectedLFSPointers: []*gitalypb.LFSPointer{
+				lfsPointers[lfsPointer1],
+				lfsPointers[lfsPointer2],
+				lfsPointers[lfsPointer3],
+				lfsPointers[lfsPointer4],
+				lfsPointers[lfsPointer5],
+				lfsPointers[lfsPointer6],
+			},
+		},
+		{
+			desc: "multiple object IDs with name filter",
+			input: strings.Join([]string{
+				lfsPointer1,
+				lfsPointer2,
+				lfsPointer3 + " x",
+				lfsPointer4,
+				lfsPointer5 + " z",
+				lfsPointer6 + " a",
+			}, "\n"),
+			filterByObjectName: true,
+			expectedLFSPointers: []*gitalypb.LFSPointer{
+				lfsPointers[lfsPointer3],
+				lfsPointers[lfsPointer5],
+				lfsPointers[lfsPointer6],
+			},
+		},
+		{
+			desc: "non-pointer object",
+			input: strings.Join([]string{
+				"60ecb67744cb56576c30214ff52294f8ce2def98",
+			}, "\n"),
+		},
+		{
+			desc: "mixed objects",
+			input: strings.Join([]string{
+				"60ecb67744cb56576c30214ff52294f8ce2def98",
+				lfsPointer2,
+			}, "\n"),
+			expectedLFSPointers: []*gitalypb.LFSPointer{
+				lfsPointers[lfsPointer2],
+			},
+		},
+		{
+			desc: "missing object",
+			input: strings.Join([]string{
+				"0101010101010101010101010101010101010101",
+			}, "\n"),
+			expectedErr: errors.New("object not found"),
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			reader := strings.NewReader(tc.input)
+
+			actualLFSPointers, err := readLFSPointers(
+				ctx, repo, gitCmdFactory, reader, tc.filterByObjectName)
+			if tc.expectedErr == nil {
+				require.NoError(t, err)
+			} else {
+				require.Contains(t, err.Error(), tc.expectedErr.Error())
+			}
+			require.ElementsMatch(t, tc.expectedLFSPointers, actualLFSPointers)
+		})
+	}
+}
+
+func TestSliceLFSPointers(t *testing.T) {
+	generateSlice := func(n, offset int) []*gitalypb.LFSPointer {
+		slice := make([]*gitalypb.LFSPointer, n)
+		for i := 0; i < n; i++ {
+			slice[i] = &gitalypb.LFSPointer{
+				Size: int64(i + offset),
+			}
+		}
+		return slice
+	}
+
+	for _, tc := range []struct {
+		desc           string
+		err            error
+		lfsPointers    []*gitalypb.LFSPointer
+		expectedSlices [][]*gitalypb.LFSPointer
+	}{
+		{
+			desc: "empty",
+		},
+		{
+			desc:        "single slice",
+			lfsPointers: generateSlice(10, 0),
+			expectedSlices: [][]*gitalypb.LFSPointer{
+				generateSlice(10, 0),
+			},
+		},
+		{
+			desc:        "two slices",
+			lfsPointers: generateSlice(101, 0),
+			expectedSlices: [][]*gitalypb.LFSPointer{
+				generateSlice(100, 0),
+				generateSlice(1, 100),
+			},
+		},
+		{
+			desc:        "many slices",
+			lfsPointers: generateSlice(635, 0),
+			expectedSlices: [][]*gitalypb.LFSPointer{
+				generateSlice(100, 0),
+				generateSlice(100, 100),
+				generateSlice(100, 200),
+				generateSlice(100, 300),
+				generateSlice(100, 400),
+				generateSlice(100, 500),
+				generateSlice(35, 600),
+			},
+		},
+		{
+			desc:        "error",
+			lfsPointers: generateSlice(500, 0),
+			err:         errors.New("foo"),
+			expectedSlices: [][]*gitalypb.LFSPointer{
+				generateSlice(100, 0),
+			},
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			var slices [][]*gitalypb.LFSPointer
+
+			err := sliceLFSPointers(tc.lfsPointers, func(slice []*gitalypb.LFSPointer) error {
+				slices = append(slices, slice)
+				return tc.err
+			})
+			require.Equal(t, tc.err, err)
+			require.Equal(t, tc.expectedSlices, slices)
+		})
+	}
+}
diff --git a/internal/gitaly/service/blob/server.go b/internal/gitaly/service/blob/server.go
index 07d414c31..42b0809ab 100644
--- a/internal/gitaly/service/blob/server.go
+++ b/internal/gitaly/service/blob/server.go
@@ -2,6 +2,7 @@ package blob
 
 import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -9,11 +10,17 @@ import (
 
 type server struct {
 	ruby          *rubyserver.Server
+	cfg           config.Cfg
 	locator       storage.Locator
 	gitCmdFactory git.CommandFactory
 }
 
 // NewServer creates a new instance of a grpc BlobServer
-func NewServer(rs *rubyserver.Server, locator storage.Locator, gitCmdFactory git.CommandFactory) gitalypb.BlobServiceServer {
-	return &server{ruby: rs, locator: locator, gitCmdFactory: gitCmdFactory}
+func NewServer(rs *rubyserver.Server, cfg config.Cfg, locator storage.Locator, gitCmdFactory git.CommandFactory) gitalypb.BlobServiceServer {
+	return &server{
+		ruby:          rs,
+		cfg:           cfg,
+		locator:       locator,
+		gitCmdFactory: gitCmdFactory,
+	}
 }
diff --git a/internal/gitaly/service/blob/testhelper_test.go b/internal/gitaly/service/blob/testhelper_test.go
index ad0583813..b3b4ddb59 100644
--- a/internal/gitaly/service/blob/testhelper_test.go
+++ b/internal/gitaly/service/blob/testhelper_test.go
@@ -45,7 +45,7 @@ func testMain(m *testing.M) int {
 func runBlobServer(t *testing.T, locator storage.Locator) (func(), string) {
 	srv := testhelper.NewServer(t, nil, nil)
 
-	gitalypb.RegisterBlobServiceServer(srv.GrpcServer(), NewServer(rubyServer, locator, git.NewExecCommandFactory(config.Config)))
+	gitalypb.RegisterBlobServiceServer(srv.GrpcServer(), NewServer(rubyServer, config.Config, locator, git.NewExecCommandFactory(config.Config)))
 	reflection.Register(srv.GrpcServer())
 
 	srv.Start(t)
diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go
index bfb673e22..df898e5ca 100644
--- a/internal/gitaly/service/register.go
+++ b/internal/gitaly/service/register.go
@@ -69,7 +69,7 @@ func RegisterAll(
 	conns *client.Pool,
 	gitCmdFactory git.CommandFactory,
 ) {
-	gitalypb.RegisterBlobServiceServer(grpcServer, blob.NewServer(rubyServer, locator, gitCmdFactory))
+	gitalypb.RegisterBlobServiceServer(grpcServer, blob.NewServer(rubyServer, cfg, locator, gitCmdFactory))
 	gitalypb.RegisterCleanupServiceServer(grpcServer, cleanup.NewServer(cfg, gitCmdFactory))
 	gitalypb.RegisterCommitServiceServer(grpcServer, commit.NewServer(cfg, locator, gitCmdFactory))
 	gitalypb.RegisterDiffServiceServer(grpcServer, diff.NewServer(cfg, locator, gitCmdFactory))
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index f8f37c38a..71fcf6581 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -28,6 +28,10 @@ var (
 	GoUserUpdateSubmodule = FeatureFlag{Name: "go_user_update_submodule", OnByDefault: false}
 	// GoUserRevert enables the Go implementation of UserRevert
 	GoUserRevert = FeatureFlag{Name: "go_user_revert", OnByDefault: false}
+	// GoGetAllLFSPointers enables the Go implementation of GetAllLFSPointers
+	GoGetAllLFSPointers = FeatureFlag{Name: "go_get_all_lfs_pointers", OnByDefault: false}
+	// GoGetLFSPointers enables the Go implementation of GetLFSPointers
+	GoGetLFSPointers = FeatureFlag{Name: "go_get_lfs_pointers", OnByDefault: false}
 
 	// TxApplyBfgObjectMapStream enables transactions for ApplyBfgObjectMapStream
 	TxApplyBfgObjectMapStream = FeatureFlag{Name: "tx_apply_bfg_object_map_stream", OnByDefault: true}
@@ -108,6 +112,8 @@ var All = []FeatureFlag{
 	GoResolveConflicts,
 	GoUserUpdateSubmodule,
 	GoUserRevert,
+	GoGetAllLFSPointers,
+	GoGetLFSPointers,
 	TxApplyBfgObjectMapStream,
 	TxApplyGitattributes,
 	TxResolveConflicts,
diff --git a/proto/blob.proto b/proto/blob.proto
index 76c417026..5798a04fe 100644
--- a/proto/blob.proto
+++ b/proto/blob.proto
@@ -21,16 +21,26 @@ service BlobService {
       op: ACCESSOR
     };
   }
+
+  // GetLFSPointers retrieves LFS pointers from a given set of object IDs.
+  // This RPC filters all requested objects and only returns those which refer
+  // to a valid LFS pointer.
   rpc GetLFSPointers(GetLFSPointersRequest) returns (stream GetLFSPointersResponse) {
     option (op_type) = {
       op: ACCESSOR
     };
   }
+
+  // GetNewLFSPointers retrieves LFS pointers for a limited subset of the
+  // commit graph. It will return all LFS pointers which are reachable by the
+  // provided revision, but not reachable by any of the limiting references.
   rpc GetNewLFSPointers(GetNewLFSPointersRequest) returns (stream GetNewLFSPointersResponse) {
     option (op_type) = {
       op: ACCESSOR
     };
   }
+
+  // GetAllLFSPointers retrieves all LFS pointers of the given repository.
   rpc GetAllLFSPointers(GetAllLFSPointersRequest) returns (stream GetAllLFSPointersResponse) {
     option (op_type) = {
       op: ACCESSOR
@@ -84,9 +94,15 @@ message GetBlobsResponse {
   ObjectType type = 8;
 }
 
+// LFSPointer is a git blob which points to an LFS object.
 message LFSPointer {
+  // Size is the size of the blob. This is not the size of the LFS object
+  // pointed to.
   int64 size = 1;
+  // Data is the bare data of the LFS pointer blob. It contains the pointer to
+  // the LFS data in the format specified by the LFS project.
   bytes data = 2;
+  // Oid is the object ID of the blob.
   string oid = 3;
 }
 
@@ -96,35 +112,57 @@ message NewBlobObject {
   bytes path = 3;
 }
 
+// GetLFSPointersRequest is a request for the GetLFSPointers RPC.
 message GetLFSPointersRequest {
-
+  // Repository is the repository for which LFS pointers should be retrieved
+  // from.
   Repository repository = 1[(target_repository)=true];
+  // BlobIds is the list of blobs to retrieve LFS pointers from. Must be a
+  // non-empty list of blobs IDs to fetch.
   repeated string blob_ids = 2;
 }
 
+// GetLFSPointersResponse is a response for the GetLFSPointers RPC.
 message GetLFSPointersResponse {
+  // LfsPointers is the list of LFS pointers which were requested.
   repeated LFSPointer lfs_pointers = 1;
 }
 
+// GetNewLFSPointersRequest is a request for the GetNewLFSPointers RPC.
 message GetNewLFSPointersRequest {
-
+  // Repository is the repository for which LFS pointers should be retrieved
+  // from.
   Repository repository = 1[(target_repository)=true];
+  // Revision is the revision for which to retrieve new LFS pointers.
   bytes revision = 2;
+  // Limit limits the number of LFS pointers returned.
   int32 limit = 3;
-  // Note: When `not_in_all` is true, `not_in_refs` is ignored
+  // NotInAll limits the revision graph to not include any commits which are
+  // referenced by a git reference. When `not_in_all` is true, `not_in_refs` is
+  // ignored.
   bool not_in_all = 4;
+  // NotInRefs is a list of references used to limit the revision graph. Any
+  // commit reachable by any commit in NotInRefs will not be searched for new
+  // LFS pointers. This is ignored if NotInAll is set to `true`.
   repeated bytes not_in_refs = 5;
 }
 
+// GetNewLFSPointersResponse is a response for the GetNewLFSPointers RPC.
 message GetNewLFSPointersResponse {
+  // LfsPointers is the list of LFS pointers which were requested.
   repeated LFSPointer lfs_pointers = 1;
 }
 
+// GetAllLFSPointersRequest is a request for the GetAllLFSPointers RPC.
 message GetAllLFSPointersRequest {
+  // Repository is the repository for which LFS pointers shoul be retrieved
+  // from.
   Repository repository = 1[(target_repository)=true];
   reserved 2;
 }
 
+// GetAllLFSPointersResponse is a response for the GetAllLFSPointers RPC.
 message GetAllLFSPointersResponse {
+  // LfsPointers is the list of LFS pointers.
   repeated LFSPointer lfs_pointers = 1;
 }
diff --git a/proto/go/gitalypb/blob.pb.go b/proto/go/gitalypb/blob.pb.go
index 128f0466a..b75143edb 100644
--- a/proto/go/gitalypb/blob.pb.go
+++ b/proto/go/gitalypb/blob.pb.go
@@ -341,9 +341,15 @@ func (m *GetBlobsResponse) GetType() ObjectType {
 	return ObjectType_UNKNOWN
 }
 
+// LFSPointer is a git blob which points to an LFS object.
 type LFSPointer struct {
-	Size                 int64    `protobuf:"varint,1,opt,name=size,proto3" json:"size,omitempty"`
-	Data                 []byte   `protobuf:"bytes,2,opt,name=data,proto3" json:"data,omitempty"`
+	// Size is the size of the blob. This is not the size of the LFS object
+	// pointed to.
+	Size int64 `protobuf:"varint,1,opt,name=size,proto3" json:"size,omitempty"`
+	// Data is the bare data of the LFS pointer blob. It contains the pointer to
+	// the LFS data in the format specified by the LFS project.
+	Data []byte `protobuf:"bytes,2,opt,name=data,proto3" json:"data,omitempty"`
+	// Oid is the object ID of the blob.
 	Oid                  string   `protobuf:"bytes,3,opt,name=oid,proto3" json:"oid,omitempty"`
 	XXX_NoUnkeyedLiteral struct{} `json:"-"`
 	XXX_unrecognized     []byte   `json:"-"`
@@ -451,12 +457,17 @@ func (m *NewBlobObject) GetPath() []byte {
 	return nil
 }
 
+// GetLFSPointersRequest is a request for the GetLFSPointers RPC.
 type GetLFSPointersRequest struct {
-	Repository           *Repository `protobuf:"bytes,1,opt,name=repository,proto3" json:"repository,omitempty"`
-	BlobIds              []string    `protobuf:"bytes,2,rep,name=blob_ids,json=blobIds,proto3" json:"blob_ids,omitempty"`
-	XXX_NoUnkeyedLiteral struct{}    `json:"-"`
-	XXX_unrecognized     []byte      `json:"-"`
-	XXX_sizecache        int32       `json:"-"`
+	// Repository is the repository for which LFS pointers should be retrieved
+	// from.
+	Repository *Repository `protobuf:"bytes,1,opt,name=repository,proto3" json:"repository,omitempty"`
+	// BlobIds is the list of blobs to retrieve LFS pointers from. Must be a
+	// non-empty list of blobs IDs to fetch.
+	BlobIds              []string `protobuf:"bytes,2,rep,name=blob_ids,json=blobIds,proto3" json:"blob_ids,omitempty"`
+	XXX_NoUnkeyedLiteral struct{} `json:"-"`
+	XXX_unrecognized     []byte   `json:"-"`
+	XXX_sizecache        int32    `json:"-"`
 }
 
 func (m *GetLFSPointersRequest) Reset()         { *m = GetLFSPointersRequest{} }
@@ -498,7 +509,9 @@ func (m *GetLFSPointersRequest) GetBlobIds() []string {
 	return nil
 }
 
+// GetLFSPointersResponse is a response for the GetLFSPointers RPC.
 type GetLFSPointersResponse struct {
+	// LfsPointers is the list of LFS pointers which were requested.
 	LfsPointers          []*LFSPointer `protobuf:"bytes,1,rep,name=lfs_pointers,json=lfsPointers,proto3" json:"lfs_pointers,omitempty"`
 	XXX_NoUnkeyedLiteral struct{}      `json:"-"`
 	XXX_unrecognized     []byte        `json:"-"`
@@ -537,12 +550,22 @@ func (m *GetLFSPointersResponse) GetLfsPointers() []*LFSPointer {
 	return nil
 }
 
+// GetNewLFSPointersRequest is a request for the GetNewLFSPointers RPC.
 type GetNewLFSPointersRequest struct {
+	// Repository is the repository for which LFS pointers should be retrieved
+	// from.
 	Repository *Repository `protobuf:"bytes,1,opt,name=repository,proto3" json:"repository,omitempty"`
-	Revision   []byte      `protobuf:"bytes,2,opt,name=revision,proto3" json:"revision,omitempty"`
-	Limit      int32       `protobuf:"varint,3,opt,name=limit,proto3" json:"limit,omitempty"`
-	// Note: When `not_in_all` is true, `not_in_refs` is ignored
-	NotInAll             bool     `protobuf:"varint,4,opt,name=not_in_all,json=notInAll,proto3" json:"not_in_all,omitempty"`
+	// Revision is the revision for which to retrieve new LFS pointers.
+	Revision []byte `protobuf:"bytes,2,opt,name=revision,proto3" json:"revision,omitempty"`
+	// Limit limits the number of LFS pointers returned.
+	Limit int32 `protobuf:"varint,3,opt,name=limit,proto3" json:"limit,omitempty"`
+	// NotInAll limits the revision graph to not include any commits which are
+	// referenced by a git reference. When `not_in_all` is true, `not_in_refs` is
+	// ignored.
+	NotInAll bool `protobuf:"varint,4,opt,name=not_in_all,json=notInAll,proto3" json:"not_in_all,omitempty"`
+	// NotInRefs is a list of references used to limit the revision graph. Any
+	// commit reachable by any commit in NotInRefs will not be searched for new
+	// LFS pointers. This is ignored if NotInAll is set to `true`.
 	NotInRefs            [][]byte `protobuf:"bytes,5,rep,name=not_in_refs,json=notInRefs,proto3" json:"not_in_refs,omitempty"`
 	XXX_NoUnkeyedLiteral struct{} `json:"-"`
 	XXX_unrecognized     []byte   `json:"-"`
@@ -609,7 +632,9 @@ func (m *GetNewLFSPointersRequest) GetNotInRefs() [][]byte {
 	return nil
 }
 
+// GetNewLFSPointersResponse is a response for the GetNewLFSPointers RPC.
 type GetNewLFSPointersResponse struct {
+	// LfsPointers is the list of LFS pointers which were requested.
 	LfsPointers          []*LFSPointer `protobuf:"bytes,1,rep,name=lfs_pointers,json=lfsPointers,proto3" json:"lfs_pointers,omitempty"`
 	XXX_NoUnkeyedLiteral struct{}      `json:"-"`
 	XXX_unrecognized     []byte        `json:"-"`
@@ -648,7 +673,10 @@ func (m *GetNewLFSPointersResponse) GetLfsPointers() []*LFSPointer {
 	return nil
 }
 
+// GetAllLFSPointersRequest is a request for the GetAllLFSPointers RPC.
 type GetAllLFSPointersRequest struct {
+	// Repository is the repository for which LFS pointers shoul be retrieved
+	// from.
 	Repository           *Repository `protobuf:"bytes,1,opt,name=repository,proto3" json:"repository,omitempty"`
 	XXX_NoUnkeyedLiteral struct{}    `json:"-"`
 	XXX_unrecognized     []byte      `json:"-"`
@@ -687,7 +715,9 @@ func (m *GetAllLFSPointersRequest) GetRepository() *Repository {
 	return nil
 }
 
+// GetAllLFSPointersResponse is a response for the GetAllLFSPointers RPC.
 type GetAllLFSPointersResponse struct {
+	// LfsPointers is the list of LFS pointers.
 	LfsPointers          []*LFSPointer `protobuf:"bytes,1,rep,name=lfs_pointers,json=lfsPointers,proto3" json:"lfs_pointers,omitempty"`
 	XXX_NoUnkeyedLiteral struct{}      `json:"-"`
 	XXX_unrecognized     []byte        `json:"-"`
@@ -808,8 +838,15 @@ type BlobServiceClient interface {
 	// response
 	GetBlob(ctx context.Context, in *GetBlobRequest, opts ...grpc.CallOption) (BlobService_GetBlobClient, error)
 	GetBlobs(ctx context.Context, in *GetBlobsRequest, opts ...grpc.CallOption) (BlobService_GetBlobsClient, error)
+	// GetLFSPointers retrieves LFS pointers from a given set of object IDs.
+	// This RPC filters all requested objects and only returns those which refer
+	// to a valid LFS pointer.
 	GetLFSPointers(ctx context.Context, in *GetLFSPointersRequest, opts ...grpc.CallOption) (BlobService_GetLFSPointersClient, error)
+	// GetNewLFSPointers retrieves LFS pointers for a limited subset of the
+	// commit graph. It will return all LFS pointers which are reachable by the
+	// provided revision, but not reachable by any of the limiting references.
 	GetNewLFSPointers(ctx context.Context, in *GetNewLFSPointersRequest, opts ...grpc.CallOption) (BlobService_GetNewLFSPointersClient, error)
+	// GetAllLFSPointers retrieves all LFS pointers of the given repository.
 	GetAllLFSPointers(ctx context.Context, in *GetAllLFSPointersRequest, opts ...grpc.CallOption) (BlobService_GetAllLFSPointersClient, error)
 }
 
@@ -988,8 +1025,15 @@ type BlobServiceServer interface {
 	// response
 	GetBlob(*GetBlobRequest, BlobService_GetBlobServer) error
 	GetBlobs(*GetBlobsRequest, BlobService_GetBlobsServer) error
+	// GetLFSPointers retrieves LFS pointers from a given set of object IDs.
+	// This RPC filters all requested objects and only returns those which refer
+	// to a valid LFS pointer.
 	GetLFSPointers(*GetLFSPointersRequest, BlobService_GetLFSPointersServer) error
+	// GetNewLFSPointers retrieves LFS pointers for a limited subset of the
+	// commit graph. It will return all LFS pointers which are reachable by the
+	// provided revision, but not reachable by any of the limiting references.
 	GetNewLFSPointers(*GetNewLFSPointersRequest, BlobService_GetNewLFSPointersServer) error
+	// GetAllLFSPointers retrieves all LFS pointers of the given repository.
 	GetAllLFSPointers(*GetAllLFSPointersRequest, BlobService_GetAllLFSPointersServer) error
 }
 
diff --git a/ruby/proto/gitaly/blob_services_pb.rb b/ruby/proto/gitaly/blob_services_pb.rb
index c602e3af9..149e27c81 100644
--- a/ruby/proto/gitaly/blob_services_pb.rb
+++ b/ruby/proto/gitaly/blob_services_pb.rb
@@ -19,8 +19,15 @@ module Gitaly
       # response
       rpc :GetBlob, Gitaly::GetBlobRequest, stream(Gitaly::GetBlobResponse)
       rpc :GetBlobs, Gitaly::GetBlobsRequest, stream(Gitaly::GetBlobsResponse)
+      # GetLFSPointers retrieves LFS pointers from a given set of object IDs.
+      # This RPC filters all requested objects and only returns those which refer
+      # to a valid LFS pointer.
       rpc :GetLFSPointers, Gitaly::GetLFSPointersRequest, stream(Gitaly::GetLFSPointersResponse)
+      # GetNewLFSPointers retrieves LFS pointers for a limited subset of the
+      # commit graph. It will return all LFS pointers which are reachable by the
+      # provided revision, but not reachable by any of the limiting references.
       rpc :GetNewLFSPointers, Gitaly::GetNewLFSPointersRequest, stream(Gitaly::GetNewLFSPointersResponse)
+      # GetAllLFSPointers retrieves all LFS pointers of the given repository.
       rpc :GetAllLFSPointers, Gitaly::GetAllLFSPointersRequest, stream(Gitaly::GetAllLFSPointersResponse)
     end