summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2018-12-25 23:34:47 -0800
committerStan Hu <stanhu@gmail.com>2018-12-27 04:24:37 -0800
commite7bd824484636fd4d4d7beb524b6bea3ecef533a (patch)
tree4b435626be5630c408e5ef4323420fe74a82118a
parent77909a88460bbc864a5f95f3fa66053eb6cab5a8 (diff)
downloadgitlab-ce-e7bd824484636fd4d4d7beb524b6bea3ecef533a.tar.gz
Fix timeout issues retrieving branches via API
47d4890d changed the order of pagination so that the full list of branches would be passed to Gitaly to determine which ones had been merged, but this operation can timeout for large repositories with many branches. We only need to determine whether the found branches have been merged, so limit the scan to those. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/55724
-rw-r--r--changelogs/unreleased/sh-fix-branches-api-timeout.yml5
-rw-r--r--lib/api/branches.rb4
-rw-r--r--spec/requests/api/branches_spec.rb12
3 files changed, 19 insertions, 2 deletions
diff --git a/changelogs/unreleased/sh-fix-branches-api-timeout.yml b/changelogs/unreleased/sh-fix-branches-api-timeout.yml
new file mode 100644
index 00000000000..8cd29a7269d
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-branches-api-timeout.yml
@@ -0,0 +1,5 @@
+---
+title: Fix timeout issues retrieving branches via API
+merge_request: 24034
+author:
+type: performance
diff --git a/lib/api/branches.rb b/lib/api/branches.rb
index e7e58ad0e66..07f529b01bb 100644
--- a/lib/api/branches.rb
+++ b/lib/api/branches.rb
@@ -34,11 +34,11 @@ module API
repository = user_project.repository
branches = BranchesFinder.new(repository, declared_params(include_missing: false)).execute
-
+ branches = ::Kaminari.paginate_array(branches)
merged_branch_names = repository.merged_branch_names(branches.map(&:name))
present(
- paginate(::Kaminari.paginate_array(branches)),
+ paginate(branches),
with: Entities::Branch,
current_user: current_user,
project: user_project,
diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb
index 93c411476bb..b38cd66986f 100644
--- a/spec/requests/api/branches_spec.rb
+++ b/spec/requests/api/branches_spec.rb
@@ -20,6 +20,12 @@ describe API::Branches do
let(:route) { "/projects/#{project_id}/repository/branches" }
shared_examples_for 'repository branches' do
+ RSpec::Matchers.define :has_merged_branch_names_count do |expected|
+ match do |actual|
+ actual[:merged_branch_names].count == expected
+ end
+ end
+
it 'returns the repository branches' do
get api(route, current_user), params: { per_page: 100 }
@@ -30,6 +36,12 @@ describe API::Branches do
expect(branch_names).to match_array(project.repository.branch_names)
end
+ it 'determines only a limited number of merged branch names' do
+ expect(API::Entities::Branch).to receive(:represent).with(anything, has_merged_branch_names_count(2))
+
+ get api(route, current_user), params: { per_page: 2 }
+ end
+
context 'when repository is disabled' do
include_context 'disabled repository'