Commit 13f42b92 authored by Grzegorz Bizon's avatar Grzegorz Bizon
Browse files

Use persisted stages to load pipelines index table

parent 09ebfeb8c174
class Projects::PipelinesController < Projects::ApplicationController class Projects::PipelinesController < Projects::ApplicationController
before_action :whitelist_query_limiting, only: [:create, :retry] # before_action :whitelist_query_limiting, only: [:create, :retry]
before_action :pipeline, except: [:index, :new, :create, :charts] before_action :pipeline, except: [:index, :new, :create, :charts]
before_action :commit, only: [:show, :builds, :failures] before_action :commit, only: [:show, :builds, :failures]
before_action :authorize_read_pipeline! before_action :authorize_read_pipeline!
...@@ -15,6 +15,7 @@ def index ...@@ -15,6 +15,7 @@ def index
@pipelines = PipelinesFinder @pipelines = PipelinesFinder
.new(project, scope: @scope) .new(project, scope: @scope)
.execute .execute
.preload(:stages)
.page(params[:page]) .page(params[:page])
.per(30) .per(30)
...@@ -23,7 +24,7 @@ def index ...@@ -23,7 +24,7 @@ def index
@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(@pipelines) Gitlab::Ci::Pipeline::Preloader.preload(@project, @pipelines)
respond_to do |format| respond_to do |format|
format.html format.html
......
...@@ -13,7 +13,7 @@ class Pipeline < ActiveRecord::Base ...@@ -13,7 +13,7 @@ class Pipeline < ActiveRecord::Base
belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline' belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline'
belongs_to :pipeline_schedule, class_name: 'Ci::PipelineSchedule' belongs_to :pipeline_schedule, class_name: 'Ci::PipelineSchedule'
has_many :stages has_many :stages, inverse_of: :pipeline # -> { order(position: :asc) }, inverse_of: :pipeline
has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline
has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline
has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent
......
...@@ -79,5 +79,23 @@ def update_status ...@@ -79,5 +79,23 @@ def update_status
end end
end end
end end
def groups
@groups ||= statuses.ordered.latest
.sort_by(&:sortable_name).group_by(&:group_name)
.map do |group_name, grouped_statuses|
Ci::Group.new(self, name: group_name, jobs: grouped_statuses)
end
end
def has_warnings?
statuses.latest.failed_but_allowed.any?
end
def detailed_status(current_user)
Gitlab::Ci::Status::Stage::Factory
.new(self, current_user)
.fabricate!
end
end end
end end
...@@ -227,6 +227,7 @@ class Project < ActiveRecord::Base ...@@ -227,6 +227,7 @@ class Project < ActiveRecord::Base
has_many :commit_statuses has_many :commit_statuses
has_many :pipelines, class_name: 'Ci::Pipeline', inverse_of: :project has_many :pipelines, class_name: 'Ci::Pipeline', inverse_of: :project
has_many :stages, class_name: 'Ci::Stage', inverse_of: :project
# Ci::Build objects store data on the file system such as artifact files and # Ci::Build objects store data on the file system such as artifact files and
# build traces. Currently there's no efficient way of removing this data in # build traces. Currently there's no efficient way of removing this data in
......
class PipelineDetailsEntity < PipelineEntity class PipelineDetailsEntity < PipelineEntity
expose :details do expose :details do
expose :legacy_stages, as: :stages, using: StageEntity ##
# TODO consider switching to persisted stages only in pipelines table
# (not necessairly in the show pipeline page because of #23257.
# Hide this behind two feature flags - enabled / disabled and only
# gitlab-ce / everywhere.
expose :stages, as: :stages, using: StageEntity
expose :artifacts, using: BuildArtifactEntity expose :artifacts, using: BuildArtifactEntity
expose :manual_actions, using: BuildActionEntity expose :manual_actions, using: BuildActionEntity
end end
......
...@@ -6,7 +6,7 @@ module Pipeline ...@@ -6,7 +6,7 @@ module Pipeline
# 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(pipelines) def self.preload(project, 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)
......
...@@ -8,7 +8,9 @@ def has_details? ...@@ -8,7 +8,9 @@ def has_details?
end end
def details_path def details_path
project_pipeline_path(subject.project, subject.pipeline, anchor: subject.name) project_pipeline_path(subject.pipeline.project,
subject.pipeline,
anchor: subject.name)
end end
def has_action? def has_action?
......
...@@ -47,6 +47,8 @@ def initialize ...@@ -47,6 +47,8 @@ def initialize
# Sends a notification based on the number of executed SQL queries. # Sends a notification based on the number of executed SQL queries.
def act_upon_results def act_upon_results
puts "XXXX\n\n\n\n\n #{count} \n\n\nXXXX"
return unless threshold_exceeded? return unless threshold_exceeded?
error = ThresholdExceededError.new(error_message) error = ThresholdExceededError.new(error_message)
......
...@@ -19,16 +19,21 @@ ...@@ -19,16 +19,21 @@
before do before do
%w(pending running created success).each_with_index do |status, index| %w(pending running created success).each_with_index do |status, index|
sha = project.commit("HEAD~#{index}") sha = project.commit("HEAD~#{index}")
create(:ci_empty_pipeline, status: status, project: project, sha: sha)
end
end
subject do pipeline = create(:ci_empty_pipeline, status: status,
get :index, namespace_id: project.namespace, project_id: project, format: :json 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' do it 'returns JSON with serialized pipelines', :request_store do
subject queries = ActiveRecord::QueryRecorder.new { get_pipelines_index_json }
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')
...@@ -39,22 +44,35 @@ ...@@ -39,22 +44,35 @@
expect(json_response['count']['running']).to eq '1' expect(json_response['count']['running']).to eq '1'
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 '1'
puts queries.log
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
subject get_pipelines_index_json
expect(json_response['pipelines'][0]).not_to include('coverage') expect(json_response['pipelines'][0]).not_to include('coverage')
end end
context 'when performing gitaly calls', :request_store do context 'when performing gitaly calls', :request_store do
it 'limits the Gitaly requests' do it 'limits the Gitaly requests' do
expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(3) expect { get_pipelines_index_json }
.to change { Gitlab::GitalyClient.get_request_count }.by(2)
end end
end end
def get_pipelines_index_json
get :index, namespace_id: project.namespace,
project_id: project,
format: :json
end
def create_build(pipeline, stage, stage_idx, name)
create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name)
end
end end
describe 'GET show JSON' do describe 'GET show.json' do
let(:pipeline) { create(:ci_pipeline_with_one_job, project: project) } let(:pipeline) { create(:ci_pipeline_with_one_job, project: project) }
it 'returns the pipeline' do it 'returns the pipeline' do
...@@ -67,6 +85,14 @@ ...@@ -67,6 +85,14 @@
end end
context 'when the pipeline has multiple stages and groups', :request_store do context 'when the pipeline has multiple stages and groups', :request_store do
let(:project) { create(:project, :repository) }
let(:pipeline) do
create(:ci_empty_pipeline, project: project,
user: user,
sha: project.commit.id)
end
before do before do
create_build('build', 0, 'build') create_build('build', 0, 'build')
create_build('test', 1, 'rspec 0') create_build('test', 1, 'rspec 0')
...@@ -74,11 +100,6 @@ ...@@ -74,11 +100,6 @@
create_build('post deploy', 3, 'pages 0') create_build('post deploy', 3, 'pages 0')
end end
let(:project) { create(:project, :repository) }
let(:pipeline) do
create(:ci_empty_pipeline, project: project, user: user, sha: project.commit.id)
end
it 'does not perform N + 1 queries' do it 'does not perform N + 1 queries' do
control_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count control_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count
...@@ -90,6 +111,7 @@ ...@@ -90,6 +111,7 @@
create_build('post deploy', 3, 'pages 2') create_build('post deploy', 3, 'pages 2')
new_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count new_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count
expect(new_count).to be_within(12).of(control_count) expect(new_count).to be_within(12).of(control_count)
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