Extract counters from accounts table to account_stats table (#9295)

This commit is contained in:
Eugen Rochko 2018-11-19 00:43:52 +01:00 committed by GitHub
parent 4fdefffb99
commit d6b9a62e0a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
25 changed files with 203 additions and 68 deletions

View File

@ -31,7 +31,7 @@ module Admin
end end
def subscribeable_accounts def subscribeable_accounts
Account.with_followers.remote.where(domain: params[:by_domain]) Account.remote.where(protocol: :ostatus).where(domain: params[:by_domain])
end end
def filter_params def filter_params

View File

@ -25,7 +25,7 @@ class Api::V1::Accounts::FollowerAccountsController < Api::BaseController
end end
def default_accounts def default_accounts
Account.includes(:active_relationships).references(:active_relationships) Account.includes(:active_relationships, :account_stat).references(:active_relationships)
end end
def paginated_follows def paginated_follows

View File

@ -25,7 +25,7 @@ class Api::V1::Accounts::FollowingAccountsController < Api::BaseController
end end
def default_accounts def default_accounts
Account.includes(:passive_relationships).references(:passive_relationships) Account.includes(:passive_relationships, :account_stat).references(:passive_relationships)
end end
def paginated_follows def paginated_follows

View File

@ -19,7 +19,7 @@ class Api::V1::BlocksController < Api::BaseController
end end
def paginated_blocks def paginated_blocks
@paginated_blocks ||= Block.eager_load(:target_account) @paginated_blocks ||= Block.eager_load(target_account: :account_stat)
.where(account: current_account) .where(account: current_account)
.paginate_by_max_id( .paginate_by_max_id(
limit_param(DEFAULT_ACCOUNTS_LIMIT), limit_param(DEFAULT_ACCOUNTS_LIMIT),

View File

@ -27,7 +27,7 @@ class Api::V1::EndorsementsController < Api::BaseController
end end
def endorsed_accounts def endorsed_accounts
current_account.endorsed_accounts current_account.endorsed_accounts.includes(:account_stat)
end end
def insert_pagination_headers def insert_pagination_headers

View File

@ -33,7 +33,7 @@ class Api::V1::FollowRequestsController < Api::BaseController
end end
def default_accounts def default_accounts
Account.includes(:follow_requests).references(:follow_requests) Account.includes(:follow_requests, :account_stat).references(:follow_requests)
end end
def paginated_follow_requests def paginated_follow_requests

View File

@ -37,9 +37,9 @@ class Api::V1::Lists::AccountsController < Api::BaseController
def load_accounts def load_accounts
if unlimited? if unlimited?
@list.accounts.all @list.accounts.includes(:account_stat).all
else else
@list.accounts.paginate_by_max_id(limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:max_id], params[:since_id]) @list.accounts.includes(:account_stat).paginate_by_max_id(limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:max_id], params[:since_id])
end end
end end

View File

@ -22,7 +22,7 @@ class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController
def default_accounts def default_accounts
Account Account
.includes(:favourites) .includes(:favourites, :account_stat)
.references(:favourites) .references(:favourites)
.where(favourites: { status_id: @status.id }) .where(favourites: { status_id: @status.id })
end end

View File

@ -21,7 +21,7 @@ class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController
end end
def default_accounts def default_accounts
Account.includes(:statuses).references(:statuses) Account.includes(:statuses, :account_stat).references(:statuses)
end end
def paginated_statuses def paginated_statuses

View File

@ -32,9 +32,6 @@
# suspended :boolean default(FALSE), not null # suspended :boolean default(FALSE), not null
# locked :boolean default(FALSE), not null # locked :boolean default(FALSE), not null
# header_remote_url :string default(""), not null # header_remote_url :string default(""), not null
# statuses_count :integer default(0), not null
# followers_count :integer default(0), not null
# following_count :integer default(0), not null
# last_webfingered_at :datetime # last_webfingered_at :datetime
# inbox_url :string default(""), not null # inbox_url :string default(""), not null
# outbox_url :string default(""), not null # outbox_url :string default(""), not null
@ -58,6 +55,7 @@ class Account < ApplicationRecord
include AccountInteractions include AccountInteractions
include Attachmentable include Attachmentable
include Paginable include Paginable
include AccountCounters
enum protocol: [:ostatus, :activitypub] enum protocol: [:ostatus, :activitypub]
@ -119,8 +117,6 @@ class Account < ApplicationRecord
scope :remote, -> { where.not(domain: nil) } scope :remote, -> { where.not(domain: nil) }
scope :local, -> { where(domain: nil) } scope :local, -> { where(domain: nil) }
scope :without_followers, -> { where(followers_count: 0) }
scope :with_followers, -> { where('followers_count > 0') }
scope :expiring, ->(time) { remote.where.not(subscription_expires_at: nil).where('subscription_expires_at < ?', time) } scope :expiring, ->(time) { remote.where.not(subscription_expires_at: nil).where('subscription_expires_at < ?', time) }
scope :partitioned, -> { order(Arel.sql('row_number() over (partition by domain)')) } scope :partitioned, -> { order(Arel.sql('row_number() over (partition by domain)')) }
scope :silenced, -> { where(silenced: true) } scope :silenced, -> { where(silenced: true) }
@ -385,7 +381,9 @@ class Account < ApplicationRecord
LIMIT ? LIMIT ?
SQL SQL
find_by_sql([sql, limit]) records = find_by_sql([sql, limit])
ActiveRecord::Associations::Preloader.new.preload(records, :account_stat)
records
end end
def advanced_search_for(terms, account, limit = 10, following = false) def advanced_search_for(terms, account, limit = 10, following = false)
@ -412,7 +410,7 @@ class Account < ApplicationRecord
LIMIT ? LIMIT ?
SQL SQL
find_by_sql([sql, account.id, account.id, account.id, limit]) records = find_by_sql([sql, account.id, account.id, account.id, limit])
else else
sql = <<-SQL.squish sql = <<-SQL.squish
SELECT SELECT
@ -428,8 +426,11 @@ class Account < ApplicationRecord
LIMIT ? LIMIT ?
SQL SQL
find_by_sql([sql, account.id, account.id, limit]) records = find_by_sql([sql, account.id, account.id, limit])
end end
ActiveRecord::Associations::Preloader.new.preload(records, :account_stat)
records
end end
private private

View File

@ -0,0 +1,26 @@
# frozen_string_literal: true
# == Schema Information
#
# Table name: account_stats
#
# id :bigint(8) not null, primary key
# account_id :bigint(8) not null
# statuses_count :bigint(8) default(0), not null
# following_count :bigint(8) default(0), not null
# followers_count :bigint(8) default(0), not null
# created_at :datetime not null
# updated_at :datetime not null
#
class AccountStat < ApplicationRecord
belongs_to :account, inverse_of: :account_stat
def increment_count!(key)
update(key => public_send(key) + 1)
end
def decrement_count!(key)
update(key => [public_send(key) - 1, 0].max)
end
end

View File

@ -0,0 +1,31 @@
# frozen_string_literal: true
module AccountCounters
extend ActiveSupport::Concern
included do
has_one :account_stat, inverse_of: :account
after_save :save_account_stat
end
delegate :statuses_count,
:statuses_count=,
:following_count,
:following_count=,
:followers_count,
:followers_count=,
:increment_count!,
:decrement_count!,
to: :account_stat
def account_stat
super || build_account_stat
end
private
def save_account_stat
return unless account_stat&.changed?
account_stat.save
end
end

View File

@ -16,11 +16,8 @@ class Follow < ApplicationRecord
include Paginable include Paginable
include RelationshipCacheable include RelationshipCacheable
belongs_to :account, counter_cache: :following_count belongs_to :account
belongs_to :target_account, class_name: 'Account'
belongs_to :target_account,
class_name: 'Account',
counter_cache: :followers_count
has_one :notification, as: :activity, dependent: :destroy has_one :notification, as: :activity, dependent: :destroy
@ -39,7 +36,9 @@ class Follow < ApplicationRecord
end end
before_validation :set_uri, only: :create before_validation :set_uri, only: :create
after_create :increment_cache_counters
after_destroy :remove_endorsements after_destroy :remove_endorsements
after_destroy :decrement_cache_counters
private private
@ -50,4 +49,14 @@ class Follow < ApplicationRecord
def remove_endorsements def remove_endorsements
AccountPin.where(target_account_id: target_account_id, account_id: account_id).delete_all AccountPin.where(target_account_id: target_account_id, account_id: account_id).delete_all
end end
def increment_cache_counters
account&.increment_count!(:following_count)
target_account&.increment_count!(:followers_count)
end
def decrement_cache_counters
account&.decrement_count!(:following_count)
target_account&.decrement_count!(:followers_count)
end
end end

View File

@ -75,7 +75,7 @@ class Notification < ApplicationRecord
return if account_ids.empty? return if account_ids.empty?
accounts = Account.where(id: account_ids).each_with_object({}) { |a, h| h[a.id] = a } accounts = Account.where(id: account_ids).includes(:account_stat).each_with_object({}) { |a, h| h[a.id] = a }
cached_items.each do |item| cached_items.each do |item|
item.from_account = accounts[item.from_account_id] item.from_account = accounts[item.from_account_id]

View File

@ -94,17 +94,16 @@ class Status < ApplicationRecord
end end
} }
cache_associated :account, cache_associated :application,
:application,
:media_attachments, :media_attachments,
:conversation, :conversation,
:status_stat, :status_stat,
:tags, :tags,
:preview_cards, :preview_cards,
:stream_entry, :stream_entry,
active_mentions: :account, account: :account_stat,
active_mentions: { account: :account_stat },
reblog: [ reblog: [
:account,
:application, :application,
:stream_entry, :stream_entry,
:tags, :tags,
@ -112,9 +111,10 @@ class Status < ApplicationRecord
:media_attachments, :media_attachments,
:conversation, :conversation,
:status_stat, :status_stat,
active_mentions: :account, account: :account_stat,
active_mentions: { account: :account_stat },
], ],
thread: :account thread: { account: :account_stat }
delegate :domain, to: :account, prefix: true delegate :domain, to: :account, prefix: true
@ -348,7 +348,7 @@ class Status < ApplicationRecord
return if account_ids.empty? return if account_ids.empty?
accounts = Account.where(id: account_ids).each_with_object({}) { |a, h| h[a.id] = a } accounts = Account.where(id: account_ids).includes(:account_stat).each_with_object({}) { |a, h| h[a.id] = a }
cached_items.each do |item| cached_items.each do |item|
item.account = accounts[item.account_id] item.account = accounts[item.account_id]
@ -475,12 +475,7 @@ class Status < ApplicationRecord
def increment_counter_caches def increment_counter_caches
return if direct_visibility? return if direct_visibility?
if association(:account).loaded? account&.increment_count!(:statuses_count)
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
reblog&.increment_count!(:reblogs_count) if reblog? reblog&.increment_count!(:reblogs_count) if reblog?
thread&.increment_count!(:replies_count) if in_reply_to_id.present? && (public_visibility? || unlisted_visibility?) thread&.increment_count!(:replies_count) if in_reply_to_id.present? && (public_visibility? || unlisted_visibility?)
end end
@ -488,12 +483,7 @@ class Status < ApplicationRecord
def decrement_counter_caches def decrement_counter_caches
return if direct_visibility? || marked_for_mass_destruction? return if direct_visibility? || marked_for_mass_destruction?
if association(:account).loaded? account&.decrement_count!(:statuses_count)
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
reblog&.decrement_count!(:reblogs_count) if reblog? reblog&.decrement_count!(:reblogs_count) if reblog?
thread&.decrement_count!(:replies_count) if in_reply_to_id.present? && (public_visibility? || unlisted_visibility?) thread&.decrement_count!(:replies_count) if in_reply_to_id.present? && (public_visibility? || unlisted_visibility?)
end end

View File

@ -22,7 +22,7 @@ class InstancePresenter
end end
def status_count def status_count
Rails.cache.fetch('local_status_count') { Account.local.sum(:statuses_count) } Rails.cache.fetch('local_status_count') { Account.local.joins(:account_stat).sum('account_stats.statuses_count') }
end end
def domain_count def domain_count

View File

@ -0,0 +1,12 @@
class CreateAccountStats < ActiveRecord::Migration[5.2]
def change
create_table :account_stats do |t|
t.belongs_to :account, null: false, foreign_key: { on_delete: :cascade }, index: { unique: true }
t.bigint :statuses_count, null: false, default: 0
t.bigint :following_count, null: false, default: 0
t.bigint :followers_count, null: false, default: 0
t.timestamps
end
end
end

View File

@ -0,0 +1,54 @@
class CopyAccountStats < ActiveRecord::Migration[5.2]
disable_ddl_transaction!
def up
safety_assured do
if supports_upsert?
up_fast
else
up_slow
end
end
end
def down
# Nothing
end
private
def supports_upsert?
version = select_one("SELECT current_setting('server_version_num') AS v")['v'].to_i
version >= 90500
end
def up_fast
say 'Upsert is available, importing counters using the fast method'
Account.unscoped.select('id').find_in_batches(batch_size: 5_000) do |accounts|
execute <<-SQL.squish
INSERT INTO account_stats (account_id, statuses_count, following_count, followers_count, created_at, updated_at)
SELECT id, statuses_count, following_count, followers_count, created_at, updated_at
FROM accounts
WHERE id IN (#{accounts.map(&:id).join(', ')})
ON CONFLICT (account_id) DO UPDATE
SET statuses_count = EXCLUDED.statuses_count, following_count = EXCLUDED.following_count, followers_count = EXCLUDED.followers_count
SQL
end
end
def up_slow
say 'Upsert is not available in PostgreSQL below 9.5, falling back to slow import of counters'
# We cannot use bulk INSERT or overarching transactions here because of possible
# uniqueness violations that we need to skip over
Account.unscoped.select('id, statuses_count, following_count, followers_count, created_at, updated_at').find_each do |account|
begin
params = [[nil, account.id], [nil, account.statuses_count], [nil, account.following_count], [nil, account.followers_count], [nil, account.created_at], [nil, account.updated_at]]
exec_insert('INSERT INTO account_stats (account_id, statuses_count, following_count, followers_count, created_at, updated_at) VALUES ($1, $2, $3, $4, $5, $6)', nil, params)
rescue ActiveRecord::RecordNotUnique
next
end
end
end
end

View File

@ -0,0 +1,13 @@
# frozen_string_literal: true
class CopyAccountStatsCleanup < ActiveRecord::Migration[5.2]
disable_ddl_transaction!
def change
safety_assured do
remove_column :accounts, :statuses_count, :integer, default: 0, null: false
remove_column :accounts, :following_count, :integer, default: 0, null: false
remove_column :accounts, :followers_count, :integer, default: 0, null: false
end
end
end

View File

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2018_10_26_034033) do ActiveRecord::Schema.define(version: 2018_11_16_184611) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -56,6 +56,16 @@ ActiveRecord::Schema.define(version: 2018_10_26_034033) do
t.index ["target_account_id"], name: "index_account_pins_on_target_account_id" t.index ["target_account_id"], name: "index_account_pins_on_target_account_id"
end end
create_table "account_stats", force: :cascade do |t|
t.bigint "account_id", null: false
t.bigint "statuses_count", default: 0, null: false
t.bigint "following_count", default: 0, null: false
t.bigint "followers_count", default: 0, null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true
end
create_table "accounts", force: :cascade do |t| create_table "accounts", force: :cascade do |t|
t.string "username", default: "", null: false t.string "username", default: "", null: false
t.string "domain" t.string "domain"
@ -85,9 +95,6 @@ ActiveRecord::Schema.define(version: 2018_10_26_034033) do
t.boolean "suspended", default: false, null: false t.boolean "suspended", default: false, null: false
t.boolean "locked", default: false, null: false t.boolean "locked", default: false, null: false
t.string "header_remote_url", default: "", null: false t.string "header_remote_url", default: "", null: false
t.integer "statuses_count", default: 0, null: false
t.integer "followers_count", default: 0, null: false
t.integer "following_count", default: 0, null: false
t.datetime "last_webfingered_at" t.datetime "last_webfingered_at"
t.string "inbox_url", default: "", null: false t.string "inbox_url", default: "", null: false
t.string "outbox_url", default: "", null: false t.string "outbox_url", default: "", null: false
@ -629,6 +636,7 @@ ActiveRecord::Schema.define(version: 2018_10_26_034033) do
add_foreign_key "account_moderation_notes", "accounts", column: "target_account_id" add_foreign_key "account_moderation_notes", "accounts", column: "target_account_id"
add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade
add_foreign_key "account_pins", "accounts", on_delete: :cascade add_foreign_key "account_pins", "accounts", on_delete: :cascade
add_foreign_key "account_stats", "accounts", on_delete: :cascade
add_foreign_key "accounts", "accounts", column: "moved_to_account_id", on_delete: :nullify add_foreign_key "accounts", "accounts", column: "moved_to_account_id", on_delete: :nullify
add_foreign_key "admin_action_logs", "accounts", on_delete: :cascade add_foreign_key "admin_action_logs", "accounts", on_delete: :cascade
add_foreign_key "backups", "users", on_delete: :nullify add_foreign_key "backups", "users", on_delete: :nullify

View File

@ -0,0 +1,6 @@
Fabricator(:account_stat) do
account nil
statuses_count ""
following_count ""
followers_count ""
end

View File

@ -754,24 +754,6 @@ RSpec.describe Account, type: :model do
expect(Account.suspended).to match_array([account_1]) expect(Account.suspended).to match_array([account_1])
end end
end end
describe 'without_followers' do
it 'returns a relation of accounts without followers' do
account_1 = Fabricate(:account)
account_2 = Fabricate(:account)
Fabricate(:follow, account: account_1, target_account: account_2)
expect(Account.without_followers).to match_array([account_1])
end
end
describe 'with_followers' do
it 'returns a relation of accounts with followers' do
account_1 = Fabricate(:account)
account_2 = Fabricate(:account)
Fabricate(:follow, account: account_1, target_account: account_2)
expect(Account.with_followers).to match_array([account_2])
end
end
end end
context 'when is local' do context 'when is local' do

View File

@ -0,0 +1,4 @@
require 'rails_helper'
RSpec.describe AccountStat, type: :model do
end

View File

@ -101,7 +101,7 @@ RSpec.describe Notification, type: :model do
before do before do
allow(accounts_with_ids).to receive(:[]).with(stale_account1.id).and_return(account1) allow(accounts_with_ids).to receive(:[]).with(stale_account1.id).and_return(account1)
allow(accounts_with_ids).to receive(:[]).with(stale_account2.id).and_return(account2) allow(accounts_with_ids).to receive(:[]).with(stale_account2.id).and_return(account2)
allow(Account).to receive_message_chain(:where, :each_with_object).and_return(accounts_with_ids) allow(Account).to receive_message_chain(:where, :includes, :each_with_object).and_return(accounts_with_ids)
end end
let(:cached_items) do let(:cached_items) do

View File

@ -1,5 +1,4 @@
require 'rails_helper' require 'rails_helper'
RSpec.describe StatusStat, type: :model do RSpec.describe StatusStat, type: :model do
pending "add some examples to (or delete) #{__FILE__}"
end end