From 029c5db9a4fd769a90cc0c35febd2181f060be8b Mon Sep 17 00:00:00 2001 From: Pavel Shutsin Date: Wed, 1 Jun 2022 17:24:43 +0200 Subject: [PATCH] Improve performance of Issuable finder Few improvements for issuable finder. It should improve performance of Merge requests and Issues Changelog: other --- app/finders/issuable_finder.rb | 4 +-- app/finders/issuable_finder/params.rb | 2 +- app/models/issue.rb | 2 +- app/models/namespaces/traversal/recursive.rb | 8 ++---- ...20220602130306_add_namespace_type_index.rb | 15 ++++++++++ db/schema_migrations/20220602130306 | 1 + db/structure.sql | 2 ++ .../cycle_analytics/summary/group/issue.rb | 7 ++--- lib/gitlab/object_hierarchy.rb | 28 +++++++++++++++++++ 9 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20220602130306_add_namespace_type_index.rb create mode 100644 db/schema_migrations/20220602130306 diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index fe07a52cbf043d..6bbbc237e62f43 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -316,10 +316,8 @@ def by_state(items) # rubocop: disable CodeReuse/ActiveRecord def by_project(items) - if params.project? + if params.project? || params.projects items.of_projects(params.projects).references_project - elsif params.projects - items.merge(params.projects.reorder(nil)).join_project else items.none end diff --git a/app/finders/issuable_finder/params.rb b/app/finders/issuable_finder/params.rb index 359a56bd39b71a..b0b5301f021927 100644 --- a/app/finders/issuable_finder/params.rb +++ b/app/finders/issuable_finder/params.rb @@ -175,7 +175,7 @@ def find_group_projects return Project.none unless group if params[:include_subgroups] - Project.where(namespace_id: group.self_and_descendants) # rubocop: disable CodeReuse/ActiveRecord + Project.where(namespace_id: group.self_and_descendant_ids) # rubocop: disable CodeReuse/ActiveRecord else group.projects end diff --git a/app/models/issue.rb b/app/models/issue.rb index ce7bac3fd5f696..7d684f7ae24e7e 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -149,7 +149,7 @@ def most_recent scope :without_hidden, -> { if Feature.enabled?(:ban_user_feature_flag) - where('NOT EXISTS (?)', Users::BannedUser.select(1).where('issues.author_id = banned_users.user_id')) + where.not(author_id: Users::BannedUser.all.select(:user_id)) else all end diff --git a/app/models/namespaces/traversal/recursive.rb b/app/models/namespaces/traversal/recursive.rb index 53eac27aa54e80..1c5d395cb3c117 100644 --- a/app/models/namespaces/traversal/recursive.rb +++ b/app/models/namespaces/traversal/recursive.rb @@ -63,19 +63,17 @@ def self_and_ancestor_ids(hierarchy_order: nil) # Returns all the descendants of the current namespace. def descendants - object_hierarchy(self.class.where(parent_id: id)) - .base_and_descendants + object_hierarchy(self.class.where(parent_id: id)).base_and_descendants end alias_method :recursive_descendants, :descendants def self_and_descendants - object_hierarchy(self.class.where(id: id)) - .base_and_descendants + object_hierarchy(self.class.where(id: id)).base_and_descendants end alias_method :recursive_self_and_descendants, :self_and_descendants def self_and_descendant_ids - recursive_self_and_descendants.select(:id) + object_hierarchy(self.class.where(id: id)).base_and_descendant_ids end alias_method :recursive_self_and_descendant_ids, :self_and_descendant_ids diff --git a/db/migrate/20220602130306_add_namespace_type_index.rb b/db/migrate/20220602130306_add_namespace_type_index.rb new file mode 100644 index 00000000000000..b20f36b3278b5b --- /dev/null +++ b/db/migrate/20220602130306_add_namespace_type_index.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddNamespaceTypeIndex < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + INDEX_NAME = 'index_groups_on_parent_id_id' + + def up + add_concurrent_index :namespaces, [:parent_id, :id], where: "type = 'Group'", name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name(:namespaces, INDEX_NAME) + end +end diff --git a/db/schema_migrations/20220602130306 b/db/schema_migrations/20220602130306 new file mode 100644 index 00000000000000..836f2385b650fc --- /dev/null +++ b/db/schema_migrations/20220602130306 @@ -0,0 +1 @@ +493009101e8b1340507ff8cf5d6add16f848d8d99f0b6091bf7b07105f711304 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 9eff36609e7838..d183e2fa054634 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -28098,6 +28098,8 @@ CREATE UNIQUE INDEX index_group_wiki_repositories_on_disk_path ON group_wiki_rep CREATE INDEX index_group_wiki_repositories_on_shard_id ON group_wiki_repositories USING btree (shard_id); +CREATE INDEX index_groups_on_parent_id_id ON namespaces USING btree (parent_id, id) WHERE ((type)::text = 'Group'::text); + CREATE INDEX index_historical_data_on_recorded_at ON historical_data USING btree (recorded_at); CREATE UNIQUE INDEX index_http_integrations_on_active_and_project_and_endpoint ON alert_management_http_integrations USING btree (active, project_id, endpoint_identifier) WHERE active; diff --git a/ee/lib/gitlab/analytics/cycle_analytics/summary/group/issue.rb b/ee/lib/gitlab/analytics/cycle_analytics/summary/group/issue.rb index 5405e863e513ba..8a1369f7f99f94 100644 --- a/ee/lib/gitlab/analytics/cycle_analytics/summary/group/issue.rb +++ b/ee/lib/gitlab/analytics/cycle_analytics/summary/group/issue.rb @@ -28,14 +28,11 @@ def value private - # rubocop: disable CodeReuse/ActiveRecord def issues_count issues = IssuesFinder.new(current_user, finder_params).execute - issues = issues.where(projects: { id: options[:projects] }) if options[:projects].present? - - ::Issue.where(id: issues.select(:id)).count + issues = issues.of_projects(options[:projects]) if options[:projects].present? + issues.count end - # rubocop: enable CodeReuse/ActiveRecord def finder_params options.dup.tap do |hash| diff --git a/lib/gitlab/object_hierarchy.rb b/lib/gitlab/object_hierarchy.rb index 693f1470d9d246..9a850246221e0c 100644 --- a/lib/gitlab/object_hierarchy.rb +++ b/lib/gitlab/object_hierarchy.rb @@ -92,6 +92,14 @@ def base_and_descendants(with_depth: false) end # rubocop: enable CodeReuse/ActiveRecord + # Returns a relation that includes ID of the descendants_base set of objects + # and all their descendants IDs (recursively). + # rubocop: disable CodeReuse/ActiveRecord + def base_and_descendant_ids + read_only(base_and_descendant_ids_cte.apply_to(unscoped_model.select(objects_table[:id]))) + end + # rubocop: enable CodeReuse/ActiveRecord + # Returns a relation that includes the base objects, their ancestors, # and the descendants of the base objects. # @@ -214,6 +222,26 @@ def base_and_descendants_cte(with_depth: false) end # rubocop: enable CodeReuse/ActiveRecord + # rubocop: disable CodeReuse/ActiveRecord + def base_and_descendant_ids_cte + cte = SQL::RecursiveCTE.new(:base_and_descendants) + + base_query = descendants_base.except(:order).select(objects_table[:id]) + + cte << base_query + + # Recursively get all the descendants of the base set. + descendants_query = unscoped_model + .select(objects_table[:id]) + .from(from_tables(cte)) + .where(descendant_conditions(cte)) + .except(:order) + + cte << descendants_query + cte + end + # rubocop: enable CodeReuse/ActiveRecord + def objects_table model.arel_table end -- GitLab