From 5333447be0c0e278d6f591bb6004fdee903a08f7 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 9 Nov 2022 14:08:19 +0100 Subject: [PATCH] Change account deletion requests to spread out over time (#20222) --- app/workers/admin/account_deletion_worker.rb | 2 +- .../suspended_user_cleanup_scheduler.rb | 38 +++++++++++++++++++ .../scheduler/user_cleanup_scheduler.rb | 7 ---- config/sidekiq.yml | 4 ++ 4 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 app/workers/scheduler/suspended_user_cleanup_scheduler.rb diff --git a/app/workers/admin/account_deletion_worker.rb b/app/workers/admin/account_deletion_worker.rb index 82f269ad6..6e0eb331b 100644 --- a/app/workers/admin/account_deletion_worker.rb +++ b/app/workers/admin/account_deletion_worker.rb @@ -3,7 +3,7 @@ class Admin::AccountDeletionWorker include Sidekiq::Worker - sidekiq_options queue: 'pull' + sidekiq_options queue: 'pull', lock: :until_executed def perform(account_id) DeleteAccountService.new.call(Account.find(account_id), reserve_username: true, reserve_email: true) diff --git a/app/workers/scheduler/suspended_user_cleanup_scheduler.rb b/app/workers/scheduler/suspended_user_cleanup_scheduler.rb new file mode 100644 index 000000000..50768f83c --- /dev/null +++ b/app/workers/scheduler/suspended_user_cleanup_scheduler.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class Scheduler::SuspendedUserCleanupScheduler + include Sidekiq::Worker + + # Each processed deletion request may enqueue an enormous + # amount of jobs in the `pull` queue, so only enqueue when + # the queue is empty or close to being so. + MAX_PULL_SIZE = 50 + + # Since account deletion is very expensive, we want to avoid + # overloading the server by queing too much at once. + # This job runs approximately once per 2 minutes, so with a + # value of `MAX_DELETIONS_PER_JOB` of 10, a server can + # handle the deletion of 7200 accounts per day, provided it + # has the capacity for it. + MAX_DELETIONS_PER_JOB = 10 + + sidekiq_options retry: 0 + + def perform + return if Sidekiq::Queue.new('pull').size > MAX_PULL_SIZE + + clean_suspended_accounts! + end + + private + + def clean_suspended_accounts! + # This should be fine because we only process a small amount of deletion requests at once and + # `id` and `created_at` should follow the same order. + AccountDeletionRequest.reorder(id: :asc).take(MAX_DELETIONS_PER_JOB).each do |deletion_request| + next unless deletion_request.created_at < AccountDeletionRequest::DELAY_TO_DELETION.ago + + Admin::AccountDeletionWorker.perform_async(deletion_request.account_id) + end + end +end diff --git a/app/workers/scheduler/user_cleanup_scheduler.rb b/app/workers/scheduler/user_cleanup_scheduler.rb index 7a6995a1f..63f9ed78c 100644 --- a/app/workers/scheduler/user_cleanup_scheduler.rb +++ b/app/workers/scheduler/user_cleanup_scheduler.rb @@ -7,7 +7,6 @@ class Scheduler::UserCleanupScheduler def perform clean_unconfirmed_accounts! - clean_suspended_accounts! clean_discarded_statuses! end @@ -22,12 +21,6 @@ class Scheduler::UserCleanupScheduler end end - def clean_suspended_accounts! - AccountDeletionRequest.where('created_at <= ?', AccountDeletionRequest::DELAY_TO_DELETION.ago).reorder(nil).find_each do |deletion_request| - Admin::AccountDeletionWorker.perform_async(deletion_request.account_id) - end - end - def clean_discarded_statuses! Status.unscoped.discarded.where('deleted_at <= ?', 30.days.ago).find_in_batches do |statuses| RemovalWorker.push_bulk(statuses) do |status| diff --git a/config/sidekiq.yml b/config/sidekiq.yml index e3156aa34..71e7cb33d 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -53,3 +53,7 @@ interval: 1 minute class: Scheduler::AccountsStatusesCleanupScheduler queue: scheduler + suspended_user_cleanup_scheduler: + interval: 1 minute + class: Scheduler::SuspendedUserCleanupScheduler + queue: scheduler