Commit 3f543cd2 authored by Kamil Trzciński's avatar Kamil Trzciński

Fix N+1 problem in `JobsController#index`

This adds missing preloads, and introduces
additional n+1 matcher to look for
duplicates.
parent 8b55aaee
...@@ -31,8 +31,12 @@ class Projects::JobsController < Projects::ApplicationController ...@@ -31,8 +31,12 @@ class Projects::JobsController < Projects::ApplicationController
@builds @builds
end end
@builds = @builds.includes([ @builds = @builds.includes([
{ pipeline: :project }, { pipeline: [:project, :user] },
:job_artifacts_archive,
:metadata,
:trigger_request,
:project, :project,
:user,
:tags :tags
]) ])
@builds = @builds.page(params[:page]).per(30).without_count @builds = @builds.page(params[:page]).per(30).without_count
......
...@@ -73,21 +73,27 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do ...@@ -73,21 +73,27 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
end end
context 'number of queries' do context 'number of queries' do
render_views
before do before do
Ci::Build::AVAILABLE_STATUSES.each do |status| Ci::Build::AVAILABLE_STATUSES.each do |status|
create_job(status, status) create_job(status, status)
end end
allow(Appearance).to receive(:current_without_cache)
.and_return(nil)
end end
it 'verifies number of queries', :request_store do it 'verifies number of queries', :request_store do
recorded = ActiveRecord::QueryRecorder.new { get_index } expect { get_index }.not_to be_n_plus_1_query.with_threshold(3)
expect(recorded.count).to be_within(5).of(7)
end end
def create_job(name, status) def create_job(name, status)
pipeline = create(:ci_pipeline, project: project) user = create(:user)
pipeline = create(:ci_pipeline, project: project, user: user)
create(:ci_build, :tags, :triggered, :artifacts, create(:ci_build, :tags, :triggered, :artifacts,
pipeline: pipeline, name: name, status: status) pipeline: pipeline, name: name, status: status,
user: user)
end end
end end
......
...@@ -35,5 +35,9 @@ module ActiveRecord ...@@ -35,5 +35,9 @@ module ActiveRecord
def log_message def log_message
@log.join("\n\n") @log.join("\n\n")
end end
def occurrences
@log.group_by(&:to_s).transform_values(&:count)
end
end end
end end
# frozen_string_literal: true
module Nplus1QueryHelpers
DEFAULT_THRESHOLD = 3
def with_threshold(threshold)
@threshold = threshold
self
end
def for_query(query)
@query = query
self
end
def threshold
@threshold || DEFAULT_THRESHOLD
end
def occurrences
@occurrences ||=
if @query
recorder.occurrences.select { |recorded, count| recorded =~ @query }
else
recorder.occurrences
end
end
def over_threshold
occurrences.select do |recorded, count|
count > threshold
end
end
def recorder
@recorder ||= ActiveRecord::QueryRecorder.new(&@subject_block)
end
def verify_count(&block)
@subject_block = block
over_threshold.present?
end
def failure_message
log_message = over_threshold.to_yaml
"The following queries are executed more than #{threshold} time(s):\n#{log_message}"
end
end
RSpec::Matchers.define :be_n_plus_1_query do
supports_block_expectations
include Nplus1QueryHelpers
match do |block|
verify_count(&block)
end
failure_message_when_negated do |actual|
failure_message
end
end
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