diff options
| author | Loïc Hoguin <lhoguin@vmware.com> | 2021-12-08 14:09:37 +0100 |
|---|---|---|
| committer | Loïc Hoguin <lhoguin@vmware.com> | 2021-12-08 15:53:47 +0100 |
| commit | 1b0eb9a4a3aad8fa72a207383afb98e0eeb7c019 (patch) | |
| tree | 830e13c79d70c739c338028009bd96e60fc3161c | |
| parent | b7cb8614f8deb3338e33462aea741813b22f094d (diff) | |
| download | rabbitmq-server-git-1b0eb9a4a3aad8fa72a207383afb98e0eeb7c019.tar.gz | |
Fix case where confirms may not be sent
A channel that first sends a mandatory publish before enabling
confirms mode may not receive confirms for messages published
after that. This is because the publish_seqno was increased
also for mandatory publishes even if confirms were disabled.
But the mandatory feature has nothing to do with publish_seqno.
The issue exists since at least
https://github.com/rabbitmq/rabbitmq-server/commit/38e5b687de76739b5419c1f0f6ddf0d8262ea16e
The test case introduced focuses for multiple=false. The issue
also exists for multiple=true but it has a different impact:
sending multiple=true,delivery_tag=2 results in both messages
1 and 2 being acked, even if message 2 doesn't exist as far
as the client is concerned. If the message does exist
it might get confirmed earlier than it should have been. The
issue is a bigger problem the more mandatory messages were
sent before enabling confirms mode.
| -rw-r--r-- | deps/rabbit/src/rabbit_channel.erl | 2 | ||||
| -rw-r--r-- | deps/rabbit/test/publisher_confirms_parallel_SUITE.erl | 12 |
2 files changed, 13 insertions, 1 deletions
diff --git a/deps/rabbit/src/rabbit_channel.erl b/deps/rabbit/src/rabbit_channel.erl index 7fee29f2c3..cd8ebe4446 100644 --- a/deps/rabbit/src/rabbit_channel.erl +++ b/deps/rabbit/src/rabbit_channel.erl @@ -1306,7 +1306,7 @@ handle_method(#'basic.publish'{exchange = ExchangeNameBin, check_expiration_header(Props), DoConfirm = Tx =/= none orelse ConfirmEnabled, {MsgSeqNo, State1} = - case DoConfirm orelse Mandatory of + case DoConfirm of false -> {undefined, State0}; true -> rabbit_global_counters:messages_received_confirm(amqp091, 1), SeqNo = State0#ch.publish_seqno, diff --git a/deps/rabbit/test/publisher_confirms_parallel_SUITE.erl b/deps/rabbit/test/publisher_confirms_parallel_SUITE.erl index 30bc5f8ba6..6d2e515da3 100644 --- a/deps/rabbit/test/publisher_confirms_parallel_SUITE.erl +++ b/deps/rabbit/test/publisher_confirms_parallel_SUITE.erl @@ -29,6 +29,7 @@ groups() -> confirm_nowait, confirm_ack, confirm_acks, + confirm_after_mandatory_bug, confirm_mandatory_unroutable, confirm_unroutable_message], [ @@ -187,6 +188,17 @@ confirm_acks(Config) -> publish(Ch, QName, [<<"msg1">>, <<"msg2">>, <<"msg3">>, <<"msg4">>]), receive_many(lists:seq(1, 4)). +confirm_after_mandatory_bug(Config) -> + {_Conn, Ch} = rabbit_ct_client_helpers:open_connection_and_channel(Config, 0), + QName = ?config(queue_name, Config), + declare_queue(Ch, Config, QName), + ok = amqp_channel:call(Ch, #'basic.publish'{routing_key = QName, + mandatory = true}, #amqp_msg{payload = <<"msg1">>}), + #'confirm.select_ok'{} = amqp_channel:call(Ch, #'confirm.select'{}), + publish(Ch, QName, [<<"msg2">>]), + true = amqp_channel:wait_for_confirms(Ch, 1), + ok. + %% For unroutable messages, the broker will issue a confirm once the exchange verifies a message %% won't route to any queue (returns an empty list of queues). %% If the message is also published as mandatory, the basic.return is sent to the client before |