Bring back Rugged implementation of ListCommitsByOidsh-backport-list-commits-by-oid-rugged

This brings back changes in
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20432.

For users using Gitaly on top of NFS, accessing the Git data directly
via Rugged may be faster than going through than Gitaly. This merge
request introduces the feature flag `rugged_list_commits_by_oid` to
activate the Rugged method.

For one customer, we saw that ListCommitsByOid was the second highest
used endpoint that may be causing increased load.

5 files changed, 65 insertions, 5 deletions

diff --git a/changelogs/unreleased/sh-backport-list-commits-by-oid-rugged.yml b/changelogs/unreleased/sh-backport-list-commits-by-oid-rugged.yml
new file mode 100644
index 00000000000..eb8774d652f
--- /dev/null
+++ b/changelogs/unreleased/sh-backport-list-commits-by-oid-rugged.yml
@@ -0,0 +1,5 @@
+---
+title: Bring back Rugged implementation of ListCommitsByOid
+merge_request: 27441
+author:
+type: performance
diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md
index bfde26dbe4a..d7b6b6aaae5 100644
--- a/doc/development/gitaly.md
+++ b/doc/development/gitaly.md
@@ -80,6 +80,8 @@ most commonly-used RPCs can be enabled via feature flags:
 * `rugged_get_tree_entries`
 * `rugged_tree_entry`
 * `rugged_commit_is_ancestor`
+* `rugged_commit_tree_entry`
+* `rugged_list_commits_by_oid`
 
 A convenience Rake task can be used to enable or disable these flags
 all together. To enable:
diff --git a/lib/gitlab/git/rugged_impl/commit.rb b/lib/gitlab/git/rugged_impl/commit.rb
index f6777dfa0c3..bce4fa14fb4 100644
--- a/lib/gitlab/git/rugged_impl/commit.rb
+++ b/lib/gitlab/git/rugged_impl/commit.rb
@@ -21,6 +21,17 @@ module Gitlab
             nil
           end
 
+          # This needs to return an array of Gitlab::Git:Commit objects
+          # instead of Rugged::Commit objects to ensure upstream models
+          # operate on a consistent interface. Unlike
+          # Gitlab::Git::Commit.find, Gitlab::Git::Commit.batch_by_oid
+          # doesn't attempt to decorate the result.
+          def rugged_batch_by_oid(repo, oids)
+            oids.map { |oid| rugged_find(repo, oid) }
+              .compact
+              .map { |commit| decorate(repo, commit) }
+          end
+
           override :find_commit
           def find_commit(repo, commit_id)
             if Feature.enabled?(:rugged_find_commit)
@@ -29,6 +40,15 @@ module Gitlab
               super
             end
           end
+
+          override :batch_by_oid
+          def batch_by_oid(repo, oids)
+            if Feature.enabled?(:rugged_list_commits_by_oid)
+              rugged_batch_by_oid(repo, oids)
+            else
+              super
+            end
+          end
         end
 
         extend ::Gitlab::Utils::Override
diff --git a/lib/gitlab/git/rugged_impl/repository.rb b/lib/gitlab/git/rugged_impl/repository.rb
index c0a91f59ab9..e91b0ddcd31 100644
--- a/lib/gitlab/git/rugged_impl/repository.rb
+++ b/lib/gitlab/git/rugged_impl/repository.rb
@@ -12,7 +12,7 @@ module Gitlab
       module Repository
         extend ::Gitlab::Utils::Override
 
-        FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor rugged_commit_tree_entry).freeze
+        FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor rugged_commit_tree_entry rugged_list_commits_by_oid).freeze
 
         def alternate_object_directories
           relative_object_directories.map { |d| File.join(path, d) }
diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb
index 507bf222810..1ae8e0457d0 100644
--- a/spec/lib/gitlab/git/commit_spec.rb
+++ b/spec/lib/gitlab/git/commit_spec.rb
@@ -380,16 +380,49 @@ describe Gitlab::Git::Commit, :seed_helper do
       end
     end
 
-    describe '#batch_by_oid' do
+    shared_examples '.batch_by_oid' do
+      context 'with multiple OIDs' do
+        let(:oids) { [SeedRepo::Commit::ID, SeedRepo::FirstCommit::ID] }
+
+        it 'returns multiple commits' do
+          commits = described_class.batch_by_oid(repository, oids)
+
+          expect(commits.count).to eq(2)
+          expect(commits).to all( be_a(Gitlab::Git::Commit) )
+          expect(commits.first.sha).to eq(SeedRepo::Commit::ID)
+          expect(commits.second.sha).to eq(SeedRepo::FirstCommit::ID)
+        end
+      end
+
       context 'when oids is empty' do
-        it 'makes no Gitaly request' do
-          expect(Gitlab::GitalyClient).not_to receive(:call)
+        it 'returns empty commits' do
+          commits = described_class.batch_by_oid(repository, [])
 
-          described_class.batch_by_oid(repository, [])
+          expect(commits.count).to eq(0)
         end
       end
     end
 
+    describe '.batch_by_oid with Gitaly enabled' do
+      it_should_behave_like '.batch_by_oid'
+
+      it 'makes no Gitaly request' do
+        expect(Gitlab::GitalyClient).not_to receive(:call)
+
+        described_class.batch_by_oid(repository, [])
+      end
+    end
+
+    describe '.batch_by_oid with Rugged enabled', :enable_rugged do
+      it_should_behave_like '.batch_by_oid'
+
+      it 'calls out to the Rugged implementation' do
+        allow_any_instance_of(Rugged).to receive(:rev_parse).with(SeedRepo::Commit::ID).and_call_original
+
+        described_class.batch_by_oid(repository, [SeedRepo::Commit::ID])
+      end
+    end
+
     shared_examples 'extracting commit signature' do
       context 'when the commit is signed' do
         let(:commit_id) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' }