From 1f2f38f59a719f7dae110835b8beb3d94fdcd94d Mon Sep 17 00:00:00 2001 From: John Cai Date: Wed, 9 Jan 2019 18:45:47 -0800 Subject: [PATCH] Add client support for count diverging commits Adds the client call for the gitaly rpc CountDivergingCommits fixing signature simplifying commit logic adding test for max-count refactoring tests --- GITALY_SERVER_VERSION | 2 +- app/models/repository.rb | 9 +- ...erformance-for-diverging-commit-counts.yml | 5 ++ lib/gitlab/git/repository.rb | 7 ++ lib/gitlab/gitaly_client/commit_service.rb | 11 +++ spec/lib/gitlab/git/repository_spec.rb | 86 +++++++++++++++++++ 6 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/improve-performance-for-diverging-commit-counts.yml diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 744068368fb..84cc529467b 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.18.0 \ No newline at end of file +1.18.0 diff --git a/app/models/repository.rb b/app/models/repository.rb index 7c50b4488e5..ed55a6e572b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -288,13 +288,16 @@ class Repository # Rugged seems to throw a `ReferenceError` when given branch_names rather # than SHA-1 hashes number_commits_behind, number_commits_ahead = - raw_repository.count_commits_between( + raw_repository.diverging_commit_count( @root_ref_hash, branch.dereferenced_target.sha, - left_right: true, max_count: MAX_DIVERGING_COUNT) - { behind: number_commits_behind, ahead: number_commits_ahead } + if number_commits_behind + number_commits_ahead >= MAX_DIVERGING_COUNT + { distance: MAX_DIVERGING_COUNT } + else + { behind: number_commits_behind, ahead: number_commits_ahead } + end end end diff --git a/changelogs/unreleased/improve-performance-for-diverging-commit-counts.yml b/changelogs/unreleased/improve-performance-for-diverging-commit-counts.yml new file mode 100644 index 00000000000..76ff15cba5b --- /dev/null +++ b/changelogs/unreleased/improve-performance-for-diverging-commit-counts.yml @@ -0,0 +1,5 @@ +--- +title: Improve performance for diverging commit counts +merge_request: 24287 +author: +type: performance diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 54bbd531398..593a3676519 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -491,6 +491,13 @@ module Gitlab end end + # Return total diverging commits count + def diverging_commit_count(from, to, max_count:) + wrapped_gitaly_errors do + gitaly_commit_client.diverging_commit_count(from, to, max_count: max_count) + end + end + # Mimic the `git clean` command and recursively delete untracked files. # Valid keys that can be passed in the +options+ hash are: # diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 4e46cb9f05c..ea12424eb4a 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -150,6 +150,17 @@ module Gitlab GitalyClient.call(@repository.storage, :commit_service, :count_commits, request, timeout: GitalyClient.medium_timeout).count end + def diverging_commit_count(from, to, max_count:) + request = Gitaly::CountDivergingCommitsRequest.new( + repository: @gitaly_repo, + from: encode_binary(from), + to: encode_binary(to), + max_count: max_count + ) + response = GitalyClient.call(@repository.storage, :commit_service, :count_diverging_commits, request, timeout: GitalyClient.medium_timeout) + [response.left_count, response.right_count] + end + def list_last_commits_for_tree(revision, path, offset: 0, limit: 25) request = Gitaly::ListLastCommitsForTreeRequest.new( repository: @gitaly_repo, diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index cf9e0cccc71..aef4e433439 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -283,6 +283,92 @@ describe Gitlab::Git::Repository, :seed_helper do end end + describe '#diverging_commit_count' do + it 'counts 0 for the same branch' do + expect(repository.diverging_commit_count('master', 'master', max_count: 1000)).to eq([0, 0]) + end + + context 'max count does not truncate results' do + where(:left, :right, :expected) do + 1 | 1 | [1, 1] + 4 | 4 | [4, 4] + 2 | 2 | [2, 2] + 2 | 4 | [2, 4] + 4 | 2 | [4, 2] + 10 | 10 | [10, 10] + end + + with_them do + before do + repository.create_branch('left-branch', 'master') + repository.create_branch('right-branch', 'master') + left.times do + new_commit_edit_new_file_on_branch(repository_rugged, 'encoding/CHANGELOG', 'left-branch', 'some more content for a', 'some stuff') + end + + right.times do + new_commit_edit_new_file_on_branch(repository_rugged, 'encoding/CHANGELOG', 'right-branch', 'some more content for b', 'some stuff') + end + end + + after do + repository.delete_branch('left-branch') + repository.delete_branch('right-branch') + end + + it 'returns the correct count bounding at max_count' do + branch_a_sha = repository_rugged.branches['left-branch'].target.oid + branch_b_sha = repository_rugged.branches['right-branch'].target.oid + expect( + repository.diverging_commit_count( + branch_a_sha, branch_b_sha, max_count: 1000) + ).to eq(expected) + end + end + end + + context 'max count truncates results' do + where(:left, :right, :max_count) do + 1 | 1 | 1 + 4 | 4 | 4 + 2 | 2 | 3 + 2 | 4 | 3 + 4 | 2 | 5 + 10 | 10 | 10 + end + + with_them do + before do + repository.create_branch('left-branch', 'master') + repository.create_branch('right-branch', 'master') + left.times do + new_commit_edit_new_file_on_branch(repository_rugged, 'encoding/CHANGELOG', 'left-branch', 'some more content for a', 'some stuff') + end + + right.times do + new_commit_edit_new_file_on_branch(repository_rugged, 'encoding/CHANGELOG', 'right-branch', 'some more content for b', 'some stuff') + end + end + + after do + repository.delete_branch('left-branch') + repository.delete_branch('right-branch') + end + + it 'returns the correct count bounding at max_count' do + branch_a_sha = repository_rugged.branches['left-branch'].target.oid + branch_b_sha = repository_rugged.branches['right-branch'].target.oid + results = repository.diverging_commit_count(branch_a_sha, branch_b_sha, max_count: max_count) + expect(results[0] + results[1]).to eq(max_count) + end + end + end + + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::CommitService, :diverging_commit_count do + subject { repository.diverging_commit_count('master', 'master', max_count: 1000) } + end + end + describe '#has_local_branches?' do context 'check for local branches' do it { expect(repository.has_local_branches?).to eq(true) }