Commit bc513a5d authored by Vitali Tatarintev's avatar Vitali Tatarintev

Merge branch '21107-optimize-sql-queries-in-settings-ci-cd-controller' into 'master'

Preload Ci::Runner tags to prevent N+1 queries [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!59625
parents 00e709db ba77dc7f
...@@ -119,12 +119,13 @@ module Projects ...@@ -119,12 +119,13 @@ module Projects
.assignable_for(project) .assignable_for(project)
.ordered .ordered
.page(params[:specific_page]).per(NUMBER_OF_RUNNERS_PER_PAGE) .page(params[:specific_page]).per(NUMBER_OF_RUNNERS_PER_PAGE)
.with_tags
@shared_runners = ::Ci::Runner.instance_type.active @shared_runners = ::Ci::Runner.instance_type.active.with_tags
@shared_runners_count = @shared_runners.count(:all) @shared_runners_count = @shared_runners.count(:all)
@group_runners = ::Ci::Runner.belonging_to_parent_group_of_project(@project.id) @group_runners = ::Ci::Runner.belonging_to_parent_group_of_project(@project.id).with_tags
end end
def define_ci_variables def define_ci_variables
......
---
title: Optimize CI Settings page to reduce N+1 queries
merge_request: 59625
author:
type: performance
...@@ -14,6 +14,10 @@ RSpec.describe Projects::Settings::CiCdController do ...@@ -14,6 +14,10 @@ RSpec.describe Projects::Settings::CiCdController do
end end
describe 'GET show' do describe 'GET show' do
let_it_be(:parent_group) { create(:group) }
let_it_be(:group) { create(:group, parent: parent_group) }
let_it_be(:other_project) { create(:project, group: group) }
it 'renders show with 200 status code' do it 'renders show with 200 status code' do
get :show, params: { namespace_id: project.namespace, project_id: project } get :show, params: { namespace_id: project.namespace, project_id: project }
...@@ -22,12 +26,9 @@ RSpec.describe Projects::Settings::CiCdController do ...@@ -22,12 +26,9 @@ RSpec.describe Projects::Settings::CiCdController do
end end
context 'with group runners' do context 'with group runners' do
let(:parent_group) { create(:group) } let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group]) }
let(:group) { create(:group, parent: parent_group) } let_it_be(:project_runner) { create(:ci_runner, :project, projects: [other_project]) }
let(:group_runner) { create(:ci_runner, :group, groups: [group]) } let_it_be(:shared_runner) { create(:ci_runner, :instance) }
let(:other_project) { create(:project, group: group) }
let!(:project_runner) { create(:ci_runner, :project, projects: [other_project]) }
let!(:shared_runner) { create(:ci_runner, :instance) }
it 'sets assignable project runners only' do it 'sets assignable project runners only' do
group.add_maintainer(user) group.add_maintainer(user)
...@@ -37,6 +38,33 @@ RSpec.describe Projects::Settings::CiCdController do ...@@ -37,6 +38,33 @@ RSpec.describe Projects::Settings::CiCdController do
expect(assigns(:assignable_runners)).to contain_exactly(project_runner) expect(assigns(:assignable_runners)).to contain_exactly(project_runner)
end end
end end
context 'prevents N+1 queries for tags' do
render_views
def show
get :show, params: { namespace_id: project.namespace, project_id: project }
end
it 'has the same number of queries with one tag or with many tags', :request_store do
group.add_maintainer(user)
show # warmup
# with one tag
create(:ci_runner, :instance, tag_list: %w(shared_runner))
create(:ci_runner, :project, projects: [other_project], tag_list: %w(project_runner))
create(:ci_runner, :group, groups: [group], tag_list: %w(group_runner))
control = ActiveRecord::QueryRecorder.new { show }
# with several tags
create(:ci_runner, :instance, tag_list: %w(shared_runner tag2 tag3))
create(:ci_runner, :project, projects: [other_project], tag_list: %w(project_runner tag2 tag3))
create(:ci_runner, :group, groups: [group], tag_list: %w(group_runner tag2 tag3))
expect { show }.not_to exceed_query_limit(control)
end
end
end end
describe '#reset_cache' do describe '#reset_cache' 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