PERF: Add indexes to speed up notifications queries by user menu (#26048)
Why this change? There are two problematic queries in question here when loading notifications in various tabs in the user menu: ``` SELECT "notifications".* FROM "notifications" LEFT JOIN topics ON notifications.topic_id = topics.id WHERE "notifications"."user_id" = 1338 AND (topics.id IS NULL OR topics.deleted_at IS NULL) ORDER BY notifications.high_priority AND NOT notifications.read DESC, NOT notifications.read AND notifications.notification_type NOT IN (5,19,25) DESC, notifications.created_at DESC LIMIT 30; ``` and ``` EXPLAIN ANALYZE SELECT "notifications".* FROM "notifications" LEFT JOIN topics ON notifications.topic_id = topics.id WHERE "notifications"."user_id" = 1338 AND (topics.id IS NULL OR topics.deleted_at IS NULL) AND "notifications"."notification_type" IN (5, 19, 25) ORDER BY notifications.high_priority AND NOT notifications.read DESC, NOT notifications.read DESC, notifications.created_at DESC LIMIT 30; ``` For a particular user, the queries takes about 40ms and 26ms respectively on one of our production instance where the user has 10K notifications while the site has 600K notifications in total. What does this change do? 1. Adds the `index_notifications_user_menu_ordering` index to the `notifications` table which is indexed on `(user_id, (high_priority AND NOT read) DESC, (NOT read) DESC, created_at DESC)`. 1. Adds a second index `index_notifications_user_menu_ordering_deprioritized_likes` to the `notifications` table which is indexed on `(user_id, (high_priority AND NOT read) DESC, (NOT read AND notification_type NOT IN (5,19,25)) DESC, created_at DESC)`. Note that we have to hardcode the like typed notifications type here as it is being used in an ordering clause. With the two indexes above, both queries complete in roughly 0.2ms. While I acknowledge that there will be some overhead in insert,update or delete operations. I believe this trade-off is worth it since viewing notifications in the user menu is something that is at the core of using a Discourse forum so we should optimise this experience as much as possible.
This commit is contained in:
parent
f935b2ca6d
commit
898b71da88
|
@ -37,6 +37,7 @@ class Notification < ActiveRecord::Base
|
|||
scope :prioritized,
|
||||
->(deprioritized_types = []) do
|
||||
scope = order("notifications.high_priority AND NOT notifications.read DESC")
|
||||
|
||||
if deprioritized_types.present?
|
||||
scope =
|
||||
scope.order(
|
||||
|
@ -48,8 +49,10 @@ class Notification < ActiveRecord::Base
|
|||
else
|
||||
scope = scope.order("NOT notifications.read DESC")
|
||||
end
|
||||
|
||||
scope.order("notifications.created_at DESC")
|
||||
end
|
||||
|
||||
scope :for_user_menu,
|
||||
->(user_id, limit: 30) do
|
||||
where(user_id: user_id).visible.prioritized.includes(:topic).limit(limit)
|
||||
|
@ -258,6 +261,9 @@ class Notification < ActiveRecord::Base
|
|||
Post.find_by(topic_id: topic_id, post_number: post_number)
|
||||
end
|
||||
|
||||
# Update `index_notifications_user_menu_ordering_deprioritized_likes` index when updating this as this is used by
|
||||
# `Notification.prioritized_list` to deprioritize like typed notifications. Also See
|
||||
# `db/migrate/20240306063428_add_indexes_to_notifications.rb`.
|
||||
def self.like_types
|
||||
[
|
||||
Notification.types[:liked],
|
||||
|
@ -421,4 +427,6 @@ end
|
|||
# index_notifications_on_user_id_and_topic_id_and_post_number (user_id,topic_id,post_number)
|
||||
# index_notifications_read_or_not_high_priority (user_id,id DESC,read,topic_id) WHERE (read OR (high_priority = false))
|
||||
# index_notifications_unique_unread_high_priority (user_id,id) UNIQUE WHERE ((NOT read) AND (high_priority = true))
|
||||
# index_notifications_user_menu_ordering (user_id, ((high_priority AND (NOT read))) DESC, ((NOT read)) DESC, created_at DESC)
|
||||
# index_notifications_user_menu_ordering_deprioritized_likes (user_id, ((high_priority AND (NOT read))) DESC, (((NOT read) AND (notification_type <> ALL (ARRAY[5, 19, 25])))) DESC, created_at DESC)
|
||||
#
|
||||
|
|
|
@ -0,0 +1,39 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class AddIndexesToNotifications < ActiveRecord::Migration[7.0]
|
||||
disable_ddl_transaction!
|
||||
|
||||
def up
|
||||
execute <<~SQL
|
||||
DROP INDEX IF EXISTS index_notifications_user_menu_ordering
|
||||
SQL
|
||||
|
||||
execute <<~SQL
|
||||
CREATE INDEX CONCURRENTLY index_notifications_user_menu_ordering
|
||||
ON notifications (
|
||||
user_id,
|
||||
(high_priority AND NOT read) DESC,
|
||||
(NOT read) DESC,
|
||||
created_at DESC
|
||||
);
|
||||
SQL
|
||||
|
||||
execute <<~SQL
|
||||
DROP INDEX IF EXISTS index_notifications_user_menu_ordering_deprioritized_likes
|
||||
SQL
|
||||
|
||||
execute <<~SQL
|
||||
CREATE INDEX CONCURRENTLY index_notifications_user_menu_ordering_deprioritized_likes
|
||||
ON notifications (
|
||||
user_id,
|
||||
(high_priority AND NOT read) DESC,
|
||||
(NOT read AND notification_type NOT IN (5,19,25)) DESC,
|
||||
created_at DESC
|
||||
);
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
|
@ -586,6 +586,7 @@ RSpec.describe Notification do
|
|||
expect(Notification.prioritized_list(user, count: 1).map(&:id)).to eq(
|
||||
[unread_high_priority_2].map(&:id),
|
||||
)
|
||||
|
||||
expect(Notification.prioritized_list(user, count: 3).map(&:id)).to eq(
|
||||
[unread_high_priority_2, unread_high_priority_1, unread_regular_2].map(&:id),
|
||||
)
|
||||
|
|
Loading…
Reference in New Issue