kernel-memory icon indicating copy to clipboard operation
kernel-memory copied to clipboard

Add poison queues support for RabbitMQ

Open marcominerva opened this issue 1 year ago • 5 comments

Motivation and Context (Why the change? What's the scenario?)

See #408.

marcominerva avatar Jun 05 '24 15:06 marcominerva

I have updated this PR using Quorum Queues and Dead Letter Queues.

There are a couple of issues:

  1. After creating the Quorum Queue:
this._channel.QueueDeclare(
    queue: this._queueName,
    durable: true,
    exclusive: false,
    autoDelete: false,
    arguments: new Dictionary<string, object>
    {
        ["x-queue-type"] = "quorum",
        ["x-delivery-limit"] = this._config.MaxRetriesBeforePoisonQueue,
        ["x-dead-letter-exchange"] = poisonExchange
    });

The delivery limit cannot be changed, otherwise an exception will be thrown. So, we cannot modify the value of MaxRetriesBeforePoisonQueue. To make this value dynamic, we should use a policy, but it seems that policies aren't supported by the RabbitMQ .NET client (the only way to configure them is making an HTTP call to RabbitMQ API).

  1. Messages are requeued instantly, because there isn't a native way to have a delayed retry.

marcominerva avatar Jun 11 '24 12:06 marcominerva

Hi @dluc! Have you got the chance to think about the issues I mentioned? Do you have any idea about how to handle them?

marcominerva avatar Jun 19 '24 08:06 marcominerva

Hi @dluc! Have you got the chance to think about the issues I mentioned? Do you have any idea about how to handle them?

not yet sorry, other projects keeping me busy. I don't know when I'll be able to look through this - I'm hoping to merge the streaming PRs first, to improve the end user experience

dluc avatar Jun 21 '24 06:06 dluc

Hi @dluc,

Given that it is not possible to change the x-delivery-limit value after queue creation, I have added a try... catch block to log the error and throw an exception that explains the problem:

https://github.com/marcominerva/kernel-memory/blob/c2f2c631757cca5f1ee093a40239968b6eb4b76d/extensions/RabbitMQ/RabbitMQPipeline.cs#L74-L92

Let me know if it could be a solution.

marcominerva avatar Jul 09 '24 13:07 marcominerva

@dluc, GitHub signals that this PR has 1 change requested, but it seems that all the request changes have been addressed, or am I missing something?

marcominerva avatar Jul 16 '24 13:07 marcominerva

@marcominerva I think the PR is ready, sorry about the long delay. About the issue with max retries, I think we can live with it for now. The only option I see would be removing "x-delivery-limit" and handling the count manually, checking the "x-delivery-count" header. It would be easy, but let's see if anyone complains about this first. I've made a few changes:

  • log the number of attempts
  • log an error on the last attempt
  • enable the consumer only after all queues and exchange are setup
  • change exchange suffix, make it shorter to allow longer queue names
  • some more validations
  • some more logging
  • added a test application showing messaging coming out of the dead letter queue

dluc avatar Oct 06 '24 02:10 dluc