Commit 501ffc0f authored by Maxime Orefice's avatar Maxime Orefice Committed by Patrick Bair

Modify project.group_runners query

This commit prevents cross database query when fetching group
runners at the project level.
parent 1118c7f3
......@@ -136,13 +136,18 @@ module Ci
.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433')
}
# deprecated
scope :belonging_to_parent_group_of_project, -> (project_id) {
raise ArgumentError, "only 1 project_id allowed for performance reasons" unless project_id.is_a?(Integer)
project_groups = ::Group.joins(:projects).where(projects: { id: project_id })
joins(:groups)
.where(namespaces: { id: project_groups.self_and_ancestors.as_ids })
.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433')
if Feature.enabled?(:ci_decompose_belonging_to_parent_group_of_project_query, default_enabled: :yaml)
belonging_to_group(project_groups.self_and_ancestors.pluck(:id))
else
joins(:groups)
.where(namespaces: { id: project_groups.self_and_ancestors.as_ids })
.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433')
end
}
# deprecated
......
---
name: ci_decompose_belonging_to_parent_group_of_project_query
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76454
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348560
milestone: '14.7'
type: development
group: group::pipeline execution
default_enabled: false
......@@ -203,28 +203,80 @@ RSpec.describe Ci::Runner do
end
end
describe '.belonging_to_parent_group_of_project' do
let(:project) { create(:project, group: group) }
let(:group) { create(:group) }
let(:runner) { create(:ci_runner, :group, groups: [group]) }
let!(:unrelated_group) { create(:group) }
let!(:unrelated_project) { create(:project, group: unrelated_group) }
let!(:unrelated_runner) { create(:ci_runner, :group, groups: [unrelated_group]) }
shared_examples '.belonging_to_parent_group_of_project' do
let!(:group1) { create(:group) }
let!(:project1) { create(:project, group: group1) }
let!(:runner1) { create(:ci_runner, :group, groups: [group1]) }
let!(:group2) { create(:group) }
let!(:project2) { create(:project, group: group2) }
let!(:runner2) { create(:ci_runner, :group, groups: [group2]) }
let(:project_id) { project1.id }
subject(:result) { described_class.belonging_to_parent_group_of_project(project_id) }
it 'returns the specific group runner' do
expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner)
expect(result).to contain_exactly(runner1)
end
context 'with a parent group with a runner' do
let(:runner) { create(:ci_runner, :group, groups: [parent_group]) }
let(:project) { create(:project, group: group) }
let(:group) { create(:group, parent: parent_group) }
let(:parent_group) { create(:group) }
context 'with a parent group with a runner', :sidekiq_inline do
before do
group1.update!(parent: group2)
end
it 'returns the group runner from the parent group' do
expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner)
it 'returns the group runner from the group and the parent group' do
expect(result).to contain_exactly(runner1, runner2)
end
end
context 'with multiple project ids' do
let(:project_id) { [project1.id, project2.id] }
it 'raises ArgumentError' do
expect { result }.to raise_error(ArgumentError)
end
end
end
context 'when ci_decompose_belonging_to_parent_group_of_project_query is enabled' do
context 'when use_traversal_ids* are enabled' do
it_behaves_like '.belonging_to_parent_group_of_project'
end
context 'when use_traversal_ids* are disabled' do
before do
stub_feature_flags(
use_traversal_ids: false,
use_traversal_ids_for_ancestors: false,
use_traversal_ids_for_ancestor_scopes: false
)
end
it_behaves_like '.belonging_to_parent_group_of_project'
end
end
context 'when ci_decompose_belonging_to_parent_group_of_project_query is disabled' do
before do
stub_feature_flags(ci_decompose_belonging_to_parent_group_of_project_query: false)
end
context 'when use_traversal_ids* are enabled' do
it_behaves_like '.belonging_to_parent_group_of_project'
end
context 'when use_traversal_ids* are disabled' do
before do
stub_feature_flags(
use_traversal_ids: false,
use_traversal_ids_for_ancestors: false,
use_traversal_ids_for_ancestor_scopes: false
)
end
it_behaves_like '.belonging_to_parent_group_of_project'
end
end
describe '.owned_or_instance_wide' do
......
......@@ -202,7 +202,7 @@ RSpec.describe PipelineSerializer do
# Existing numbers are high and require performance optimization
# Ongoing issue:
# https://gitlab.com/gitlab-org/gitlab/-/issues/225156
expected_queries = Gitlab.ee? ? 74 : 70
expected_queries = Gitlab.ee? ? 78 : 74
expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0)
......
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