diff options
| -rw-r--r-- | app/finders/admin/runners_finder.rb | 2 | ||||
| -rw-r--r-- | app/models/ci/runner.rb | 1 | ||||
| -rw-r--r-- | app/views/admin/runners/_runner.html.haml | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/sh-remove-nplusone-admin-runners-tags.yml | 5 | ||||
| -rw-r--r-- | spec/controllers/admin/runners_controller_spec.rb | 19 |
5 files changed, 26 insertions, 3 deletions
diff --git a/app/finders/admin/runners_finder.rb b/app/finders/admin/runners_finder.rb index fbb1cfc5c66..8d936b8121c 100644 --- a/app/finders/admin/runners_finder.rb +++ b/app/finders/admin/runners_finder.rb @@ -14,7 +14,7 @@ class Admin::RunnersFinder < UnionFinder sort! paginate! - @runners + @runners.with_tags end def sort_key diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 5aae31de6e2..d82e11bbb89 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -97,6 +97,7 @@ module Ci scope :order_contacted_at_asc, -> { order(contacted_at: :asc) } scope :order_created_at_desc, -> { order(created_at: :desc) } + scope :with_tags, -> { preload(:tags) } validate :tag_constraints validates :access_level, presence: true diff --git a/app/views/admin/runners/_runner.html.haml b/app/views/admin/runners/_runner.html.haml index 4641986cb56..ecf2b1d60ba 100644 --- a/app/views/admin/runners/_runner.html.haml +++ b/app/views/admin/runners/_runner.html.haml @@ -49,7 +49,7 @@ .table-section.section-10.section-wrap .table-mobile-header{ role: 'rowheader' }= _('Tags') .table-mobile-content - - runner.tag_list.sort.each do |tag| + - runner.tags.map(&:name).sort.each do |tag| %span.badge.badge-primary = tag diff --git a/changelogs/unreleased/sh-remove-nplusone-admin-runners-tags.yml b/changelogs/unreleased/sh-remove-nplusone-admin-runners-tags.yml new file mode 100644 index 00000000000..f8ac345bc95 --- /dev/null +++ b/changelogs/unreleased/sh-remove-nplusone-admin-runners-tags.yml @@ -0,0 +1,5 @@ +--- +title: Remove N+1 query for tags in /admin/runners page +merge_request: 25572 +author: +type: performance diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index 4cf14030ca1..82e24213408 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -1,18 +1,35 @@ require 'spec_helper' describe Admin::RunnersController do - let(:runner) { create(:ci_runner) } + let!(:runner) { create(:ci_runner) } before do sign_in(create(:admin)) end describe '#index' do + render_views + it 'lists all runners' do get :index expect(response).to have_gitlab_http_status(200) end + + it 'avoids N+1 queries', :request_store do + get :index + + control_count = ActiveRecord::QueryRecorder.new { get :index }.count + + create(:ci_runner, :tagged_only) + + # There is still an N+1 query for `runner.builds.count` + expect { get :index }.not_to exceed_query_limit(control_count + 1) + + expect(response).to have_gitlab_http_status(200) + expect(response.body).to have_content('tag1') + expect(response.body).to have_content('tag2') + end end describe '#show' do |