Commit e7bd8244 authored by Stan Hu's avatar Stan Hu

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
parent 77909a88
---
title: Fix timeout issues retrieving branches via API
merge_request: 24034
author:
type: performance
...@@ -34,11 +34,11 @@ module API ...@@ -34,11 +34,11 @@ module API
repository = user_project.repository repository = user_project.repository
branches = BranchesFinder.new(repository, declared_params(include_missing: false)).execute 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)) merged_branch_names = repository.merged_branch_names(branches.map(&:name))
present( present(
paginate(::Kaminari.paginate_array(branches)), paginate(branches),
with: Entities::Branch, with: Entities::Branch,
current_user: current_user, current_user: current_user,
project: user_project, project: user_project,
......
...@@ -20,6 +20,12 @@ describe API::Branches do ...@@ -20,6 +20,12 @@ describe API::Branches do
let(:route) { "/projects/#{project_id}/repository/branches" } let(:route) { "/projects/#{project_id}/repository/branches" }
shared_examples_for 'repository branches' do 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 it 'returns the repository branches' do
get api(route, current_user), params: { per_page: 100 } get api(route, current_user), params: { per_page: 100 }
...@@ -30,6 +36,12 @@ describe API::Branches do ...@@ -30,6 +36,12 @@ describe API::Branches do
expect(branch_names).to match_array(project.repository.branch_names) expect(branch_names).to match_array(project.repository.branch_names)
end 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 context 'when repository is disabled' do
include_context 'disabled repository' include_context 'disabled repository'
......
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