Commit ce57dc70 authored by Grzegorz Bizon's avatar Grzegorz Bizon

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

Resolve "Timeout searching group issues"

Closes #46648

See merge request gitlab-org/gitlab-ce!19429
parents 6e614bd5 c03386c3
...@@ -95,12 +95,7 @@ module IssuableCollections ...@@ -95,12 +95,7 @@ module IssuableCollections
elsif @group elsif @group
@filter_params[:group_id] = @group.id @filter_params[:group_id] = @group.id
@filter_params[:include_subgroups] = true @filter_params[:include_subgroups] = true
else @filter_params[:use_cte_for_search] = true
# 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
end end
@filter_params.permit(finder_type.valid_params) @filter_params.permit(finder_type.valid_params)
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
# created_before: datetime # created_before: datetime
# updated_after: datetime # updated_after: datetime
# updated_before: datetime # updated_before: datetime
# use_cte_for_search: boolean
# #
class IssuableFinder class IssuableFinder
prepend FinderWithCrossProjectAccess prepend FinderWithCrossProjectAccess
...@@ -54,6 +55,7 @@ class IssuableFinder ...@@ -54,6 +55,7 @@ class IssuableFinder
sort sort
state state
include_subgroups include_subgroups
use_cte_for_search
] ]
end end
...@@ -74,19 +76,21 @@ class IssuableFinder ...@@ -74,19 +76,21 @@ class IssuableFinder
items = init_collection items = init_collection
items = filter_items(items) 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 # This has to be last as we may use a CTE as an optimization fence by
items = by_project(items) # passing the use_cte_for_search param
# https://www.postgresql.org/docs/current/static/queries-with.html
items = by_search(items)
sort(items) sort(items)
end end
def filter_items(items) def filter_items(items)
items = by_project(items)
items = by_scope(items) items = by_scope(items)
items = by_created_at(items) items = by_created_at(items)
items = by_updated_at(items) items = by_updated_at(items)
items = by_state(items) items = by_state(items)
items = by_group(items) items = by_group(items)
items = by_search(items)
items = by_assignee(items) items = by_assignee(items)
items = by_author(items) items = by_author(items)
items = by_non_archived(items) items = by_non_archived(items)
...@@ -107,7 +111,6 @@ class IssuableFinder ...@@ -107,7 +111,6 @@ class IssuableFinder
# #
def count_by_state def count_by_state
count_params = params.merge(state: nil, sort: nil) 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) finder = self.class.new(current_user, count_params)
counts = Hash.new(0) counts = Hash.new(0)
...@@ -116,6 +119,11 @@ class IssuableFinder ...@@ -116,6 +119,11 @@ class IssuableFinder
# per issuable, so we have to count those in Ruby - which is bad, but still # per issuable, so we have to count those in Ruby - which is bad, but still
# better than performing multiple queries. # 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| finder.execute.reorder(nil).group(:state).count.each do |key, value|
counts[Array(key).last.to_sym] += value / labels_count counts[Array(key).last.to_sym] += value / labels_count
end end
...@@ -159,10 +167,7 @@ class IssuableFinder ...@@ -159,10 +167,7 @@ class IssuableFinder
finder_options = { include_subgroups: params[:include_subgroups], only_owned: true } finder_options = { include_subgroups: params[:include_subgroups], only_owned: true }
GroupProjectsFinder.new(group: group, current_user: current_user, options: finder_options).execute GroupProjectsFinder.new(group: group, current_user: current_user, options: finder_options).execute
else else
opts = { current_user: current_user } ProjectsFinder.new(current_user: current_user).execute
opts[:project_ids_relation] = item_project_ids(items) if items
ProjectsFinder.new(opts).execute
end end
@projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil) @projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil)
...@@ -329,8 +334,24 @@ class IssuableFinder ...@@ -329,8 +334,24 @@ class IssuableFinder
items items
end 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) 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 end
def by_iids(items) def by_iids(items)
......
...@@ -136,8 +136,4 @@ class IssuesFinder < IssuableFinder ...@@ -136,8 +136,4 @@ class IssuesFinder < IssuableFinder
items items
end end
end end
def item_project_ids(items)
items&.reorder(nil)&.select(:project_id)
end
end end
...@@ -56,8 +56,4 @@ class MergeRequestsFinder < IssuableFinder ...@@ -56,8 +56,4 @@ class MergeRequestsFinder < IssuableFinder
items.where(target_branch: target_branch) items.where(target_branch: target_branch)
end end
def item_project_ids(items)
items&.reorder(nil)&.select(:target_project_id)
end
end end
---
title: Improve performance of group issues filtering on GitLab.com
merge_request: 19429
author:
type: performance
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