fix: Optimize message reindexing to reduce sidekiq job creation (#12618)
Changes searchkick callback behavior to check `should_index?` before creating reindex jobs, preventing unnecessary job creation for messages that don't need indexing (activity messages, unpaid accounts, etc.). Previously, `callbacks: :async` created reindex jobs for all messages (~5,100/min or 7.3M/day in production), which were then filtered by `should_index?` inside the job worker - resulting in 98% wasted jobs, Redis memory pressure, and avoidable p0 alerts. Now, `should_index?` is checked before job creation via `after_commit` callback, reducing job creation to actual incoming/outgoing messages from paid accounts. Changes: - Disable automatic searchkick callbacks - Add manual `after_commit` callback with `should_index?` condition - Add specs to verify callback behavior Expected impact: - 98% reduction in sidekiq job creation (~7.3M → ~150K jobs/day) - Reduced redis memory usage - Same async indexing behavior for eligible messages
This commit is contained in:
@@ -39,7 +39,7 @@
|
||||
#
|
||||
|
||||
class Message < ApplicationRecord
|
||||
searchkick callbacks: :async if ChatwootApp.advanced_search_allowed?
|
||||
searchkick callbacks: false if ChatwootApp.advanced_search_allowed?
|
||||
|
||||
include MessageFilterHelpers
|
||||
include Liquidable
|
||||
@@ -135,6 +135,7 @@ class Message < ApplicationRecord
|
||||
after_create_commit :execute_after_create_commit_callbacks
|
||||
|
||||
after_update_commit :dispatch_update_event
|
||||
after_commit :reindex_for_search, if: :should_index?, on: [:create, :update]
|
||||
|
||||
def channel_token
|
||||
@token ||= inbox.channel.try(:page_access_token)
|
||||
@@ -436,6 +437,10 @@ class Message < ApplicationRecord
|
||||
conversation.update_columns(last_activity_at: created_at)
|
||||
# rubocop:enable Rails/SkipsModelValidations
|
||||
end
|
||||
|
||||
def reindex_for_search
|
||||
reindex(mode: :async)
|
||||
end
|
||||
end
|
||||
|
||||
Message.prepend_mod_with('Message')
|
||||
|
||||
@@ -4,6 +4,12 @@ require 'rails_helper'
|
||||
require Rails.root.join 'spec/models/concerns/liquidable_shared.rb'
|
||||
|
||||
RSpec.describe Message do
|
||||
before do
|
||||
# rubocop:disable RSpec/AnyInstance
|
||||
allow_any_instance_of(described_class).to receive(:reindex_for_search).and_return(true)
|
||||
# rubocop:enable RSpec/AnyInstance
|
||||
end
|
||||
|
||||
context 'with validations' do
|
||||
it { is_expected.to validate_presence_of(:inbox_id) }
|
||||
it { is_expected.to validate_presence_of(:conversation_id) }
|
||||
@@ -678,4 +684,54 @@ RSpec.describe Message do
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#reindex_for_search callback' do
|
||||
let(:account) { create(:account) }
|
||||
let(:conversation) { create(:conversation, account: account) }
|
||||
|
||||
before do
|
||||
allow(ChatwootApp).to receive(:advanced_search_allowed?).and_return(true)
|
||||
account.enable_features('advanced_search_indexing')
|
||||
end
|
||||
|
||||
context 'when message should be indexed' do
|
||||
it 'calls reindex_for_search for incoming message on create' do
|
||||
message = build(:message, conversation: conversation, account: account, message_type: :incoming)
|
||||
expect(message).to receive(:reindex_for_search)
|
||||
message.save!
|
||||
end
|
||||
|
||||
it 'calls reindex_for_search for outgoing message on update' do
|
||||
# rubocop:disable RSpec/AnyInstance
|
||||
allow_any_instance_of(described_class).to receive(:reindex_for_search).and_return(true)
|
||||
# rubocop:enable RSpec/AnyInstance
|
||||
message = create(:message, conversation: conversation, account: account, message_type: :outgoing)
|
||||
expect(message).to receive(:reindex_for_search).and_return(true)
|
||||
message.update!(content: 'Updated content')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when message should not be indexed' do
|
||||
it 'does not call reindex_for_search for activity message' do
|
||||
message = build(:message, conversation: conversation, account: account, message_type: :activity)
|
||||
expect(message).not_to receive(:reindex_for_search)
|
||||
message.save!
|
||||
end
|
||||
|
||||
it 'does not call reindex_for_search for unpaid account on cloud' do
|
||||
allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true)
|
||||
account.disable_features('advanced_search_indexing')
|
||||
message = build(:message, conversation: conversation, account: account, message_type: :incoming)
|
||||
expect(message).not_to receive(:reindex_for_search)
|
||||
message.save!
|
||||
end
|
||||
|
||||
it 'does not call reindex_for_search when advanced search is not allowed' do
|
||||
allow(ChatwootApp).to receive(:advanced_search_allowed?).and_return(false)
|
||||
message = build(:message, conversation: conversation, account: account, message_type: :incoming)
|
||||
expect(message).not_to receive(:reindex_for_search)
|
||||
message.save!
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user