Commit f1f39db1 authored by Nick Thomas's avatar Nick Thomas

Use the ListCommits RPC for Gitlab::Git::Commit.between

The CommitsBetween RPC is deprecated in favour of the new ListCommits
RPC, so switch to that.

ListCommits has a working pagination implementation, so this change
helps us to reduce the number of commits we pull from Gitaly.
parent edf88a73
---
name: between_uses_list_commits
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67591
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/337960
milestone: '14.2'
type: development
group: group::source code
default_enabled: false
......@@ -111,10 +111,20 @@ module Gitlab
# Commit.between(repo, '29eda46b', 'master')
#
def between(repo, base, head)
# In either of these cases, we are guaranteed to return no commits, so
# shortcut the RPC call
return [] if Gitlab::Git.blank_ref?(base) || Gitlab::Git.blank_ref?(head)
wrapped_gitaly_errors do
if Feature.enabled?(:between_uses_list_commits, default_enabled: :yaml)
revisions = [head, "^#{base}"] # base..head
repo.gitaly_commit_client.list_commits(revisions, reverse: true)
else
repo.gitaly_commit_client.between(base, head)
end
end
end
# Returns commits collection
#
......
......@@ -255,10 +255,11 @@ module Gitlab
consume_commits_response(response)
end
def list_commits(revisions)
def list_commits(revisions, reverse: false)
request = Gitaly::ListCommitsRequest.new(
repository: @gitaly_repo,
revisions: Array.wrap(revisions)
revisions: Array.wrap(revisions),
reverse: reverse
)
response = GitalyClient.call(@repository.storage, :commit_service, :list_commits, request, timeout: GitalyClient.medium_timeout)
......
......@@ -369,11 +369,15 @@ RSpec.describe Gitlab::Git::Commit, :seed_helper do
commits.map { |c| c.id }
end
it 'has 1 element' do
expect(subject.size).to eq(1)
it { is_expected.to contain_exactly(SeedRepo::Commit::ID) }
context 'between_uses_list_commits FF disabled' do
before do
stub_feature_flags(between_uses_list_commits: false)
end
it { is_expected.to contain_exactly(SeedRepo::Commit::ID) }
end
it { is_expected.to include(SeedRepo::Commit::ID) }
it { is_expected.not_to include(SeedRepo::FirstCommit::ID) }
end
describe '.shas_with_signatures' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment