PERF: defer user notifications (#5827)

app/services/post_alerter.rb

@@ -3,8 +3,7 @@ require_dependency 'user_action_creator'
 
 class PostAlerter
   def self.post_created(post, opts = {})
-    alerter = PostAlerter.new(opts)
+    PostAlerter.new(opts).after_save_post(post, true)
-    alerter.after_save_post(post, true)
     post
   end
 
@@ -90,30 +89,8 @@ class PostAlerter
     # private messages
     if new_record
       if post.topic.private_message?
-        # users that aren't part of any mentioned groups
+        notify_pm_users(post, reply_to_user, notified)
-        users = directly_targeted_users(post).reject { |u| notified.include?(u) }
-        DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
-        users.each do |user|
-          notification_level = TopicUser.get(post.topic, user).try(:notification_level)
-          if reply_to_user == user || notification_level == TopicUser.notification_levels[:watching] || user.staged?
-            create_notification(user, Notification.types[:private_message], post)
-          end
-        end
-        # users that are part of all mentionned groups
-        users = indirectly_targeted_users(post).reject { |u| notified.include?(u) }
-        DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
-        users.each do |user|
-          # only create a notification when watching the group
-          notification_level = TopicUser.get(post.topic, user).try(:notification_level)
-
-          if notification_level == TopicUser.notification_levels[:watching]
-            create_notification(user, Notification.types[:private_message], post)
-          elsif notification_level == TopicUser.notification_levels[:tracking]
-            notify_group_summary(user, post)
-          end
-        end
       elsif post.post_type == Post.types[:regular]
-        # If it's not a private message and it's not an automatic post caused by a moderator action, notify the users
         notify_post_users(post, notified)
       end
     end
@@ -153,10 +130,12 @@ class PostAlerter
     # Don't notify the OP
     user_ids -= [post.user_id]
     users = User.where(id: user_ids)
-    DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
 
-    users.each do |user|
+    Scheduler::Defer.later("Notify First Post Watchers") do
-      create_notification(user, Notification.types[:watching_first_post], post)
+      DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
+      users.each do |user|
+        create_notification(user, Notification.types[:watching_first_post], post)
+      end
     end
   end
 
@@ -222,7 +201,6 @@ class PostAlerter
   end
 
   def notify_group_summary(user, post)
-
     @group_stats ||= {}
     stats = (@group_stats[post.topic_id] ||= group_stats(post.topic))
     return unless stats
@@ -262,12 +240,9 @@ class PostAlerter
   end
 
   def should_notify_like?(user, notification)
-
     return true if user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:always]
-
     return true if user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:first_time_and_daily] && notification.created_at < 1.day.ago
-
+    false
-    return false
   end
 
   def should_notify_previous?(user, notification, opts)
@@ -504,59 +479,101 @@ class PostAlerter
     users = [users] unless users.is_a?(Array)
     users = users.reject { |u| u.staged? } if post.topic&.private_message?
 
-    DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
+    Scheduler::Defer.later("Notify Users") do
-    users.each do |u|
+      DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
-      create_notification(u, Notification.types[type], post, opts)
+      users.each do |u|
+        create_notification(u, Notification.types[type], post, opts)
+      end
     end
 
     users
   end
 
+  def notify_pm_users(post, reply_to_user, notified)
+    return unless post.topic
+
+    Scheduler::Defer.later("Notify PM Users") do
+      # users that aren't part of any mentioned groups
+      users = directly_targeted_users(post).reject { |u| notified.include?(u) }
+      DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
+      users.each do |user|
+        notification_level = TopicUser.get(post.topic, user)&.notification_level
+        if reply_to_user == user || notification_level == TopicUser.notification_levels[:watching] || user.staged?
+          create_notification(user, Notification.types[:private_message], post)
+        end
+      end
+
+      # users that are part of all mentionned groups
+      users = indirectly_targeted_users(post).reject { |u| notified.include?(u) }
+      DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
+      users.each do |user|
+        case TopicUser.get(post.topic, user)&.notification_level
+        when TopicUser.notification_levels[:watching]
+          # only create a notification when watching the group
+          create_notification(user, Notification.types[:private_message], post)
+        when TopicUser.notification_levels[:tracking]
+          notify_group_summary(user, post)
+        end
+      end
+    end
+  end
+
   def notify_post_users(post, notified)
     return unless post.topic
 
-    condition = <<SQL
+    condition = <<~SQL
+      id IN (
+        SELECT user_id
+          FROM topic_users
+         WHERE notification_level = :watching
+           AND topic_id = :topic_id
 
-    id IN (
+         UNION
-      SELECT user_id FROM topic_users
-        WHERE notification_level = :watching AND topic_id = :topic_id
 
-      UNION ALL
+        SELECT cu.user_id
+          FROM category_users cu
+     LEFT JOIN topic_users tu ON tu.user_id = cu.user_id
+                             AND tu.topic_id = :topic_id
+         WHERE cu.notification_level = :watching
+           AND cu.category_id = :category_id
+           AND tu.user_id IS NULL
 
-      SELECT cu.user_id FROM category_users cu
+        /*tags*/
-      LEFT JOIN topic_users tu ON tu.user_id = cu.user_id AND tu.topic_id = :topic_id
+      )
-      WHERE cu.notification_level = :watching AND cu.category_id = :category_id AND tu.user_id IS NULL
+    SQL
-
-      /*tags*/
-    )
-SQL
 
     tag_ids = post.topic.topic_tags.pluck('topic_tags.tag_id')
-    if tag_ids.present?
-      condition.sub! "/*tags*/", <<SQL
-      UNION ALL
 
-      SELECT tag_users.user_id FROM tag_users
+    if tag_ids.present?
-      LEFT JOIN topic_users tu ON tu.user_id = tag_users.user_id AND tu.topic_id = :topic_id
+      condition.sub! "/*tags*/", <<~SQL
-      WHERE tag_users.notification_level = :watching AND tag_users.tag_id IN (:tag_ids) AND tu.user_id IS NULL
+        UNION
-SQL
+
+        SELECT tag_users.user_id
+          FROM tag_users
+     LEFT JOIN topic_users tu ON tu.user_id = tag_users.user_id
+                             AND tu.topic_id = :topic_id
+         WHERE tag_users.notification_level = :watching
+           AND tag_users.tag_id IN (:tag_ids)
+           AND tu.user_id IS NULL
+      SQL
     end
 
     notify = User.where(condition,
-                          watching: TopicUser.notification_levels[:watching],
+      watching: TopicUser.notification_levels[:watching],
-                          topic_id: post.topic_id,
+      topic_id: post.topic_id,
-                          category_id: post.topic.category_id,
+      category_id: post.topic.category_id,
-                          tag_ids: tag_ids
+      tag_ids: tag_ids
-                       )
+    )
 
     exclude_user_ids = notified.map(&:id)
     notify = notify.where("id NOT IN (?)", exclude_user_ids) if exclude_user_ids.present?
 
-    DiscourseEvent.trigger(:before_create_notifications_for_users, notify, post)
+    Scheduler::Defer.later("Notify Post Users") do
-
+      DiscourseEvent.trigger(:before_create_notifications_for_users, notify, post)
-    notify.pluck(:id).each do |user_id|
+      notify.pluck(:id).each do |user_id|
-      user = User.find_by(id: user_id)
+        user = User.find_by(id: user_id)
-      create_notification(user, Notification.types[:posted], post)
+        create_notification(user, Notification.types[:posted], post)
+      end
     end
   end
 

lib/scheduler/defer.rb

@@ -28,7 +28,7 @@ module Scheduler
 
     def later(desc = nil, db = RailsMultisite::ConnectionManagement.current_db, &blk)
       if @async
-        start_thread unless (@thread && @thread.alive?) || @paused
+        start_thread unless @thread&.alive? || @paused
         @queue << [db, blk, desc]
       else
         blk.call
@@ -36,13 +36,13 @@ module Scheduler
     end
 
     def stop!
-      @thread.kill if @thread && @thread.alive?
+      @thread.kill if @thread&.alive?
       @thread = nil
     end
 
     # test only
     def stopped?
-      !(@thread && @thread.alive?)
+      !@thread&.alive?
     end
 
     def do_all_work
@@ -55,12 +55,8 @@ module Scheduler
 
     def start_thread
       @mutex.synchronize do
-        return if @thread && @thread.alive?
+        return if @thread&.alive?
-        @thread = Thread.new {
+        @thread = Thread.new { do_work while true }
-          while true
-            do_work
-          end
-        }
       end
     end