Merge branch 'optimize-get-blobs' into 'master'

GetBlobs: don't create blob reader if limit is zero

Closes #1179

See merge request gitlab-org/gitaly!706

2 files changed, 25 insertions, 19 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 67c699650..7f5735856 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 UNRELEASED
 
+- GetBlobs: don't create blob reader if limit is zero
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/706
 - Implement SearchFilesBy{Content,Name}
   https://gitlab.com/gitlab-org/gitaly/merge_requests/677
 - Introduce feature flag package based on gRPC metadata
diff --git a/internal/service/blob/get_blobs.go b/internal/service/blob/get_blobs.go
index 4bad57c74..e52170bec 100644
--- a/internal/service/blob/get_blobs.go
+++ b/internal/service/blob/get_blobs.go
@@ -55,11 +55,6 @@ func sendGetBlobsResponse(req *pb.GetBlobsRequest, stream pb.BlobService_GetBlob
 			return status.Errorf(codes.InvalidArgument, "GetBlobs: object at %s:%s is %s, not blob", revision, path, objectInfo.Type)
 		}
 
-		blobReader, err := c.Blob(objectInfo.Oid)
-		if err != nil {
-			return status.Errorf(codes.Internal, "GetBlobs: %v", err)
-		}
-
 		response.Size = objectInfo.Size
 
 		var readLimit int64
@@ -69,27 +64,36 @@ func sendGetBlobsResponse(req *pb.GetBlobsRequest, stream pb.BlobService_GetBlob
 			readLimit = req.Limit
 		}
 
+		// For correctness it does not matter, but for performance, the order is
+		// important: first check if readlimit == 0, if not, only then create
+		// blobReader.
 		if readLimit == 0 {
 			if err := stream.Send(response); err != nil {
 				return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err)
 			}
-		} else {
-			sw := streamio.NewWriter(func(p []byte) error {
-				msg := &pb.GetBlobsResponse{}
-				if response != nil {
-					msg = response
-					response = nil
-				}
-
-				msg.Data = p
+			continue
+		}
 
-				return stream.Send(msg)
-			})
+		blobReader, err := c.Blob(objectInfo.Oid)
+		if err != nil {
+			return status.Errorf(codes.Internal, "GetBlobs: %v", err)
+		}
 
-			_, err := io.CopyN(sw, blobReader, readLimit)
-			if err != nil {
-				return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err)
+		sw := streamio.NewWriter(func(p []byte) error {
+			msg := &pb.GetBlobsResponse{}
+			if response != nil {
+				msg = response
+				response = nil
 			}
+
+			msg.Data = p
+
+			return stream.Send(msg)
+		})
+
+		_, err = io.CopyN(sw, blobReader, readLimit)
+		if err != nil {
+			return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err)
 		}
 
 		if _, err := io.Copy(ioutil.Discard, blobReader); err != nil {