Fix account counters being overwritten by parallel writes (#12045)

lolsob-rspec
Eugen Rochko 2019-10-02 04:59:37 +02:00 committed by GitHub
parent 216594fe26
commit ced431f02f
4 changed files with 92 additions and 1 deletions

View File

@ -11,6 +11,7 @@
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# last_status_at :datetime # last_status_at :datetime
# lock_version :integer default(0), not null
# #
class AccountStat < ApplicationRecord class AccountStat < ApplicationRecord
@ -20,10 +21,26 @@ class AccountStat < ApplicationRecord
def increment_count!(key) def increment_count!(key)
update(attributes_for_increment(key)) update(attributes_for_increment(key))
rescue ActiveRecord::StaleObjectError
begin
reload_with_id
rescue ActiveRecord::RecordNotFound
# Nothing to do
else
retry
end
end end
def decrement_count!(key) def decrement_count!(key)
update(key => [public_send(key) - 1, 0].max) update(key => [public_send(key) - 1, 0].max)
rescue ActiveRecord::StaleObjectError
begin
reload_with_id
rescue ActiveRecord::RecordNotFound
# Nothing to do
else
retry
end
end end
private private
@ -33,4 +50,9 @@ class AccountStat < ApplicationRecord
attrs[:last_status_at] = Time.now.utc if key == :statuses_count attrs[:last_status_at] = Time.now.utc if key == :statuses_count
attrs attrs
end end
def reload_with_id
self.id = find_by!(account: account).id if new_record?
reload
end
end end

View File

@ -0,0 +1,15 @@
require Rails.root.join('lib', 'mastodon', 'migration_helpers')
class AddLockVersionToAccountStats < ActiveRecord::Migration[5.2]
include Mastodon::MigrationHelpers
disable_ddl_transaction!
def up
safety_assured { add_column_with_default :account_stats, :lock_version, :integer, allow_null: false, default: 0 }
end
def down
remove_column :account_stats, :lock_version
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: 2019_09_27_232842) do ActiveRecord::Schema.define(version: 2019_10_01_213028) 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"
@ -97,6 +97,7 @@ ActiveRecord::Schema.define(version: 2019_09_27_232842) do
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.datetime "last_status_at" t.datetime "last_status_at"
t.integer "lock_version", default: 0, null: false
t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true
end end

View File

@ -1,4 +1,57 @@
require 'rails_helper' require 'rails_helper'
RSpec.describe AccountStat, type: :model do RSpec.describe AccountStat, type: :model do
describe '#increment_count!' do
it 'increments the count' do
account_stat = AccountStat.create(account: Fabricate(:account))
expect(account_stat.followers_count).to eq 0
account_stat.increment_count!(:followers_count)
expect(account_stat.followers_count).to eq 1
end
it 'increments the count in multi-threaded an environment' do
account_stat = AccountStat.create(account: Fabricate(:account), statuses_count: 0)
increment_by = 15
wait_for_start = true
threads = Array.new(increment_by) do
Thread.new do
true while wait_for_start
AccountStat.find(account_stat.id).increment_count!(:statuses_count)
end
end
wait_for_start = false
threads.each(&:join)
expect(account_stat.reload.statuses_count).to eq increment_by
end
end
describe '#decrement_count!' do
it 'decrements the count' do
account_stat = AccountStat.create(account: Fabricate(:account), followers_count: 15)
expect(account_stat.followers_count).to eq 15
account_stat.decrement_count!(:followers_count)
expect(account_stat.followers_count).to eq 14
end
it 'decrements the count in multi-threaded an environment' do
account_stat = AccountStat.create(account: Fabricate(:account), statuses_count: 15)
decrement_by = 10
wait_for_start = true
threads = Array.new(decrement_by) do
Thread.new do
true while wait_for_start
AccountStat.find(account_stat.id).decrement_count!(:statuses_count)
end
end
wait_for_start = false
threads.each(&:join)
expect(account_stat.reload.statuses_count).to eq 5
end
end
end end