Commit 6c63f96e authored by Grzegorz Bizon's avatar Grzegorz Bizon

Preload number of warnings in every stage in a pipeline

This makes it possible to avoid N+1 queries when loading pipelines
table.
parent 9520d2ff
...@@ -24,7 +24,7 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -24,7 +24,7 @@ class Projects::PipelinesController < Projects::ApplicationController
@finished_count = limited_pipelines_count(project, 'finished') @finished_count = limited_pipelines_count(project, 'finished')
@pipelines_count = limited_pipelines_count(project) @pipelines_count = limited_pipelines_count(project)
Gitlab::Ci::Pipeline::Preloader.preload(@project, @pipelines) Gitlab::Ci::Pipeline::Preloader.preload(@pipelines)
respond_to do |format| respond_to do |format|
format.html format.html
......
...@@ -411,7 +411,7 @@ module Ci ...@@ -411,7 +411,7 @@ module Ci
def number_of_warnings def number_of_warnings
BatchLoader.for(id).batch(default_value: 0) do |pipeline_ids, loader| BatchLoader.for(id).batch(default_value: 0) do |pipeline_ids, loader|
Build.where(commit_id: pipeline_ids) ::Ci::Build.where(commit_id: pipeline_ids)
.latest .latest
.failed_but_allowed .failed_but_allowed
.group(:commit_id) .group(:commit_id)
......
...@@ -89,7 +89,18 @@ module Ci ...@@ -89,7 +89,18 @@ module Ci
end end
def has_warnings? def has_warnings?
statuses.latest.failed_but_allowed.any? number_of_warnings.positive?
end
def number_of_warnings
BatchLoader.for(id).batch(default_value: 0) do |stage_ids, loader|
::Ci::Build.where(stage_id: stage_ids)
.latest
.failed_but_allowed
.group(:stage_id)
.count
.each { |id, amount| loader.call(id, amount) }
end
end end
def detailed_status(current_user) def detailed_status(current_user)
......
...@@ -6,7 +6,7 @@ module Gitlab ...@@ -6,7 +6,7 @@ module Gitlab
# Class for preloading data associated with pipelines such as commit # Class for preloading data associated with pipelines such as commit
# authors. # authors.
module Preloader module Preloader
def self.preload(project, pipelines) def self.preload(pipelines)
# This ensures that all the pipeline commits are eager loaded before we # This ensures that all the pipeline commits are eager loaded before we
# start using them. # start using them.
pipelines.each(&:commit) pipelines.each(&:commit)
...@@ -20,6 +20,11 @@ module Gitlab ...@@ -20,6 +20,11 @@ module Gitlab
# that Ci::Pipeline#has_warnings? doesn't execute any additional # that Ci::Pipeline#has_warnings? doesn't execute any additional
# queries. # queries.
pipeline.number_of_warnings pipeline.number_of_warnings
# This preloads the number of warnings for every stage, ensuring
# that Ci::Stage#has_warnings? doesn't execute any additional
# queries.
pipeline.stages.each { |stage| stage.number_of_warnings }
end end
end end
end end
......
...@@ -17,35 +17,27 @@ describe Projects::PipelinesController do ...@@ -17,35 +17,27 @@ describe Projects::PipelinesController do
describe 'GET index.json' do describe 'GET index.json' do
before do before do
%w(pending running created success).each_with_index do |status, index| %w(pending running running success canceled)
sha = project.commit("HEAD~#{index}") .each_with_index do |status, index|
create_pipeline(status, project.commit("HEAD~#{index}"))
pipeline = create(:ci_empty_pipeline, status: status, end
project: project,
sha: sha)
create_build(pipeline, 'test', 1, 'unit')
create_build(pipeline, 'test', 1, 'feature')
create_build(pipeline, 'review', 2, 'staging')
create_build(pipeline, 'deploy', 3, 'production')
end
end end
it 'returns JSON with serialized pipelines', :request_store do it 'returns JSON with serialized pipelines', :request_store do
queries = ActiveRecord::QueryRecorder.new { get_pipelines_index_json } queries = ActiveRecord::QueryRecorder.new do
get_pipelines_index_json
end
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('pipeline') expect(response).to match_response_schema('pipeline')
expect(json_response).to include('pipelines') expect(json_response).to include('pipelines')
expect(json_response['pipelines'].count).to eq 4 expect(json_response['pipelines'].count).to eq 5
expect(json_response['count']['all']).to eq '4' expect(json_response['count']['all']).to eq '5'
expect(json_response['count']['running']).to eq '1' expect(json_response['count']['running']).to eq '2'
expect(json_response['count']['pending']).to eq '1' expect(json_response['count']['pending']).to eq '1'
expect(json_response['count']['finished']).to eq '1' expect(json_response['count']['finished']).to eq '2'
puts queries.log expect(queries.count).to be < 32
expect(queries.count).to be < 25
end end
it 'does not include coverage data for the pipelines' do it 'does not include coverage data for the pipelines' do
...@@ -67,8 +59,19 @@ describe Projects::PipelinesController do ...@@ -67,8 +59,19 @@ describe Projects::PipelinesController do
format: :json format: :json
end end
def create_pipeline(status, sha)
pipeline = create(:ci_empty_pipeline, status: status,
project: project,
sha: sha)
create_build(pipeline, 'build', 1, 'build')
create_build(pipeline, 'test', 2, 'test')
create_build(pipeline, 'deploy', 3, 'deploy')
end
def create_build(pipeline, stage, stage_idx, name) def create_build(pipeline, stage, stage_idx, name)
create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name) status = %w[created running pending success failed canceled].sample
create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name, status: status)
end end
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