Allow controlling nack requeue on plugin level

gor181 commented 4 years ago

Proposed Changes

👋

Why: Various companies are using RMQ with third party software which doesn't offer the ability to alter the source code related to how the STOMP consumer behaves on the frame level, e.g. there's no possibility to actually change the frame requeue header to false as the frame is created by the 3rd party software.

This has proven to be an issue on a project I have been working on as the current stomp plugin frame requeue implementation defaults to true if the requeue header doesn't exist on the frame. The side-effect of this behaviour is that we ended in a loop of massive re-deliveries as the messages were always re-queued, while, what we wanted was to avoid re-queueing.

In order to get around this issue, we decided to expose the stomp.default_nack_requeue configuration option which would serve as a default for the requeue frame option.

This PR tries to:

retain the existing frame requeue logic and defaults
add the stomp.default_nack_requeue option which overrides the default frame requeue value

Side-effects:

no changes will be noticed by the consumers which are setting this on the message level, or not setting at all
exposes the setting for those who can't use the frame message header but can configure their stomp plugins

Types of Changes

What types of changes does your code introduce to this project? Put an x in the boxes that apply

[ ] Bugfix (non-breaking change which fixes issue #NNNN)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Documentation (correction or otherwise)
[ ] Cosmetics (whitespace, appearance)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask on the mailing list. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

[x] I have read the CONTRIBUTING.md document
[x] I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
[ ] All tests pass locally with my changes
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I have added necessary documentation (if appropriate)
[ ] Any dependent changes have been merged and published in related repositories

Further Comments

When running tests locally I get the following errors (regardless whether I'm on this branch or on rabbitmq-stomp master branch):

I have been running:

make
make check

What I can confirm is that the functionality works when the plugin is released with make dist.

TODO

[ ] Understand why the tests are unable to run
[ ] Add spec for the newly added config option

pivotal-issuemaster commented 4 years ago

@gor181 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

pivotal-issuemaster commented 4 years ago

@gor181 Thank you for signing the Contributor License Agreement!

michaelklishin commented 4 years ago

Thank you!

michaelklishin commented 4 years ago

Quorum queues with redelivery limit are worth mentioning here.

rabbitmq / rabbitmq-stomp