PERF: Dematerialize topic_reply_count (#9769)

* PERF: Dematerialize topic_reply_count

It's only ever used for trust level promotions that run daily, or compared to 0. We don't need to track it on every post creation.

* UX: Add symbol in TL3 report if topic reply count is capped

* DEV: Drop user_stats.topic_reply_count column
This commit is contained in:
Kane York
2020-05-14 15:42:00 -07:00
committed by GitHub
parent 675c9c38c8
commit 869f9b20a2
19 changed files with 73 additions and 119 deletions

View File

@ -12,6 +12,11 @@ export default EmberObject.extend({
return Math.round((minDaysVisited * 100) / timePeriod); return Math.round((minDaysVisited * 100) / timePeriod);
}, },
@discourseComputed("num_topics_replied_to", "min_topics_replied_to")
capped_topics_replied_to(numReplied, minReplied) {
return numReplied > minReplied;
},
@discourseComputed( @discourseComputed(
"days_visited", "days_visited",
"min_days_visited", "min_days_visited",

View File

@ -33,7 +33,7 @@
<tr> <tr>
<th>{{i18n "admin.user.tl3_requirements.topics_replied_to"}}</th> <th>{{i18n "admin.user.tl3_requirements.topics_replied_to"}}</th>
<td>{{check-icon model.tl3Requirements.met.topics_replied_to}}</td> <td>{{check-icon model.tl3Requirements.met.topics_replied_to}}</td>
<td>{{model.tl3Requirements.num_topics_replied_to}}</td> <td>{{#if model.tl3Requirements.capped_topics_replied_to}}&ge; {{/if}}{{model.tl3Requirements.num_topics_replied_to}}</td>
<td>{{model.tl3Requirements.min_topics_replied_to}}</td> <td>{{model.tl3Requirements.min_topics_replied_to}}</td>
</tr> </tr>
<tr> <tr>

View File

@ -20,21 +20,19 @@ export default DesktopNotificationConfig.extend({
"isNotSupported", "isNotSupported",
"isEnabled", "isEnabled",
"bannerDismissed", "bannerDismissed",
"currentUser.reply_count", "currentUser.any_posts"
"currentUser.topic_count"
) )
showNotificationPromptBanner( showNotificationPromptBanner(
isNotSupported, isNotSupported,
isEnabled, isEnabled,
bannerDismissed, bannerDismissed,
replyCount, anyPosts
topicCount
) { ) {
return ( return (
this.siteSettings.push_notifications_prompt && this.siteSettings.push_notifications_prompt &&
!isNotSupported && !isNotSupported &&
this.currentUser && this.currentUser &&
replyCount + topicCount > 0 && anyPosts &&
Notification.permission !== "denied" && Notification.permission !== "denied" &&
Notification.permission !== "granted" && Notification.permission !== "granted" &&
!isEnabled && !isEnabled &&

View File

@ -732,12 +732,7 @@ export default Controller.extend({
this.close(); this.close();
const currentUser = this.currentUser; this.currentUser.set("any_posts", true);
if (composer.creatingTopic) {
currentUser.set("topic_count", currentUser.topic_count + 1);
} else {
currentUser.set("reply_count", currentUser.reply_count + 1);
}
const post = result.target; const post = result.target;
if (post && !staged) { if (post && !staged) {

View File

@ -34,7 +34,6 @@ module Jobs
).where.not(id: demoted_user_ids) ).where.not(id: demoted_user_ids)
.joins(:user_stat) .joins(:user_stat)
.where("user_stats.days_visited >= ?", SiteSetting.tl3_requires_days_visited) .where("user_stats.days_visited >= ?", SiteSetting.tl3_requires_days_visited)
.where("user_stats.topic_reply_count >= ?", SiteSetting.tl3_requires_topics_replied_to)
.where("user_stats.topics_entered >= ?", SiteSetting.tl3_requires_topics_viewed_all_time) .where("user_stats.topics_entered >= ?", SiteSetting.tl3_requires_topics_viewed_all_time)
.where("user_stats.posts_read_count >= ?", SiteSetting.tl3_requires_posts_read_all_time) .where("user_stats.posts_read_count >= ?", SiteSetting.tl3_requires_posts_read_all_time)
.where("user_stats.likes_given >= ?", SiteSetting.tl3_requires_likes_given) .where("user_stats.likes_given >= ?", SiteSetting.tl3_requires_likes_given)

View File

@ -143,11 +143,7 @@ class TrustLevel3Requirements
end end
def num_topics_replied_to def num_topics_replied_to
@user.posts @user.user_stat.calc_topic_reply_count!(min_topics_replied_to + 1, time_period.days.ago)
.public_posts
.where("posts.created_at > ? AND posts.post_number > 1", time_period.days.ago)
.select("distinct topic_id")
.count
end end
def min_topics_replied_to def min_topics_replied_to

View File

@ -4,6 +4,9 @@ class UserStat < ActiveRecord::Base
belongs_to :user belongs_to :user
after_save :trigger_badges after_save :trigger_badges
# TODO(2021-05-13): Remove
self.ignored_columns = ["topic_reply_count"]
def self.ensure_consistency!(last_seen = 1.hour.ago) def self.ensure_consistency!(last_seen = 1.hour.ago)
reset_bounce_scores reset_bounce_scores
update_distinct_badge_count update_distinct_badge_count
@ -151,12 +154,30 @@ class UserStat < ActiveRecord::Base
end end
# topic_reply_count is a count of posts in other users' topics # topic_reply_count is a count of posts in other users' topics
def update_topic_reply_count def calc_topic_reply_count!(max, start_time = nil)
self.topic_reply_count = Topic sql = <<~SQL
.joins("INNER JOIN posts ON topics.id = posts.topic_id AND topics.user_id <> posts.user_id") SELECT COUNT(*) count
.where("posts.deleted_at IS NULL AND posts.user_id = ?", self.user_id) FROM (
.distinct SELECT DISTINCT posts.topic_id
.count FROM posts
INNER JOIN topics ON topics.id = posts.topic_id
WHERE posts.user_id = ?
AND topics.user_id <> posts.user_id
AND posts.deleted_at IS NULL AND topics.deleted_at IS NULL
AND topics.archetype <> 'private_message'
#{start_time.nil? ? '' : 'AND posts.created_at > ?'}
LIMIT ?
) as user_topic_replies
SQL
if start_time.nil?
DB.query_single(sql, self.user_id, max).first
else
DB.query_single(sql, self.user_id, start_time, max).first
end
end
def any_posts
user.posts.exists?
end end
MAX_TIME_READ_DIFF = 100 MAX_TIME_READ_DIFF = 100
@ -212,7 +233,6 @@ end
# posts_read_count :integer default(0), not null # posts_read_count :integer default(0), not null
# likes_given :integer default(0), not null # likes_given :integer default(0), not null
# likes_received :integer default(0), not null # likes_received :integer default(0), not null
# topic_reply_count :integer default(0), not null
# new_since :datetime not null # new_since :datetime not null
# read_faq :datetime # read_faq :datetime
# first_post_created_at :datetime # first_post_created_at :datetime

View File

@ -12,8 +12,7 @@ class CurrentUserSerializer < BasicUserSerializer
:moderator?, :moderator?,
:staff?, :staff?,
:title, :title,
:reply_count, :any_posts,
:topic_count,
:enable_quoting, :enable_quoting,
:enable_defer, :enable_defer,
:external_links_in_new_tab, :external_links_in_new_tab,
@ -66,12 +65,8 @@ class CurrentUserSerializer < BasicUserSerializer
object.user_stat.read_faq? object.user_stat.read_faq?
end end
def topic_count def any_posts
object.user_stat.topic_count object.user_stat.any_posts
end
def reply_count
object.user_stat.topic_reply_count
end end
def hide_profile_and_presence def hide_profile_and_presence

View File

@ -0,0 +1,19 @@
# frozen_string_literal: true
class DropTopicReplyCount < ActiveRecord::Migration[6.0]
DROPPED_COLUMNS ||= {
user_stats: %i{
topic_reply_count
}
}
def up
DROPPED_COLUMNS.each do |table, columns|
Migration::ColumnDropper.execute_drop(table, columns)
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -539,11 +539,6 @@ class PostCreator
@user.user_stat.topic_count += 1 if @post.is_first_post? @user.user_stat.topic_count += 1 if @post.is_first_post?
end end
# We don't count replies to your own topics
if !@opts[:import_mode] && @user.id != @topic.user_id
@user.user_stat.update_topic_reply_count
end
@user.user_stat.save! @user.user_stat.save!
if !@topic.private_message? && @post.post_type != Post.types[:whisper] if !@topic.private_message? && @post.post_type != Post.types[:whisper]

View File

@ -350,11 +350,6 @@ class PostDestroyer
author.user_stat.topic_count -= 1 if @post.is_first_post? author.user_stat.topic_count -= 1 if @post.is_first_post?
end end
# We don't count replies to your own topics in topic_reply_count
if @topic && author.id != @topic.user_id
author.user_stat.update_topic_reply_count
end
author.user_stat.save! author.user_stat.save!
if @post.created_at == author.last_posted_at if @post.created_at == author.last_posted_at

View File

@ -394,7 +394,6 @@ class PostRevisor
prev_owner_user_stat.topic_count -= 1 if @post.is_first_post? prev_owner_user_stat.topic_count -= 1 if @post.is_first_post?
prev_owner_user_stat.likes_received -= likes prev_owner_user_stat.likes_received -= likes
end end
prev_owner_user_stat.update_topic_reply_count
if @post.created_at == prev_owner.user_stat.first_post_created_at if @post.created_at == prev_owner.user_stat.first_post_created_at
prev_owner_user_stat.first_post_created_at = prev_owner.posts.order('created_at ASC').first.try(:created_at) prev_owner_user_stat.first_post_created_at = prev_owner.posts.order('created_at ASC').first.try(:created_at)
@ -408,7 +407,6 @@ class PostRevisor
new_owner_user_stat.topic_count += 1 if @post.is_first_post? new_owner_user_stat.topic_count += 1 if @post.is_first_post?
new_owner_user_stat.likes_received += likes new_owner_user_stat.likes_received += likes
end end
new_owner_user_stat.update_topic_reply_count
new_owner_user_stat.save! new_owner_user_stat.save!
end end
end end

View File

@ -91,7 +91,7 @@ class Promotion
return false if stat.days_visited < SiteSetting.tl2_requires_days_visited return false if stat.days_visited < SiteSetting.tl2_requires_days_visited
return false if stat.likes_received < SiteSetting.tl2_requires_likes_received return false if stat.likes_received < SiteSetting.tl2_requires_likes_received
return false if stat.likes_given < SiteSetting.tl2_requires_likes_given return false if stat.likes_given < SiteSetting.tl2_requires_likes_given
return false if stat.topic_reply_count < SiteSetting.tl2_requires_topic_reply_count return false if stat.calc_topic_reply_count!(SiteSetting.tl2_requires_topic_reply_count) < SiteSetting.tl2_requires_topic_reply_count
true true
end end

View File

@ -227,7 +227,6 @@ def update_user_stats
# TODO: topic_count is counting all topics you replied in as if you started the topic. # TODO: topic_count is counting all topics you replied in as if you started the topic.
# TODO: post_count is counting first posts. # TODO: post_count is counting first posts.
# TODO: topic_reply_count is never used, and is counting PMs here.
DB.exec <<-SQL DB.exec <<-SQL
WITH X AS ( WITH X AS (
SELECT p.user_id SELECT p.user_id
@ -235,20 +234,6 @@ def update_user_stats
, COUNT(DISTINCT p.topic_id) topics , COUNT(DISTINCT p.topic_id) topics
, MIN(p.created_at) min_created_at , MIN(p.created_at) min_created_at
, COALESCE(COUNT(DISTINCT DATE(p.created_at)), 0) days , COALESCE(COUNT(DISTINCT DATE(p.created_at)), 0) days
, COALESCE((
SELECT COUNT(*)
FROM topics
WHERE id IN (
SELECT topic_id
FROM posts p2
JOIN topics t2 ON t2.id = p2.topic_id
WHERE p2.deleted_at IS NULL
AND p2.post_type = 1
AND NOT COALESCE(p2.hidden, 't')
AND p2.user_id <> t2.user_id
AND p2.user_id = p.user_id
)
), 0) topic_replies
FROM posts p FROM posts p
JOIN topics t ON t.id = p.topic_id JOIN topics t ON t.id = p.topic_id
WHERE p.deleted_at IS NULL WHERE p.deleted_at IS NULL
@ -268,7 +253,6 @@ def update_user_stats
, topics_entered = X.topics , topics_entered = X.topics
, first_post_created_at = X.min_created_at , first_post_created_at = X.min_created_at
, days_visited = X.days , days_visited = X.days
, topic_reply_count = X.topic_replies
FROM X FROM X
WHERE user_stats.user_id = X.user_id WHERE user_stats.user_id = X.user_id
AND (post_count <> X.posts AND (post_count <> X.posts
@ -278,7 +262,7 @@ def update_user_stats
OR topics_entered <> X.topics OR topics_entered <> X.topics
OR COALESCE(first_post_created_at, '1970-01-01') <> X.min_created_at OR COALESCE(first_post_created_at, '1970-01-01') <> X.min_created_at
OR days_visited <> X.days OR days_visited <> X.days
OR topic_reply_count <> X.topic_replies) )
SQL SQL
end end

View File

@ -58,7 +58,6 @@ class ImportScripts::Base
update_post_timings update_post_timings
update_feature_topic_users update_feature_topic_users
update_category_featured_topics update_category_featured_topics
update_topic_count_replies
reset_topic_counters reset_topic_counters
end end
@ -690,19 +689,6 @@ class ImportScripts::Base
end end
def update_user_stats def update_user_stats
puts "", "Updating topic reply counts..."
count = 0
total = User.real.count
User.real.find_each do |u|
u.create_user_stat if u.user_stat.nil?
us = u.user_stat
us.update_topic_reply_count
us.save
print_status(count += 1, total, get_start_time("user_stats"))
end
puts "", "Updating first_post_created_at..." puts "", "Updating first_post_created_at..."
DB.exec <<~SQL DB.exec <<~SQL
@ -807,19 +793,6 @@ class ImportScripts::Base
end end
end end
def update_topic_count_replies
puts "", "Updating user topic reply counts"
count = 0
total = User.real.count
User.real.find_each do |u|
u.user_stat.update_topic_reply_count
u.user_stat.save!
print_status(count += 1, total, get_start_time("topic_count_replies"))
end
end
def update_tl0 def update_tl0
puts "", "Setting users with no posts to trust level 0" puts "", "Setting users with no posts to trust level 0"

View File

@ -253,28 +253,6 @@ describe PostCreator do
creator_with_image_sizes.create creator_with_image_sizes.create
end end
it 'increases topic response counts' do
first_post = creator.create
# ensure topic user is correct
topic_user = first_post.user.topic_users.find_by(topic_id: first_post.topic_id)
expect(topic_user).to be_present
expect(topic_user).to be_posted
expect(topic_user.last_read_post_number).to eq(first_post.post_number)
expect(topic_user.highest_seen_post_number).to eq(first_post.post_number)
user2 = Fabricate(:coding_horror)
expect(user2.user_stat.topic_reply_count).to eq(0)
expect(first_post.user.user_stat.reload.topic_reply_count).to eq(0)
PostCreator.new(user2, topic_id: first_post.topic_id, raw: "this is my test post 123").create
expect(first_post.user.user_stat.reload.topic_reply_count).to eq(0)
expect(user2.user_stat.reload.topic_reply_count).to eq(1)
end
it 'sets topic excerpt if first post, but not second post' do it 'sets topic excerpt if first post, but not second post' do
first_post = creator.create first_post = creator.create
topic = first_post.topic.reload topic = first_post.topic.reload

View File

@ -124,6 +124,8 @@ describe Promotion do
context "that has done the requisite things" do context "that has done the requisite things" do
before do before do
SiteSetting.tl2_requires_topic_reply_count = 3
stat = user.user_stat stat = user.user_stat
stat.topics_entered = SiteSetting.tl2_requires_topics_entered stat.topics_entered = SiteSetting.tl2_requires_topics_entered
stat.posts_read_count = SiteSetting.tl2_requires_read_posts stat.posts_read_count = SiteSetting.tl2_requires_read_posts
@ -131,7 +133,10 @@ describe Promotion do
stat.days_visited = SiteSetting.tl2_requires_days_visited * 60 stat.days_visited = SiteSetting.tl2_requires_days_visited * 60
stat.likes_received = SiteSetting.tl2_requires_likes_received stat.likes_received = SiteSetting.tl2_requires_likes_received
stat.likes_given = SiteSetting.tl2_requires_likes_given stat.likes_given = SiteSetting.tl2_requires_likes_given
stat.topic_reply_count = SiteSetting.tl2_requires_topic_reply_count SiteSetting.tl2_requires_topic_reply_count.times do |_|
topic = Fabricate(:topic)
reply = Fabricate(:post, topic: topic, user: user, post_number: 2)
end
@result = promotion.review @result = promotion.review
end end
@ -148,6 +153,7 @@ describe Promotion do
context "when the account hasn't existed long enough" do context "when the account hasn't existed long enough" do
it "does not promote the user" do it "does not promote the user" do
user.created_at = 1.minute.ago user.created_at = 1.minute.ago
SiteSetting.tl2_requires_topic_reply_count = 3
stat = user.user_stat stat = user.user_stat
stat.topics_entered = SiteSetting.tl2_requires_topics_entered stat.topics_entered = SiteSetting.tl2_requires_topics_entered
@ -156,7 +162,10 @@ describe Promotion do
stat.days_visited = SiteSetting.tl2_requires_days_visited * 60 stat.days_visited = SiteSetting.tl2_requires_days_visited * 60
stat.likes_received = SiteSetting.tl2_requires_likes_received stat.likes_received = SiteSetting.tl2_requires_likes_received
stat.likes_given = SiteSetting.tl2_requires_likes_given stat.likes_given = SiteSetting.tl2_requires_likes_given
stat.topic_reply_count = SiteSetting.tl2_requires_topic_reply_count SiteSetting.tl2_requires_topic_reply_count.times do |_|
topic = Fabricate(:topic)
reply = Fabricate(:post, topic: topic, user: user, post_number: 2)
end
result = promotion.review result = promotion.review
expect(result).to eq(false) expect(result).to eq(false)

View File

@ -8,7 +8,6 @@ describe Jobs::Tl3Promotions do
user.create_user_stat if user.user_stat.nil? user.create_user_stat if user.user_stat.nil?
user.user_stat.update!( user.user_stat.update!(
days_visited: 1000, days_visited: 1000,
topic_reply_count: 1000,
topics_entered: 1000, topics_entered: 1000,
posts_read_count: 1000, posts_read_count: 1000,
likes_given: 1000, likes_given: 1000,

View File

@ -111,14 +111,12 @@ describe PostOwnerChanger do
topic_count: 1, topic_count: 1,
post_count: 1, post_count: 1,
first_post_created_at: p1.created_at, first_post_created_at: p1.created_at,
topic_reply_count: 0
) )
p2user.user_stat.update!( p2user.user_stat.update!(
topic_count: 0, topic_count: 0,
post_count: 1, post_count: 1,
first_post_created_at: p2.created_at, first_post_created_at: p2.created_at,
topic_reply_count: 1
) )
UserAction.create!(action_type: UserAction::NEW_TOPIC, user_id: p1user.id, acting_user_id: p1user.id, UserAction.create!(action_type: UserAction::NEW_TOPIC, user_id: p1user.id, acting_user_id: p1user.id,
@ -155,13 +153,11 @@ describe PostOwnerChanger do
p1_user_stat = p1user.user_stat p1_user_stat = p1user.user_stat
expect(p1_user_stat.first_post_created_at).to eq(nil) expect(p1_user_stat.first_post_created_at).to eq(nil)
expect(p1_user_stat.topic_reply_count).to eq(0)
expect(p1_user_stat.likes_received).to eq(0) expect(p1_user_stat.likes_received).to eq(0)
p2_user_stat = p2user.user_stat p2_user_stat = p2user.user_stat
expect(p2_user_stat.first_post_created_at).to eq(nil) expect(p2_user_stat.first_post_created_at).to eq(nil)
expect(p2_user_stat.topic_reply_count).to eq(0)
user_a_stat = user_a.user_stat user_a_stat = user_a.user_stat