From 19b4c666f7f760986be4215ea874f951c617a0df Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Thu, 31 May 2018 17:09:09 +0200
Subject: 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
---
 .../20180528141303_fix_accounts_unique_index.rb    | 42 +++++++++++-----------
 1 file changed, 22 insertions(+), 20 deletions(-)

(limited to 'db')

diff --git a/db/migrate/20180528141303_fix_accounts_unique_index.rb b/db/migrate/20180528141303_fix_accounts_unique_index.rb
index aadb5b7db..e949436dc 100644
--- a/db/migrate/20180528141303_fix_accounts_unique_index.rb
+++ b/db/migrate/20180528141303_fix_accounts_unique_index.rb
@@ -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
     reference_account = accounts.shift
 
-    accounts.each do |other_account|
-      if other_account.public_key == reference_account.public_key
-        # The accounts definitely point to the same resource, so
-        # it's safe to re-attribute content and relationships
-        merge_accounts!(reference_account, other_account)
-      elsif other_account.local?
-        # Since domain is in the GROUP BY clause, both accounts
-        # are always either going to be local or not local, so only
-        # one check is needed. Since we cannot support two users with
-        # the same username locally, one has to go. 😢
-        other_account.user&.destroy
-      end
+    say_with_time "Deduplicating @#{reference_account.acct} (#{accounts.size} duplicates)..." do
+      accounts.each do |other_account|
+        if other_account.public_key == reference_account.public_key
+          # The accounts definitely point to the same resource, so
+          # it's safe to re-attribute content and relationships
+          merge_accounts!(reference_account, other_account)
+        elsif other_account.local?
+          # Since domain is in the GROUP BY clause, both accounts
+          # are always either going to be local or not local, so only
+          # one check is needed. Since we cannot support two users with
+          # the same username locally, one has to go. 😢
+          other_account.user&.destroy
+        end
 
-      other_account.destroy
+        other_account.destroy
+      end
     end
   end
 
   def merge_accounts!(main_account, duplicate_account)
-    [Status, Favourite, Mention, StatusPin, StreamEntry].each do |klass|
-      klass.where(account_id: duplicate_account.id).update_all(account_id: main_account.id)
+    [Status, Mention, StatusPin, StreamEntry].each do |klass|
+      klass.where(account_id: duplicate_account.id).in_batches.update_all(account_id: main_account.id)
     end
 
     # 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
     # account already, therefore mass update will not work and we need
     # 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|
         begin
-          record.update(account_id: main_account.id)
-        rescue ActiveRecord::RecordNotUnique
+          record.update_attribute(:account_id, main_account.id)
+        rescue PG::UniqueViolation
           next
         end
       end
 
       klass.where(target_account_id: duplicate_account.id).find_each do |record|
         begin
-          record.update(target_account_id: main_account.id)
-        rescue ActiveRecord::RecordNotUnique
+          record.update_attribute(:target_account_id, main_account.id)
+        rescue PG::UniqueViolation
           next
         end
       end
-- 
cgit 


From fb1ae0152d399db863457963890310a500863b02 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Thu, 31 May 2018 17:22:33 +0200
Subject: Wrong exception class: ActiveRecord::RecordNotUnique, not
 PG::UniqueViolation (#7688)

* Wrong exception class: ActiveRecord::RecordNotUnique, not PG::UniqueViolation

It's completely not obvious but PG::UniqueViolation is just a string inside the exception message, not the actual class of the exception

* Favourite does not have target_account_id
---
 db/migrate/20180528141303_fix_accounts_unique_index.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'db')

diff --git a/db/migrate/20180528141303_fix_accounts_unique_index.rb b/db/migrate/20180528141303_fix_accounts_unique_index.rb
index e949436dc..96cee37f9 100644
--- a/db/migrate/20180528141303_fix_accounts_unique_index.rb
+++ b/db/migrate/20180528141303_fix_accounts_unique_index.rb
@@ -73,15 +73,17 @@ class FixAccountsUniqueIndex < ActiveRecord::Migration[5.2]
       klass.where(account_id: duplicate_account.id).find_each do |record|
         begin
           record.update_attribute(:account_id, main_account.id)
-        rescue PG::UniqueViolation
+        rescue ActiveRecord::RecordNotUnique
           next
         end
       end
+    end
 
+    [Follow, FollowRequest, Block, Mute].each do |klass|
       klass.where(target_account_id: duplicate_account.id).find_each do |record|
         begin
           record.update_attribute(:target_account_id, main_account.id)
-        rescue PG::UniqueViolation
+        rescue ActiveRecord::RecordNotUnique
           next
         end
       end
-- 
cgit