From 1b6f753f7315e99475fb230140b3fde75d9237bf Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 23 Apr 2021 13:21:30 -0400 Subject: [PATCH] Performance improvements to comments search (#13489) * Explicitly pluck user_id from comments table * Change comment query * Remove indexes and add partial indexes * Update schema --- app/services/search/postgres/comment.rb | 31 ++++++------------- ...0423162805_remove_indexes_from_comments.rb | 15 +++++++++ ...62847_add_partial_indexes_from_comments.rb | 13 ++++++++ db/schema.rb | 7 ++--- 4 files changed, 40 insertions(+), 26 deletions(-) create mode 100644 db/migrate/20210423162805_remove_indexes_from_comments.rb create mode 100644 db/migrate/20210423162847_add_partial_indexes_from_comments.rb diff --git a/app/services/search/postgres/comment.rb b/app/services/search/postgres/comment.rb index 2a94f3078..76b9f83e6 100644 --- a/app/services/search/postgres/comment.rb +++ b/app/services/search/postgres/comment.rb @@ -23,13 +23,6 @@ module Search ].freeze private_constant :USER_ATTRIBUTES - ARTICLE_COMMENTABLE_QUERY = <<-SQL.freeze - LEFT JOIN articles - ON comments.commentable_id = articles.id - AND comments.commentable_type = 'Article' - SQL - private_constant :ARTICLE_COMMENTABLE_QUERY - DEFAULT_PER_PAGE = 60 private_constant :DEFAULT_PER_PAGE @@ -42,19 +35,6 @@ module Search MAX_PER_PAGE = 120 # to avoid querying too many items, we set a maximum amount for a page private_constant :MAX_PER_PAGE - # We filter comments for those that are: - # 1. On Articles - # 2. Not deleted - # 3. Not hidden by commentable user (i.e. an Article author didn't hide the comment) - # 4. Are attached to published articles - QUERY_FILTER = <<-SQL.freeze - comments.commentable_type = 'Article' AND - comments.deleted = false AND - comments.hidden_by_commentable_user = false AND - articles.published = true - SQL - private_constant :QUERY_FILTER - def self.search_documents( page: 0, per_page: DEFAULT_PER_PAGE, @@ -74,7 +54,14 @@ module Search page = page.to_i + 1 per_page = [(per_page || DEFAULT_PER_PAGE).to_i, MAX_PER_PAGE].min - relation = ::Comment.joins(ARTICLE_COMMENTABLE_QUERY).where(QUERY_FILTER) + relation = ::Comment + .where( + deleted: false, + hidden_by_commentable_user: false, + commentable_type: "Article", + ) + .joins("join articles on articles.id = comments.commentable_id") + .where("articles.published": true) relation = relation.search_comments(term).with_pg_search_highlight if term.present? @@ -92,7 +79,7 @@ module Search # (see https://github.com/forem/forem/pull/4744#discussion_r345698674 # and https://github.com/rails/rails/issues/15185#issuecomment-351868335 # for additional context) - user_ids = results.pluck(:user_id) + user_ids = results.pluck("comments.user_id") users = find_users(user_ids) serialize(results, users) diff --git a/db/migrate/20210423162805_remove_indexes_from_comments.rb b/db/migrate/20210423162805_remove_indexes_from_comments.rb new file mode 100644 index 000000000..f1c734584 --- /dev/null +++ b/db/migrate/20210423162805_remove_indexes_from_comments.rb @@ -0,0 +1,15 @@ +class RemoveIndexesFromComments < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def up + remove_index :comments, column: :commentable_type, algorithm: :concurrently if index_exists?(:comments, :commentable_type) + remove_index :comments, column: :deleted, algorithm: :concurrently if index_exists?(:comments, :deleted) + remove_index :comments, column: :hidden_by_commentable_user, algorithm: :concurrently if index_exists?(:comments, :hidden_by_commentable_user) + end + + def down + add_index :comments, :commentable_type, algorithm: :concurrently unless index_exists?(:comments, :commentable_type) + add_index :comments, :deleted, algorithm: :concurrently unless index_exists?(:comments, :deleted) + add_index :comments, :hidden_by_commentable_user, algorithm: :concurrently unless index_exists?(:comments, :hidden_by_commentable_user) + end +end diff --git a/db/migrate/20210423162847_add_partial_indexes_from_comments.rb b/db/migrate/20210423162847_add_partial_indexes_from_comments.rb new file mode 100644 index 000000000..3466e8634 --- /dev/null +++ b/db/migrate/20210423162847_add_partial_indexes_from_comments.rb @@ -0,0 +1,13 @@ +class AddPartialIndexesFromComments < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def up + add_index :comments, :deleted, where: "deleted = false", algorithm: :concurrently unless index_exists?(:comments, :deleted) + add_index :comments, :hidden_by_commentable_user,where: "hidden_by_commentable_user = false", algorithm: :concurrently unless index_exists?(:comments, :hidden_by_commentable_user) + end + + def down + remove_index :comments, column: :deleted, algorithm: :concurrently if index_exists?(:comments, :deleted) + remove_index :comments, column: :hidden_by_commentable_user, algorithm: :concurrently if index_exists?(:comments, :hidden_by_commentable_user) + end +end diff --git a/db/schema.rb b/db/schema.rb index f4bbd33af..7519cfda1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_04_22_171642) do +ActiveRecord::Schema.define(version: 2021_04_23_162847) do # These are extensions that must be enabled in order to support this database enable_extension "citext" @@ -392,10 +392,9 @@ ActiveRecord::Schema.define(version: 2021_04_22_171642) do t.index ["ancestry"], name: "index_comments_on_ancestry" t.index ["ancestry"], name: "index_comments_on_ancestry_trgm", opclass: :gin_trgm_ops, using: :gin t.index ["commentable_id", "commentable_type"], name: "index_comments_on_commentable_id_and_commentable_type" - t.index ["commentable_type"], name: "index_comments_on_commentable_type" t.index ["created_at"], name: "index_comments_on_created_at" - t.index ["deleted"], name: "index_comments_on_deleted" - t.index ["hidden_by_commentable_user"], name: "index_comments_on_hidden_by_commentable_user" + t.index ["deleted"], name: "index_comments_on_deleted", where: "(deleted = false)" + t.index ["hidden_by_commentable_user"], name: "index_comments_on_hidden_by_commentable_user", where: "(hidden_by_commentable_user = false)" t.index ["score"], name: "index_comments_on_score" t.index ["user_id"], name: "index_comments_on_user_id" end