mirror of
https://github.com/lingble/chatwoot.git
synced 2025-10-29 18:22:53 +00:00
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
This commit is contained in:
@@ -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')
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user