From 55315089cf0d5ed8d1bd067c53d110fa80696a45 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Thu, 11 Sep 2025 18:43:36 +0530 Subject: [PATCH] fix(delete_object_job): pre-purge heavy associations before destroy to prevent timeout (#12408) Deleting large Accounts/Inboxes with object.destroy! can time out and create heavy destroy_async fan-out; this change adds a simple pre-purge that batch-destroys heavy associations first . ``` Account: conversations, contacts Inbox: conversations, contact_inboxes ``` We use find_in_batches(5000), then proceeds with destroy!, reducing DB pressure and race conditions while preserving callbacks and leaving the behavior for non heavy models unchanged. The change is also done in a way to easily add additional objects or relations to the list. fixes: https://linear.app/chatwoot/issue/CW-3106/inbox-deletion-process-update-the-flow --- app/jobs/delete_object_job.rb | 28 +++++++++++ spec/jobs/delete_object_job_spec.rb | 78 ++++++++++++++++++++++++----- 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/app/jobs/delete_object_job.rb b/app/jobs/delete_object_job.rb index 49a7e4752..756d0feb1 100644 --- a/app/jobs/delete_object_job.rb +++ b/app/jobs/delete_object_job.rb @@ -1,12 +1,40 @@ class DeleteObjectJob < ApplicationJob queue_as :low + BATCH_SIZE = 5_000 + HEAVY_ASSOCIATIONS = { + Account => %i[conversations contacts inboxes reporting_events], + Inbox => %i[conversations contact_inboxes reporting_events] + }.freeze + def perform(object, user = nil, ip = nil) + # Pre-purge heavy associations for large objects to avoid + # timeouts & race conditions due to destroy_async fan-out. + purge_heavy_associations(object) object.destroy! process_post_deletion_tasks(object, user, ip) end def process_post_deletion_tasks(object, user, ip); end + + private + + def purge_heavy_associations(object) + klass = HEAVY_ASSOCIATIONS.keys.find { |k| object.is_a?(k) } + return unless klass + + HEAVY_ASSOCIATIONS[klass].each do |assoc| + next unless object.respond_to?(assoc) + + batch_destroy(object.public_send(assoc)) + end + end + + def batch_destroy(relation) + relation.find_in_batches(batch_size: BATCH_SIZE) do |batch| + batch.each(&:destroy!) + end + end end DeleteObjectJob.prepend_mod_with('DeleteObjectJob') diff --git a/spec/jobs/delete_object_job_spec.rb b/spec/jobs/delete_object_job_spec.rb index 854ff1da2..cb01ceb34 100644 --- a/spec/jobs/delete_object_job_spec.rb +++ b/spec/jobs/delete_object_job_spec.rb @@ -1,20 +1,74 @@ require 'rails_helper' -RSpec.describe DeleteObjectJob do - subject(:job) { described_class.perform_later(account) } +RSpec.describe DeleteObjectJob, type: :job do + describe '#perform' do + context 'when object is heavy (Inbox)' do + let!(:account) { create(:account) } + let!(:inbox) { create(:inbox, account: account) } - let(:account) { create(:account) } + before do + create_list(:conversation, 3, account: account, inbox: inbox) + ReportingEvent.create!(account: account, inbox: inbox, name: 'inbox_metric', value: 1.0) + end - it 'enqueues the job' do - expect { job }.to have_enqueued_job(described_class) - .with(account) - .on_queue('low') - end + it 'enqueues on the low queue' do + expect { described_class.perform_later(inbox) } + .to have_enqueued_job(described_class).with(inbox).on_queue('low') + end - context 'when an object is passed to the job' do - it 'is deleted' do - described_class.perform_now(account) - expect { account.reload }.to raise_error(ActiveRecord::RecordNotFound) + it 'pre-deletes heavy associations and then destroys the object' do + conv_ids = inbox.conversations.pluck(:id) + ci_ids = inbox.contact_inboxes.pluck(:id) + contact_ids = inbox.contacts.pluck(:id) + re_ids = inbox.reporting_events.pluck(:id) + + described_class.perform_now(inbox) + + expect(Conversation.where(id: conv_ids)).to be_empty + expect(ContactInbox.where(id: ci_ids)).to be_empty + expect(ReportingEvent.where(id: re_ids)).to be_empty + # Contacts should not be deleted for inbox destroy + expect(Contact.where(id: contact_ids)).not_to be_empty + expect { inbox.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'when object is heavy (Account)' do + let!(:account) { create(:account) } + let!(:inbox1) { create(:inbox, account: account) } + let!(:inbox2) { create(:inbox, account: account) } + + before do + create_list(:conversation, 2, account: account, inbox: inbox1) + create_list(:conversation, 1, account: account, inbox: inbox2) + ReportingEvent.create!(account: account, name: 'acct_metric', value: 2.5) + ReportingEvent.create!(account: account, inbox: inbox1, name: 'acct_inbox_metric', value: 3.5) + end + + it 'pre-deletes conversations, contacts, inboxes and reporting events and then destroys the account' do + conv_ids = account.conversations.pluck(:id) + contact_ids = account.contacts.pluck(:id) + inbox_ids = account.inboxes.pluck(:id) + re_ids = account.reporting_events.pluck(:id) + + described_class.perform_now(account) + + expect(Conversation.where(id: conv_ids)).to be_empty + expect(Contact.where(id: contact_ids)).to be_empty + expect(Inbox.where(id: inbox_ids)).to be_empty + expect(ReportingEvent.where(id: re_ids)).to be_empty + expect { account.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'when object is regular (Label)' do + it 'just destroys the object' do + label = create(:label) + + described_class.perform_now(label) + + expect { label.reload }.to raise_error(ActiveRecord::RecordNotFound) + end end end end