Commit c80cd3a8 authored by Igor Drozdov's avatar Igor Drozdov

Optimize query for CI pipelines of merge request

Use CTE with joins instead of passing an array of shas
to IN condition
parent 7572b7e0
...@@ -12,15 +12,18 @@ class MergeRequest::Pipelines ...@@ -12,15 +12,18 @@ class MergeRequest::Pipelines
attr_reader :merge_request attr_reader :merge_request
delegate :all_commit_shas, :source_project, :source_branch, to: :merge_request delegate :commit_shas, :source_project, :source_branch, to: :merge_request
def all def all
return Ci::Pipeline.none unless source_project
strong_memoize(:all_pipelines) do strong_memoize(:all_pipelines) do
pipelines = Ci::Pipeline.from_union( next Ci::Pipeline.none unless source_project
[source_pipelines, detached_pipelines, triggered_for_branch],
remove_duplicates: false) pipelines =
if merge_request.persisted?
pipelines_using_cte
else
triggered_for_branch.for_sha(commit_shas)
end
sort(pipelines) sort(pipelines)
end end
...@@ -28,38 +31,55 @@ class MergeRequest::Pipelines ...@@ -28,38 +31,55 @@ class MergeRequest::Pipelines
private private
def triggered_by_merge_request def pipelines_using_cte
source_project.ci_pipelines cte = Gitlab::SQL::CTE.new(:shas, merge_request.all_commits.select(:sha))
.where(source: :merge_request_event, merge_request: merge_request)
source_pipelines_join = cte.table[:sha].eq(Ci::Pipeline.arel_table[:source_sha])
source_pipelines = filter_by(triggered_by_merge_request, cte, source_pipelines_join)
detached_pipelines = filter_by_sha(triggered_by_merge_request, cte)
pipelines_for_branch = filter_by_sha(triggered_for_branch, cte)
Ci::Pipeline.with(cte.to_arel)
.from_union([source_pipelines, detached_pipelines, pipelines_for_branch])
end
def filter_by_sha(pipelines, cte)
hex = Arel::Nodes::SqlLiteral.new("'hex'")
string_sha = Arel::Nodes::NamedFunction.new('encode', [cte.table[:sha], hex])
join_condition = string_sha.eq(Ci::Pipeline.arel_table[:sha])
filter_by(pipelines, cte, join_condition)
end end
def detached_pipelines def filter_by(pipelines, cte, join_condition)
triggered_by_merge_request.for_sha(all_commit_shas) shas_table =
Ci::Pipeline.arel_table
.join(cte.table, Arel::Nodes::InnerJoin)
.on(join_condition)
.join_sources
pipelines.joins(shas_table)
end end
def source_pipelines def triggered_by_merge_request
triggered_by_merge_request.for_source_sha(all_commit_shas) source_project.ci_pipelines
.where(source: :merge_request_event, merge_request: merge_request)
end end
def triggered_for_branch def triggered_for_branch
source_project.ci_pipelines source_project.ci_pipelines
.where(source: branch_pipeline_sources, ref: source_branch, tag: false) .where(source: branch_pipeline_sources, ref: source_branch, tag: false)
.for_sha(all_commit_shas)
end
def sources
::Ci::Pipeline.sources
end end
def branch_pipeline_sources def branch_pipeline_sources
strong_memoize(:branch_pipeline_sources) do strong_memoize(:branch_pipeline_sources) do
sources.reject { |source| source == EVENT }.values Ci::Pipeline.sources.reject { |source| source == EVENT }.values
end end
end end
def sort(pipelines) def sort(pipelines)
sql = 'CASE ci_pipelines.source WHEN (?) THEN 0 ELSE 1 END, ci_pipelines.id DESC' sql = 'CASE ci_pipelines.source WHEN (?) THEN 0 ELSE 1 END, ci_pipelines.id DESC'
query = ApplicationRecord.send(:sanitize_sql_array, [sql, sources[:merge_request_event]]) # rubocop:disable GitlabSecurity/PublicSend query = ApplicationRecord.send(:sanitize_sql_array, [sql, Ci::Pipeline.sources[:merge_request_event]]) # rubocop:disable GitlabSecurity/PublicSend
pipelines.order(Arel.sql(query)) pipelines.order(Arel.sql(query))
end end
......
---
title: Optimize query for CI pipelines of merge request
merge_request: 19653
author:
type: performance
# frozen_string_literal: true
FactoryBot.define do
factory :merge_request_diff_commit do
association :merge_request_diff
sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
relative_order { 0 }
end
end
...@@ -75,7 +75,9 @@ describe MergeRequest::Pipelines do ...@@ -75,7 +75,9 @@ describe MergeRequest::Pipelines do
let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) } let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) }
before do before do
allow(merge_request).to receive(:all_commit_shas) { shas } create(:merge_request_diff_commit,
merge_request_diff: merge_request.merge_request_diff,
sha: shas.second, relative_order: 1)
end end
it 'returns merge request pipeline first' do it 'returns merge request pipeline first' do
...@@ -119,7 +121,11 @@ describe MergeRequest::Pipelines do ...@@ -119,7 +121,11 @@ describe MergeRequest::Pipelines do
end end
before do before do
allow(merge_request_2).to receive(:all_commit_shas) { shas } shas.each.with_index do |sha, index|
create(:merge_request_diff_commit,
merge_request_diff: merge_request_2.merge_request_diff,
sha: sha, relative_order: index)
end
end end
it 'returns only related merge request pipelines' do it 'returns only related merge request pipelines' 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