diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2019-04-05 11:49:13 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2019-04-05 11:49:13 +0300 |
commit | 43713d976a93677f3c90f1f1e926bf7d519e02bf (patch) | |
tree | 0758d4c905e618997c904f89e940f1786e0b0b86 /app | |
parent | 9a77ca11403d9223fcd549bac8c7d2aa52d9554c (diff) | |
parent | 10ceb33ba271f603fa09d4a4b5fdca03fd7ea333 (diff) |
Merge branch 'extend-cte-optimisations-to-projects' into 'master'
Extend CTE search optimisation to projects
Closes #55170
See merge request gitlab-org/gitlab-ce!26908
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/concerns/issuable_collections.rb | 1 | ||||
-rw-r--r-- | app/finders/issuable_finder.rb | 46 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 4 | ||||
-rw-r--r-- | app/models/concerns/sortable.rb | 28 |
4 files changed, 40 insertions, 39 deletions
diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index c529aabf797..6d6e0cc6c7f 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -100,6 +100,7 @@ module IssuableCollections if @project options[:project_id] = @project.id + options[:attempt_project_search_optimizations] = true elsif @group options[:group_id] = @group.id options[:include_subgroups] = true diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index b6be2895d85..64c88505a16 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -83,7 +83,7 @@ class IssuableFinder # https://www.postgresql.org/docs/current/static/queries-with.html items = by_search(items) - items = sort(items) unless use_cte_for_count? + items = sort(items) items end @@ -91,7 +91,6 @@ class IssuableFinder def filter_items(items) items = by_project(items) items = by_group(items) - items = by_subquery(items) items = by_scope(items) items = by_created_at(items) items = by_updated_at(items) @@ -131,10 +130,12 @@ class IssuableFinder # 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. # - # We always use CTE when searching in Groups if the feature flag is enabled, - # but never when searching in Projects. + # Groups and projects have separate feature flags to suggest the use + # of a CTE. The CTE will not be used if the sort doesn't support it, + # but will always be used for the counts here as we ignore sorting + # anyway. labels_count = label_names.any? ? label_names.count : 1 - labels_count = 1 if use_cte_for_count? + labels_count = 1 if use_cte_for_search? finder.execute.reorder(nil).group(:state).count.each do |key, value| counts[count_key(key)] += value / labels_count @@ -308,15 +309,14 @@ class IssuableFinder end # rubocop: enable CodeReuse/ActiveRecord - def use_subquery_for_search? - strong_memoize(:use_subquery_for_search) do - !force_cte? && attempt_group_search_optimizations? - end - end + def use_cte_for_search? + strong_memoize(:use_cte_for_search) do + next false unless search + next false unless Gitlab::Database.postgresql? + # Only simple unsorted & simple sorts can use CTE + next false if params[:sort].present? && !params[:sort].in?(klass.simple_sorts.keys) - def use_cte_for_count? - strong_memoize(:use_cte_for_count) do - force_cte? && attempt_group_search_optimizations? + attempt_group_search_optimizations? || attempt_project_search_optimizations? end end @@ -331,12 +331,15 @@ class IssuableFinder end def attempt_group_search_optimizations? - search && - Gitlab::Database.postgresql? && - params[:attempt_group_search_optimizations] && + params[:attempt_group_search_optimizations] && Feature.enabled?(:attempt_group_search_optimizations, default_enabled: true) end + def attempt_project_search_optimizations? + params[:attempt_project_search_optimizations] && + Feature.enabled?(:attempt_project_search_optimizations) + end + def count_key(value) Array(value).last.to_sym end @@ -407,20 +410,11 @@ class IssuableFinder end # rubocop: enable CodeReuse/ActiveRecord - # Wrap projects and groups in a subquery if the conditions are met. - def by_subquery(items) - if use_subquery_for_search? - klass.where(id: items.select(:id)) # rubocop: disable CodeReuse/ActiveRecord - else - items - end - end - # rubocop: disable CodeReuse/ActiveRecord def by_search(items) return items unless search - if use_cte_for_count? + if use_cte_for_search? cte = Gitlab::SQL::RecursiveCTE.new(klass.table_name) cte << items diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 51a8395c013..17f94b4bd9b 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -172,6 +172,10 @@ module Issuable fuzzy_search(query, matched_columns) end + def simple_sorts + super.except('name_asc', 'name_desc') + end + def sort_by_attribute(method, excluded_labels: []) sorted = case method.to_s diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb index 29e48f0c5f7..df1a9e3fe6e 100644 --- a/app/models/concerns/sortable.rb +++ b/app/models/concerns/sortable.rb @@ -21,19 +21,21 @@ module Sortable class_methods do def order_by(method) - case method.to_s - when 'created_asc' then order_created_asc - when 'created_date' then order_created_desc - when 'created_desc' then order_created_desc - when 'id_asc' then order_id_asc - when 'id_desc' then order_id_desc - when 'name_asc' then order_name_asc - when 'name_desc' then order_name_desc - when 'updated_asc' then order_updated_asc - when 'updated_desc' then order_updated_desc - else - all - end + simple_sorts.fetch(method.to_s, -> { all }).call + end + + def simple_sorts + { + 'created_asc' => -> { order_created_asc }, + 'created_date' => -> { order_created_desc }, + 'created_desc' => -> { order_created_desc }, + 'id_asc' => -> { order_id_asc }, + 'id_desc' => -> { order_id_desc }, + 'name_asc' => -> { order_name_asc }, + 'name_desc' => -> { order_name_desc }, + 'updated_asc' => -> { order_updated_asc }, + 'updated_desc' => -> { order_updated_desc } + } end private |