Improve account index migration (#7684)

* Improve account index migration

- Display more progress in stdout
- Catch PG::UniqueViolation when re-attributing favourites
- Skip callbacks and validations when re-attributing other relationships

* Use in_batches to reduce table lock-up during account merge

* Use #say_with_time to benchmark each deduplication
This commit is contained in:
Eugen Rochko 2018-05-31 17:09:09 +02:00 committed by GitHub
parent 1e938b966e
commit 19b4c666f7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -39,26 +39,28 @@ class FixAccountsUniqueIndex < ActiveRecord::Migration[5.2]
accounts = accounts.first.local? ? accounts.sort_by(&:created_at) : accounts.sort_by(&:updated_at).reverse accounts = accounts.first.local? ? accounts.sort_by(&:created_at) : accounts.sort_by(&:updated_at).reverse
reference_account = accounts.shift reference_account = accounts.shift
accounts.each do |other_account| say_with_time "Deduplicating @#{reference_account.acct} (#{accounts.size} duplicates)..." do
if other_account.public_key == reference_account.public_key accounts.each do |other_account|
# The accounts definitely point to the same resource, so if other_account.public_key == reference_account.public_key
# it's safe to re-attribute content and relationships # The accounts definitely point to the same resource, so
merge_accounts!(reference_account, other_account) # it's safe to re-attribute content and relationships
elsif other_account.local? merge_accounts!(reference_account, other_account)
# Since domain is in the GROUP BY clause, both accounts elsif other_account.local?
# are always either going to be local or not local, so only # Since domain is in the GROUP BY clause, both accounts
# one check is needed. Since we cannot support two users with # are always either going to be local or not local, so only
# the same username locally, one has to go. 😢 # one check is needed. Since we cannot support two users with
other_account.user&.destroy # the same username locally, one has to go. 😢
end other_account.user&.destroy
end
other_account.destroy other_account.destroy
end
end end
end end
def merge_accounts!(main_account, duplicate_account) def merge_accounts!(main_account, duplicate_account)
[Status, Favourite, Mention, StatusPin, StreamEntry].each do |klass| [Status, Mention, StatusPin, StreamEntry].each do |klass|
klass.where(account_id: duplicate_account.id).update_all(account_id: main_account.id) klass.where(account_id: duplicate_account.id).in_batches.update_all(account_id: main_account.id)
end end
# Since it's the same remote resource, the remote resource likely # Since it's the same remote resource, the remote resource likely
@ -67,19 +69,19 @@ class FixAccountsUniqueIndex < ActiveRecord::Migration[5.2]
# of the index bug users could have *also* followed the reference # of the index bug users could have *also* followed the reference
# account already, therefore mass update will not work and we need # account already, therefore mass update will not work and we need
# to check for (and skip past) uniqueness errors # to check for (and skip past) uniqueness errors
[Follow, FollowRequest, Block, Mute].each do |klass| [Favourite, Follow, FollowRequest, Block, Mute].each do |klass|
klass.where(account_id: duplicate_account.id).find_each do |record| klass.where(account_id: duplicate_account.id).find_each do |record|
begin begin
record.update(account_id: main_account.id) record.update_attribute(:account_id, main_account.id)
rescue ActiveRecord::RecordNotUnique rescue PG::UniqueViolation
next next
end end
end end
klass.where(target_account_id: duplicate_account.id).find_each do |record| klass.where(target_account_id: duplicate_account.id).find_each do |record|
begin begin
record.update(target_account_id: main_account.id) record.update_attribute(:target_account_id, main_account.id)
rescue ActiveRecord::RecordNotUnique rescue PG::UniqueViolation
next next
end end
end end