From 7c5bb343c67ae371ee47eeb4fadb354794a0505d Mon Sep 17 00:00:00 2001
From: Vishnu Narayanan <iamwishnu@gmail.com>
Date: Thu, 9 Oct 2025 16:04:50 +0530
Subject: [PATCH] fix: Optimize message reindexing to reduce sidekiq job
 creation (#12618)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 app/models/message.rb       |  7 ++++-
 spec/models/message_spec.rb | 56 +++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/app/models/message.rb b/app/models/message.rb
index dbab19df3..2079e31a9 100644
--- a/app/models/message.rb
+++ b/app/models/message.rb
@@ -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')
diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb
index a0bd48e39..c5e080677 100644
--- a/spec/models/message_spec.rb
+++ b/spec/models/message_spec.rb
@@ -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