Improve counter caches on Status and Account (#7644)

Do not touch statuses_count on accounts table when mass-destroying
statuses to reduce load when removing accounts, same for
reblogs_count and favourites_count

Do not count statuses with direct visibility in statuses_count

Fix #828
This commit is contained in:
Eugen Rochko 2018-05-30 02:50:23 +02:00 committed by GitHub
parent dc7bf28635
commit 9e3f9b8250
6 changed files with 126 additions and 7 deletions

View file

@ -16,7 +16,7 @@ class Favourite < ApplicationRecord
update_index('statuses#status', :status) if Chewy.enabled?
belongs_to :account, inverse_of: :favourites
belongs_to :status, inverse_of: :favourites, counter_cache: true
belongs_to :status, inverse_of: :favourites
has_one :notification, as: :activity, dependent: :destroy
@ -25,4 +25,27 @@ class Favourite < ApplicationRecord
before_validation do
self.status = status.reblog if status&.reblog?
end
after_create :increment_cache_counters
after_destroy :decrement_cache_counters
private
def increment_cache_counters
if association(:status).loaded?
status.update_attribute(:favourites_count, status.favourites_count + 1)
else
Status.where(id: status_id).update_all('favourites_count = COALESCE(favourites_count, 0) + 1')
end
end
def decrement_cache_counters
return if association(:status).loaded? && (status.marked_for_destruction? || status.marked_for_mass_destruction?)
if association(:status).loaded?
status.update_attribute(:favourites_count, [status.favourites_count - 1, 0].max)
else
Status.where(id: status_id).update_all('favourites_count = GREATEST(COALESCE(favourites_count, 0) - 1, 0)')
end
end
end

View file

@ -41,12 +41,12 @@ class Status < ApplicationRecord
belongs_to :application, class_name: 'Doorkeeper::Application', optional: true
belongs_to :account, inverse_of: :statuses, counter_cache: true
belongs_to :account, inverse_of: :statuses
belongs_to :in_reply_to_account, foreign_key: 'in_reply_to_account_id', class_name: 'Account', optional: true
belongs_to :conversation, optional: true
belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies, optional: true
belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, counter_cache: :reblogs_count, optional: true
belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, optional: true
has_many :favourites, inverse_of: :status, dependent: :destroy
has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy
@ -167,6 +167,17 @@ class Status < ApplicationRecord
@emojis ||= CustomEmoji.from_text([spoiler_text, text].join(' '), account.domain)
end
def mark_for_mass_destruction!
@marked_for_mass_destruction = true
end
def marked_for_mass_destruction?
@marked_for_mass_destruction
end
after_create :increment_counter_caches
after_destroy :decrement_counter_caches
after_create_commit :store_uri, if: :local?
after_create_commit :update_statistics, if: :local?
@ -388,4 +399,40 @@ class Status < ApplicationRecord
return unless public_visibility? || unlisted_visibility?
ActivityTracker.increment('activity:statuses:local')
end
def increment_counter_caches
return if direct_visibility?
if association(:account).loaded?
account.update_attribute(:statuses_count, account.statuses_count + 1)
else
Account.where(id: account_id).update_all('statuses_count = COALESCE(statuses_count, 0) + 1')
end
return unless reblog?
if association(:reblog).loaded?
reblog.update_attribute(:reblogs_count, reblog.reblogs_count + 1)
else
Status.where(id: reblog_of_id).update_all('reblogs_count = COALESCE(reblogs_count, 0) + 1')
end
end
def decrement_counter_caches
return if direct_visibility? || marked_for_mass_destruction?
if association(:account).loaded?
account.update_attribute(:statuses_count, [account.statuses_count - 1, 0].max)
else
Account.where(id: account_id).update_all('statuses_count = GREATEST(COALESCE(statuses_count, 0) - 1, 0)')
end
return unless reblog?
if association(:reblog).loaded?
reblog.update_attribute(:reblogs_count, [reblog.reblogs_count - 1, 0].max)
else
Status.where(id: reblog_of_id).update_all('reblogs_count = GREATEST(COALESCE(reblogs_count, 0) - 1, 0)')
end
end
end

View file

@ -21,7 +21,10 @@ class BatchedRemoveStatusService < BaseService
@activity_xml = {}
# Ensure that rendered XML reflects destroyed state
statuses.each(&:destroy)
statuses.each do |status|
status.mark_for_mass_destruction!
status.destroy
end
# Batch by source account
statuses.group_by(&:account_id).each_value do |account_statuses|

View file

@ -41,9 +41,10 @@ class SuspendAccountService < BaseService
end
def purge_profile!
@account.suspended = true
@account.display_name = ''
@account.note = ''
@account.suspended = true
@account.display_name = ''
@account.note = ''
@account.statuses_count = 0
@account.avatar.destroy
@account.header.destroy
@account.save!

View file

@ -525,6 +525,37 @@ RSpec.describe Account, type: :model do
end
end
describe '#statuses_count' do
subject { Fabricate(:account) }
it 'counts statuses' do
Fabricate(:status, account: subject)
Fabricate(:status, account: subject)
expect(subject.statuses_count).to eq 2
end
it 'does not count direct statuses' do
Fabricate(:status, account: subject, visibility: :direct)
expect(subject.statuses_count).to eq 0
end
it 'is decremented when status is removed' do
status = Fabricate(:status, account: subject)
expect(subject.statuses_count).to eq 1
status.destroy
expect(subject.statuses_count).to eq 0
end
it 'is decremented when status is removed when account is not preloaded' do
status = Fabricate(:status, account: subject)
expect(subject.reload.statuses_count).to eq 1
clean_status = Status.find(status.id)
expect(clean_status.association(:account).loaded?).to be false
clean_status.destroy
expect(subject.reload.statuses_count).to eq 0
end
end
describe '.following_map' do
it 'returns an hash' do
expect(Account.following_map([], 1)).to be_a Hash

View file

@ -175,6 +175,13 @@ RSpec.describe Status, type: :model do
expect(subject.reblogs_count).to eq 2
end
it 'is decremented when reblog is removed' do
reblog = Fabricate(:status, account: bob, reblog: subject)
expect(subject.reblogs_count).to eq 1
reblog.destroy
expect(subject.reblogs_count).to eq 0
end
end
describe '#favourites_count' do
@ -184,6 +191,13 @@ RSpec.describe Status, type: :model do
expect(subject.favourites_count).to eq 2
end
it 'is decremented when favourite is removed' do
favourite = Fabricate(:favourite, account: bob, status: subject)
expect(subject.favourites_count).to eq 1
favourite.destroy
expect(subject.favourites_count).to eq 0
end
end
describe '#proper' do