Commit 82253cc6 authored by Patrick Bair's avatar Patrick Bair

Merge branch 'mo-fix-for-project-any_online_runners-generates-cross-database-query' into 'master'

Fix for Runner#belonging_to_parent_group_of_project cross dabatase query

See merge request gitlab-org/gitlab!76454
parents a387a751 501ffc0f
...@@ -136,13 +136,18 @@ module Ci ...@@ -136,13 +136,18 @@ module Ci
.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433')
} }
# deprecated
scope :belonging_to_parent_group_of_project, -> (project_id) { 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 }) project_groups = ::Group.joins(:projects).where(projects: { id: project_id })
joins(:groups) if Feature.enabled?(:ci_decompose_belonging_to_parent_group_of_project_query, default_enabled: :yaml)
.where(namespaces: { id: project_groups.self_and_ancestors.as_ids }) belonging_to_group(project_groups.self_and_ancestors.pluck(:id))
.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433') 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 # 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 ...@@ -203,28 +203,80 @@ RSpec.describe Ci::Runner do
end end
end end
describe '.belonging_to_parent_group_of_project' do shared_examples '.belonging_to_parent_group_of_project' do
let(:project) { create(:project, group: group) } let!(:group1) { create(:group) }
let(:group) { create(:group) } let!(:project1) { create(:project, group: group1) }
let(:runner) { create(:ci_runner, :group, groups: [group]) } let!(:runner1) { create(:ci_runner, :group, groups: [group1]) }
let!(:unrelated_group) { create(:group) }
let!(:unrelated_project) { create(:project, group: unrelated_group) } let!(:group2) { create(:group) }
let!(:unrelated_runner) { create(:ci_runner, :group, groups: [unrelated_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 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 end
context 'with a parent group with a runner' do context 'with a parent group with a runner', :sidekiq_inline do
let(:runner) { create(:ci_runner, :group, groups: [parent_group]) } before do
let(:project) { create(:project, group: group) } group1.update!(parent: group2)
let(:group) { create(:group, parent: parent_group) } end
let(:parent_group) { create(:group) }
it 'returns the group runner from the parent group' do it 'returns the group runner from the group and the parent group' do
expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner) expect(result).to contain_exactly(runner1, runner2)
end end
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 end
describe '.owned_or_instance_wide' do describe '.owned_or_instance_wide' do
......
...@@ -202,7 +202,7 @@ RSpec.describe PipelineSerializer do ...@@ -202,7 +202,7 @@ RSpec.describe PipelineSerializer do
# Existing numbers are high and require performance optimization # Existing numbers are high and require performance optimization
# Ongoing issue: # Ongoing issue:
# https://gitlab.com/gitlab-org/gitlab/-/issues/225156 # 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.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0) 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