From 70269c7c9727cf9c4d2f7d7fd4ce607257f37196 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Fri, 4 Jan 2019 09:24:46 +1100
Subject: [PATCH] FEATURE: tighter limits on per cluster post rebakes

We have the periodical job that regularly will rebake old posts. This is
used to trickle in update to cooked markdown. The problem is that each rebake
can issue multiple background jobs (post process and pull hotlinked images)

Previously we had no per-cluster limit so cluster running 100s of sites could
flood the sidekiq queue with rebake related jobs.

New system introduces a hard limit of 300 rebakes per 15 minutes across a
cluster to ensure the sidekiq job is not dominated by this.

We also reduced `rebake_old_posts_count` to 80, which is a safer default.
---
 app/models/post.rb             | 19 +++++++++++++++++++
 config/discourse_defaults.conf |  5 +++++
 config/site_settings.yml       |  2 +-
 spec/models/post_spec.rb       | 21 +++++++++++++++++++++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/app/models/post.rb b/app/models/post.rb
index 7279a0ff271..ff6c32890b4 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -516,13 +516,32 @@ class Post < ActiveRecord::Base
   end
 
   def self.rebake_old(limit)
+
+    limiter = RateLimiter.new(
+      nil,
+      "global_periodical_rebake_limit",
+      GlobalSetting.max_old_rebakes_per_15_minutes,
+      900,
+      global: true
+    )
+
     problems = []
     Post.where('baked_version IS NULL OR baked_version < ?', BAKED_VERSION)
       .order('id desc')
       .limit(limit).pluck(:id).each do |id|
       begin
+
+        break if !limiter.can_perform?
+
         post = Post.find(id)
         post.rebake!
+
+        begin
+          limiter.performed!
+        rescue RateLimiter::LimitExceeded
+          break
+        end
+
       rescue => e
         problems << { post: post, ex: e }
 
diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf
index 43ce40ba2d9..cde2c3666d1 100644
--- a/config/discourse_defaults.conf
+++ b/config/discourse_defaults.conf
@@ -202,3 +202,8 @@ force_anonymous_min_queue_seconds = 1
 # only trigger anon if we see more than N requests for this path in last 10 seconds
 force_anonymous_min_per_10_seconds = 3
 
+# maximum number of posts rebaked across the cluster in the periodical job
+# rebake process is very expensive, on multisite we have to make sure we never
+# flood the queue
+max_old_rebakes_per_15_minutes = 300
+
diff --git a/config/site_settings.yml b/config/site_settings.yml
index ebfd493e877..afaa044a15b 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1411,7 +1411,7 @@ developer:
   top_topics_formula_least_likes_per_post_multiplier:
     default: 3
   rebake_old_posts_count:
-    default: 100
+    default: 80
     min: 1
   migrate_to_new_scheme:
     hidden: true
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index dce839102db..9ddfccfb3fd 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -1153,6 +1153,27 @@ describe Post do
       post.reload
       expect(post.baked_at).to eq(baked)
     end
+
+    it "will rate limit globally" do
+
+      post1 = create_post
+      post2 = create_post
+      post3 = create_post
+
+      Post.where(id: [post1.id, post2.id, post3.id]).update_all(baked_version: -1)
+
+      global_setting :max_old_rebakes_per_15_minutes, 2
+
+      RateLimiter.clear_all_global!
+      RateLimiter.enable
+
+      Post.rebake_old(100)
+
+      expect(post3.reload.baked_version).not_to eq(-1)
+      expect(post2.reload.baked_version).not_to eq(-1)
+      expect(post1.reload.baked_version).to eq(-1)
+
+    end
   end
 
   describe ".unhide!" do