mirror of
https://github.com/discourse/discourse.git
synced 2024-11-22 13:09:18 +08:00
PERF: Keep track of first unread PM and first unread group PM for user.
This optimization helps to filter away topics so that the joins on related tables when querying for unread messages is not expensive.
This commit is contained in:
parent
0398271f87
commit
9b75d95fc6
|
@ -35,6 +35,9 @@ module Jobs
|
|||
UserStat.ensure_consistency!(13.hours.ago)
|
||||
measure(UserStat)
|
||||
|
||||
GroupUser.ensure_consistency!(13.hours.ago)
|
||||
measure(GroupUser)
|
||||
|
||||
Rails.logger.debug(format_measure)
|
||||
nil
|
||||
end
|
||||
|
|
|
@ -19,6 +19,52 @@ class GroupUser < ActiveRecord::Base
|
|||
NotificationLevels.all
|
||||
end
|
||||
|
||||
def self.ensure_consistency!(last_seen = 1.hour.ago)
|
||||
update_first_unread_pm(last_seen)
|
||||
end
|
||||
|
||||
def self.update_first_unread_pm(last_seen, limit: 10_000)
|
||||
DB.exec(<<~SQL, archetype: Archetype.private_message, last_seen: last_seen, limit: limit)
|
||||
UPDATE group_users gu
|
||||
SET first_unread_pm_at = Y.min_date
|
||||
FROM (
|
||||
SELECT
|
||||
X.group_id,
|
||||
X.user_id,
|
||||
X.min_date
|
||||
FROM (
|
||||
SELECT
|
||||
gu2.group_id,
|
||||
gu2.user_id,
|
||||
MIN(t.updated_at) min_date
|
||||
FROM group_users gu2
|
||||
INNER JOIN topic_allowed_groups tag ON tag.group_id = gu2.group_id
|
||||
INNER JOIN topics t ON t.id = tag.topic_id
|
||||
INNER JOIN users u ON u.id = gu2.user_id
|
||||
LEFT JOIN topic_users tu ON t.id = tu.topic_id AND tu.user_id = gu2.user_id
|
||||
WHERE t.deleted_at IS NULL
|
||||
AND t.archetype = :archetype
|
||||
AND tu.last_read_post_number < CASE
|
||||
WHEN u.admin OR u.moderator
|
||||
THEN t.highest_staff_post_number
|
||||
ELSE t.highest_post_number
|
||||
END
|
||||
AND (COALESCE(tu.notification_level, 1) >= 2)
|
||||
GROUP BY gu2.user_id, gu2.group_id
|
||||
) AS X
|
||||
WHERE X.user_id IN (
|
||||
SELECT id
|
||||
FROM users
|
||||
WHERE last_seen_at IS NOT NULL
|
||||
AND last_seen_at > :last_seen
|
||||
ORDER BY last_seen_at DESC
|
||||
LIMIT :limit
|
||||
)
|
||||
) Y
|
||||
WHERE gu.user_id = Y.user_id AND gu.group_id = Y.group_id
|
||||
SQL
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def set_notification_level
|
||||
|
|
|
@ -75,7 +75,9 @@ class PostTiming < ActiveRecord::Base
|
|||
|
||||
topic.posts.find_by(post_number: post_number).decrement!(:reads)
|
||||
|
||||
if !topic.private_message?
|
||||
if topic.private_message?
|
||||
set_minimum_first_unread_pm!(topic: topic, user_id: user.id, date: topic.updated_at)
|
||||
else
|
||||
set_minimum_first_unread!(user_id: user.id, date: topic.updated_at)
|
||||
end
|
||||
end
|
||||
|
@ -101,6 +103,27 @@ class PostTiming < ActiveRecord::Base
|
|||
end
|
||||
end
|
||||
|
||||
def self.set_minimum_first_unread_pm!(topic:, user_id:, date:)
|
||||
if topic.topic_allowed_users.exists?(user_id: user_id)
|
||||
UserStat.where("first_unread_pm_at > ? AND user_id = ?", date, user_id)
|
||||
.update_all(first_unread_pm_at: date)
|
||||
else
|
||||
DB.exec(<<~SQL, date: date, user_id: user_id, topic_id: topic.id)
|
||||
UPDATE group_users gu
|
||||
SET first_unread_pm_at = :date
|
||||
FROM (
|
||||
SELECT
|
||||
gu2.user_id,
|
||||
gu2.group_id
|
||||
FROM group_users gu2
|
||||
INNER JOIN topic_allowed_groups tag ON tag.group_id = gu2.group_id AND tag.topic_id = :topic_id
|
||||
WHERE gu2.user_id = :user_id
|
||||
) Y
|
||||
WHERE gu.user_id = Y.user_id AND gu.group_id = Y.group_id
|
||||
SQL
|
||||
end
|
||||
end
|
||||
|
||||
def self.set_minimum_first_unread!(user_id:, date:)
|
||||
DB.exec(<<~SQL, date: date, user_id: user_id)
|
||||
UPDATE user_stats
|
||||
|
|
|
@ -12,10 +12,59 @@ class UserStat < ActiveRecord::Base
|
|||
update_distinct_badge_count
|
||||
update_view_counts(last_seen)
|
||||
update_first_unread(last_seen)
|
||||
update_first_unread_pm(last_seen)
|
||||
end
|
||||
|
||||
def self.update_first_unread(last_seen, limit: 10_000)
|
||||
DB.exec(<<~SQL, min_date: last_seen, limit: limit, now: 10.minutes.ago)
|
||||
UPDATE_UNREAD_MINUTES_AGO = 10
|
||||
UPDATE_UNREAD_USERS_LIMIT = 10_000
|
||||
|
||||
def self.update_first_unread_pm(last_seen, limit: UPDATE_UNREAD_USERS_LIMIT)
|
||||
DB.exec(<<~SQL, archetype: Archetype.private_message, now: UPDATE_UNREAD_MINUTES_AGO.minutes.ago, last_seen: last_seen, limit: limit)
|
||||
UPDATE user_stats us
|
||||
SET first_unread_pm_at = COALESCE(Z.min_date, :now)
|
||||
FROM (
|
||||
SELECT
|
||||
Y.user_id,
|
||||
Y.min_date
|
||||
FROM (
|
||||
SELECT
|
||||
u1.id user_id,
|
||||
X.min_date
|
||||
FROM users u1
|
||||
LEFT JOIN (
|
||||
SELECT
|
||||
tau.user_id,
|
||||
MIN(t.updated_at) min_date
|
||||
FROM topic_allowed_users tau
|
||||
INNER JOIN topics t ON t.id = tau.topic_id
|
||||
INNER JOIN users u ON u.id = tau.user_id
|
||||
LEFT JOIN topic_users tu ON t.id = tu.topic_id AND tu.user_id = tau.user_id
|
||||
WHERE t.deleted_at IS NULL
|
||||
AND t.archetype = :archetype
|
||||
AND tu.last_read_post_number < CASE
|
||||
WHEN u.admin OR u.moderator
|
||||
THEN t.highest_staff_post_number
|
||||
ELSE t.highest_post_number
|
||||
END
|
||||
AND (COALESCE(tu.notification_level, 1) >= 2)
|
||||
GROUP BY tau.user_id
|
||||
) AS X ON X.user_id = u1.id
|
||||
) AS Y
|
||||
WHERE Y.user_id IN (
|
||||
SELECT id
|
||||
FROM users
|
||||
WHERE last_seen_at IS NOT NULL
|
||||
AND last_seen_at > :last_seen
|
||||
ORDER BY last_seen_at DESC
|
||||
LIMIT :limit
|
||||
)
|
||||
) AS Z
|
||||
WHERE us.user_id = Z.user_id
|
||||
SQL
|
||||
end
|
||||
|
||||
def self.update_first_unread(last_seen, limit: UPDATE_UNREAD_USERS_LIMIT)
|
||||
DB.exec(<<~SQL, min_date: last_seen, limit: limit, now: UPDATE_UNREAD_MINUTES_AGO.minutes.ago)
|
||||
UPDATE user_stats us
|
||||
SET first_unread_at = COALESCE(Y.min_date, :now)
|
||||
FROM (
|
||||
|
|
|
@ -0,0 +1,16 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class AddFirstUnreadPmAToGroupUser < ActiveRecord::Migration[6.0]
|
||||
def up
|
||||
add_column :group_users, :first_unread_pm_at, :datetime, null: false, default: -> { 'CURRENT_TIMESTAMP' }
|
||||
|
||||
execute <<~SQL
|
||||
UPDATE group_users gu
|
||||
SET first_unread_pm_at = gu.created_at
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
|
@ -0,0 +1,18 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class AddFirstUnreadPmAtToUserStats < ActiveRecord::Migration[6.0]
|
||||
def up
|
||||
add_column :user_stats, :first_unread_pm_at, :datetime, null: false, default: -> { 'CURRENT_TIMESTAMP' }
|
||||
|
||||
execute <<~SQL
|
||||
UPDATE user_stats us
|
||||
SET first_unread_pm_at = u.created_at
|
||||
FROM users u
|
||||
WHERE u.id = us.user_id
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
|
@ -0,0 +1,18 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class AddIndexTopicsOnTimestampsPrivate < ActiveRecord::Migration[6.0]
|
||||
disable_ddl_transaction!
|
||||
|
||||
def up
|
||||
execute <<~SQL
|
||||
CREATE INDEX CONCURRENTLY IF NOT EXISTS
|
||||
index_topics_on_timestamps_private
|
||||
ON topics (bumped_at, created_at, updated_at)
|
||||
WHERE deleted_at IS NULL AND archetype = 'private_message'
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
|
@ -960,9 +960,19 @@ class TopicQuery
|
|||
def unread_messages(params)
|
||||
query = TopicQuery.unread_filter(
|
||||
messages_for_groups_or_user(params[:my_group_ids]),
|
||||
@user&.id,
|
||||
staff: @user&.staff?)
|
||||
.limit(params[:count])
|
||||
@user.id,
|
||||
staff: @user.staff?
|
||||
)
|
||||
|
||||
first_unread_pm_at =
|
||||
if params[:my_group_ids].present?
|
||||
GroupUser.where(user_id: @user.id, group_id: params[:my_group_ids]).minimum(:first_unread_pm_at)
|
||||
else
|
||||
UserStat.where(user_id: @user.id).pluck(:first_unread_pm_at).first
|
||||
end
|
||||
|
||||
query = query.where("topics.updated_at >= ?", first_unread_pm_at) if first_unread_pm_at
|
||||
query = query.limit(params[:count]) if params[:count]
|
||||
query
|
||||
end
|
||||
|
||||
|
|
|
@ -160,4 +160,64 @@ describe GroupUser do
|
|||
expect(TagUser.lookup(user, :watching_first_post).pluck(:tag_id)).to eq([tag4.id])
|
||||
end
|
||||
end
|
||||
|
||||
describe '#ensure_consistency!' do
|
||||
fab!(:group) { Fabricate(:group) }
|
||||
|
||||
fab!(:pm_post) { Fabricate(:private_message_post) }
|
||||
|
||||
fab!(:pm_topic) do
|
||||
pm_post.topic.tap { |t| t.allowed_groups << group }
|
||||
end
|
||||
|
||||
fab!(:user) do
|
||||
Fabricate(:user, last_seen_at: Time.zone.now).tap do |u|
|
||||
group.add(u)
|
||||
|
||||
TopicUser.change(u.id, pm_topic.id,
|
||||
notification_level: TopicUser.notification_levels[:tracking],
|
||||
last_read_post_number: pm_post.post_number
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
# User that is not tracking topic
|
||||
fab!(:user_2) do
|
||||
Fabricate(:user, last_seen_at: Time.zone.now).tap do |u|
|
||||
group.add(u)
|
||||
|
||||
TopicUser.change(u.id, pm_topic.id,
|
||||
notification_level: TopicUser.notification_levels[:regular],
|
||||
last_read_post_number: pm_post.post_number
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
# User that has not been seen
|
||||
fab!(:user_3) do
|
||||
Fabricate(:user).tap do |u|
|
||||
group.add(u)
|
||||
|
||||
TopicUser.change(u.id, pm_topic.id,
|
||||
notification_level: TopicUser.notification_levels[:tracking],
|
||||
last_read_post_number: pm_post.post_number
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
it 'updates first unread pm timestamp correctly' do
|
||||
freeze_time 10.minutes.from_now
|
||||
|
||||
post = create_post(
|
||||
user: pm_topic.user,
|
||||
topic_id: pm_topic.id
|
||||
)
|
||||
|
||||
GroupUser.ensure_consistency!
|
||||
|
||||
expect(group.group_users.find_by(user_id: user.id).first_unread_pm_at).to eq_time(post.topic.updated_at)
|
||||
expect(group.group_users.find_by(user_id: user_2.id).first_unread_pm_at).to_not eq_time(post.topic.updated_at)
|
||||
expect(group.group_users.find_by(user_id: user_3.id).first_unread_pm_at).to_not eq_time(post.topic.updated_at)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -184,4 +184,41 @@ describe PostTiming do
|
|||
expect(post.reload.reads).to eq initial_read_count
|
||||
end
|
||||
end
|
||||
|
||||
describe '.destroy_last_for' do
|
||||
it 'updates first unread for a user correctly when topic is public' do
|
||||
post = Fabricate(:post)
|
||||
post.topic.update!(updated_at: 10.minutes.ago)
|
||||
PostTiming.process_timings(post.user, post.topic_id, 1, [[post.post_number, 100]])
|
||||
|
||||
PostTiming.destroy_last_for(post.user, post.topic_id)
|
||||
|
||||
expect(post.user.user_stat.reload.first_unread_at).to eq_time(post.topic.updated_at)
|
||||
end
|
||||
|
||||
it 'updates first unread for a user correctly when topic is a pm' do
|
||||
post = Fabricate(:private_message_post)
|
||||
post.topic.update!(updated_at: 10.minutes.ago)
|
||||
PostTiming.process_timings(post.user, post.topic_id, 1, [[post.post_number, 100]])
|
||||
|
||||
PostTiming.destroy_last_for(post.user, post.topic_id)
|
||||
|
||||
expect(post.user.user_stat.reload.first_unread_pm_at).to eq_time(post.topic.updated_at)
|
||||
end
|
||||
|
||||
it 'updates first unread for a user correctly when topic is a group pm' do
|
||||
topic = Fabricate(:private_message_topic, updated_at: 10.minutes.ago)
|
||||
post = Fabricate(:post, topic: topic)
|
||||
user = Fabricate(:user)
|
||||
group = Fabricate(:group)
|
||||
group.add(user)
|
||||
topic.allowed_groups << group
|
||||
PostTiming.process_timings(user, topic.id, 1, [[post.post_number, 100]])
|
||||
|
||||
PostTiming.destroy_last_for(user, topic.id)
|
||||
|
||||
expect(GroupUser.find_by(user: user, group: group).first_unread_pm_at)
|
||||
.to eq_time(post.topic.updated_at)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -88,6 +88,46 @@ describe UserStat do
|
|||
post.user.user_stat.reload
|
||||
expect(post.user.user_stat.first_unread_at).to eq_time(Time.zone.now)
|
||||
end
|
||||
|
||||
it 'updates first unread pm timestamp correctly' do
|
||||
pm_topic = Fabricate(:private_message_topic)
|
||||
user = pm_topic.user
|
||||
user.update!(last_seen_at: Time.zone.now)
|
||||
create_post(user: user, topic_id: pm_topic.id)
|
||||
|
||||
TopicUser.change(user.id, pm_topic.id,
|
||||
notification_level: TopicUser.notification_levels[:tracking]
|
||||
)
|
||||
|
||||
# user that is not tracking PM topic
|
||||
user_2 = Fabricate(:user, last_seen_at: Time.zone.now)
|
||||
pm_topic.allowed_users << user_2
|
||||
|
||||
TopicUser.change(user_2.id, pm_topic.id,
|
||||
notification_level: TopicUser.notification_levels[:regular]
|
||||
)
|
||||
|
||||
# User that has not been seen
|
||||
user_3 = Fabricate(:user)
|
||||
pm_topic.allowed_users << user_3
|
||||
|
||||
TopicUser.change(user_3.id, pm_topic.id,
|
||||
notification_level: TopicUser.notification_levels[:tracking]
|
||||
)
|
||||
|
||||
freeze_time 10.minutes.from_now
|
||||
|
||||
post = create_post(
|
||||
user: Fabricate(:admin),
|
||||
topic_id: pm_topic.id
|
||||
)
|
||||
|
||||
UserStat.ensure_consistency!
|
||||
|
||||
expect(user.user_stat.reload.first_unread_pm_at).to eq_time(post.topic.updated_at)
|
||||
expect(user_2.user_stat.reload.first_unread_pm_at).to_not eq_time(post.topic.updated_at)
|
||||
expect(user_3.user_stat.reload.first_unread_pm_at).to_not eq_time(post.topic.updated_at)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'update_time_read!' do
|
||||
|
|
Loading…
Reference in New Issue
Block a user