From e7bd824484636fd4d4d7beb524b6bea3ecef533a Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 25 Dec 2018 23:34:47 -0800 Subject: 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 --- changelogs/unreleased/sh-fix-branches-api-timeout.yml | 5 +++++ lib/api/branches.rb | 4 ++-- spec/requests/api/branches_spec.rb | 12 ++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/sh-fix-branches-api-timeout.yml 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' -- cgit v1.2.1