Merge branch '46648-timeout-searching-group-issues' into 'master'

Resolve "Timeout searching group issues"

Closes #46648

See merge request gitlab-org/gitlab-ce!19429

5 files changed, 36 insertions, 23 deletions

diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb
index ca1b80a36a0..2ef2ee76855 100644
--- a/app/controllers/concerns/issuable_collections.rb
+++ b/app/controllers/concerns/issuable_collections.rb
@@ -95,12 +95,7 @@ module IssuableCollections
     elsif @group
       @filter_params[:group_id] = @group.id
       @filter_params[:include_subgroups] = true
-    else
-      # TODO: this filter ignore issues/mr created in public or
-      # internal repos where you are not a member. Enable this filter
-      # or improve current implementation to filter only issues you
-      # created or assigned or mentioned
-      # @filter_params[:authorized_only] = true
+      @filter_params[:use_cte_for_search] = true
     end
 
     @filter_params.permit(finder_type.valid_params)
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index c6ef79ce15e..5d5f72c4d86 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -23,6 +23,7 @@
 #     created_before: datetime
 #     updated_after: datetime
 #     updated_before: datetime
+#     use_cte_for_search: boolean
 #
 class IssuableFinder
   prepend FinderWithCrossProjectAccess
@@ -54,6 +55,7 @@ class IssuableFinder
       sort
       state
       include_subgroups
+      use_cte_for_search
     ]
   end
 
@@ -74,19 +76,21 @@ class IssuableFinder
     items = init_collection
     items = filter_items(items)
 
-    # Filtering by project HAS TO be the last because we use the project IDs yielded by the issuable query thus far
-    items = by_project(items)
+    # This has to be last as we may use a CTE as an optimization fence by
+    # passing the use_cte_for_search param
+    # https://www.postgresql.org/docs/current/static/queries-with.html
+    items = by_search(items)
 
     sort(items)
   end
 
   def filter_items(items)
+    items = by_project(items)
     items = by_scope(items)
     items = by_created_at(items)
     items = by_updated_at(items)
     items = by_state(items)
     items = by_group(items)
-    items = by_search(items)
     items = by_assignee(items)
     items = by_author(items)
     items = by_non_archived(items)
@@ -107,7 +111,6 @@ class IssuableFinder
   #
   def count_by_state
     count_params = params.merge(state: nil, sort: nil)
-    labels_count = label_names.any? ? label_names.count : 1
     finder = self.class.new(current_user, count_params)
     counts = Hash.new(0)
 
@@ -116,6 +119,11 @@ class IssuableFinder
     # per issuable, so we have to count those in Ruby - which is bad, but still
     # better than performing multiple queries.
     #
+    # This does not apply when we are using a CTE for the search, as the labels
+    # GROUP BY is inside the subquery in that case, so we set labels_count to 1.
+    labels_count = label_names.any? ? label_names.count : 1
+    labels_count = 1 if use_cte_for_search?
+
     finder.execute.reorder(nil).group(:state).count.each do |key, value|
       counts[Array(key).last.to_sym] += value / labels_count
     end
@@ -159,10 +167,7 @@ class IssuableFinder
         finder_options = { include_subgroups: params[:include_subgroups], only_owned: true }
         GroupProjectsFinder.new(group: group, current_user: current_user, options: finder_options).execute
       else
-        opts = { current_user: current_user }
-        opts[:project_ids_relation] = item_project_ids(items) if items
-
-        ProjectsFinder.new(opts).execute
+        ProjectsFinder.new(current_user: current_user).execute
       end
 
     @projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil)
@@ -329,8 +334,24 @@ class IssuableFinder
     items
   end
 
+  def use_cte_for_search?
+    return false unless search
+    return false unless Gitlab::Database.postgresql?
+
+    params[:use_cte_for_search]
+  end
+
   def by_search(items)
-    search ? items.full_search(search) : items
+    return items unless search
+
+    if use_cte_for_search?
+      cte = Gitlab::SQL::RecursiveCTE.new(klass.table_name)
+      cte << items
+
+      items = klass.with(cte.to_arel).from(klass.table_name)
+    end
+
+    items.full_search(search)
   end
 
   def by_iids(items)
diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb
index 3626670d141..24a6b9349a0 100644
--- a/app/finders/issues_finder.rb
+++ b/app/finders/issues_finder.rb
@@ -136,8 +136,4 @@ class IssuesFinder < IssuableFinder
       items
     end
   end
-
-  def item_project_ids(items)
-    items&.reorder(nil)&.select(:project_id)
-  end
 end
diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb
index e2240e5e0d8..8d84ed4bdfb 100644
--- a/app/finders/merge_requests_finder.rb
+++ b/app/finders/merge_requests_finder.rb
@@ -56,8 +56,4 @@ class MergeRequestsFinder < IssuableFinder
 
     items.where(target_branch: target_branch)
   end
-
-  def item_project_ids(items)
-    items&.reorder(nil)&.select(:target_project_id)
-  end
 end
diff --git a/changelogs/unreleased/46648-timeout-searching-group-issues.yml b/changelogs/unreleased/46648-timeout-searching-group-issues.yml
new file mode 100644
index 00000000000..54401edf5cc
--- /dev/null
+++ b/changelogs/unreleased/46648-timeout-searching-group-issues.yml
@@ -0,0 +1,5 @@
+---
+title: Improve performance of group issues filtering on GitLab.com
+merge_request: 19429
+author:
+type: performance