From 3d9c320aabd54c3cce1bb4e9f29769f0d7c4c9ab Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Thu, 9 Apr 2020 15:42:24 +0300 Subject: [PATCH] PERF: Cache Category.subcategory_ids (#9350) Also reset category cache after backup restore. --- app/models/category.rb | 44 ++++++++++++------- app/models/topic.rb | 10 ++--- .../initializers/014-track-setting-changes.rb | 2 + lib/backup_restore/restorer.rb | 7 +++ lib/topic_query.rb | 7 +-- 5 files changed, 43 insertions(+), 27 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index 5fb020c5562..44d115cad0a 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -74,6 +74,7 @@ class Category < ActiveRecord::Base after_save :publish_discourse_stylesheet after_save :publish_category after_save :reset_topic_ids_cache + after_save :clear_subcategory_ids after_save :clear_url_cache after_save :index_search after_save :update_reviewables @@ -153,23 +154,36 @@ class Category < ActiveRecord::Base Category.reset_topic_ids_cache end + @@subcategory_ids = DistributedCache.new('subcategory_ids') + def self.subcategory_ids(category_id) - sql = <<~SQL - WITH RECURSIVE subcategories AS ( - SELECT :category_id id, 1 depth - UNION - SELECT categories.id, (subcategories.depth + 1) depth - FROM categories - JOIN subcategories ON subcategories.id = categories.parent_category_id - WHERE subcategories.depth < :max_category_nesting + @@subcategory_ids[category_id] ||= + begin + sql = <<~SQL + WITH RECURSIVE subcategories AS ( + SELECT :category_id id, 1 depth + UNION + SELECT categories.id, (subcategories.depth + 1) depth + FROM categories + JOIN subcategories ON subcategories.id = categories.parent_category_id + WHERE subcategories.depth < :max_category_nesting + ) + SELECT id FROM subcategories + SQL + DB.query_single( + sql, + category_id: category_id, + max_category_nesting: SiteSetting.max_category_nesting ) - SELECT id FROM subcategories - SQL - DB.query_single( - sql, - category_id: category_id.to_i, - max_category_nesting: SiteSetting.max_category_nesting - ) + end + end + + def self.clear_subcategory_ids + @@subcategory_ids.clear + end + + def clear_subcategory_ids + Category.clear_subcategory_ids end def self.scoped_to_permissions(guardian, permission_types) diff --git a/app/models/topic.rb b/app/models/topic.rb index dd9460ce3a2..d080fbee325 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -191,9 +191,7 @@ class Topic < ActiveRecord::Base } scope :in_category_and_subcategories, lambda { |category_id| - where("topics.category_id = ? OR topics.category_id IN (SELECT id FROM categories WHERE categories.parent_category_id = ?)", - category_id, - category_id) if category_id + where("topics.category_id IN (?)", Category.subcategory_ids(category_id.to_i)) if category_id } scope :with_subtype, ->(subtype) { where('topics.subtype = ?', subtype) } @@ -1296,7 +1294,7 @@ class Topic < ActiveRecord::Base builder = DB.build(sql) builder.where("t.created_at >= :start_date", start_date: opts[:start_date]) if opts[:start_date] builder.where("t.created_at < :end_date", end_date: opts[:end_date]) if opts[:end_date] - builder.where("t.category_id IN (:subcategory_ids)", subcategory_ids: Category.subcategory_ids(opts[:category_id])) if opts[:category_id] + builder.where("t.category_id IN (:subcategory_ids)", subcategory_ids: Category.subcategory_ids(opts[:category_id].to_i)) if opts[:category_id] builder.where("t.archetype <> '#{Archetype.private_message}'") builder.where("t.deleted_at IS NULL") builder.where("p.deleted_at IS NULL") @@ -1335,7 +1333,7 @@ class Topic < ActiveRecord::Base builder = DB.build(WITH_NO_RESPONSE_SQL) builder.where("t.created_at >= :start_date", start_date: start_date) if start_date builder.where("t.created_at < :end_date", end_date: end_date) if end_date - builder.where("t.category_id IN (:subcategory_ids)", subcategory_ids: Category.subcategory_ids(category_id)) if category_id + builder.where("t.category_id IN (:subcategory_ids)", subcategory_ids: Category.subcategory_ids(category_id.to_i)) if category_id builder.where("t.archetype <> '#{Archetype.private_message}'") builder.where("t.deleted_at IS NULL") builder.query_hash @@ -1355,7 +1353,7 @@ class Topic < ActiveRecord::Base def self.with_no_response_total(opts = {}) builder = DB.build(WITH_NO_RESPONSE_TOTAL_SQL) - builder.where("t.category_id IN (:subcategory_ids)", subcategory_ids: Category.subcategory_ids(opts[:category_id])) if opts[:category_id] + builder.where("t.category_id IN (:subcategory_ids)", subcategory_ids: Category.subcategory_ids(opts[:category_id].to_i)) if opts[:category_id] builder.where("t.archetype <> '#{Archetype.private_message}'") builder.where("t.deleted_at IS NULL") builder.query_single.first.to_i diff --git a/config/initializers/014-track-setting-changes.rb b/config/initializers/014-track-setting-changes.rb index d3156d46d94..c469af48fc4 100644 --- a/config/initializers/014-track-setting-changes.rb +++ b/config/initializers/014-track-setting-changes.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true DiscourseEvent.on(:site_setting_changed) do |name, old_value, new_value| + Category.clear_subcategory_ids if name === :max_category_nesting + # Enabling `must_approve_users` on an existing site is odd, so we assume that the # existing users are approved. if name == :must_approve_users && new_value == true diff --git a/lib/backup_restore/restorer.rb b/lib/backup_restore/restorer.rb index eaf3ee4083d..7a2b37d01a4 100644 --- a/lib/backup_restore/restorer.rb +++ b/lib/backup_restore/restorer.rb @@ -52,6 +52,7 @@ module BackupRestore @system.disable_readonly_mode + clear_category_cache clear_emoji_cache clear_theme_cache @@ -119,6 +120,12 @@ module BackupRestore end end + def clear_category_cache + log "Clearing category cache..." + Category.reset_topic_ids_cache + Category.clear_subcategory_ids + end + def clear_emoji_cache log "Clearing emoji cache..." Emoji.clear_cache diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 24a35b021e2..fa851e916ab 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -669,7 +669,7 @@ class TopicQuery if options[:no_subcategories] result = result.where('categories.id = ?', category_id) else - result = result.where(<<~SQL, subcategory_ids: subcategory_ids(category_id), category_id: category_id) + result = result.where(<<~SQL, subcategory_ids: Category.subcategory_ids(category_id), category_id: category_id) categories.id in (:subcategory_ids) AND ( categories.topic_id <> topics.id OR categories.id = :category_id ) @@ -1052,11 +1052,6 @@ class TopicQuery private - def subcategory_ids(category_id) - @subcategory_ids ||= {} - @subcategory_ids[category_id] ||= Category.subcategory_ids(category_id) - end - def sanitize_sql_array(input) ActiveRecord::Base.public_send(:sanitize_sql_array, input.join(',')) end