Support Nack functionality for avoiding loosing messages on errors #3371
Conversation
… - supporting cases of transient failures, that messages we can't process- won't be lost by ack, and not re-published back to the queue, but published to DLQ (in some queue implementation).
| @@ -134,6 +134,10 @@ public void handle(ObservableQueue queue, Message msg) { | |||
| queue.publish(Collections.singletonList(msg)); | |||
| LOGGER.debug("Message: {} published to queue: {}", msg.getId(), queue.getName()); | |||
| } | |||
| else { | |||
There was a problem hiding this comment.
Hi @AvitalOfstein , Thanks for submitting the PR. Should we also need to change the logic at line 127 to not ack when executionFailed is true?
There was a problem hiding this comment.
Hi @manan164, Yes, but we would prefer to split the executionFailed issue handling to different PR (bug fix), from this one (adding Nack feature), since there are already many users of this code, with different queue types, and we wouldn't like to cause potential regression in changing the conditions in this logic in the same PR
There was a problem hiding this comment.
hi @AvitalOfstein , could you explain what the nack will do? The default implementation is empty. It would also help to which queue implementation are you planning to use nack with.
There was a problem hiding this comment.
Hi @aravindanr ,
Sure, in case we have an error while processing the event (e.g. line 194), it won't get into the first 2 conditions (queue.ack and queue.publish).
In RabbitMQ implementation, the consumner will do nothing with the message (will be idle) and after some time it will get disconnected.
So in AMQP implementation (community repo) I would like to add the specific implementation of Nack with possibility not to requeue (so it can go to DLQ if defined), other implementations can keep with current behavior.
|
This PR is stale, because it has been open for 45 days with no activity. Remove the stale label or comment, or this will be closed in 7 days. |
|
Hi @aravindanr , can you please review the changes? cc @AvitalOfstein |
Pull Request type
Changes in this PR
Describe the new behavior from this PR, and why it's needed
Support Nack functionality for avoiding loosing messages when handling messages and getting unexpected errors