Commit 320d6793 authored by Adam Hegyi's avatar Adam Hegyi Committed by Heinrich Lee Yu

Fix performance of GroupsWithTemplatesFinder query

- Introduce OptimizedGroupsWithTemplatesFinder that queries group
templates more efficiently.
- Introduce a feature flag to enable the optimization:
`optimized_groups_with_templates_finder`
parent a7813e74
...@@ -21,15 +21,30 @@ class GroupsWithTemplatesFinder ...@@ -21,15 +21,30 @@ class GroupsWithTemplatesFinder
attr_reader :group_id attr_reader :group_id
# Cleanup issue: https://gitlab.com/gitlab-org/gitlab/issues/35733
def extended_group_search def extended_group_search
groups = Group.with_feature_available_in_plan(:group_project_templates) if ::Feature.enabled?(:optimized_groups_with_templates_finder)
Gitlab::ObjectHierarchy.new(groups).base_and_descendants groups = Group.with_project_templates_optimized
groups_with_plan = Gitlab::ObjectHierarchy
.new(groups)
.base_and_ancestors
.with_feature_available_in_plan(:group_project_templates)
Gitlab::ObjectHierarchy.new(groups_with_plan).base_and_descendants
else
groups = Group.with_feature_available_in_plan(:group_project_templates)
Gitlab::ObjectHierarchy.new(groups).base_and_descendants
end
end end
def simple_group_search(groups) def simple_group_search(groups)
groups = group_id ? groups.find_by(id: group_id)&.self_and_ancestors : groups # rubocop: disable CodeReuse/ActiveRecord groups = group_id ? groups.find_by(id: group_id)&.self_and_ancestors : groups # rubocop: disable CodeReuse/ActiveRecord
return Group.none unless groups return Group.none unless groups
groups.with_project_templates if ::Feature.enabled?(:optimized_groups_with_templates_finder)
groups.with_project_templates_optimized
else
groups.with_project_templates
end
end end
end end
...@@ -59,9 +59,11 @@ module EE ...@@ -59,9 +59,11 @@ module EE
scope :with_project_templates, -> do scope :with_project_templates, -> do
joins("INNER JOIN projects ON projects.namespace_id = namespaces.custom_project_templates_group_id") joins("INNER JOIN projects ON projects.namespace_id = namespaces.custom_project_templates_group_id")
.distinct .distinct
end end
scope :with_project_templates_optimized, -> { where.not(custom_project_templates_group_id: nil) }
scope :with_custom_file_templates, -> do scope :with_custom_file_templates, -> do
preload( preload(
file_template_project: :route, file_template_project: :route,
......
...@@ -207,14 +207,24 @@ module EE ...@@ -207,14 +207,24 @@ module EE
end end
def available_subgroups_with_custom_project_templates(group_id = nil) def available_subgroups_with_custom_project_templates(group_id = nil)
groups = GroupsWithTemplatesFinder.new(group_id).execute found_groups = GroupsWithTemplatesFinder.new(group_id).execute
GroupsFinder.new(self, min_access_level: ::Gitlab::Access::DEVELOPER) if ::Feature.enabled?(:optimized_groups_with_templates_finder)
.execute GroupsFinder.new(self, min_access_level: ::Gitlab::Access::DEVELOPER)
.where(id: groups.select(:custom_project_templates_group_id)) .execute
.includes(:projects) .where(id: found_groups.select(:custom_project_templates_group_id))
.reorder(nil) .preload(:projects)
.distinct .joins(:projects)
.reorder(nil)
.distinct
else
GroupsFinder.new(self, min_access_level: ::Gitlab::Access::DEVELOPER)
.execute
.where(id: found_groups.select(:custom_project_templates_group_id))
.includes(:projects)
.reorder(nil)
.distinct
end
end end
def roadmap_layout def roadmap_layout
......
---
title: Fix new project page load performance
merge_request: 18180
author:
type: performance
...@@ -269,7 +269,7 @@ describe 'New project' do ...@@ -269,7 +269,7 @@ describe 'New project' do
end end
end end
context 'when custom project group template is set' do shared_context 'when custom project group template is set' do
let(:group1) { create(:group) } let(:group1) { create(:group) }
let(:group2) { create(:group) } let(:group2) { create(:group) }
let(:group3) { create(:group) } let(:group3) { create(:group) }
...@@ -429,6 +429,23 @@ describe 'New project' do ...@@ -429,6 +429,23 @@ describe 'New project' do
end end
end end
# Cleanup issue: https://gitlab.com/gitlab-org/gitlab/issues/35733
context 'when `optimized_groups_with_templates_finder` feature flag is enabled' do
before do
stub_feature_flags(optimized_groups_with_templates_finder: true)
end
include_context 'when custom project group template is set'
end
context 'when `optimized_groups_with_templates_finder` feature flag is disabled' do
before do
stub_feature_flags(optimized_groups_with_templates_finder: false)
end
include_context 'when custom project group template is set'
end
context 'when group template is not set' do context 'when group template is not set' do
it_behaves_like 'group templates displayed' do it_behaves_like 'group templates displayed' do
let(:template_number) { 0 } let(:template_number) { 0 }
......
...@@ -26,94 +26,112 @@ describe GroupsWithTemplatesFinder do ...@@ -26,94 +26,112 @@ describe GroupsWithTemplatesFinder do
create(:gitlab_subscription, :silver, namespace: group_2) create(:gitlab_subscription, :silver, namespace: group_2)
end end
describe 'without group id' do shared_examples 'GroupsWithTemplatesFinder examples' do
it 'returns all groups' do describe 'without group id' do
expect(described_class.new.execute).to contain_exactly(group_1, group_2, group_3) it 'returns all groups' do
end expect(described_class.new.execute).to contain_exactly(group_1, group_2, group_3)
context 'when namespace checked' do
before do
allow(Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?) { true }
end end
it 'returns all groups before cut-off date' do context 'when namespace checked' do
Timecop.freeze(described_class::CUT_OFF_DATE - 1.day) do before do
expect(described_class.new.execute).to contain_exactly(group_1, group_2, group_3) allow(Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?) { true }
end
end
it 'returns groups on gold/silver plan after cut-off date' do
Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do
expect(described_class.new.execute).to contain_exactly(group_1, group_2)
end end
end
context 'with subgroup with template' do it 'returns all groups before cut-off date' do
before do Timecop.freeze(described_class::CUT_OFF_DATE - 1.day) do
subgroup_4.update!(custom_project_templates_group_id: subgroup_5.id) expect(described_class.new.execute).to contain_exactly(group_1, group_2, group_3)
create(:project, namespace: subgroup_5) end
end end
it 'returns groups on gold/silver plan after cut-off date' do it 'returns groups on gold/silver plan after cut-off date' do
Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do
expect(described_class.new.execute).to contain_exactly(group_1, group_2, subgroup_4) expect(described_class.new.execute).to contain_exactly(group_1, group_2)
end end
end end
end
end
end
describe 'with group id' do
it 'returns given group with it descendants' do
expect(described_class.new(group_1.id).execute).to contain_exactly(group_1)
end
context 'with subgroup with template' do context 'with subgroup with template' do
before do before do
subgroup_4.update!(custom_project_templates_group_id: subgroup_5.id) subgroup_4.update!(custom_project_templates_group_id: subgroup_5.id)
create(:project, namespace: subgroup_5) create(:project, namespace: subgroup_5)
end end
it 'returns only chosen group' do it 'returns groups on gold/silver plan after cut-off date' do
expect(described_class.new(group_1.id).execute).to contain_exactly(group_1) Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do
expect(described_class.new.execute).to contain_exactly(group_1, group_2, subgroup_4)
end
end
end
end end
end end
context 'when namespace checked' do describe 'with group id' do
before do it 'returns given group with it descendants' do
allow(Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?) { true } expect(described_class.new(group_1.id).execute).to contain_exactly(group_1)
end end
it 'returns given group with it descendants before cut-off date' do context 'with subgroup with template' do
Timecop.freeze(described_class::CUT_OFF_DATE - 1.day) do before do
expect(described_class.new(group_3.id).execute).to contain_exactly(group_3) subgroup_4.update!(custom_project_templates_group_id: subgroup_5.id)
create(:project, namespace: subgroup_5)
end end
end
it 'does not return the group after the cut-off date' do it 'returns only chosen group' do
Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do expect(described_class.new(group_1.id).execute).to contain_exactly(group_1)
expect(described_class.new(group_3.id).execute).to be_empty
end end
end end
context 'with subgroup with template' do context 'when namespace checked' do
before do before do
subgroup_4.update!(custom_project_templates_group_id: subgroup_5.id) allow(Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?) { true }
create(:project, namespace: subgroup_5)
end end
it 'returns only chosen group' do it 'returns given group with it descendants before cut-off date' do
Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do Timecop.freeze(described_class::CUT_OFF_DATE - 1.day) do
expect(described_class.new(group_1.id).execute).to contain_exactly(group_1) expect(described_class.new(group_3.id).execute).to contain_exactly(group_3)
end end
end end
it 'returns only chosen subgroup' do it 'does not return the group after the cut-off date' do
Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do
expect(described_class.new(subgroup_4.id).execute).to contain_exactly(group_1, subgroup_4) expect(described_class.new(group_3.id).execute).to be_empty
end
end
context 'with subgroup with template' do
before do
subgroup_4.update!(custom_project_templates_group_id: subgroup_5.id)
create(:project, namespace: subgroup_5)
end
it 'returns only chosen group' do
Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do
expect(described_class.new(group_1.id).execute).to contain_exactly(group_1)
end
end
it 'returns only chosen subgroup' do
Timecop.freeze(described_class::CUT_OFF_DATE + 1.day) do
expect(described_class.new(subgroup_4.id).execute).to contain_exactly(group_1, subgroup_4)
end
end end
end end
end end
end end
end end
context 'when `optimized_groups_with_templates_finder` feature flag is enabled' do
before do
stub_feature_flags(optimized_groups_with_templates_finder: true)
end
include_examples 'GroupsWithTemplatesFinder examples'
end
context 'when `optimized_groups_with_templates_finder` feature flag is disabled' do
before do
stub_feature_flags(optimized_groups_with_templates_finder: false)
end
include_examples 'GroupsWithTemplatesFinder examples'
end
end 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