Rewrite ListCommitsByOid in Go

3 files changed, 36 insertions, 17 deletions

diff --git a/changelogs/unreleased/list-commits-by-oid-go.yml b/changelogs/unreleased/list-commits-by-oid-go.yml
new file mode 100644
index 000000000..1f6ec4587
--- /dev/null
+++ b/changelogs/unreleased/list-commits-by-oid-go.yml
@@ -0,0 +1,5 @@
+---
+title: Rewrite ListCommitsByOid in Go
+merge_request: 787
+author:
+type: performance
diff --git a/internal/service/commit/list_commits_by_oid.go b/internal/service/commit/list_commits_by_oid.go
index 9be6eb96f..2035debb4 100644
--- a/internal/service/commit/list_commits_by_oid.go
+++ b/internal/service/commit/list_commits_by_oid.go
@@ -1,36 +1,49 @@
 package commit
 
 import (
-	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
-
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+	gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log"
 )
 
+const batchSizeListCommitsByOid = 20
+
 func (s *server) ListCommitsByOid(in *pb.ListCommitsByOidRequest, stream pb.CommitService_ListCommitsByOidServer) error {
 	ctx := stream.Context()
 
-	client, err := s.CommitServiceClient(ctx)
+	c, err := catfile.New(ctx, in.Repository)
 	if err != nil {
 		return err
 	}
 
-	clientCtx, err := rubyserver.SetHeaders(ctx, in.GetRepository())
-	if err != nil {
-		return err
+	send := func(commits []*pb.GitCommit) error {
+		return stream.Send(&pb.ListCommitsByOidResponse{Commits: commits})
 	}
 
-	rubyStream, err := client.ListCommitsByOid(clientCtx, in)
-	if err != nil {
-		return err
-	}
-
-	return rubyserver.Proxy(func() error {
-		resp, err := rubyStream.Recv()
+	var commits []*pb.GitCommit
+	for _, oid := range in.Oid {
+		commit, err := gitlog.GetCommitCatfile(c, oid)
 		if err != nil {
-			md := rubyStream.Trailer()
-			stream.SetTrailer(md)
 			return err
 		}
-		return stream.Send(resp)
-	})
+
+		if commit == nil {
+			continue
+		}
+
+		commits = append(commits, commit)
+
+		if len(commits) == batchSizeListCommitsByOid {
+			if err := send(commits); err != nil {
+				return err
+			}
+			commits = nil
+		}
+	}
+
+	if len(commits) > 0 {
+		return send(commits)
+	}
+
+	return nil
 }
diff --git a/ruby/lib/gitaly_server/commit_service.rb b/ruby/lib/gitaly_server/commit_service.rb
index 1f5afb4d8..9e4ccd261 100644
--- a/ruby/lib/gitaly_server/commit_service.rb
+++ b/ruby/lib/gitaly_server/commit_service.rb
@@ -21,6 +21,7 @@ module GitalyServer
       end
     end
 
+    # TODO remove this implementation in GitLab 11.3
     def list_commits_by_oid(request, call)
       bridge_exceptions do
         repository = Gitlab::Git::Repository.from_gitaly(request.repository, call)