PERF: do not include suggested topics when loading new posts

PERF: do not include suggested topics when loading new posts

When a new post is triggered via message bus post stream will attempt to load it, previously the /topic/TOPIC_ID/posts.json would unconditionally include suggested topics, this caused excessive load on the server.

New pattern defaults to exclude suggested and related topics from this API unless people explicitly ask for suggested.

diff --git a/app/assets/javascripts/discourse/models/post-stream.js.es6 b/app/assets/javascripts/discourse/models/post-stream.js.es6
index 8cc53ea..4188998 100644
--- a/app/assets/javascripts/discourse/models/post-stream.js.es6
+++ b/app/assets/javascripts/discourse/models/post-stream.js.es6
@@ -907,10 +907,13 @@ export default RestModel.extend({
   },
 
   fetchNextWindow(postNumber, asc, callback) {
+    let includeSuggested = !this.get("topic.suggested_topics");
+
     const url = `/t/${this.get("topic.id")}/posts.json`;
     let data = {
       post_number: postNumber,
-      asc: asc
+      asc: asc,
+      include_suggested: includeSuggested
     };
 
     data = _.merge(data, this.get("streamFilters"));
@@ -950,8 +953,10 @@ export default RestModel.extend({
       return Ember.RSVP.resolve([]);
     }
 
+    let includeSuggested = !this.get("topic.suggested_topics");
+
     const url = "/t/" + this.get("topic.id") + "/posts.json";
-    const data = { post_ids: postIds };
+    const data = { post_ids: postIds, include_suggested: includeSuggested };
     const store = this.store;
 
     return ajax(url, { data }).then(result => {
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 33b0ebe..e311798 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -197,13 +197,17 @@ class TopicsController < ApplicationController
 
   def posts
     params.require(:topic_id)
-    params.permit(:post_ids, :post_number, :username_filters, :filter)
+    params.permit(:post_ids, :post_number, :username_filters, :filter, :include_suggested)
+
+    include_suggested = params[:include_suggested] == "true"
 
     options = {
       filter_post_number: params[:post_number],
       post_ids: params[:post_ids],
       asc: ActiveRecord::Type::Boolean.new.deserialize(params[:asc]),
-      filter: params[:filter]
+      filter: params[:filter],
+      include_suggested: include_suggested,
+      include_related: include_suggested,
     }
 
     fetch_topic_view(options)
diff --git a/lib/topic_view.rb b/lib/topic_view.rb
index 059b111..f9b29c4 100644
--- a/lib/topic_view.rb
+++ b/lib/topic_view.rb
@@ -51,6 +51,9 @@ class TopicView
     @post_number = [@post_number.to_i, 1].max
     @page = [@page.to_i, 1].max
 
+    @include_suggested = options.fetch(:include_suggested) { true }
+    @include_related = options.fetch(:include_related) { true }
+
     @chunk_size =
       case
       when @print then TopicView.print_chunk_size
@@ -402,11 +405,19 @@ class TopicView
   end
 
   def suggested_topics
-    @suggested_topics ||= TopicQuery.new(@user).list_suggested_for(topic, pm_params: pm_params)
+    if @include_suggested
+      @suggested_topics ||= TopicQuery.new(@user).list_suggested_for(topic, pm_params: pm_params)
+    else
+      nil
+    end
   end
 
   def related_messages
-    @related_messages ||= TopicQuery.new(@user).list_related_for(topic, pm_params: pm_params)
+    if @include_related
+      @related_messages ||= TopicQuery.new(@user).list_related_for(topic, pm_params: pm_params)
+    else
+      nil
+    end
   end
 
   # This is pending a larger refactor, that allows custom orders
diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb
index c7851de..738d17e 100644
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@@ -1720,6 +1720,9 @@ RSpec.describe TopicsController do
     let(:topic) { post.topic }
 
     it 'returns first post of the topic' do
+      # we need one for suggested
+      create_post
+
       get "/t/#{topic.id}/posts.json"
 
       expect(response.status).to eq(200)
@@ -1727,6 +1730,13 @@ RSpec.describe TopicsController do
       body = JSON.parse(response.body)
 
       expect(body["post_stream"]["posts"].first["id"]).to eq(post.id)
+
+      expect(body["suggested_topics"]).to eq(nil)
+
+      get "/t/#{topic.id}/posts.json?include_suggested=true"
+      body = JSON.parse(response.body)
+
+      expect(body["suggested_topics"]).not_to eq(nil)
     end
 
     describe 'filtering by post number with filters' do

GitHub sha: 31d41f53

1 Like

The need for this comment hints to me that our assertions is not complete enough. Instead of not_to eq(nil), I think we should just check for some resemblance of the actual payload.

I worry though, the more testing I add here the more fragile it gets, I don’t mind if there are 2 suggested, also given the way random works this could return a post that I did not even create.

I guess we can flush the random cache first and then run the spec, so it can be improved … for sure

1 Like