Fix AccountDeletionWorker crashing and clogging sidekiq queues (#15380)

* Fix account deletion workers being queued multiple times for a single account

* Fix poll votes being unnecessarily instantiated on poll deletion

* Fix favourites being unnecessarily instantiated on status deletion

* Remove inaccurate comments

* Delete polls instead of destroying them

Co-authored-by: Claire <claire.github-309c@sitedethib.com>

diff --git a/app/models/poll.rb b/app/models/poll.rb
index b5deafcc21fbe442f54b86fb3a0c7bb592c7aac9..e1ca5525219e4eaa67caf0538f69f79a026eb9ed 100644 (file)
--- a/app/models/poll.rb
+++ b/app/models/poll.rb
@@ -25,7 +25,7 @@ class Poll < ApplicationRecord
   belongs_to :account
   belongs_to :status
 
-  has_many :votes, class_name: 'PollVote', inverse_of: :poll, dependent: :destroy
+  has_many :votes, class_name: 'PollVote', inverse_of: :poll, dependent: :delete_all
 
   has_many :notifications, as: :activity, dependent: :destroy
 

diff --git a/app/models/status.rb b/app/models/status.rb
index 96d90e1c27e216d4fb9beb47501b9cee96892653..f1b3b75ce8e9bcc2fff96293dea7b3c0800f791e 100644 (file)
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -56,7 +56,7 @@ class Status < ApplicationRecord
   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, optional: true
 
-  has_many :favourites, inverse_of: :status, dependent: :destroy
+  has_many :favourites, inverse_of: :status, dependent: :delete_all
   has_many :bookmarks, inverse_of: :status, dependent: :destroy
   has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy
   has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread

diff --git a/app/services/batched_remove_status_service.rb b/app/services/batched_remove_status_service.rb
index 2295a01dc3d613bf2499af71d6672c8b64bbfa1d..28e5468b389386ecbaed7c16f7b4372225a3b548 100644 (file)
--- a/app/services/batched_remove_status_service.rb
+++ b/app/services/batched_remove_status_service.rb
@@ -4,8 +4,6 @@ class BatchedRemoveStatusService < BaseService
   include Redisable
 
   # Delete given statuses and reblogs of them
-  # Dispatch PuSH updates of the deleted statuses, but only local ones
-  # Dispatch Salmon deletes, unique per domain, of the deleted statuses, but only local ones
   # Remove statuses from home feeds
   # Push delete events to streaming API for home feeds and public feeds
   # @param [Enumerable<Status>] statuses A preferably batched array of statuses
@@ -19,7 +17,6 @@ class BatchedRemoveStatusService < BaseService
 
     @json_payloads = statuses.each_with_object({}) { |s, h| h[s.id] = Oj.dump(event: :delete, payload: s.id.to_s) }
 
-    # Ensure that rendered XML reflects destroyed state
     statuses.each do |status|
       status.mark_for_mass_destruction!
       status.destroy

diff --git a/app/services/delete_account_service.rb b/app/services/delete_account_service.rb
index 9cb80c95aeabc4e4c0358e53e7656ffc01891395..fe9b30b1746aed5a3e94eac33c79ea2ff22f32f7 100644 (file)
--- a/app/services/delete_account_service.rb
+++ b/app/services/delete_account_service.rb
@@ -122,7 +122,9 @@ class DeleteAccountService < BaseService
     @account.polls.reorder(nil).find_each do |poll|
       next if @options[:reserve_username] && reported_status_ids.include?(poll.status_id)
 
-      poll.destroy
+      # We can safely delete the poll rather than destroy it, as any non-reported
+      # status should have been deleted already
+      poll.delete
     end
 
     associations_for_destruction.each do |association_name|

diff --git a/app/workers/account_deletion_worker.rb b/app/workers/account_deletion_worker.rb
index 98b67419d013813918e9931ab169e1517cd16740..fdf013e01043b10fa26025bcfa46cf87565408b0 100644 (file)
--- a/app/workers/account_deletion_worker.rb
+++ b/app/workers/account_deletion_worker.rb
@@ -3,7 +3,7 @@
 class AccountDeletionWorker
   include Sidekiq::Worker
 
-  sidekiq_options queue: 'pull'
+  sidekiq_options queue: 'pull', lock: :until_executed
 
   def perform(account_id, options = {})
     reserve_username = options.with_indifferent_access.fetch(:reserve_username, true)