me/glitchier-soc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix AccountDeletionWorker crashing and clogging sidekiq queues (#15380)

Browse source 
* 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>
This commit is contained in:
ThibG 2020-12-20 18:25:00 +01:00 committed by GitHub
parent 9734c9b6fe
commit 7bf3c6e57b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 6 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/models/poll.rb
View file

@ -25,7 +25,7 @@ class Poll < ApplicationRecord
belongs_to :account belongs_to :account
belongs_to :status 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 has_many :notifications, as: :activity, dependent: :destroy

2
app/models/status.rb
View file

@ -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 :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 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 :bookmarks, inverse_of: :status, dependent: :destroy
has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, 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 has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread

3
app/services/batched_remove_status_service.rb
View file

@ -4,8 +4,6 @@ class BatchedRemoveStatusService < BaseService
include Redisable include Redisable
# Delete given statuses and reblogs of them # 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 # Remove statuses from home feeds
# Push delete events to streaming API for home feeds and public feeds # Push delete events to streaming API for home feeds and public feeds
# @param [Enumerable<Status>] statuses A preferably batched array of statuses # @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) } @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| statuses.each do |status|
status.mark_for_mass_destruction! status.mark_for_mass_destruction!
status.destroy status.destroy

4
app/services/delete_account_service.rb
View file

@ -122,7 +122,9 @@ class DeleteAccountService < BaseService
@account.polls.reorder(nil).find_each do |poll| @account.polls.reorder(nil).find_each do |poll|
next if @options[:reserve_username] && reported_status_ids.include?(poll.status_id) 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 end
associations_for_destruction.each do |association_name| associations_for_destruction.each do |association_name|

2
app/workers/account_deletion_worker.rb
View file

@ -3,7 +3,7 @@
class AccountDeletionWorker class AccountDeletionWorker
include Sidekiq::Worker include Sidekiq::Worker
sidekiq_options queue: 'pull' sidekiq_options queue: 'pull', lock: :until_executed
def perform(account_id, options = {}) def perform(account_id, options = {})
reserve_username = options.with_indifferent_access.fetch(:reserve_username, true) reserve_username = options.with_indifferent_access.fetch(:reserve_username, true)