Merge branch 'feature/migrate-get-lfs-ptrs-to-gitaly' into 'master'

Migrate .batch_lfs_pointers to Gitaly

Closes gitaly#921

See merge request gitlab-org/gitlab-ce!16517

lib/gitlab/git/blob.rb

@@ -34,7 +34,7 @@ module Gitlab
         def raw(repository, sha)
           Gitlab::GitalyClient.migrate(:git_blob_raw) do |is_enabled|
             if is_enabled
-              Gitlab::GitalyClient::BlobService.new(repository).get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE)
+              repository.gitaly_blob_client.get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE)
             else
               rugged_raw(repository, sha, limit: MAX_DATA_DISPLAY_SIZE)
             end
@@ -70,11 +70,19 @@ module Gitlab
         # Returns array of Gitlab::Git::Blob
         # Does not guarantee blob data will be set
         def batch_lfs_pointers(repository, blob_ids)
-          blob_ids.lazy
-                  .select { |sha| possible_lfs_blob?(repository, sha) }
-                  .map { |sha| rugged_raw(repository, sha, limit: LFS_POINTER_MAX_SIZE) }
-                  .select(&:lfs_pointer?)
-                  .force
+          return [] if blob_ids.empty?
+
+          repository.gitaly_migrate(:batch_lfs_pointers) do |is_enabled|
+            if is_enabled
+              repository.gitaly_blob_client.batch_lfs_pointers(blob_ids)
+            else
+              blob_ids.lazy
+                      .select { |sha| possible_lfs_blob?(repository, sha) }
+                      .map { |sha| rugged_raw(repository, sha, limit: LFS_POINTER_MAX_SIZE) }
+                      .select(&:lfs_pointer?)
+                      .force
+            end
+          end
         end
 
         def binary?(data)
@@ -258,7 +266,7 @@ module Gitlab
         Gitlab::GitalyClient.migrate(:git_blob_load_all_data) do |is_enabled|
           @data = begin
             if is_enabled
-              Gitlab::GitalyClient::BlobService.new(repository).get_blob(oid: id, limit: -1).data
+              repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data
             else
               repository.lookup(id).content
             end

lib/gitlab/git/repository.rb

@@ -1331,6 +1331,10 @@ module Gitlab
         @gitaly_remote_client ||= Gitlab::GitalyClient::RemoteService.new(self)
       end
 
+      def gitaly_blob_client
+        @gitaly_blob_client ||= Gitlab::GitalyClient::BlobService.new(self)
+      end
+
       def gitaly_conflicts_client(our_commit_oid, their_commit_oid)
         Gitlab::GitalyClient::ConflictsService.new(self, our_commit_oid, their_commit_oid)
       end

lib/gitlab/gitaly_client/blob_service.rb

@@ -32,6 +32,26 @@ module Gitlab
           binary: Gitlab::Git::Blob.binary?(data)
         )
       end
+
+      def batch_lfs_pointers(blob_ids)
+        request = Gitaly::GetLFSPointersRequest.new(
+          repository: @gitaly_repo,
+          blob_ids: blob_ids
+        )
+
+        response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_lfs_pointers, request)
+
+        response.flat_map do |message|
+          message.lfs_pointers.map do |lfs_pointer|
+            Gitlab::Git::Blob.new(
+              id: lfs_pointer.oid,
+              size: lfs_pointer.size,
+              data: lfs_pointer.data,
+              binary: Gitlab::Git::Blob.binary?(lfs_pointer.data)
+            )
+          end
+        end
+      end
     end
   end
 end

spec/lib/gitlab/git/blob_spec.rb

@@ -260,29 +260,42 @@ describe Gitlab::Git::Blob, seed_helper: true do
       )
     end
 
-    it 'returns a list of Gitlab::Git::Blob' do
-      blobs = described_class.batch_lfs_pointers(repository, [lfs_blob.id])
+    shared_examples 'fetching batch of LFS pointers' do
+      it 'returns a list of Gitlab::Git::Blob' do
+        blobs = described_class.batch_lfs_pointers(repository, [lfs_blob.id])
 
-      expect(blobs.count).to eq(1)
-      expect(blobs).to all( be_a(Gitlab::Git::Blob) )
-    end
+        expect(blobs.count).to eq(1)
+        expect(blobs).to all( be_a(Gitlab::Git::Blob) )
+      end
 
-    it 'silently ignores tree objects' do
-      blobs = described_class.batch_lfs_pointers(repository, [tree_object.oid])
+      it 'silently ignores tree objects' do
+        blobs = described_class.batch_lfs_pointers(repository, [tree_object.oid])
 
-      expect(blobs).to eq([])
-    end
+        expect(blobs).to eq([])
+      end
+
+      it 'silently ignores non lfs objects' do
+        blobs = described_class.batch_lfs_pointers(repository, [non_lfs_blob.id])
 
-    it 'silently ignores non lfs objects' do
-      blobs = described_class.batch_lfs_pointers(repository, [non_lfs_blob.id])
+        expect(blobs).to eq([])
+      end
+
+      it 'avoids loading large blobs into memory' do
+        # This line could call `lookup` on `repository`, so do here before mocking.
+        non_lfs_blob_id = non_lfs_blob.id
+
+        expect(repository).not_to receive(:lookup)
 
-      expect(blobs).to eq([])
+        described_class.batch_lfs_pointers(repository, [non_lfs_blob_id])
+      end
     end
 
-    it 'avoids loading large blobs into memory' do
-      expect(repository).not_to receive(:lookup)
+    context 'when Gitaly batch_lfs_pointers is enabled' do
+      it_behaves_like 'fetching batch of LFS pointers'
+    end
 
-      described_class.batch_lfs_pointers(repository, [non_lfs_blob.id])
+    context 'when Gitaly batch_lfs_pointers is disabled', :disable_gitaly do
+      it_behaves_like 'fetching batch of LFS pointers'
     end
   end