Commit 929ab2ad authored by Fabio Pitino's avatar Fabio Pitino

Split same_family_pipeline_ids query to reduce complexity

Ci::Pipeline#same_family_pipeline_ids uses PipelineObjectHierarchy
to recursively find all pipelines (parent + children) given
a pipeline object.

This previously was finding recursively all ancestors and then
all descendants. We can instead fire two queries, one to find
the root ancestor and then use that as a baseline to find all
descendants.
parent 6ff76f6a
......@@ -845,9 +845,15 @@ module Ci
end
def same_family_pipeline_ids
::Gitlab::Ci::PipelineObjectHierarchy.new(
base_and_ancestors(same_project: true), options: { same_project: true }
).base_and_descendants.select(:id)
if Feature.enabled?(:ci_root_ancestor_for_pipeline_family, project, default_enabled: false)
::Gitlab::Ci::PipelineObjectHierarchy.new(
self.class.where(id: root_ancestor), options: { same_project: true }
).base_and_descendants.select(:id)
else
::Gitlab::Ci::PipelineObjectHierarchy.new(
base_and_ancestors(same_project: true), options: { same_project: true }
).base_and_descendants.select(:id)
end
end
def build_with_artifacts_in_self_and_descendants(name)
......@@ -869,6 +875,15 @@ module Ci
.base_and_descendants
end
def root_ancestor
return self unless child?
Gitlab::Ci::PipelineObjectHierarchy
.new(self.class.unscoped.where(id: id), options: { same_project: true })
.base_and_ancestors(hierarchy_order: :desc)
.first
end
def bridge_triggered?
source_bridge.present?
end
......@@ -878,7 +893,8 @@ module Ci
end
def child?
parent_pipeline.present?
parent_pipeline? && # child pipelines have `parent_pipeline` source
parent_pipeline.present?
end
def parent?
......
---
name: ci_root_ancestor_for_pipeline_family
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46575
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/287812
milestone: '13.7'
type: development
group: group::continuous integration
default_enabled: false
......@@ -16,6 +16,7 @@ FactoryBot.define do
transient { head_pipeline_of { nil } }
transient { child_of { nil } }
transient { upstream_of { nil } }
after(:build) do |pipeline, evaluator|
if evaluator.child_of
......@@ -30,9 +31,12 @@ FactoryBot.define do
if evaluator.child_of
bridge = create(:ci_bridge, pipeline: evaluator.child_of)
create(:ci_sources_pipeline,
source_job: bridge,
pipeline: pipeline)
create(:ci_sources_pipeline, source_job: bridge, pipeline: pipeline)
end
if evaluator.upstream_of
bridge = create(:ci_bridge, pipeline: pipeline)
create(:ci_sources_pipeline, source_job: bridge, pipeline: evaluator.upstream_of)
end
end
......
......@@ -269,7 +269,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
let!(:older_other_pipeline) { create(:ci_pipeline, project: project) }
let!(:upstream_pipeline) { create(:ci_pipeline, project: project) }
let!(:child_pipeline) { create(:ci_pipeline, project: project) }
let!(:child_pipeline) { create(:ci_pipeline, child_of: upstream_pipeline) }
let!(:other_pipeline) { create(:ci_pipeline, project: project) }
......@@ -2754,13 +2754,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end
context 'when pipeline is child' do
let(:parent) { create(:ci_pipeline, project: pipeline.project) }
let(:sibling) { create(:ci_pipeline, project: pipeline.project) }
before do
create_source_pipeline(parent, pipeline)
create_source_pipeline(parent, sibling)
end
let(:parent) { create(:ci_pipeline, project: project) }
let!(:pipeline) { create(:ci_pipeline, child_of: parent) }
let!(:sibling) { create(:ci_pipeline, child_of: parent) }
it 'returns parent sibling and self ids' do
expect(subject).to contain_exactly(parent.id, pipeline.id, sibling.id)
......@@ -2768,11 +2764,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end
context 'when pipeline is parent' do
let(:child) { create(:ci_pipeline, project: pipeline.project) }
before do
create_source_pipeline(pipeline, child)
end
let!(:child) { create(:ci_pipeline, child_of: pipeline) }
it 'returns self and child ids' do
expect(subject).to contain_exactly(pipeline.id, child.id)
......@@ -2780,17 +2772,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end
context 'when pipeline is a child of a child pipeline' do
let(:ancestor) { create(:ci_pipeline, project: pipeline.project) }
let(:parent) { create(:ci_pipeline, project: pipeline.project) }
let(:cousin_parent) { create(:ci_pipeline, project: pipeline.project) }
let(:cousin) { create(:ci_pipeline, project: pipeline.project) }
before do
create_source_pipeline(ancestor, parent)
create_source_pipeline(ancestor, cousin_parent)
create_source_pipeline(parent, pipeline)
create_source_pipeline(cousin_parent, cousin)
end
let(:ancestor) { create(:ci_pipeline, project: project) }
let!(:parent) { create(:ci_pipeline, child_of: ancestor) }
let!(:pipeline) { create(:ci_pipeline, child_of: parent) }
let!(:cousin_parent) { create(:ci_pipeline, child_of: ancestor) }
let!(:cousin) { create(:ci_pipeline, child_of: cousin_parent) }
it 'returns all family ids' do
expect(subject).to contain_exactly(
......@@ -2800,11 +2786,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end
context 'when pipeline is a triggered pipeline' do
let(:upstream) { create(:ci_pipeline, project: create(:project)) }
before do
create_source_pipeline(upstream, pipeline)
end
let!(:upstream) { create(:ci_pipeline, project: create(:project), upstream_of: pipeline)}
it 'returns self id' do
expect(subject).to contain_exactly(pipeline.id)
......@@ -2812,6 +2794,46 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end
end
describe '#root_ancestor' do
subject { pipeline.root_ancestor }
let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
context 'when pipeline is child of child pipeline' do
let!(:root_ancestor) { create(:ci_pipeline, project: project) }
let!(:parent_pipeline) { create(:ci_pipeline, child_of: root_ancestor) }
let!(:pipeline) { create(:ci_pipeline, child_of: parent_pipeline) }
it 'returns the root ancestor' do
expect(subject).to eq(root_ancestor)
end
end
context 'when pipeline is root ancestor' do
let!(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) }
it 'returns itself' do
expect(subject).to eq(pipeline)
end
end
context 'when pipeline is standalone' do
it 'returns itself' do
expect(subject).to eq(pipeline)
end
end
context 'when pipeline is multi-project downstream pipeline' do
let!(:upstream_pipeline) do
create(:ci_pipeline, project: create(:project), upstream_of: pipeline)
end
it 'ignores cross project ancestors' do
expect(subject).to eq(pipeline)
end
end
end
describe '#stuck?' do
before do
create(:ci_build, :pending, pipeline: pipeline)
......@@ -3552,18 +3574,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
describe '#parent_pipeline' do
let_it_be(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, project: project) }
context 'when pipeline is triggered by a pipeline from the same project' do
let(:upstream_pipeline) { create(:ci_pipeline, project: pipeline.project) }
before do
create(:ci_sources_pipeline,
source_pipeline: upstream_pipeline,
source_project: project,
pipeline: pipeline,
project: project)
end
let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: project) }
let_it_be(:pipeline) { create(:ci_pipeline, child_of: upstream_pipeline) }
it 'returns the parent pipeline' do
expect(pipeline.parent_pipeline).to eq(upstream_pipeline)
......@@ -3575,15 +3588,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end
context 'when pipeline is triggered by a pipeline from another project' do
let(:upstream_pipeline) { create(:ci_pipeline) }
before do
create(:ci_sources_pipeline,
source_pipeline: upstream_pipeline,
source_project: upstream_pipeline.project,
pipeline: pipeline,
project: project)
end
let(:pipeline) { create(:ci_pipeline, project: project) }
let!(:upstream_pipeline) { create(:ci_pipeline, project: create(:project), upstream_of: pipeline) }
it 'returns nil' do
expect(pipeline.parent_pipeline).to be_nil
......@@ -3595,6 +3601,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end
context 'when pipeline is not triggered by a pipeline' do
let_it_be(:pipeline) { create(:ci_pipeline) }
it 'returns nil' do
expect(pipeline.parent_pipeline).to be_nil
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