PERF: Perform user filtering in SQL (#13358)

Notifying about a tag change sometimes resulted in loading a large
number of users in memory just to perform an exclusion. This commit
prefers to do inclusion (i.e. instead of exclude users X, do include
users in groups Y) and does it in SQL to avoid fetching unnecessary
data that is later discarded.
This commit is contained in:
Dan Ungureanu 2021-06-11 03:55:50 +03:00 committed by GitHub
parent fbfd54a941
commit fa02775095
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 13 deletions

View File

@ -7,23 +7,20 @@ module Jobs
if post&.topic&.visible?
post_alerter = PostAlerter.new
post_alerter.notify_post_users(post, excluded_users(args), include_topic_watchers: !post.topic.private_message?, include_category_watchers: false)
post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]),
group_ids: all_tags_in_hidden_groups?(args) ? tag_group_ids(args) : nil,
include_topic_watchers: !post.topic.private_message?,
include_category_watchers: false
)
post_alerter.notify_first_post_watchers(post, post_alerter.tag_watchers(post.topic))
end
end
private
def excluded_users(args)
if !args[:diff_tags] || !all_tags_in_hidden_groups?(args)
return User.where(id: args[:notified_user_ids])
end
group_users_join = DB.sql_fragment("LEFT JOIN group_users ON group_users.user_id = users.id AND group_users.group_id IN (:group_ids)", group_ids: tag_group_ids(args))
condition = DB.sql_fragment("group_users.id IS NULL OR users.id IN (:notified_user_ids)", notified_user_ids: args[:notified_user_ids])
User.joins(group_users_join).where(condition)
end
def all_tags_in_hidden_groups?(args)
return false if args[:diff_tags].blank?
Tag
.where(name: args[:diff_tags])
.joins(tag_groups: :tag_group_permissions)

View File

@ -651,13 +651,13 @@ class PostAlerter
emails_to_skip_send
end
def notify_post_users(post, notified, include_topic_watchers: true, include_category_watchers: true, include_tag_watchers: true, new_record: false)
def notify_post_users(post, notified, group_ids: nil, include_topic_watchers: true, include_category_watchers: true, include_tag_watchers: true, new_record: false)
return unless post.topic
warn_if_not_sidekiq
condition = +<<~SQL
id IN (
users.id IN (
SELECT id FROM users WHERE false
/*topic*/
/*category*/
@ -711,12 +711,16 @@ class PostAlerter
tag_ids: tag_ids
)
if group_ids.present?
notify = notify.joins(:group_users).where("group_users.group_id IN (?)", group_ids)
end
if post.topic.private_message?
notify = notify.where(staged: false).staff
end
exclude_user_ids = notified.map(&:id)
notify = notify.where("id NOT IN (?)", exclude_user_ids) if exclude_user_ids.present?
notify = notify.where("users.id NOT IN (?)", exclude_user_ids) if exclude_user_ids.present?
DiscourseEvent.trigger(:before_create_notifications_for_users, notify, post)