Commit 07e0b8a7 authored by Laura Montemayor's avatar Laura Montemayor Committed by James Lopez

Improve n+1 in pipeline serializer for triggered pipelines

parent 8f59c173
......@@ -42,6 +42,7 @@ module Ci
has_many :stages, -> { order(position: :asc) }, inverse_of: :pipeline
has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline
has_many :latest_statuses_ordered_by_stage, -> { latest.order(:stage_idx, :stage) }, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline
has_many :latest_statuses, -> { latest }, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline
has_many :processables, class_name: 'Ci::Processable', foreign_key: :commit_id, inverse_of: :pipeline
has_many :bridges, class_name: 'Ci::Bridge', foreign_key: :commit_id, inverse_of: :pipeline
has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline
......@@ -577,11 +578,11 @@ module Ci
end
def retried
@retried ||= (statuses.order(id: :desc) - statuses.latest)
@retried ||= (statuses.order(id: :desc) - latest_statuses)
end
def coverage
coverage_array = statuses.latest.map(&:coverage).compact
coverage_array = latest_statuses.map(&:coverage).compact
if coverage_array.size >= 1
'%.2f' % (coverage_array.reduce(:+) / coverage_array.size)
end
......
......@@ -47,6 +47,7 @@ class PipelineSerializer < BaseSerializer
:retryable_builds,
:scheduled_actions,
:stages,
:latest_statuses,
:trigger_requests,
:user,
{
......@@ -62,7 +63,14 @@ class PipelineSerializer < BaseSerializer
pending_builds: :project,
project: [:route, { namespace: :route }],
triggered_by_pipeline: [{ project: [:route, { namespace: :route }] }, :user],
triggered_pipelines: [{ project: [:route, { namespace: :route }] }, :user, :source_job]
triggered_pipelines: [
{
project: [:route, { namespace: :route }]
},
:source_job,
:latest_statuses,
:user
]
}
]
end
......
......@@ -31,14 +31,14 @@ module Ci
# rubocop: disable CodeReuse/ActiveRecord
def update_retried
# find the latest builds for each name
latest_statuses = pipeline.statuses.latest
latest_statuses = pipeline.latest_statuses
.group(:name)
.having('count(*) > 1')
.pluck(Arel.sql('MAX(id)'), 'name')
# mark builds that are retried
if latest_statuses.any?
pipeline.statuses.latest
pipeline.latest_statuses
.where(name: latest_statuses.map(&:second))
.where.not(id: latest_statuses.map(&:first))
.update_all(retried: true)
......
......@@ -46,4 +46,4 @@
%td{ style: "font-family: 'Menlo','Liberation Mono','Consolas','DejaVu Sans Mono','Ubuntu Mono','Courier New','andale mono','lucida console',monospace; font-size: 14px; line-height: 1.4; vertical-align: baseline; padding:0 8px;" }
API
= render 'notify/failed_builds', pipeline: @pipeline, failed: @pipeline.statuses.latest.failed
= render 'notify/failed_builds', pipeline: @pipeline, failed: @pipeline.latest_statuses.failed
......@@ -7,7 +7,7 @@ The Auto DevOps pipeline failed for pipeline <%= @pipeline.iid %> (<%= pipeline_
<% else -%>
Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by API
<% end -%>
<% failed = @pipeline.statuses.latest.failed -%>
<% failed = @pipeline.latest_statuses.failed -%>
had <%= failed.size %> failed <%= 'build'.pluralize(failed.size) %>.
<% failed.each do |build| -%>
......
......@@ -108,4 +108,4 @@
%td{ style: "font-family:'Menlo','Liberation Mono','Consolas','DejaVu Sans Mono','Ubuntu Mono','Courier New','andale mono','lucida console',monospace;font-size:14px;line-height:1.4;vertical-align:baseline;padding:0 5px;" }
API
= render 'notify/failed_builds', pipeline: @pipeline, failed: @pipeline.statuses.latest.failed
= render 'notify/failed_builds', pipeline: @pipeline, failed: @pipeline.latest_statuses.failed
......@@ -27,7 +27,7 @@ Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by <%
<% else -%>
Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by API
<% end -%>
<% failed = @pipeline.statuses.latest.failed -%>
<% failed = @pipeline.latest_statuses.failed -%>
had <%= failed.size %> failed <%= 'build'.pluralize(failed.size) %>.
<% failed.each do |build| -%>
......
---
title: Improve n+1 in pipeline serializer for triggered pipelines
merge_request: 42421
author:
type: performance
......@@ -242,6 +242,7 @@ ci_pipelines:
- latest_builds_report_results
- messages
- pipeline_artifacts
- latest_statuses
ci_refs:
- project
- ci_pipelines
......
......@@ -2436,7 +2436,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end
describe '#retry_failed' do
let(:latest_status) { pipeline.statuses.latest.pluck(:status) }
let(:latest_status) { pipeline.latest_statuses.pluck(:status) }
before do
stub_not_protect_default_branch
......
......@@ -155,7 +155,7 @@ RSpec.describe PipelineSerializer do
it 'verifies number of queries', :request_store do
recorded = ActiveRecord::QueryRecorder.new { subject }
expected_queries = Gitlab.ee? ? 43 : 40
expected_queries = Gitlab.ee? ? 39 : 36
expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0)
......@@ -176,7 +176,7 @@ RSpec.describe PipelineSerializer do
# pipeline. With the same ref this check is cached but if refs are
# different then there is an extra query per ref
# https://gitlab.com/gitlab-org/gitlab-foss/issues/46368
expected_queries = Gitlab.ee? ? 49 : 46
expected_queries = Gitlab.ee? ? 42 : 39
expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0)
......@@ -199,11 +199,10 @@ RSpec.describe PipelineSerializer do
it 'verifies number of queries', :request_store do
recorded = ActiveRecord::QueryRecorder.new { subject }
# 99 queries by default + 2 related to preloading
# :source_pipeline and :source_job
# Existing numbers are high and require performance optimization
# Ongoing issue:
# https://gitlab.com/gitlab-org/gitlab/-/issues/225156
expected_queries = Gitlab.ee? ? 95 : 86
expected_queries = Gitlab.ee? ? 85 : 76
expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0)
......
......@@ -43,12 +43,12 @@ RSpec.shared_context 'Pipeline Processing Service Tests With Yaml' do
{
pipeline: pipeline.status,
stages: pipeline.stages.pluck(:name, :status).to_h,
jobs: pipeline.statuses.latest.pluck(:name, :status).to_h
jobs: pipeline.latest_statuses.pluck(:name, :status).to_h
}
end
def event_on_jobs(event, job_names)
statuses = pipeline.statuses.latest.by_name(job_names).to_a
statuses = pipeline.latest_statuses.by_name(job_names).to_a
expect(statuses.count).to eq(job_names.count) # ensure that we have the same counts
statuses.each { |status| status.public_send("#{event}!") }
......
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