summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohn Cai <jcai@gitlab.com>2019-05-06 16:57:44 -0700
committerJohn Cai <jcai@gitlab.com>2019-05-14 12:13:19 -0500
commitf86797b5a91791875ccd1a02806d4cab236c1ac6 (patch)
treee48b1e9bcf1e068baf84d4780e65c25b5c9a9110
parente1aa7f5074aab8190d16a6f25121d33b99a07238 (diff)
downloadgitlab-ce-jc-omit-count-diverging-commits-max.tar.gz
Omit max-count for diverging_commit_counts behind feature flagjc-omit-count-diverging-commits-max
We want to optimize the query for the CountDivergingCommits rpc by removing the --max-count argument now that we have commit graphs enabled for all repositories during housekeeping. However, we want to test this first behind a feature flag.
-rw-r--r--app/models/repository.rb23
-rw-r--r--changelogs/unreleased/jc-omit-count-diverging-commits-max.yml5
-rw-r--r--lib/gitlab/git/repository.rb2
-rw-r--r--spec/models/repository_spec.rb37
4 files changed, 63 insertions, 4 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb
index be17b54ff12..5a003f25944 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -283,14 +283,19 @@ class Repository
end
def diverging_commit_counts(branch)
+ return diverging_commit_counts_without_max(branch) if Feature.enabled?('gitaly_count_diverging_commits_no_max')
+
+ ## TODO: deprecate the below code after 12.0
@root_ref_hash ||= raw_repository.commit(root_ref).id
cache.fetch(:"diverging_commit_counts_#{branch.name}") do
# Rugged seems to throw a `ReferenceError` when given branch_names rather
# than SHA-1 hashes
+ branch_sha = branch.dereferenced_target.sha
+
number_commits_behind, number_commits_ahead =
raw_repository.diverging_commit_count(
@root_ref_hash,
- branch.dereferenced_target.sha,
+ branch_sha,
max_count: MAX_DIVERGING_COUNT)
if number_commits_behind + number_commits_ahead >= MAX_DIVERGING_COUNT
@@ -301,6 +306,22 @@ class Repository
end
end
+ def diverging_commit_counts_without_max(branch)
+ @root_ref_hash ||= raw_repository.commit(root_ref).id
+ cache.fetch(:"diverging_commit_counts_without_max_#{branch.name}") do
+ # Rugged seems to throw a `ReferenceError` when given branch_names rather
+ # than SHA-1 hashes
+ branch_sha = branch.dereferenced_target.sha
+
+ number_commits_behind, number_commits_ahead =
+ raw_repository.diverging_commit_count(
+ @root_ref_hash,
+ branch_sha)
+
+ { behind: number_commits_behind, ahead: number_commits_ahead }
+ end
+ end
+
def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:, path: nil)
raw_repository.archive_metadata(
ref,
diff --git a/changelogs/unreleased/jc-omit-count-diverging-commits-max.yml b/changelogs/unreleased/jc-omit-count-diverging-commits-max.yml
new file mode 100644
index 00000000000..23235060a98
--- /dev/null
+++ b/changelogs/unreleased/jc-omit-count-diverging-commits-max.yml
@@ -0,0 +1,5 @@
+---
+title: Omit max-count for diverging_commit_counts behind feature flag
+merge_request: 28157
+author:
+type: other
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index fc9bcbdcca2..455588f3c66 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -500,7 +500,7 @@ module Gitlab
end
# Return total diverging commits count
- def diverging_commit_count(from, to, max_count:)
+ def diverging_commit_count(from, to, max_count: 0)
wrapped_gitaly_errors do
gitaly_commit_client.diverging_commit_count(from, to, max_count: max_count)
end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 9ff0f355fd4..c5ab7e57272 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -2286,12 +2286,45 @@ describe Repository do
end
describe '#diverging_commit_counts' do
+ let(:diverged_branch) { repository.find_branch('fix') }
+ let(:root_ref_sha) { repository.raw_repository.commit(repository.root_ref).id }
+ let(:diverged_branch_sha) { diverged_branch.dereferenced_target.sha }
+
it 'returns the commit counts behind and ahead of default branch' do
- result = repository.diverging_commit_counts(
- repository.find_branch('fix'))
+ result = repository.diverging_commit_counts(diverged_branch)
expect(result).to eq(behind: 29, ahead: 2)
end
+
+ context 'when gitaly_count_diverging_commits_no_max is enabled' do
+ before do
+ stub_feature_flags(gitaly_count_diverging_commits_no_max: true)
+ end
+
+ it 'calls diverging_commit_count without max count' do
+ expect(repository.raw_repository)
+ .to receive(:diverging_commit_count)
+ .with(root_ref_sha, diverged_branch_sha)
+ .and_return([29, 2])
+
+ repository.diverging_commit_counts(diverged_branch)
+ end
+ end
+
+ context 'when gitaly_count_diverging_commits_no_max is disabled' do
+ before do
+ stub_feature_flags(gitaly_count_diverging_commits_no_max: false)
+ end
+
+ it 'calls diverging_commit_count with max count' do
+ expect(repository.raw_repository)
+ .to receive(:diverging_commit_count)
+ .with(root_ref_sha, diverged_branch_sha, max_count: Repository::MAX_DIVERGING_COUNT)
+ .and_return([29, 2])
+
+ repository.diverging_commit_counts(diverged_branch)
+ end
+ end
end
describe '#refresh_method_caches' do