FEATURE: limit number of notifications per user to 10,000

FEATURE: limit number of notifications per user to 10,000

Introduces a new site setting max_notifications_per_user.

Out-of-the-box this is set to 10,000. If a user exceeds this number of notifications, we will delete the oldest notifications keeping only 10,000.

To disable this safeguard set the setting to 0.

Enforcement happens weekly.

This is in place to protect the system from pathological states where a single user has enormous amounts of notifications causing various queries to time out. In practice nobody looks back more than a few hundred notifications.

diff --git a/app/jobs/scheduled/weekly.rb b/app/jobs/scheduled/weekly.rb
index 164650e0e7..f149684b2b 100644
--- a/app/jobs/scheduled/weekly.rb
+++ b/app/jobs/scheduled/weekly.rb
@@ -14,6 +14,7 @@ module Jobs
       UserAuthToken.cleanup!
       Upload.reset_unknown_extensions!
       Email::Cleaner.delete_rejected!
+      Notification.purge_old!
     end
   end
 end
diff --git a/app/models/notification.rb b/app/models/notification.rb
index fc0e60805b..b9443d89fb 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -44,6 +44,25 @@ class Notification < ActiveRecord::Base
     send_email unless NotificationConsolidator.new(self).consolidate!
   end
 
+  def self.purge_old!
+    return if SiteSetting.max_notifications_per_user == 0
+
+    DB.exec(<<~SQL, SiteSetting.max_notifications_per_user)
+      DELETE FROM notifications n1
+      USING (
+        SELECT * FROM (
+          SELECT
+            user_id,
+            id,
+            rank() OVER (PARTITION BY user_id ORDER BY id DESC)
+          FROM notifications
+        ) AS X
+        WHERE rank = ?
+      ) n2
+      WHERE n1.user_id = n2.user_id AND n1.id < n2.id
+    SQL
+  end
+
   def self.ensure_consistency!
     DB.exec(<<~SQL, Notification.types[:private_message])
       DELETE
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 1b250cdb78..577180be98 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1994,6 +1994,8 @@ en:
 
     user_selected_primary_groups: "Allow users to set their own primary group"
 
+    max_notifications_per_user: "Maximum amount of notifications per user, if this number is exceeded old notifications will be deleted. Enforced weekly. Set to 0 to disable"
+
     user_website_domains_whitelist: "User website will be verified against these domains. Pipe-delimited list."
 
     allow_profile_backgrounds: "Allow users to upload profile backgrounds."
diff --git a/config/site_settings.yml b/config/site_settings.yml
index d35b80c3f6..6ea8733db4 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -592,6 +592,8 @@ users:
   user_selected_primary_groups:
     default: false
     client: true
+  max_notifications_per_user:
+    default: 10000
 
 groups:
   enable_group_directory:
diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb
index 0139780c19..f8bf62a82f 100644
--- a/spec/models/notification_spec.rb
+++ b/spec/models/notification_spec.rb
@@ -414,4 +414,26 @@ describe Notification do
       end
     end
   end
+
+  describe "purge_old!" do
+    fab!(:user) { Fabricate(:user) }
+    fab!(:notification1) { Fabricate(:notification, user: user) }
+    fab!(:notification2) { Fabricate(:notification, user: user) }
+    fab!(:notification3) { Fabricate(:notification, user: user) }
+    fab!(:notification4) { Fabricate(:notification, user: user) }
+
+    it "does nothing if set to 0" do
+      SiteSetting.max_notifications_per_user = 0
+      Notification.purge_old!
+
+      expect(Notification.where(user_id: user.id).count).to eq(4)
+    end
+
+    it "correctly limits" do
+      SiteSetting.max_notifications_per_user = 2
+      Notification.purge_old!
+
+      expect(Notification.where(user_id: user.id).pluck(:id)).to contain_exactly(notification4.id, notification3.id)
+    end
+  end
 end

GitHub sha: 372f6f4f

2 Likes