Merge branch 'codex/cw-7615-intercom-query-timeout-retries' into codex/cw-7615-intercom-message-batches
# Conflicts: # app/services/data_imports/intercom/importer.rb
This commit is contained in:
@@ -162,6 +162,7 @@ class DataImports::Intercom::Importer
|
||||
mapped_conversation = mapping&.chatwoot_record
|
||||
if mapped_conversation && mapping.data_import_id != @data_import.id
|
||||
skip_already_imported_item(item, mapping, already_handled: already_handled)
|
||||
reconcile_item_stats('conversation') if already_handled
|
||||
return unless import_conversation_messages(conversation, mapped_conversation, contact)
|
||||
|
||||
update_conversation_activity(mapped_conversation)
|
||||
@@ -173,7 +174,11 @@ class DataImports::Intercom::Importer
|
||||
record_mapping('conversation', source_id, chatwoot_conversation, metadata: conversation_metadata(conversation, inbox, source_type))
|
||||
end
|
||||
item.update!(status: :imported, chatwoot_record_type: 'Conversation', chatwoot_record_id: chatwoot_conversation.id)
|
||||
increment_stat('conversations', 'imported') unless already_handled
|
||||
if already_handled
|
||||
reconcile_item_stats('conversation')
|
||||
else
|
||||
increment_stat('conversations', 'imported')
|
||||
end
|
||||
|
||||
return unless import_conversation_messages(conversation, chatwoot_conversation, contact)
|
||||
|
||||
@@ -194,8 +199,8 @@ class DataImports::Intercom::Importer
|
||||
end
|
||||
|
||||
def continue_import_with_heartbeat?
|
||||
return true if @data_import.updated_at > HEARTBEAT_INTERVAL.ago
|
||||
return false if import_stopped?
|
||||
return true if @data_import.updated_at > HEARTBEAT_INTERVAL.ago
|
||||
|
||||
@data_import.touch if @data_import.updated_at <= HEARTBEAT_INTERVAL.ago
|
||||
true
|
||||
@@ -412,6 +417,8 @@ class DataImports::Intercom::Importer
|
||||
|
||||
import_message(chatwoot_conversation, contact, entry)
|
||||
end
|
||||
return false if import_stopped?
|
||||
|
||||
true
|
||||
end
|
||||
|
||||
@@ -704,19 +711,21 @@ class DataImports::Intercom::Importer
|
||||
end
|
||||
|
||||
def skip_already_imported_item(item, mapping, already_handled:)
|
||||
item.update!(
|
||||
status: :skipped,
|
||||
chatwoot_record_type: mapping.chatwoot_record_type,
|
||||
chatwoot_record_id: mapping.chatwoot_record_id,
|
||||
last_error_code: ALREADY_IMPORTED_ERROR_CODE,
|
||||
last_error_message: 'Already imported in a previous import.'
|
||||
)
|
||||
record_already_imported_log(
|
||||
data_import_item: item,
|
||||
source_object_type: item.source_object_type,
|
||||
source_object_id: item.source_object_id,
|
||||
mapping: mapping
|
||||
)
|
||||
DataImportItem.transaction do
|
||||
item.update!(
|
||||
status: :skipped,
|
||||
chatwoot_record_type: mapping.chatwoot_record_type,
|
||||
chatwoot_record_id: mapping.chatwoot_record_id,
|
||||
last_error_code: ALREADY_IMPORTED_ERROR_CODE,
|
||||
last_error_message: 'Already imported in a previous import.'
|
||||
)
|
||||
record_already_imported_log(
|
||||
data_import_item: item,
|
||||
source_object_type: item.source_object_type,
|
||||
source_object_id: item.source_object_id,
|
||||
mapping: mapping
|
||||
)
|
||||
end
|
||||
increment_stat(stat_group_for(item.source_object_type), 'skipped') unless already_handled
|
||||
end
|
||||
|
||||
@@ -988,17 +997,19 @@ class DataImports::Intercom::Importer
|
||||
def reconcile_dirty_stats
|
||||
return if @dirty_stat_groups.empty?
|
||||
|
||||
@dirty_stat_groups.each_key do |group|
|
||||
case group
|
||||
when 'contacts'
|
||||
reconcile_item_stats('contact')
|
||||
when 'messages'
|
||||
reconcile_message_stats
|
||||
else
|
||||
raise ArgumentError, "Unsupported Intercom import stat group: #{group}"
|
||||
with_query_timeout_retry do
|
||||
@dirty_stat_groups.each_key do |group|
|
||||
case group
|
||||
when 'contacts'
|
||||
reconcile_item_stats('contact')
|
||||
when 'messages'
|
||||
reconcile_message_stats
|
||||
else
|
||||
raise ArgumentError, "Unsupported Intercom import stat group: #{group}"
|
||||
end
|
||||
end
|
||||
persist_stats
|
||||
end
|
||||
persist_stats
|
||||
@dirty_stat_groups.clear
|
||||
end
|
||||
|
||||
|
||||
@@ -212,6 +212,28 @@ RSpec.describe DataImports::Intercom::Importer do
|
||||
expect(data_import.reload.stats.dig('messages', 'imported')).to eq(3)
|
||||
end
|
||||
|
||||
it 'retries a query timeout during deferred message stats reconciliation', :aggregate_failures do
|
||||
described_class.new(data_import: data_import).import_conversations_page
|
||||
stats = data_import.reload.stats.deep_dup
|
||||
stats['messages']['imported'] = 0
|
||||
data_import.update!(stats: stats)
|
||||
importer = described_class.new(data_import: data_import)
|
||||
reconciliation_attempts = 0
|
||||
allow(importer).to receive(:sleep)
|
||||
allow(importer).to receive(:reconcile_message_stats).and_wrap_original do |method|
|
||||
reconciliation_attempts += 1
|
||||
raise ActiveRecord::QueryCanceled, 'statement timeout' if reconciliation_attempts == 1
|
||||
|
||||
method.call
|
||||
end
|
||||
|
||||
importer.import_conversations_page
|
||||
|
||||
expect(reconciliation_attempts).to eq(2)
|
||||
expect(importer).to have_received(:sleep).with(be_between(0.2, 0.5)).once
|
||||
expect(data_import.reload.stats.dig('messages', 'imported')).to eq(3)
|
||||
end
|
||||
|
||||
it 'indexes imported messages for advanced search' do
|
||||
allow(ChatwootApp).to receive(:advanced_search_allowed?).and_return(true)
|
||||
allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(false)
|
||||
@@ -328,8 +350,7 @@ RSpec.describe DataImports::Intercom::Importer do
|
||||
allow(data_import).to receive(:touch).and_call_original
|
||||
importer = described_class.new(data_import: data_import, run_id: run_id)
|
||||
allow(importer).to receive(:create_message) do
|
||||
data_import.update!(source_metadata: { DataImport::ACTIVE_INTERCOM_IMPORT_RUN_ID_KEY => 'new-run' })
|
||||
travel 1.minute
|
||||
DataImport.find(data_import.id).update!(source_metadata: { DataImport::ACTIVE_INTERCOM_IMPORT_RUN_ID_KEY => 'new-run' })
|
||||
end
|
||||
|
||||
importer.import_conversations_page
|
||||
@@ -340,6 +361,49 @@ RSpec.describe DataImports::Intercom::Importer do
|
||||
end
|
||||
end
|
||||
|
||||
it 'does not persist stale stats when a newer run takes over during the final part', :aggregate_failures do
|
||||
run_id = 'intercom-run-1'
|
||||
data_import.update!(source_metadata: { DataImport::ACTIVE_INTERCOM_IMPORT_RUN_ID_KEY => run_id })
|
||||
importer = described_class.new(data_import: data_import, run_id: run_id)
|
||||
allow(importer).to receive(:create_message).and_wrap_original do |method, *args|
|
||||
method.call(*args).tap do
|
||||
entry = args[2]
|
||||
next unless entry.part['id'] == 'part_2'
|
||||
|
||||
DataImport.find(data_import.id).update!(source_metadata: { DataImport::ACTIVE_INTERCOM_IMPORT_RUN_ID_KEY => 'new-run' })
|
||||
end
|
||||
end
|
||||
|
||||
importer.import_conversations_page
|
||||
|
||||
conversation = account.conversations.find_by!(identifier: 'intercom:conversation_1')
|
||||
expect(conversation.messages.count).to eq(3)
|
||||
expect(data_import.reload.stats.dig('conversations', 'imported')).to eq(0)
|
||||
end
|
||||
|
||||
it 'reconciles conversation stats when a superseded run is retried', :aggregate_failures do
|
||||
freeze_time do
|
||||
run_id = 'intercom-run-1'
|
||||
next_run_id = 'intercom-run-2'
|
||||
data_import.update!(source_metadata: { DataImport::ACTIVE_INTERCOM_IMPORT_RUN_ID_KEY => run_id })
|
||||
importer = described_class.new(data_import: data_import, run_id: run_id)
|
||||
allow(importer).to receive(:create_message) do
|
||||
data_import.update!(source_metadata: { DataImport::ACTIVE_INTERCOM_IMPORT_RUN_ID_KEY => next_run_id })
|
||||
travel 1.minute
|
||||
end
|
||||
|
||||
importer.import_conversations_page
|
||||
expect(data_import.reload.stats.dig('conversations', 'imported')).to eq(0)
|
||||
|
||||
retry_importer = described_class.new(data_import: data_import, run_id: next_run_id)
|
||||
retry_importer.import_conversations_page
|
||||
retry_importer.finish!
|
||||
|
||||
expect(data_import.reload.stats.dig('conversations', 'imported')).to eq(1)
|
||||
expect(data_import.processed_records).to eq(5)
|
||||
end
|
||||
end
|
||||
|
||||
it 'rolls back a newly inserted conversation when mapping persistence fails', :aggregate_failures do
|
||||
importer = described_class.new(data_import: data_import)
|
||||
allow(importer).to receive(:record_mapping).and_wrap_original do |method, object_type, source_id, record, metadata:|
|
||||
@@ -550,6 +614,55 @@ RSpec.describe DataImports::Intercom::Importer do
|
||||
expect(next_data_import.import_errors.skip_logs.pluck(:details).map { |details| details['reason'] }.uniq).to eq(['already_imported'])
|
||||
end
|
||||
|
||||
it 'reconciles skipped conversation stats when a superseded run is retried' do
|
||||
described_class.new(data_import: data_import).perform
|
||||
run_id = 'intercom-run-1'
|
||||
next_run_id = 'intercom-run-2'
|
||||
next_data_import.update!(source_metadata: { DataImport::ACTIVE_INTERCOM_IMPORT_RUN_ID_KEY => run_id })
|
||||
|
||||
freeze_time do
|
||||
importer = described_class.new(data_import: next_data_import, run_id: run_id)
|
||||
allow(importer).to receive(:skip_existing_message_mapping).and_wrap_original do |method, *args|
|
||||
method.call(*args)
|
||||
next_data_import.update!(source_metadata: { DataImport::ACTIVE_INTERCOM_IMPORT_RUN_ID_KEY => next_run_id })
|
||||
travel 1.minute
|
||||
end
|
||||
|
||||
importer.import_conversations_page
|
||||
expect(next_data_import.reload.stats.dig('conversations', 'skipped')).to eq(0)
|
||||
|
||||
retry_importer = described_class.new(data_import: next_data_import, run_id: next_run_id)
|
||||
retry_importer.import_conversations_page
|
||||
retry_importer.finish!
|
||||
|
||||
expect(next_data_import.reload.stats.dig('conversations', 'skipped')).to eq(1)
|
||||
end
|
||||
end
|
||||
|
||||
it 'keeps skipped contact stats when a query timeout is retried after the item update', :aggregate_failures do
|
||||
described_class.new(data_import: data_import).perform
|
||||
importer = described_class.new(data_import: next_data_import)
|
||||
contact_log_attempts = 0
|
||||
allow(importer).to receive(:sleep)
|
||||
allow(importer).to receive(:record_already_imported_log).and_wrap_original do |method, **attributes|
|
||||
if attributes[:source_object_type] == 'contact'
|
||||
contact_log_attempts += 1
|
||||
raise ActiveRecord::QueryCanceled, 'statement timeout' if contact_log_attempts == 1
|
||||
end
|
||||
|
||||
method.call(**attributes)
|
||||
end
|
||||
|
||||
importer.import_contacts_page
|
||||
|
||||
contact_item = next_data_import.items.find_by!(source_object_type: 'contact', source_object_id: 'contact_1')
|
||||
expect(contact_log_attempts).to eq(2)
|
||||
expect(importer).to have_received(:sleep).with(be_between(0.2, 0.5)).once
|
||||
expect(contact_item).to be_skipped
|
||||
expect(next_data_import.reload.stats.dig('contacts', 'skipped')).to eq(1)
|
||||
expect(next_data_import.import_errors.skip_logs.exists?(data_import_item: contact_item)).to be(true)
|
||||
end
|
||||
|
||||
it 'recreates messages when existing message mappings point to deleted records', :aggregate_failures do
|
||||
described_class.new(data_import: data_import).perform
|
||||
conversation = account.conversations.find_by!(identifier: 'intercom:conversation_1')
|
||||
|
||||
Reference in New Issue
Block a user