PERF: optimise pinned handling on home page

Old query used to scan the full topics table, on home page

Instead we now perform 2 queries, one for pinned and one for unpinned and merge
results in a 10x improvement on a 1 million topic DB
This commit is contained in:
Sam 2015-02-23 16:50:52 +11:00
parent a55a6fb703
commit 8370b26cba
3 changed files with 49 additions and 44 deletions

View file

@ -25,7 +25,7 @@ class CategoryFeaturedTopic < ActiveRecord::Base
visible: true, visible: true,
no_definitions: true) no_definitions: true)
results = query.list_category(c).topic_ids.uniq results = query.list_category_topic_ids(c).uniq
return if results == existing return if results == existing

View file

@ -32,16 +32,7 @@ class TopicList
def topics def topics
return @topics if @topics.present? return @topics if @topics.present?
# copy side-loaded data (allowed users) before dumping it with the .to_a @topics = @topics_input
@topics_input.each do |t|
t.allowed_user_ids = if @filter == :private_messages
t.allowed_users.map { |u| u.id }.to_a
else
[]
end
end
@topics = @topics_input.to_a
# Attach some data for serialization to each topic # Attach some data for serialization to each topic
@topic_lookup = TopicUser.lookup_for(@current_user, @topics) if @current_user @topic_lookup = TopicUser.lookup_for(@current_user, @topics) if @current_user
@ -88,11 +79,6 @@ class TopicList
@topics @topics
end end
def topic_ids
return [] unless @topics_input
@topics_input.pluck(:id)
end
def attributes def attributes
{'more_topics_url' => page} {'more_topics_url' => page}
end end

View file

@ -62,7 +62,7 @@ class TopicQuery
end end
builder.add_results(random_suggested(topic, builder.results_left, builder.excluded_topic_ids)) unless builder.full? builder.add_results(random_suggested(topic, builder.results_left, builder.excluded_topic_ids)) unless builder.full?
create_list(:suggested, {}, builder.results) create_list(:suggested, {unordered: true}, builder.results)
end end
# The latest view of topics # The latest view of topics
@ -77,11 +77,11 @@ class TopicQuery
end end
def list_new def list_new
create_list(:new, {}, new_results) create_list(:new, {unordered: true}, new_results)
end end
def list_unread def list_unread
create_list(:unread, {}, unread_results) create_list(:unread, {unordered: true}, unread_results)
end end
def list_posted def list_posted
@ -128,13 +128,11 @@ class TopicQuery
end end
def list_category(category) def list_category(category)
create_list(:category, unordered: true, category: category.id) do |list| create_list(:category, unordered: true, category: category.id)
if @user end
list.order(TopicQuerySQL.order_with_pinned_sql)
else def list_category_topic_ids(category)
list.order(TopicQuerySQL.order_basic_bumped) default_results(unordered: true, category: category.id).pluck(:id)
end
end
end end
def list_new_in_category(category) def list_new_in_category(category)
@ -154,10 +152,43 @@ class TopicQuery
.where("COALESCE(tu.notification_level, :regular) >= :tracking", regular: TopicUser.notification_levels[:regular], tracking: TopicUser.notification_levels[:tracking]) .where("COALESCE(tu.notification_level, :regular) >= :tracking", regular: TopicUser.notification_levels[:regular], tracking: TopicUser.notification_levels[:tracking])
end end
def prioritize_pinned_topics(topics, options)
pinned_clause = options[:category] ? "" : "pinned_globally AND "
pinned_clause << " pinned_at IS NOT NULL "
if @user
pinned_clause << " AND (topics.pinned_at > tu.cleared_pinned_at OR tu.cleared_pinned_at IS NULL)"
end
unpinned_topics = topics.where("NOT ( #{pinned_clause} )")
pinned_topics = topics.where(pinned_clause)
per_page = options[:per_page] || per_page_setting
limit = per_page unless options[:limit] == false
page = options[:page].to_i
if page == 0
(pinned_topics + unpinned_topics)[0...limit] if limit
else
unpinned_topics.offset((page * per_page - pinned_topics.count) - 1).to_a
end
end
def create_list(filter, options={}, topics = nil) def create_list(filter, options={}, topics = nil)
topics ||= default_results(options) topics ||= default_results(options)
topics = yield(topics) if block_given? topics = yield(topics) if block_given?
list = TopicList.new(filter, @user, topics, options.merge(@options))
options = options.merge(@options)
if (options[:order] || "activity") == "activity" && !options[:unordered]
topics = prioritize_pinned_topics(topics, options)
end
topics = topics.to_a.each do |t|
t.allowed_user_ids = filter == :private_messags ? t.allowed_users.map{|u| u.id} : []
end
list = TopicList.new(filter, @user, topics.to_a, options.merge(@options))
list.per_page = per_page_setting list.per_page = per_page_setting
list list
end end
@ -171,11 +202,12 @@ class TopicQuery
def unread_results(options={}) def unread_results(options={})
result = TopicQuery.unread_filter(default_results(options.reverse_merge(:unordered => true))) result = TopicQuery.unread_filter(default_results(options.reverse_merge(:unordered => true)))
.order('CASE WHEN topics.user_id = tu.user_id THEN 1 ELSE 2 END') .order('CASE WHEN topics.user_id = tu.user_id THEN 1 ELSE 2 END')
suggested_ordering(result, options) suggested_ordering(result, options)
end end
def new_results(options={}) def new_results(options={})
# TODO does this make sense or should it be ordered on created_at
# it is ordering on bumped_at now
result = TopicQuery.new_filter(default_results(options.reverse_merge(:unordered => true)), @user.treat_as_new_topic_start_date) result = TopicQuery.new_filter(default_results(options.reverse_merge(:unordered => true)), @user.treat_as_new_topic_start_date)
result = remove_muted_categories(result, @user, exclude: options[:category]) result = remove_muted_categories(result, @user, exclude: options[:category])
suggested_ordering(result, options) suggested_ordering(result, options)
@ -196,7 +228,7 @@ class TopicQuery
result = Topic.includes(:allowed_users) result = Topic.includes(:allowed_users)
.where("topics.id IN (SELECT topic_id FROM topic_allowed_users WHERE user_id = #{user.id.to_i})") .where("topics.id IN (SELECT topic_id FROM topic_allowed_users WHERE user_id = #{user.id.to_i})")
.joins("LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{user.id.to_i})") .joins("LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{user.id.to_i})")
.order(TopicQuerySQL.order_nocategory_basic_bumped) .order("topics.bumped_at")
.private_messages .private_messages
result = result.limit(options[:per_page]) unless options[:limit] == false result = result.limit(options[:per_page]) unless options[:limit] == false
@ -205,18 +237,6 @@ class TopicQuery
result result
end end
def default_ordering(result, options)
# If we're logged in, we have to pay attention to our pinned settings
if @user
result = options[:category].blank? ? result.order(TopicQuerySQL.order_nocategory_with_pinned_sql) :
result.order(TopicQuerySQL.order_with_pinned_sql)
else
result = options[:category].blank? ? result.order(TopicQuerySQL.order_nocategory_basic_bumped) :
result.order(TopicQuerySQL.order_basic_bumped)
end
result
end
def apply_ordering(result, options) def apply_ordering(result, options)
sort_column = SORTABLE_MAPPING[options[:order]] || 'default' sort_column = SORTABLE_MAPPING[options[:order]] || 'default'
sort_dir = (options[:ascending] == "true") ? "ASC" : "DESC" sort_dir = (options[:ascending] == "true") ? "ASC" : "DESC"
@ -228,14 +248,13 @@ class TopicQuery
# If something requires a custom order, for example "unread" which sorts the least read # If something requires a custom order, for example "unread" which sorts the least read
# to the top, do nothing # to the top, do nothing
return result if options[:unordered] return result if options[:unordered]
# Otherwise apply our default ordering
return default_ordering(result, options)
end end
sort_column = 'bumped_at' sort_column = 'bumped_at'
end end
# If we are sorting by category, actually use the name # If we are sorting by category, actually use the name
if sort_column == 'category_id' if sort_column == 'category_id'
# TODO forces a table scan, slow
return result.references(:categories).order(TopicQuerySQL.order_by_category_sql(sort_dir)) return result.references(:categories).order(TopicQuerySQL.order_by_category_sql(sort_dir))
end end
@ -404,6 +423,6 @@ class TopicQuery
result = result.order("CASE WHEN topics.category_id = #{options[:topic].category_id.to_i} THEN 0 ELSE 1 END") result = result.order("CASE WHEN topics.category_id = #{options[:topic].category_id.to_i} THEN 0 ELSE 1 END")
end end
result.order(TopicQuerySQL.order_nocategory_with_pinned_sql) result.order('topics.bumped_at DESC')
end end
end end