Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dead lettering crash (backport #12935) #12938

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Dec 13, 2024

Fixes #12933

The assumption that x-last-death-* annotations must have been set whenever the deaths annotation is set was wrong.

Reproducation steps, Option 1:

  1. In v3.13.7, dead letter a message from Q1 to Q2 (both can be classic queues).
  2. Re-publish the message including its x-death header from Q2 back to Q1. (RabbitMQ 3.13.7 will interpret this x-death header and set the deaths annotation.)
  3. Upgrade to v4.0.4
  4. Dead letter the message from Q1 to Q2 will cause the following crash:
crasher:
  initial call: rabbit_amqqueue_process:init/1
  pid: <0.577.0>
  registered_name: []
  exception exit: {{badkey,<<"x-last-death-exchange">>},
                   [{mc,record_death,4,[{file,"mc.erl"},{line,410}]},
                    {rabbit_dead_letter,publish,5,
                        [{file,"rabbit_dead_letter.erl"},{line,38}]},
                    {rabbit_amqqueue_process,'-dead_letter_msgs/4-fun-0-',
                        7,
                        [{file,"rabbit_amqqueue_process.erl"},{line,1060}]},
                    {rabbit_variable_queue,'-ackfold/4-fun-0-',3,
                        [{file,"rabbit_variable_queue.erl"},{line,655}]},
                    {lists,foldl,3,[{file,"lists.erl"},{line,2146}]},
                    {rabbit_variable_queue,ackfold,4,
                        [{file,"rabbit_variable_queue.erl"},{line,652}]},
                    {rabbit_priority_queue,ackfold,4,
                        [{file,"rabbit_priority_queue.erl"},{line,309}]},
                    {rabbit_amqqueue_process,
                        '-dead_letter_rejected_msgs/3-fun-0-',5,
                        [{file,"rabbit_amqqueue_process.erl"},
                         {line,1038}]}]}

Reproduction steps, Option 2:

  1. Run a 4.0.4 / 3.13.7 mixed version cluster where both queues Q1 and Q2 are hosted on the 4.0.4 node.
  2. Send a message to Q1 which dead letters to Q2.
  3. Re-publish a message with the x-death AMQP 0.9.1 header from Q2 to Q1. However, this time make sure to publish to the 3.13.7 node which forwards this message to Q1 on the 4.0.4 node.
  4. Subsequently dead lettering this message from Q1 to Q2 (happening on the 4.0.4 node) will also cause the crash.

The modified test case in this commit was able to repro this crash via Option 2 in the mixed version cluster tests on the v4.0.x branch.


This is an automatic backport of pull request #12935 done by Mergify.

Fixes #12933

The assumption that `x-last-death-*` annotations must have been set
whenever the `deaths` annotation is set was wrong.

Reproducation steps, Option 1:
1. In v3.13.7, dead letter a message from Q1 to Q2 (both can be classic queues).
2. Re-publish the message including its x-death header from Q2 back to Q1.
(RabbitMQ 3.13.7 will interpret this x-death header and set the deaths annotation.)
3. Upgrade to v4.0.4
4. Dead letter the message from Q1 to Q2 will cause the following crash:
```
crasher:
  initial call: rabbit_amqqueue_process:init/1
  pid: <0.577.0>
  registered_name: []
  exception exit: {{badkey,<<"x-last-death-exchange">>},
                   [{mc,record_death,4,[{file,"mc.erl"},{line,410}]},
                    {rabbit_dead_letter,publish,5,
                        [{file,"rabbit_dead_letter.erl"},{line,38}]},
                    {rabbit_amqqueue_process,'-dead_letter_msgs/4-fun-0-',
                        7,
                        [{file,"rabbit_amqqueue_process.erl"},{line,1060}]},
                    {rabbit_variable_queue,'-ackfold/4-fun-0-',3,
                        [{file,"rabbit_variable_queue.erl"},{line,655}]},
                    {lists,foldl,3,[{file,"lists.erl"},{line,2146}]},
                    {rabbit_variable_queue,ackfold,4,
                        [{file,"rabbit_variable_queue.erl"},{line,652}]},
                    {rabbit_priority_queue,ackfold,4,
                        [{file,"rabbit_priority_queue.erl"},{line,309}]},
                    {rabbit_amqqueue_process,
                        '-dead_letter_rejected_msgs/3-fun-0-',5,
                        [{file,"rabbit_amqqueue_process.erl"},
                         {line,1038}]}]}
```

Reproduction steps, Option 2:
1. Run a 4.0.4 / 3.13.7 mixed version cluster where both queues Q1 and Q2
   are hosted on the 4.0.4 node.
2. Send a message to Q1 which dead letters to Q2.
3. Re-publish a message with the x-death AMQP 0.9.1 header from Q2 to
   Q1. However, this time make sure to publish to the 3.13.7 node which
   forwards this message to Q1 on the 4.0.4 node.
4. Subsequently dead lettering this message from Q1 to Q2 (happening on
   the 4.0.4 node) will also cause the crash.

The modified test case in this commit was able to repro this crash via
Option 2 in the mixed version cluster tests on the `v4.0.x` branch.

(cherry picked from commit b6027ec)
@mergify mergify bot assigned ansd Dec 13, 2024
@michaelklishin michaelklishin added this to the 4.0.5 milestone Dec 13, 2024
@ansd ansd merged commit 92275ff into v4.0.x Dec 13, 2024
8 of 10 checks passed
@ansd ansd deleted the mergify/bp/v4.0.x/pr-12935 branch December 13, 2024 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants