From 5bffbad5917195c4eff72570a5798e5a5aa81d29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= <fjlopez@gitlab.com>
Date: Mon, 20 Sep 2021 13:06:13 +0200
Subject: [PATCH] Use GroupPlansPreloader linear ancestor scopes
In this commit we're replacing the recursive ancestors scope in
the `GroupPlansPreloader` class, for their linear version.
Changelog: performance
EE: true
---
..._group_plans_preloaded_ancestor_scopes.yml | 8 +++
ee/lib/gitlab/group_plans_preloader.rb | 12 +++--
.../lib/gitlab/group_plans_preloader_spec.rb | 50 ++++++++++++-------
3 files changed, 48 insertions(+), 22 deletions(-)
create mode 100644 config/feature_flags/development/linear_group_plans_preloaded_ancestor_scopes.yml
diff --git a/config/feature_flags/development/linear_group_plans_preloaded_ancestor_scopes.yml b/config/feature_flags/development/linear_group_plans_preloaded_ancestor_scopes.yml
new file mode 100644
index 0000000000000000..d45b8d71a20cacc6
--- /dev/null
+++ b/config/feature_flags/development/linear_group_plans_preloaded_ancestor_scopes.yml
@@ -0,0 +1,8 @@
+---
+name: linear_group_plans_preloaded_ancestor_scopes
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70685
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/341349
+milestone: '14.4'
+type: development
+group: group::access
+default_enabled: false
diff --git a/ee/lib/gitlab/group_plans_preloader.rb b/ee/lib/gitlab/group_plans_preloader.rb
index fb28ad93b056a0f1..ae18d578c1d2a294 100644
--- a/ee/lib/gitlab/group_plans_preloader.rb
+++ b/ee/lib/gitlab/group_plans_preloader.rb
@@ -57,9 +57,15 @@ def preload(groups)
# Returns an ActiveRecord::Relation that includes the given groups, and all
# their (recursive) ancestors.
def groups_and_ancestors_for(groups)
- Gitlab::ObjectHierarchy
- .new(groups)
- .base_and_ancestors
+ groups_and_ancestors = if ::Feature.enabled?(:linear_group_plans_preloaded_ancestor_scopes, default_enabled: :yaml)
+ groups.self_and_ancestors
+ else
+ Gitlab::ObjectHierarchy
+ .new(groups)
+ .base_and_ancestors
+ end
+
+ groups_and_ancestors
.join_gitlab_subscription
.select('namespaces.id', 'namespaces.parent_id', 'gitlab_subscriptions.hosted_plan_id')
end
diff --git a/ee/spec/lib/gitlab/group_plans_preloader_spec.rb b/ee/spec/lib/gitlab/group_plans_preloader_spec.rb
index 2a7f68f5d3e10b1b..d9a185f30e56aedd 100644
--- a/ee/spec/lib/gitlab/group_plans_preloader_spec.rb
+++ b/ee/spec/lib/gitlab/group_plans_preloader_spec.rb
@@ -23,30 +23,42 @@
create(:group, name: 'group-3', parent: group1)
end
- it 'only executes three SQL queries to preload the data' do
- amount = ActiveRecord::QueryRecorder
- .new { preloaded_groups }
- .count
-
- # One query to get the groups and their ancestors, one query to get their
- # plans, and one query to _just_ get the groups.
- expect(amount).to eq(3)
- end
+ shared_examples 'preloading cases' do
+ it 'only executes three SQL queries to preload the data' do
+ amount = ActiveRecord::QueryRecorder
+ .new { preloaded_groups }
+ .count
+
+ # One query to get the groups and their ancestors, one query to get their
+ # plans, and one query to _just_ get the groups.
+ expect(amount).to eq(3)
+ end
+
+ it 'associates the correct plans with the correct groups' do
+ expect(preloaded_groups[0].plans).to match_array([plan1])
+ expect(preloaded_groups[1].plans).to match_array([plan2])
+ expect(preloaded_groups[2].plans).to match_array([plan1])
+ end
+
+ it 'does not execute any queries for preloaded plans' do
+ preloaded_groups
+
+ amount = ActiveRecord::QueryRecorder
+ .new { preloaded_groups.each(&:plans) }
+ .count
- it 'associates the correct plans with the correct groups' do
- expect(preloaded_groups[0].plans).to eq([plan1])
- expect(preloaded_groups[1].plans).to eq([plan2])
- expect(preloaded_groups[2].plans).to eq([plan1])
+ expect(amount).to be_zero
+ end
end
- it 'does not execute any queries for preloaded plans' do
- preloaded_groups
+ it_behaves_like 'preloading cases'
- amount = ActiveRecord::QueryRecorder
- .new { preloaded_groups.each(&:plans) }
- .count
+ context 'when feature flag :linear_group_plans_preloaded_ancestor_scopes is disabled' do
+ before do
+ stub_feature_flags(linear_group_plans_preloaded_ancestor_scopes: false)
+ end
- expect(amount).to be_zero
+ it_behaves_like 'preloading cases'
end
end
end
--
GitLab