Merge branch 'feature/migrate-commit-count-to-gitaly' into 'master'

Migrate Gitlab::Git::Repository#commit_count to Gitaly

Closes gitaly#355

See merge request !12688

GITALY_SERVER_VERSION

-0.14.0
+0.15.0

Gemfile

@@ -386,7 +386,7 @@ gem 'vmstat', '~> 2.3.0'
 gem 'sys-filesystem', '~> 1.1.6'
 
 # Gitaly GRPC client
-gem 'gitaly', '~> 0.9.0'
+gem 'gitaly', '~> 0.13.0'
 
 gem 'toml-rb', '~> 0.3.15', require: false
 

Gemfile.lock

@@ -278,7 +278,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly (0.9.0)
+    gitaly (0.13.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -980,7 +980,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.2.0)
-  gitaly (~> 0.9.0)
+  gitaly (~> 0.13.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-markup (~> 1.5.1)

lib/gitlab/git/repository.rb

@@ -568,11 +568,17 @@ module Gitlab
 
       # Return total commits count accessible from passed ref
       def commit_count(ref)
-        walker = Rugged::Walker.new(rugged)
-        walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE)
-        oid = rugged.rev_parse_oid(ref)
-        walker.push(oid)
-        walker.count
+        gitaly_migrate(:commit_count) do |is_enabled|
+          if is_enabled
+            gitaly_commit_client.commit_count(ref)
+          else
+            walker = Rugged::Walker.new(rugged)
+            walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE)
+            oid = rugged.rev_parse_oid(ref)
+            walker.push(oid)
+            walker.count
+          end
+        end
       end
 
       # Sets HEAD to the commit specified by +ref+; +ref+ can be a branch or

lib/gitlab/gitaly_client/commit.rb

@@ -56,6 +56,15 @@ module Gitlab
         entry
       end
 
+      def commit_count(ref)
+        request = Gitaly::CountCommitsRequest.new(
+          repository: @gitaly_repo,
+          revision: ref
+        )
+
+        GitalyClient.call(@repository.storage, :commit_service, :count_commits, request).count
+      end
+
       private
 
       def commit_diff_request_params(commit, options = {})

spec/lib/gitlab/git/repository_spec.rb

@@ -3,6 +3,20 @@ require "spec_helper"
 describe Gitlab::Git::Repository, seed_helper: true do
   include Gitlab::EncodingHelper
 
+  shared_examples 'wrapping gRPC errors' do |gitaly_client_class, gitaly_client_method|
+    it 'wraps gRPC not found error' do
+      expect_any_instance_of(gitaly_client_class).to receive(gitaly_client_method)
+        .and_raise(GRPC::NotFound)
+      expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
+    end
+
+    it 'wraps gRPC unknown error' do
+      expect_any_instance_of(gitaly_client_class).to receive(gitaly_client_method)
+        .and_raise(GRPC::Unknown)
+      expect { subject }.to raise_error(Gitlab::Git::CommandError)
+    end
+  end
+
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) }
 
   describe "Respond to" do
@@ -35,16 +49,8 @@ describe Gitlab::Git::Repository, seed_helper: true do
       repository.root_ref
     end
 
-    it 'wraps GRPC not found' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
-        .and_raise(GRPC::NotFound)
-      expect { repository.root_ref }.to raise_error(Gitlab::Git::Repository::NoRepository)
-    end
-
-    it 'wraps GRPC exceptions' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
-        .and_raise(GRPC::Unknown)
-      expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
+    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :default_branch_name do
+      subject { repository.root_ref }
     end
   end
 
@@ -130,17 +136,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
       subject
     end
 
-    it 'wraps GRPC not found' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
-        .and_raise(GRPC::NotFound)
-      expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
-    end
-
-    it 'wraps GRPC other exceptions' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
-        .and_raise(GRPC::Unknown)
-      expect { subject }.to raise_error(Gitlab::Git::CommandError)
-    end
+    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :branch_names
   end
 
   describe '#tag_names' do
@@ -168,17 +164,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
       subject
     end
 
-    it 'wraps GRPC not found' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
-        .and_raise(GRPC::NotFound)
-      expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
-    end
-
-    it 'wraps GRPC exceptions' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
-        .and_raise(GRPC::Unknown)
-      expect { subject }.to raise_error(Gitlab::Git::CommandError)
-    end
+    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :tag_names
   end
 
   shared_examples 'archive check' do |extenstion|
@@ -438,8 +424,21 @@ describe Gitlab::Git::Repository, seed_helper: true do
   end
 
   describe '#commit_count' do
-    it { expect(repository.commit_count("master")).to eq(25) }
-    it { expect(repository.commit_count("feature")).to eq(9) }
+    shared_examples 'counting commits' do
+      it { expect(repository.commit_count("master")).to eq(25) }
+      it { expect(repository.commit_count("feature")).to eq(9) }
+    end
+
+    context 'when Gitaly commit_count feature is enabled' do
+      it_behaves_like 'counting commits'
+      it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Commit, :commit_count do
+        subject { repository.commit_count('master') }
+      end
+    end
+
+    context 'when Gitaly commit_count feature is disabled', skip_gitaly_mock: true  do
+      it_behaves_like 'counting commits'
+    end
   end
 
   describe "#reset" do
@@ -1298,16 +1297,8 @@ describe Gitlab::Git::Repository, seed_helper: true do
       @repo.local_branches
     end
 
-    it 'wraps GRPC not found' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
-        .and_raise(GRPC::NotFound)
-      expect { @repo.local_branches }.to raise_error(Gitlab::Git::Repository::NoRepository)
-    end
-
-    it 'wraps GRPC exceptions' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
-        .and_raise(GRPC::Unknown)
-      expect { @repo.local_branches }.to raise_error(Gitlab::Git::CommandError)
+    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :local_branches do
+      subject { @repo.local_branches }
     end
   end