PERF: backoff background requests when overloaded (#10888)

PERF: backoff background requests when overloaded (#10888)

When the server gets overloaded and lots of requests start queuing server will attempt to shed load by returning 429 errors on background requests.

The client can flag a request as background by setting the header: Discourse-Background to true

Out-of-the-box we shed load when the queue time goes above 0.5 seconds.

The only request we shed at the moment is the request to load up a new post when someone posts to a topic.

We can extend this as we go with a more general pattern on the client.

Previous to this change, rate limiting would “break” the post stream which would make suggested topics vanish and users would have to scroll the page to see more posts in the topic.

Server needs this protection for cases where tons of clients are navigated to a topic and a new post is made. This can lead to a self inflicted denial of service if enough clients are viewing the topic.

Due to the internal security design of Discourse it is hard for a large number of clients to share a channel where we would pass the full post body via the message bus.

It also renames (and deprecates) triggerNewPostInStream to triggerNewPostsInStream

This allows us to load a batch of new posts cleanly, so the controller can keep track of a backlog

Co-authored-by: Joffrey JAFFEUX j.jaffeux@gmail.com

diff --git a/.prettierignore b/.prettierignore
index 0d02324..8329465 100644
--- a/.prettierignore
+++ b/.prettierignore
@@ -23,3 +23,4 @@ app/assets/javascripts/discourse/tests/fixtures
 app/assets/javascripts/discourse/tests/helpers/assertions.js
 node_modules/
 dist/
+**/*.rb
diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js
index 03f90ce..c7b91cd 100644
--- a/app/assets/javascripts/discourse/app/controllers/topic.js
+++ b/app/assets/javascripts/discourse/app/controllers/topic.js
@@ -2,7 +2,7 @@ import I18n from "I18n";
 import { isPresent, isEmpty } from "@ember/utils";
 import { or, and, not, alias } from "@ember/object/computed";
 import EmberObject from "@ember/object";
-import { next, schedule } from "@ember/runloop";
+import { next, schedule, later } from "@ember/runloop";
 import Controller, { inject as controller } from "@ember/controller";
 import { bufferedProperty } from "discourse/mixins/buffered-content";
 import Composer from "discourse/models/composer";
@@ -30,6 +30,8 @@ import { deepMerge } from "discourse-common/lib/object";
 
 let customPostMessageCallbacks = {};
 
+const RETRIES_ON_RATE_LIMIT = 4;
+
 export function resetCustomPostMessageCallbacks() {
   customPostMessageCallbacks = {};
 }
@@ -1292,6 +1294,42 @@ export default Controller.extend(bufferedProperty("model"), {
     this.model.destroy(this.currentUser);
   },
 
+  retryOnRateLimit(times, promise, topicId) {
+    const currentTopicId = this.get("model.id");
+    topicId = topicId || currentTopicId;
+    if (topicId !== currentTopicId) {
+      // we navigated to another topic, so skip
+      return;
+    }
+
+    if (this.retryRateLimited || times <= 0) {
+      return;
+    }
+
+    promise().catch((e) => {
+      const xhr = e.jqXHR;
+      if (
+        xhr &&
+        xhr.status === 429 &&
+        xhr.responseJSON &&
+        xhr.responseJSON.extras &&
+        xhr.responseJSON.extras.wait_seconds
+      ) {
+        let waitSeconds = xhr.responseJSON.extras.wait_seconds;
+        if (waitSeconds < 5) {
+          waitSeconds = 5;
+        }
+
+        this.retryRateLimited = true;
+
+        later(() => {
+          this.retryRateLimited = false;
+          this.retryOnRateLimit(times - 1, promise, topicId);
+        }, waitSeconds * 1000);
+      }
+    });
+  },
+
   subscribe() {
     this.unsubscribe();
 
@@ -1363,7 +1401,22 @@ export default Controller.extend(bufferedProperty("model"), {
             break;
           }
           case "created": {
-            postStream.triggerNewPostInStream(data.id).then(() => refresh());
+            this.newPostsInStream = this.newPostsInStream || [];
+            this.newPostsInStream.push(data.id);
+
+            this.retryOnRateLimit(RETRIES_ON_RATE_LIMIT, () => {
+              const postIds = this.newPostsInStream;
+              this.newPostsInStream = [];
+
+              return postStream
+                .triggerNewPostsInStream(postIds, { background: true })
+                .then(() => refresh())
+                .catch((e) => {
+                  this.newPostsInStream = postIds.concat(this.newPostsInStream);
+                  throw e;
+                });
+            });
+
             if (this.get("currentUser.id") !== data.user_id) {
               this.documentTitle.incrementBackgroundContextCount();
             }
diff --git a/app/assets/javascripts/discourse/app/models/post-stream.js b/app/assets/javascripts/discourse/app/models/post-stream.js
index 340b1b2..d68c2ca 100644
--- a/app/assets/javascripts/discourse/app/models/post-stream.js
+++ b/app/assets/javascripts/discourse/app/models/post-stream.js
@@ -11,6 +11,7 @@ import { loadTopicView } from "discourse/models/topic";
 import { Promise } from "rsvp";
 import User from "discourse/models/user";
 import { deepMerge } from "discourse-common/lib/object";
+import deprecated from "discourse-common/lib/deprecated";
 
 export default RestModel.extend({
   _identityMap: null,
@@ -599,15 +600,25 @@ export default RestModel.extend({
     });
   },
 
+  /* mainly for backwards compatability with plugins, used in quick messages plugin
+   * TODO: remove July 2021
+   * */
+  triggerNewPostInStream(postId, opts) {
+    deprecated(
+      "Please use triggerNewPostsInStream, this method will be removed July 2021"
+    );
+    return this.triggerNewPostsInStream([postId], opts);
+  },
+
   /**
-    Finds and adds a post to the stream by id. Typically this would happen if we receive a message
+    Finds and adds posts to the stream by id. Typically this would happen if we receive a message
     from the message bus indicating there's a new post. We'll only insert it if we currently
     have no filters.
   **/
-  triggerNewPostInStream(postId) {
+  triggerNewPostsInStream(postIds, opts) {
     const resolved = Promise.resolve();
 
-    if (!postId) {
+    if (!postIds || postIds.length === 0) {
       return resolved;
     }
 
@@ -617,27 +628,46 @@ export default RestModel.extend({
     }
 
     const loadedAllPosts = this.loadedAllPosts;
+    this._loadingPostIds = this._loadingPostIds || [];
 
-    if (this.stream.indexOf(postId) === -1) {
-      this.stream.addObject(postId);
-      if (loadedAllPosts) {
-        this.set("loadingLastPost", true);
-        return this.findPostsByIds([postId])
-          .then((posts) => {
-            const ignoredUsers =
-              User.current() && User.current().get("ignored_users");
-            posts.forEach((p) => {
-              if (ignoredUsers && ignoredUsers.includes(p.username)) {
-                this.stream.removeObject(postId);
-                return;
-              }
-              this.appendPost(p);
-            });
-          })
-          .finally(() => {
-            this.set("loadingLastPost", false);
-          });
+    let missingIds = [];
+
+    postIds.forEach((postId) => {
+      if (postId && this.stream.indexOf(postId) === -1) {
+        missingIds.push(postId);
       }
+    });
+
+    if (missingIds.length === 0) {
+      return resolved;
+    }
+
+    if (loadedAllPosts) {
+      missingIds.forEach((postId) => {
+        if (this._loadingPostIds.indexOf(postId) === -1) {
+          this._loadingPostIds.push(postId);
+        }
+      });
+      this.set("loadingLastPost", true);
+      return this.findPostsByIds(this._loadingPostIds, opts)
+        .then((posts) => {
+          this._loadingPostIds = null;
+          const ignoredUsers =
+            User.current() && User.current().get("ignored_users");
+          posts.forEach((p) => {
+            if (ignoredUsers && ignoredUsers.includes(p.username)) {
+              this.stream.removeObject(p.id);
+              return;
+            }
+            this.stream.addObject(p.id);
+            this.appendPost(p);
+          });
+        })
+        .finally(() => {
+          this.set("loadingLastPost", false);
+        });
+    } else {
+      missingIds.forEach((postId) => this.stream.addObject(postId));
     }
 
     return resolved;
@@ -789,11 +819,11 @@ export default RestModel.extend({
   // Get the index in the stream of a post id. (Use this for the topic progress bar.)
   progressIndexOfPostId(post) {
     const postId = post.get("id");
-    const index = this.stream.indexOf(postId);
 
     if (this.isMegaTopic) {
       return post.get("post_number");
     } else {
+      const index = this.stream.indexOf(postId);
       return index + 1;
     }
   },
@@ -972,17 +1002,17 @@ export default RestModel.extend({
     });
   },
 
-  findPostsByIds(postIds) {
+  findPostsByIds(postIds, opts) {
     const identityMap = this._identityMap;
     const unloaded = postIds.filter((p) => !identityMap[p]);
 
     // Load our unloaded posts by id
-    return this.loadIntoIdentityMap(unloaded).then(() => {
+    return this.loadIntoIdentityMap(unloaded, opts).then(() => {
       return postIds.map((p) => identityMap[p]).compact();
     });
   },
 

[... diff too long, it was truncated ...]

GitHub sha: 32393f72

This commit appears in #10888 which was approved by ZogStriP. It was merged by SamSaffron.