Commit ebb284c0 authored by Stan Hu's avatar Stan Hu

Remove N+1 query for tags in /admin/runners page

As discussed in https://github.com/mbleigh/acts-as-taggable-on/issues/91,
we can avoid N+1 queries if we use `tags` instead of `tag_list`.

Seen while reviewing
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19740.
parent 3395eacb
......@@ -14,7 +14,7 @@ class Admin::RunnersFinder < UnionFinder
sort!
paginate!
@runners
@runners.with_tags
end
def sort_key
......
......@@ -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
......
......@@ -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
......
---
title: Remove N+1 query for tags in /admin/runners page
merge_request: 25572
author:
type: performance
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
......
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