rabbitmq / rabbitmq-delayed-message-exchange

Delayed Messaging for RabbitMQ
Other
2.02k stars 149 forks source link

Delayed publishing results in immediate alternate exchange routing #36

Open maximede opened 8 years ago

maximede commented 8 years ago

Hello,

I've encountered a small problem with the delayed exchange. I have and exchange defined like this :

Map<String, Object> arguments = new HashMap<String, Object>();
arguments.put("alternate-exchange", "my-alternate-exchange" )
arguments.put("x-delayed-type", "fanout");
channel.exchangeDeclare("my-delayed-exchange", "x-delayed-message", true, false, arguments);

The problem is that every time a message is pushed to my-delayed-exchange it's also directly pushed to my-alternate-exchange. I was expecting the message to be pushed to the alternate exchange only if there was no queue bound to my-delayed-exchange and after the x-delay timeout was respected.

Did I miss something ?

Thanks,

Maxime

michaelklishin commented 8 years ago

Your expectations are reasonable.

maximede commented 8 years ago

Also, I'm pretty sure it's not transferred by the plugin as the message I get in the queue attached to my-alternate-exchange contains a x-delay header without the minus sign.

Running on RabbitMq 3.6.0 btw

michaelklishin commented 7 years ago

@ovipopnz this is not a support forum. I'm deleting all comments not related to the original issue.

ovipopnz commented 7 years ago

@michaelklishin The issue I have is identical to the one above. Is this the right place to raise delayed plugin issues?

michaelklishin commented 7 years ago

It is the right place but it is not a place for questions. If you have e.g. an example that can be used to reproduce or a hypothesis as to what specifically is going on that can be verified, post them here.

michaelarnauts commented 6 years ago

This could be the same issue as #64

I see the message being published to my alternate-exchange as soon as it enters the delayed-exchange, then after the x-delay, it gets published again to my alternate-exchange (since i don't have bindings in my delayed-exchange). In the end, the message gets forwarded twice.

The message that gets published immediatly has a positive x-delay, the message after the delay has a negative x-delay.

huoshan12345 commented 6 years ago

same issue here

lukebakken commented 6 years ago

@huoshan12345 please see @michaelklishin's comment. Simply stating "same issue here" is not helpful at all. Do you have more information or a way to reliably reproduce this issue?

jarodriguez-itsoft commented 4 years ago

I'm really surprised this issue is still present as of 2020. It can be reproduced on version 3.8.2, Windows by following these steps:

  1. Create 2 queues: "normal-queue" -the queue where we want to publish- and "deadletter-queue" -the queue dead-letters should go-.

DELAYED_01

DELAYED_02

  1. Now create the "delayed-exchange" exchange. It will have "x-delayed-type"="direct" and "alternate-exhange"="deadletter-exchange"

DELAYED_03

  1. Next, we bind "delayed-exchange" with "normal-queue"

DELAYED_04

  1. We now create the "deadletter-exchange" exchange. This will be the alternate exchange for our delayed exchange. Exchange type will be "fanout" so undeliverable messages go to the binded queues.

DELAYED_05

  1. Finally, we bind the "deadletter-queue" with the "deadletter-exchange"

DELAYED_06

  1. Now we publish a message to "delayed-exchange" using routing key "normal-queue". "x-delay" will be 60000 so it is published into "normal-queue" after 60 seconds.

DELAYED_07

  1. Delayed message instantly appears in "deadletter-queue", which is an unexpected behaviour:

DELAYED_08

DELAYED_09

  1. As expected, it also appears at "normal-queue" 60 seconds after publishing:

DELAYED_10

michaelklishin commented 4 years ago

@jarodriguez-itsoft this is open source software. Pull requests with a fix would be considered. My guess is that since this exchange immediately routes to no queues, RabbitMQ core considers those messages to be eligible for AE routing, so this is not a trivial issue it may seem at first.

jarodriguez-itsoft commented 4 years ago

Hi Michael, I have never worked on erlang before but I downloaded the plugin, compiled it and made some tests with it. Your guess seems correct, it seems it is due to rabbit core detecting it has not been properly routed and therefore sending it to the alternate exchange. This seems to be due to the plugin retuning an empty list in the route() method at abbit_exchange_type_delayed_message.erl (line 58)

route(X,` Delivery) ->
    case delay_message(X, Delivery) of
        nodelay ->
            %% route the message using proxy module
            ?EXCHANGE(X):route(X, Delivery);
        _ ->
            []
    end.

If rabbitmq added some kind of standard "amqp.blackhole" exchange which simply silently accepts messages ignoing routing keys and without any real queuing, the plugin could just route to that exchange.

Something like:

route(X,` Delivery) ->
    case delay_message(X, Delivery) of
        nodelay ->
            %% route the message using proxy module
            ?EXCHANGE(X):route(X, Delivery);
        _ ->
            ?EXCHANGE(X):route(BLACKHOLE_EXCHANGE, Delivery)
    end.

Another option that could work without a new feature in rabbitmq is modifying expiration property of the message

route(X,` Delivery) ->
    case delay_message(X, Delivery) of
        nodelay ->
            %% route the message using proxy module
            ?EXCHANGE(X):route(X, Delivery);
        _ ->
            expire_message(Delivery),
            []
    end.

I don't know if this later approach is feasible (modifying message on the fly). I also don't know if a negative value of the expiration makes it to be also routed to the alternate exchange, but I tested sending original messages with a positive short expiration (1) an messages are removed from the dead-letter queue. Problem with expiration is I think they don't get expired until they reach the head of the queue, so if there are older messages there, they will stay there for a while.

I definitely like the amqp.blachole exchange approach. Do you think opening a feature request at rabbitmq has any chance of success?

michaelklishin commented 4 years ago

Simply returning a special value that would tell the core to not perform AE routing would be enough. No need for artificially created black holes ;) Thank you for diving in to confirm my hypothesis. It should be a fairly small change.

jarodriguez-itsoft commented 4 years ago

Yeah, sounds like the best solution ;) Will this be added to the core anytime soon? I can test it as soon as it is available.

michaelklishin commented 4 years ago

We can do a spike early next week and see. If we don't learn anything new I'd expect it could go into 3.8.3 and would require a new release of this plugin.

jarodriguez-itsoft commented 4 years ago

Nice! I will stay tuned :)

jarodriguez-itsoft commented 4 years ago

Hi Michael, any news on this matter?

michaelklishin commented 4 years ago

There are no updates.

jarodriguez-itsoft commented 2 years ago

I don't want to seem like I'm bugging you but I think it's time to ask again for an ETA about this fix xD We can currently workaround this by periodically purging the alternate queue, but it would be nice if we could get rid of that hack.

DaveOHenry commented 2 years ago

This issue has another side effect: It pollutes the "unroutable messages" metrics. The counter is increased for every delayed message that is sent. See https://github.com/rabbitmq/rabbitmq-prometheus/blob/master/metrics.md


From: jarodriguez-itsoft @.> Sent: Wednesday, October 13, 2021 12:58:15 PM To: rabbitmq/rabbitmq-delayed-message-exchange @.> Cc: David Heinrich @.>; Manual @.> Subject: Re: [rabbitmq/rabbitmq-delayed-message-exchange] Delayed publishing results in immediate alternate exchange routing (#36)

I don't want to seem like I'm bugging you but I think it's time to ask again for an ETA about this fix xD We can currently workaround this by periodically purging the alternate queue, but it would be nice if we could get rid of that hack.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/rabbitmq/rabbitmq-delayed-message-exchange/issues/36#issuecomment-942179409, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AE77SLPKSUIAIYDLP45TNBDUGVQ4PANCNFSM4BZHBWRA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Jojoooo1 commented 2 years ago

also having a lot of false alert on unroutable message metric, any news on the matter ?

michaelklishin commented 2 years ago

This is open source software so everyone who really needs this change is welcome to contribute it. Asking others to do the work because you need it in a piece of software you get for free is not how open source works.