Skip to content

Commit 9b8663f

Browse files
committed
feat: add max_retry to drop a batch after too many failed retries
Add a new producer option `max_retry` (default `infinity`, i.e. retry forever). When set to a non-negative integer, a batch that keeps receiving Kafka error responses (e.g. `not_leader_for_partition`) is dropped once its attempt counter reaches `max_retry + 1` — the initial send plus `max_retry` retries have all failed. `max_retry = 0` drops on the first error. The check is applied in the Kafka-error path of `do_handle_kafka_ack/3`: when the front in-flight request has been attempted enough times, it is popped and its callers are acked with reason `max_retry_exceeded` (via `clear_sent_and_ack_callers/3`) and the `dropped` counter is bumped, instead of incrementing the attempt counter and retrying. The producer still reconnects afterwards, since the error means the current connection is stale for the remaining in-flight requests. This bounds retries on Kafka error responses only; resends triggered purely by connection loss are bounded by `max_batch_age`, not `max_retry`.
1 parent 48af322 commit 9b8663f

4 files changed

Lines changed: 114 additions & 10 deletions

File tree

changelog.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
* 4.3.0
2+
- Add `max_retry` producer option (default `infinity`, i.e. retry forever).
3+
When set to a non-negative integer, a batch that keeps getting Kafka error
4+
responses (e.g. `not_leader_for_partition`) is dropped once it has been
5+
attempted `max_retry + 1` times (the initial send plus `max_retry` retries).
6+
`max_retry = 0` drops on the first error. Each dropped message has its ack
7+
callback evaluated with reason `max_retry_exceeded` and bumps the `dropped`
8+
counter. This bounds retries on Kafka error responses; resends triggered
9+
purely by connection loss remain bounded by `max_batch_age`, not `max_retry`.
10+
111
* 4.2.0
212
- Add `max_batch_age` producer option (default `infinity`, i.e. disabled).
313
When set to a number of milliseconds, a batch is dropped instead of being

include/wolff.hrl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
-define(buffer_overflow_discarded, buffer_overflow_discarded).
88
-define(partition_lost, partition_lost).
99
-define(message_expired, message_expired).
10+
-define(max_retry_exceeded, max_retry_exceeded).
1011

1112
%% Kafka has default batch size limit 1000012.
1213
%% Since Kafka 2.4, it has been extended to 1048588.

src/wolff_producer.erl

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
compression |
5151
drop_if_highmem |
5252
max_batch_age |
53+
max_retry |
5354
telemetry_meta_data |
5455
max_partitions.
5556

@@ -67,6 +68,7 @@
6768
compression => kpro:compress_option(),
6869
drop_if_highmem => boolean(),
6970
max_batch_age => timeout(),
71+
max_retry => infinity | non_neg_integer(),
7072
telemetry_meta_data => map(),
7173
max_partitions => pos_integer()
7274
}.
@@ -84,6 +86,7 @@
8486
compression => kpro:compress_option(),
8587
drop_if_highmem => boolean(),
8688
max_batch_age => timeout(),
89+
max_retry => infinity | non_neg_integer(),
8790
telemetry_meta_data => map(),
8891
max_partitions => pos_integer()
8992
}.
@@ -170,6 +173,15 @@
170173
%% on reconnect). Each dropped message has its ack callback evaluated with
171174
%% reason `message_expired' and bumps the `dropped' and `dropped_expired'
172175
%% counters. Set to `infinity' to disable (never drop by age).
176+
%% * `max_retry': default `infinity'. Maximum number of times a batch is retried
177+
%% after a Kafka error response (e.g. `not_leader_for_partition') before it is
178+
%% dropped. A batch is dropped once its attempt counter reaches
179+
%% `max_retry + 1' (i.e. the initial send plus `max_retry' retries have all
180+
%% failed). Each dropped message has its ack callback evaluated with reason
181+
%% `max_retry_exceeded' and bumps the `dropped' counter. `max_retry = 0' drops
182+
%% on the first error; `infinity' (the default) retries forever. Note this
183+
%% counts Kafka error responses only; resends triggered purely by connection
184+
%% loss are bounded by `max_batch_age', not by `max_retry'.
173185
-spec start_link(wolff:client_id(), topic(), partition(), pid() | ?conn_down(any()), config_in()) ->
174186
{ok, pid()} | {error, any()}.
175187
start_link(ClientId, Topic, Partition, MaybeConnPid, Config) ->
@@ -437,6 +449,7 @@ use_defaults(Config) ->
437449
{max_send_ahead, 0},
438450
{compression, no_compression},
439451
{max_batch_age, infinity},
452+
{max_retry, infinity},
440453
{reconnect_delay_ms, 2000}
441454
]).
442455

@@ -787,20 +800,45 @@ do_handle_kafka_ack(EC, _BaseOffset, St) when EC =:= ?message_too_large orelse E
787800
end;
788801
do_handle_kafka_ack(ErrorCode, _BaseOffset, St) ->
789802
%% Other errors, such as not_leader_for_partition
790-
#{sent_reqs := SentReqs} = St,
791-
{value, #{attempts := Attempts, q_items := Items}} = queue:peek(SentReqs),
792-
#{topic := Topic,
803+
#{sent_reqs := SentReqs,
804+
topic := Topic,
793805
partition := Partition,
794806
config := Config
795807
} = St,
808+
{value, #{attempts := Attempts, q_items := Items}} = queue:peek(SentReqs),
796809
NrOfCalls = count_calls(Items),
797-
inc_sent_failed(Config, NrOfCalls, Attempts),
798-
log_warn(Topic, Partition, "error_in_produce_response",
799-
#{error_code => ErrorCode,
800-
batch_size => count_msgs(Items),
801-
attempts => Attempts}),
802-
St1 = increment_attempt(St),
803-
erlang:throw({kafka_error, ErrorCode, St1}).
810+
MaxRetry = maps:get(max_retry, Config, infinity),
811+
case is_max_retry_reached(Attempts, MaxRetry) of
812+
true ->
813+
%% Give up: drop the front batch instead of retrying it again.
814+
log_error(Topic, Partition, "dropped_produce_request_reached_max_retry",
815+
#{error_code => ErrorCode,
816+
batch_size => count_msgs(Items),
817+
attempts => Attempts,
818+
max_retry => MaxRetry}),
819+
wolff_metrics:dropped_inc(Config, NrOfCalls),
820+
%% clear_sent_and_ack_callers/3 pops the front request, bumps the
821+
%% failed/retried_failed counter, and acks the callers with the reason.
822+
St1 = clear_sent_and_ack_callers(ErrorCode, ?max_retry_exceeded, St),
823+
%% Still reconnect: the error (e.g. not_leader_for_partition) means the
824+
%% current connection is stale for the remaining in-flight requests.
825+
erlang:throw({kafka_error, ErrorCode, St1});
826+
false ->
827+
inc_sent_failed(Config, NrOfCalls, Attempts),
828+
log_warn(Topic, Partition, "error_in_produce_response",
829+
#{error_code => ErrorCode,
830+
batch_size => count_msgs(Items),
831+
attempts => Attempts}),
832+
St1 = increment_attempt(St),
833+
erlang:throw({kafka_error, ErrorCode, St1})
834+
end.
835+
836+
%% A batch is dropped once it has been attempted `max_retry + 1' times, i.e. the
837+
%% initial send plus `max_retry' retries have all failed.
838+
is_max_retry_reached(_Attempts, infinity) ->
839+
false;
840+
is_max_retry_reached(Attempts, MaxRetry) ->
841+
Attempts >= MaxRetry + 1.
804842

805843
%% Since we only handle Kafka ack for the first sent request in the sent queue
806844
%% we only bump the first itme with attempt counter
@@ -1329,6 +1367,15 @@ is_batch_expired_test_() ->
13291367
?_assert(is_batch_expired(Sent([Item(50)]), 50, Now))}
13301368
].
13311369

1370+
is_max_retry_reached_test_() ->
1371+
[ {"infinity never reached", ?_assertNot(is_max_retry_reached(1000000, infinity))}
1372+
%% max_retry = 0 -> drop on the first attempt's failure
1373+
, {"0: first attempt reached", ?_assert(is_max_retry_reached(1, 0))}
1374+
%% max_retry = 3 -> allow attempts 1..4, drop when attempts reaches 4
1375+
, {"3: attempt 3 not reached", ?_assertNot(is_max_retry_reached(3, 3))}
1376+
, {"3: attempt 4 reached", ?_assert(is_max_retry_reached(4, 3))}
1377+
].
1378+
13321379
maybe_log_discard_test_() ->
13331380
[ {"no-increment", fun() -> maybe_log_discard(undefined, 0, ?DISCARD_OVERFLOW) end}
13341381
, {"fake-last-old",

test/wolff_tests.erl

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,52 @@ drop_expired_queued_batch() ->
414414
ets:delete(CntrEventsTable),
415415
deinstall_event_logging(?FUNCTION_NAME).
416416

417+
%% With max_retry = 0 a batch is dropped on the first Kafka error response,
418+
%% instead of being retried, and the caller is acked with `max_retry_exceeded'.
419+
drop_batch_on_max_retry_test_() ->
420+
{timeout, 30, fun drop_batch_on_max_retry/0}.
421+
422+
drop_batch_on_max_retry() ->
423+
CntrEventsTable = ets:new(cntr_events, [public]),
424+
install_event_logging(?FUNCTION_NAME, CntrEventsTable, false),
425+
ClientCfg = client_config(),
426+
{ok, Client} = start_client(<<"client-1">>, ?HOSTS, ClientCfg),
427+
%% mem-only replayq; max_retry => 0 drops on the first error
428+
ProducerCfg = #{max_retry => 0, max_send_ahead => 0, reconnect_delay_ms => 0},
429+
{ok, Producers} = wolff:start_producers(Client, <<"test-topic">>, ProducerCfg),
430+
Producer = wolff:get_producer(Producers, 0),
431+
TesterPid = self(),
432+
%% Swallow the produce request so we can inject a fake error response below.
433+
ok = meck:new(kpro, [no_history, no_link, passthrough]),
434+
meck:expect(kpro, send, fun(_Conn, _Req) -> TesterPid ! sent_to_kafka, ok end),
435+
Ref = make_ref(),
436+
AckFun = fun(_Partition, Result) -> TesterPid ! {ack, Ref, Result}, ok end,
437+
Msg = #{key => ?KEY, value => <<"value">>},
438+
try
439+
_ = wolff:send(Producers, [Msg], AckFun),
440+
receive sent_to_kafka -> ok after 5000 -> error(timeout_waiting_send) end,
441+
%% grab the connection and the in-flight request ref from the producer state
442+
#{conn := Conn, sent_reqs := SentReqs} = sys:get_state(Producer),
443+
{value, #{req_ref := ReqRef}} = queue:peek(SentReqs),
444+
%% inject a Kafka error response for that request
445+
ErrBody = #{responses =>
446+
[#{partition_responses =>
447+
[#{error_code => not_leader_for_partition,
448+
base_offset => -1}]}]},
449+
Producer ! {msg, Conn, #kpro_rsp{api = produce, ref = ReqRef, msg = ErrBody}},
450+
receive
451+
{ack, Ref, Result} -> ?assertEqual(max_retry_exceeded, Result)
452+
after 5000 -> error(timeout_waiting_ack)
453+
end
454+
after
455+
meck:unload(kpro),
456+
ok = wolff:stop_producers(Producers),
457+
ok = stop_client(Client)
458+
end,
459+
?assertEqual([1], get_telemetry_seq(CntrEventsTable, [wolff, dropped])),
460+
ets:delete(CntrEventsTable),
461+
deinstall_event_logging(?FUNCTION_NAME).
462+
417463
zero_ack_test() ->
418464
CntrEventsTable = ets:new(cntr_events, [public]),
419465
install_event_logging(?FUNCTION_NAME, CntrEventsTable, false),

0 commit comments

Comments
 (0)