Skip to content

Commit 3a18cbe

Browse files
Argonusclaude
andauthored
fix(produce): honour the acks option and make acks=0 fire-and-forget work (#593)
Since the 1.0 rewrite every produce went out at acks: -1: the option was read under a name the request builder never used. Resolve :acks / :required_acks once to the wire (:acks canonical, :required_acks a deprecated alias until 2.0), reject values the int16 encoder cannot carry with {:error, :invalid_acks}, and accept :all / :any as -1. acks: 0 now takes a dedicated fire-and-forget path (sent async, never retried, returns base_offset: nil). A failed async send closes the socket, and the client gains {:tcp_error} / {:ssl_error} clauses plus a catch-all handle_info/2 so an unmatched message no longer crashes it together with every broker socket. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3f1c5a6 commit 3a18cbe

9 files changed

Lines changed: 400 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,24 @@
11
# KafkaEx Changelog
22

3+
## 1.1.2 (unreleased)
4+
5+
### Changed (Breaking)
6+
7+
* **`acks` / `required_acks` now reaches the broker.** Since 1.0 every produce ran at `acks: -1`
8+
regardless of the option. `required_acks: 1` now means leader-only and `required_acks: 0` means
9+
fire-and-forget (records can be lost, `base_offset: nil`). Pass `acks: -1` to keep the old
10+
behaviour. `:acks` is canonical; `:required_acks` is a deprecated alias (removed in 2.0).
11+
12+
### Fixed
13+
14+
* **The `acks` option is honoured again (regression since 1.0).** Resolved once to the wire;
15+
invalid values are rejected with `{:error, :invalid_acks}` instead of crashing Kayrock's int16
16+
encoder; `:all` / `:any` map to `-1`.
17+
* **`acks: 0` fire-and-forget works.** Sent asynchronously, never retried, returns
18+
`base_offset: nil`. A failed async send closes the socket, and the client now handles
19+
`{:tcp_error}` / `{:ssl_error}` plus a catch-all `handle_info/2` instead of crashing on an
20+
unmatched message.
21+
322
## 1.1.1 (2026-07-24)
423

524
### Fixed

lib/kafka_ex/api.ex

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -810,8 +810,17 @@ defmodule KafkaEx.API do
810810
* `messages` - List of message maps with `:value`, optional `:key`, `:timestamp`, `:headers`
811811
* `opts` - Options including:
812812
* `:api_version` - API version to use. If omitted, resolved via `:api_versions` app-config or broker-negotiated max (`min(broker_max, kayrock_max)`). See CHANGELOG § 3-tier API version resolution.
813-
* `:required_acks` - Number of acks required (default: 1)
814-
* `:timeout` - Request timeout
813+
* `:acks` - Acknowledgements required: `-1` (default) all in-sync replicas, `1` leader only,
814+
`0` fire-and-forget. `:all` / `:any` (the Java/librdkafka spelling) are accepted as `-1`.
815+
Any other value is rejected with `{:error, :invalid_acks}`.
816+
With `0` the broker sends no response at all, so `{:ok, _}` means only that the batch was
817+
handed to the socket, the call is never retried, the returned `RecordMetadata` has
818+
`base_offset: nil`, and a broker-side rejection (stale leader, oversized batch) arrives as
819+
a closed connection — records produced before the client notices are lost.
820+
* `:required_acks` - Deprecated alias for `:acks`, honoured only when `:acks` is absent.
821+
Will be removed in 2.0.
822+
* `:timeout` - How long the broker waits for the required acknowledgements, in ms
823+
(default 5000). Only has an effect with `acks: -1`.
815824
* `:partitioner` - Custom partitioner module (default: configured or `KafkaEx.Producer.Partitioner.Default`)
816825
817826
## Partitioning

lib/kafka_ex/client/client.ex

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ defmodule KafkaEx.Client do
2727
alias KafkaEx.Cluster.ClusterMetadata
2828
alias KafkaEx.Messages.Fetch
2929
alias KafkaEx.Messages.FindCoordinator, as: FindCoordinatorMsg
30+
alias KafkaEx.Messages.RecordMetadata
3031
alias KafkaEx.Support.OptionalDeps
3132
alias KafkaEx.Support.Retry
3233

@@ -395,6 +396,21 @@ defmodule KafkaEx.Client do
395396
{:noreply, state_out}
396397
end
397398

399+
def handle_info({:tcp_error, socket, reason}, state) do
400+
{:noreply, close_broker_by_socket(state, socket, :recv_error, reason)}
401+
end
402+
403+
def handle_info({:ssl_error, socket, reason}, state) do
404+
{:noreply, close_broker_by_socket(state, socket, :recv_error, reason)}
405+
end
406+
407+
# Defining any handle_info/2 replaces the catch-all use GenServer injects; without one an
408+
# unmatched message would crash the client.
409+
def handle_info(message, state) do
410+
Logger.warning("#{inspect(__MODULE__)} ignoring unexpected message: #{inspect(message)}")
411+
{:noreply, state}
412+
end
413+
398414
defp update_metadata_with_retry(_state, retries_left) when retries_left <= 0 do
399415
raise KafkaEx.MetadataUpdateError, attempts: @max_metadata_update_retries
400416
end
@@ -629,16 +645,14 @@ defmodule KafkaEx.Client do
629645
end
630646

631647
defp produce_request(topic, partition, messages, opts, state) do
632-
message_count = length(messages)
633-
required_acks = Keyword.get(opts, :required_acks, 1)
634-
client_id = Config.client_id()
635-
636-
metadata = Telemetry.produce_metadata(topic, partition, client_id, required_acks)
637-
start_measurements = %{message_count: message_count}
648+
with {:ok, acks} <- resolve_acks(opts) do
649+
metadata = Telemetry.produce_metadata(topic, partition, Config.client_id(), acks)
650+
start_measurements = %{message_count: length(messages)}
638651

639-
Telemetry.span([:kafka_ex, :produce], Map.merge(metadata, start_measurements), fn ->
640-
do_produce_request(topic, partition, messages, opts, state, metadata)
641-
end)
652+
Telemetry.span([:kafka_ex, :produce], Map.merge(metadata, start_measurements), fn ->
653+
do_produce_request(topic, partition, messages, Keyword.put(opts, :acks, acks), state, metadata)
654+
end)
655+
end
642656
end
643657

644658
defp do_produce_request(topic, partition, messages, opts, state, metadata) do
@@ -655,6 +669,16 @@ defmodule KafkaEx.Client do
655669
end
656670
end
657671

672+
# Reject a bad acks value here; unvalidated it reaches Kayrock's int16 encoder and crashes the client.
673+
defp resolve_acks(opts) do
674+
case Keyword.get(opts, :acks, Keyword.get(opts, :required_acks, -1)) do
675+
acks when acks in [-1, 0, 1] -> {:ok, acks}
676+
# Java/librdkafka spelling for "all in-sync replicas"; accept it as the -1 the wire carries.
677+
acks when acks in [:all, :any] -> {:ok, -1}
678+
_ -> {:error, :invalid_acks}
679+
end
680+
end
681+
658682
defp add_offset_to_metadata(metadata, result) do
659683
case Map.get(result, :base_offset) do
660684
nil -> metadata
@@ -847,6 +871,20 @@ defmodule KafkaEx.Client do
847871
|> handle_request_with_retry(state, @coordinator_max_attempts)
848872
end
849873

874+
# acks=0: the broker sends no response, and a resend would duplicate records.
875+
defp handle_produce_request(%{acks: 0} = request, node_selector, state) do
876+
%NodeSelector{topic: topic, partition: partition} = node_selector
877+
878+
case network_request(request, node_selector, state) do
879+
{{:ok, :no_response}, updated_state} ->
880+
{{:ok, RecordMetadata.build(topic: topic, partition: partition, base_offset: nil)}, updated_state}
881+
882+
{{:error, reason}, updated_state} ->
883+
Logger.warning("Fire-and-forget produce to #{topic}/#{partition} failed with #{inspect(reason)}")
884+
{{:error, build_transport_error(reason)}, updated_state}
885+
end
886+
end
887+
850888
defp handle_produce_request(request, node_selector, state) do
851889
# Produce requests should only retry on leadership errors to avoid duplicates.
852890
# Timeout errors are NOT safe to retry because the message may have been written
@@ -1394,6 +1432,9 @@ defmodule KafkaEx.Client do
13941432
{{:error, reason}, broker} ->
13951433
{{:error, reason}, 0, broker_to_telemetry_info(broker)}
13961434

1435+
{:ok, broker} ->
1436+
{{:ok, :no_response}, 0, broker_to_telemetry_info(broker)}
1437+
13971438
{data, broker} when synchronous ->
13981439
{deserialize(data, client_request), byte_size(data), broker_to_telemetry_info(broker)}
13991440

@@ -1507,10 +1548,10 @@ defmodule KafkaEx.Client do
15071548
{topic_metadata, %{updated_state | allow_auto_topic_creation: allow_auto_topic_creation}}
15081549
end
15091550

1510-
defp close_broker_by_socket(state, socket, reason \\ :remote_closed) do
1551+
defp close_broker_by_socket(state, socket, reason \\ :remote_closed, detail \\ nil) do
15111552
State.update_brokers(state, fn broker ->
15121553
if Broker.has_socket?(broker, socket) do
1513-
Logger.debug("#{Broker.to_string(broker)} closed connection")
1554+
log_connection_close(broker, detail)
15141555
# Socket is already closed (received :tcp_closed/:ssl_closed), just emit telemetry
15151556
NetworkClient.close_socket(broker, socket, reason)
15161557
Broker.put_socket(broker, nil)
@@ -1519,4 +1560,9 @@ defmodule KafkaEx.Client do
15191560
end
15201561
end)
15211562
end
1563+
1564+
defp log_connection_close(broker, nil), do: Logger.debug("#{Broker.to_string(broker)} closed connection")
1565+
1566+
defp log_connection_close(broker, detail),
1567+
do: Logger.warning("#{Broker.to_string(broker)} closed connection: #{inspect(detail)}")
15221568
end

lib/kafka_ex/messages/record_metadata.ex

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ defmodule KafkaEx.Messages.RecordMetadata do
1111
1212
* `:topic` - The topic the messages were produced to
1313
* `:partition` - The partition the messages were produced to
14-
* `:base_offset` - The offset assigned to the first message in the batch
14+
* `:base_offset` - The offset assigned to the first message in the batch; `nil` when
15+
produced with `acks: 0`, where the broker sends no response
1516
* `:log_append_time` - The timestamp assigned by the broker (v2+, -1 if not available)
1617
* `:log_start_offset` - The start offset of the log (v5+)
1718
* `:throttle_time_ms` - Time in ms the request was throttled (v3+)
@@ -29,7 +30,7 @@ defmodule KafkaEx.Messages.RecordMetadata do
2930
@type t :: %__MODULE__{
3031
topic: String.t(),
3132
partition: non_neg_integer(),
32-
base_offset: non_neg_integer(),
33+
base_offset: non_neg_integer() | nil,
3334
log_append_time: integer() | nil,
3435
log_start_offset: non_neg_integer() | nil,
3536
throttle_time_ms: non_neg_integer() | nil
@@ -42,7 +43,7 @@ defmodule KafkaEx.Messages.RecordMetadata do
4243
4344
* `:topic` - (required) The topic name
4445
* `:partition` - (required) The partition number
45-
* `:base_offset` - (required) The base offset assigned to the batch
46+
* `:base_offset` - (required) The base offset assigned to the batch, `nil` for `acks: 0`
4647
* `:log_append_time` - The broker-assigned timestamp (-1 if not using LogAppendTime)
4748
* `:log_start_offset` - The log start offset (v5+)
4849
* `:throttle_time_ms` - Request throttle time in milliseconds (v3+)
@@ -64,7 +65,7 @@ defmodule KafkaEx.Messages.RecordMetadata do
6465
6566
This is an alias for `base_offset` to match Java API.
6667
"""
67-
@spec offset(t()) :: non_neg_integer()
68+
@spec offset(t()) :: non_neg_integer() | nil
6869
def offset(%__MODULE__{base_offset: offset}), do: offset
6970

7071
@doc """

lib/kafka_ex/network/network_client.ex

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ defmodule KafkaEx.Network.NetworkClient do
102102
{_, reason} ->
103103
broker_str = inspect_broker(broker.host, broker.port)
104104
Logger.error("Asynchronously sending data to broker #{broker_str} failed with #{inspect(reason)}")
105-
reason
105+
# A self-close is silent (no {:tcp_closed} to the owner), so emit the close event here.
106+
close_socket(broker, socket, :send_error)
107+
{:error, reason}
106108
end
107109
end
108110

test/integration/produce/reliability_test.exs

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,56 @@ defmodule KafkaEx.Integration.Produce.ReliabilityTest do
1717
end
1818

1919
describe "produce with different acks settings" do
20-
test "produce with acks=0 (fire and forget) succeeds", %{client: client} do
20+
test "produce with acks=0 (fire and forget) returns no offset", %{client: client} do
2121
topic_name = generate_random_string()
2222
_ = create_topic(client, topic_name)
2323

24+
# A fire-and-forget batch is dropped without a trace while the partition leader is still
25+
# being elected, so establish the leader with an acknowledged produce first.
26+
{:ok, %RecordMetadata{base_offset: base_offset}} =
27+
API.produce(client, topic_name, 0, [%{value: "leader-warmup"}], acks: -1)
28+
2429
messages = [%{value: "acks-0-message"}]
2530

26-
{:ok, result} = API.produce(client, topic_name, 0, messages, required_acks: 0)
31+
{:ok, result} = API.produce(client, topic_name, 0, messages, acks: 0)
32+
{:ok, _} = API.produce(client, topic_name, 0, messages, acks: 0)
2733

2834
assert %RecordMetadata{} = result
2935
assert result.topic == topic_name
3036
assert result.partition == 0
37+
assert result.base_offset == nil
38+
39+
# Two async sends followed by a synchronous request on the same socket: proves the
40+
# response-less produce leaves no bytes behind to desynchronise the next request.
41+
wait_for(
42+
fn ->
43+
{:ok, latest} = API.latest_offset(client, topic_name, 0)
44+
latest >= base_offset + 3
45+
end,
46+
200,
47+
25
48+
)
49+
end
3150

32-
# Wait for message to arrive and verify via fetch
33-
Process.sleep(500)
34-
{:ok, latest} = API.latest_offset(client, topic_name, 0)
35-
assert latest >= 1
51+
test "produce with the deprecated required_acks: 0 alias still fires and forgets", %{client: client} do
52+
topic_name = generate_random_string()
53+
_ = create_topic(client, topic_name)
54+
55+
{:ok, %RecordMetadata{base_offset: base_offset}} =
56+
API.produce(client, topic_name, 0, [%{value: "leader-warmup"}], acks: -1)
57+
58+
{:ok, result} = API.produce(client, topic_name, 0, [%{value: "alias-message"}], required_acks: 0)
59+
60+
assert %RecordMetadata{base_offset: nil} = result
61+
62+
wait_for(
63+
fn ->
64+
{:ok, latest} = API.latest_offset(client, topic_name, 0)
65+
latest >= base_offset + 2
66+
end,
67+
200,
68+
25
69+
)
3670
end
3771

3872
test "produce with acks=1 (leader only) succeeds", %{client: client} do

0 commit comments

Comments
 (0)