mirror of
https://github.com/discourse/discourse.git
synced 2024-11-22 11:23:25 +08:00
PERF: Fix N+1 queries when serializing topic posters (#19545)
At the time of writing, this is how the `TopicPosterSerializer` looks like: ``` class TopicPosterSerializer < ApplicationSerializer attributes :extras, :description has_one :user, serializer: PosterSerializer has_one :primary_group, serializer: PrimaryGroupSerializer has_one :flair_group, serializer: FlairGroupSerializer end ``` Within `PosterSerializer`, the `primary_group` and `flair_group` association is requested on the `user` object. However, the associations have not been loaded on the `user` object at this point leading to the N+1 queries problem. One may wonder why the associations have not been loaded when the `TopicPosterSerializer` has `has_one :primary_group` and `has_one :flair_group`. It turns out that `TopicPoster` is just a struct containing the `user`, `primary_group` and `flair_group` objects. The `primary_group` and `flair_group` ActiveRecord objects are loaded seperately in `UserLookup` and not preloaded when querying for the users. This is done for performance reason so that we are able to load the `primary_group` and `flair_group` records in a single query without duplication.
This commit is contained in:
parent
6bcf558bae
commit
4eee1320b0
|
@ -21,7 +21,9 @@ class UserLookup
|
|||
def primary_groups
|
||||
@primary_groups ||= users.values.each_with_object({}) do |user, hash|
|
||||
if user.primary_group_id
|
||||
hash[user.id] = groups[user.primary_group_id]
|
||||
group = groups[user.primary_group_id]
|
||||
set_user_group_preload(user, group, :primary_group)
|
||||
hash[user.id] = group
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -29,13 +31,20 @@ class UserLookup
|
|||
def flair_groups
|
||||
@flair_groups ||= users.values.each_with_object({}) do |user, hash|
|
||||
if user.flair_group_id
|
||||
hash[user.id] = groups[user.flair_group_id]
|
||||
group = groups[user.flair_group_id]
|
||||
set_user_group_preload(user, group, :flair_group)
|
||||
hash[user.id] = group
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def set_user_group_preload(user, group, group_association_name)
|
||||
association = user.association(group_association_name)
|
||||
association.target = group
|
||||
end
|
||||
|
||||
def users
|
||||
@users ||= User
|
||||
.where(id: @user_ids)
|
||||
|
|
|
@ -99,6 +99,46 @@ RSpec.describe ListController do
|
|||
expect(first_item.css('[itemprop="position"]')[0]['content']).to eq('1')
|
||||
expect(first_item.css('[itemprop="url"]')[0]['href']).to eq(topic.url)
|
||||
end
|
||||
|
||||
it 'does not N+1 queries when topic featured users have different primary groups' do
|
||||
user.update!(primary_group: group)
|
||||
|
||||
# warm up
|
||||
get "/latest.json"
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
initial_sql_queries_count = track_sql_queries do
|
||||
get "/latest.json"
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
body = response.parsed_body
|
||||
|
||||
expect(body["topic_list"]["topics"].map { |t| t["id"] }).to contain_exactly(topic.id)
|
||||
expect(body["topic_list"]["topics"][0]["posters"].map { |p| p["user_id"] }).to contain_exactly(user.id)
|
||||
end.count
|
||||
|
||||
group2 = Fabricate(:group)
|
||||
user2 = Fabricate(:user, primary_group: group2)
|
||||
topic.update!(last_post_user_id: user2.id)
|
||||
|
||||
group3 = Fabricate(:group)
|
||||
user3 = Fabricate(:user, flair_group: group3)
|
||||
topic.update!(featured_user3_id: user3.id)
|
||||
|
||||
new_sql_queries_count = track_sql_queries do
|
||||
get "/latest.json"
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
body = response.parsed_body
|
||||
|
||||
expect(body["topic_list"]["topics"].map { |t| t["id"] }).to contain_exactly(topic.id)
|
||||
expect(body["topic_list"]["topics"][0]["posters"].map { |p| p["user_id"] }).to contain_exactly(user.id, user2.id, user3.id)
|
||||
end.count
|
||||
|
||||
expect(new_sql_queries_count).to be <= initial_sql_queries_count
|
||||
end
|
||||
end
|
||||
|
||||
describe "categories and X" do
|
||||
|
|
Loading…
Reference in New Issue
Block a user