Skip to content

Commit 6babf51

Browse files
authored
fix: Handle batch size too large in kinesis batch sender (#64)
* handle batch size too large errors * rubocop fixes * fix bug in metric for failed events * fix test data
1 parent 5de3c3a commit 6babf51

10 files changed

Lines changed: 126 additions & 15 deletions

File tree

Gemfile.lock

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
PATH
22
remote: .
33
specs:
4-
journaled (6.2.4)
4+
journaled (6.2.5)
55
activejob
66
activerecord
77
activesupport
@@ -231,7 +231,7 @@ GEM
231231
uncruft (0.5.0)
232232
railties (>= 6.1.0)
233233
unicode-display_width (2.6.0)
234-
uri (1.0.4)
234+
uri (1.1.1)
235235
useragent (0.16.11)
236236
webmock (3.24.0)
237237
addressable (>= 2.8.0)

app/models/journaled/outbox/event.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class Event < Journaled.outbox_base_class_name.constantize
1919
attribute :event_data, :json
2020

2121
validates :event_type, :event_data, :partition_key, :stream_name, presence: true
22+
validate :failed_at_and_failure_reason_must_be_consistent
2223

2324
scope :ready_to_process, -> {
2425
where(failed_at: nil)
@@ -63,6 +64,14 @@ def requeue!
6364
def self.oldest_non_failed_timestamp
6465
ready_to_process.order(:id).limit(1).pick(:created_at)
6566
end
67+
68+
private
69+
70+
def failed_at_and_failure_reason_must_be_consistent
71+
if failed_at.present? != failure_reason.present?
72+
errors.add(:base, 'failed_at and failure_reason must both be present or both be absent')
73+
end
74+
end
6675
end
6776
end
6877
end

gemfiles/rails_7_2.gemfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
PATH
22
remote: ..
33
specs:
4-
journaled (6.2.4)
4+
journaled (6.2.5)
55
activejob
66
activerecord
77
activesupport

gemfiles/rails_8_0.gemfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
PATH
22
remote: ..
33
specs:
4-
journaled (6.2.4)
4+
journaled (6.2.5)
55
activejob
66
activerecord
77
activesupport

lib/journaled/kinesis_batch_sender.rb

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ class KinesisBatchSender
1515
'ValidationException',
1616
].freeze
1717

18+
BATCH_TOO_LARGE_PATTERN = /too large/i
19+
1820
# Send a batch of database events to Kinesis
1921
#
2022
# Uses put_records batch API. Groups events by stream and sends each group as a batch.
@@ -40,11 +42,10 @@ def send_stream_batch(stream_name, stream_events)
4042
begin
4143
response = kinesis_client.put_records(stream_name:, records:)
4244
process_response(response, stream_events)
43-
rescue Aws::Kinesis::Errors::ValidationException
44-
# Re-raise batch-level validation errors (configuration issues)
45-
# These indicate invalid stream name, batch too large, etc.
46-
# Not event data problems - requires manual intervention
47-
raise
45+
rescue Aws::Kinesis::Errors::ValidationException => e
46+
raise unless e.message.match?(BATCH_TOO_LARGE_PATTERN)
47+
48+
handle_batch_too_large(stream_name, stream_events)
4849
rescue StandardError => e
4950
# Handle transient errors (throttling, network issues, service unavailable)
5051
handle_transient_batch_error(e, stream_events)
@@ -93,6 +94,36 @@ def create_failed_event(event, error_code:, error_message:, transient:)
9394
)
9495
end
9596

97+
def handle_batch_too_large(stream_name, stream_events)
98+
if stream_events.size <= 1
99+
# Single event exceeds payload limit — treat as permanent failure
100+
return {
101+
succeeded: [],
102+
failed: stream_events.map do |event|
103+
create_failed_event(
104+
event,
105+
error_code: 'ValidationException',
106+
error_message: 'Record exceeds Kinesis payload limit',
107+
transient: false,
108+
)
109+
end,
110+
}
111+
end
112+
113+
Rails.logger.warn(
114+
"[journaled] Batch too large for Kinesis (#{stream_events.size} events), splitting in half and retrying",
115+
)
116+
117+
mid = stream_events.size / 2
118+
first_half = send_stream_batch(stream_name, stream_events[...mid])
119+
second_half = send_stream_batch(stream_name, stream_events[mid...])
120+
121+
{
122+
succeeded: first_half[:succeeded] + second_half[:succeeded],
123+
failed: first_half[:failed] + second_half[:failed],
124+
}
125+
end
126+
96127
def handle_transient_batch_error(error, stream_events)
97128
Rails.logger.error("Kinesis batch send failed (transient): #{error.class} - #{error.message}")
98129

lib/journaled/outbox/metric_emitter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ def calculate_queue_metrics
7373
Event.select(
7474
'COUNT(*) AS total_count',
7575
'COUNT(*) FILTER (WHERE failed_at IS NULL) AS workable_count',
76-
'COUNT(*) FILTER (WHERE failure_reason IS NOT NULL AND failed_at IS NULL) AS failed_count',
76+
'COUNT(*) FILTER (WHERE failed_at IS NOT NULL) AS failed_count',
7777
'MIN(created_at) FILTER (WHERE failed_at IS NULL) AS oldest_non_failed_timestamp',
7878
).to_sql,
7979
)

lib/journaled/version.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# frozen_string_literal: true
22

33
module Journaled
4-
VERSION = "6.2.4"
4+
VERSION = "6.2.5"
55
end

spec/lib/journaled/kinesis_batch_sender_spec.rb

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,75 @@
125125
end
126126
end
127127

128+
context 'when batch exceeds total payload limit' do
129+
before do
130+
skip "Database event tests require PostgreSQL" unless ActiveRecord::Base.connection.adapter_name == 'PostgreSQL'
131+
end
132+
133+
let(:large_data) { { event_type: 'test', payload: 'x' * 3_000_000 } }
134+
let(:event_1) { create_database_event(stream_name: 'stream1', event_data: large_data) }
135+
let(:event_2) { create_database_event(stream_name: 'stream1', event_data: large_data) }
136+
let(:events) { [event_1, event_2] }
137+
138+
before do
139+
allow(Rails.logger).to receive(:warn)
140+
141+
call_count = 0
142+
allow(kinesis_client).to receive(:put_records) do |args|
143+
call_count += 1
144+
if call_count == 1
145+
raise Aws::Kinesis::Errors::ValidationException.new(nil, 'Request Payload is too large')
146+
end
147+
148+
mock_put_records_response(
149+
args[:records].map { { error_code: nil, error_message: nil } },
150+
)
151+
end
152+
end
153+
154+
it 'splits the batch and sends each half separately' do
155+
result = subject.send_batch(events)
156+
157+
expect(result[:succeeded]).to contain_exactly(event_1, event_2)
158+
expect(result[:failed]).to be_empty
159+
# 1 failed call + 2 successful half-batch calls
160+
expect(kinesis_client).to have_received(:put_records).exactly(3).times
161+
end
162+
163+
it 'logs a warning about the split' do
164+
subject.send_batch(events)
165+
166+
expect(Rails.logger).to have_received(:warn).with(/Batch too large.*splitting in half/)
167+
end
168+
end
169+
170+
context 'when a single event exceeds the payload limit' do
171+
before do
172+
skip "Database event tests require PostgreSQL" unless ActiveRecord::Base.connection.adapter_name == 'PostgreSQL'
173+
end
174+
175+
let(:event) { create_database_event(stream_name: 'stream1') }
176+
let(:events) { [event] }
177+
178+
before do
179+
allow(kinesis_client).to receive(:put_records)
180+
.and_raise(Aws::Kinesis::Errors::ValidationException.new(nil, 'Request Payload is too large'))
181+
end
182+
183+
it 'marks the event as a permanent failure' do
184+
result = subject.send_batch(events)
185+
186+
expect(result[:succeeded]).to be_empty
187+
expect(result[:failed].length).to eq(1)
188+
189+
failure = result[:failed].first
190+
expect(failure.event).to eq(event)
191+
expect(failure.error_code).to eq('ValidationException')
192+
expect(failure.error_message).to eq('Record exceeds Kinesis payload limit')
193+
expect(failure.permanent?).to be true
194+
end
195+
end
196+
128197
context 'with permanent error' do
129198
before do
130199
skip "Database event tests require PostgreSQL" unless ActiveRecord::Base.connection.adapter_name == 'PostgreSQL'

spec/lib/journaled/outbox/worker_spec.rb

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,7 @@
341341
partition_key: 'key2',
342342
stream_name: 'test_stream',
343343
failure_reason: 'Some error',
344+
failed_at: Time.current,
344345
)
345346

346347
# Start worker, then travel forward in time
@@ -405,6 +406,7 @@
405406
event_data: { test: 'data' },
406407
partition_key: 'key2',
407408
stream_name: 'test_stream',
409+
failure_reason: 'Some error',
408410
failed_at: Time.current,
409411
)
410412

@@ -433,21 +435,20 @@
433435
expect(emitted['journaled.worker.queue_workable_count'][:value]).to eq(1) # Only non-failed events
434436
end
435437

436-
it 'counts erroring events correctly' do
438+
it 'counts failed events correctly' do
437439
Journaled::Outbox::Event.create!(
438440
event_type: 'test_event',
439441
event_data: { test: 'data' },
440442
partition_key: 'key1',
441443
stream_name: 'test_stream',
442-
failure_reason: 'Error but not failed',
443444
)
444445

445446
Journaled::Outbox::Event.create!(
446447
event_type: 'test_event',
447448
event_data: { test: 'data' },
448449
partition_key: 'key2',
449450
stream_name: 'test_stream',
450-
failure_reason: 'Error and failed',
451+
failure_reason: 'Permanent failure',
451452
failed_at: Time.current,
452453
)
453454

@@ -472,7 +473,7 @@
472473
sleep 0.1 until emitted.key?('journaled.worker.queue_failed_count') || Time.current > timeout
473474
end
474475

475-
expect(emitted['journaled.worker.queue_failed_count'][:value]).to eq(1) # Only events with error but not failed
476+
expect(emitted['journaled.worker.queue_failed_count'][:value]).to eq(1)
476477
end
477478
end
478479
end

spec/models/journaled/outbox/event_spec.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
partition_key: 'key1',
6767
stream_name: 'test_stream',
6868
failed_at: Time.current,
69+
failure_reason: 'Some error',
6970
)
7071

7172
# Create a non-failed event

0 commit comments

Comments
 (0)