search

smallrye / smallrye-reactive-messaging

SmallRye Reactive Messaging
http://www.smallrye.io/smallrye-reactive-messaging/
Apache License 2.0
241 stars 179 forks source link

RabbitMQ should allow configure explicitly Retry Failure for nack when using Publish Confirm #2726

Closed gcuisinier closed 2 months ago

gcuisinier commented 3 months ago

Use case : Sending a message with rabbit mq ( with publish-confirm ) on a queue configured with max-lenght and `x-overflow : reject-publish'

Expected As the queue is full, the message is rejected and the error should get raise directly or quickly.

Current Behaviour As the nack is considered as Failure, the Retry on Failure came into action :

https://github.com/smallrye/smallrye-reactive-messaging/blob/c176b3ec6f4a65c456a9d17d4215316b60d37eb5/smallrye-reactive-messaging-rabbitmq/src/main/java/io/smallrye/reactive/messaging/rabbitmq/internals/RabbitMQMessageSender.java#L285

But the Retry attemps / Retry interval are reusing the "ReconnectAttempts" and "ReconnectInterval" :

https://github.com/smallrye/smallrye-reactive-messaging/blob/c176b3ec6f4a65c456a9d17d4215316b60d37eb5/smallrye-reactive-messaging-rabbitmq/src/main/java/io/smallrye/reactive/messaging/rabbitmq/internals/RabbitMQMessageSender.java#L264-L265

The default configuration it quite high ( 100 tentative, delay 10 sec ==> 1000 seconds in total). That fine to validate the connection itself, but a bit long for the nack check.

It think it should be great to have dedicated configuration options to be able to configure the retry on publish explicitly to shorter time.

Sample project https://github.com/gcuisinier/quarkus-rabbitmq-tests/tree/rabbitOverflowNotNacked

ozangunalp commented 3 months ago

That definitely makes sense, would you want to open a PR to change that @gcuisinier ?

gcuisinier commented 3 months ago

Yes, I can do that :)