Improve rabbit_backing_queue:is_duplicate behaviour

[noxdafox]

Proposed Changes

The rabbit_backing_queue:is_duplicate callback was modified in PR 1774 in order to support both the de-duplication plugin and the mirroring queues.

Since implementation of mirroring queues has been removed, the only user for the is_duplicate callback remains the de-duplication plugin. Hence, this PR simplifies the callback signature removing dead code.

Neither the classic variable nor the priority queues are making use of the is_duplicate callback.

On top of that, in the event the message is deemed duplicated, it is re-routed to DLX if present. This would satisfy issue noxdafox/rabbitmq-message-deduplication#106 on the plugin itself.

Types of Changes

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

• Bug fix (non-breaking change which fixes issue #NNNN)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• Documentation improvements (corrections, new content, etc)
• Cosmetic change (whitespace, formatting, etc)
• Build system and/or CI

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.

• I have read the CONTRIBUTING.md document
• I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
• I have added tests that prove my fix is effective or that my feature works
• All tests pass locally with my changes
• If relevant, I have added necessary documentation to https://github.com/rabbitmq/rabbitmq-website
• If relevant, I have added this change to the first version(s) in release-notes that I expect to introduce it

[michaelklishin]

@noxdafox thank you. I will re-submit so that Actions can run with access to all the secrets.

[michaelklishin]

Voilà: #12914.

I have asked a couple of team members to review this and the feasibility of a backport to v4.0.x.

[mkuratczyk]

The CI is very red with #12914. I've tried to set up deduplication but queues fail with a function_cluase even without the deduplication plugin. All it takes is:

1. git checkout rabbitmq-server-12913
2. make run-broker
3. perf-test -u q -r 1 -y 0

Messages are initially delivered to the queue but then the queue crashes.

[email protected]

[noxdafox]

Hello,

I run local tests for both priority and classic queues and did not spot any issue.

The plugin is not yet supporting the new function signature, this is why you see it failing with a function_clause error. I will make a backwards compatible release which will support the new rabbit_backing_queue:is_duplicate ASAP.

Could you point me out the problematic CI tests which I should look at?

[mkuratczyk]

The deduplication plugin is not needed for things to go wrong. I just start RMQ from a branch with your changes, start perf-test as described above and the queues crashes after a few seconds. I don't have the plugin installed/enabled for this at all.

You can see a lot of tests failing here: https://github.com/rabbitmq/rabbitmq-server/actions/runs/12265974908/job/34232064015?pr=12914. I didn't even look into this - with classic queues crashing, obviously a lot of tests will fail

[noxdafox]

Thanks for the pointer, I will look into those and fix the issues.

EDIT: indeed the issue is clear, a bit surprised local tests did not catch it. I will fix it this evening.

[noxdafox]

@mkuratczyk I updated the PR with the relevant fixes.

I ran the following tests which are now passing

• ct-queue_parallel
• ct-classic_queue
• ct-priority_queue
• ct-quorum_queue
• ct-queue_length_limits
• ct-dead_lettering

Before I limited myself to classic and priority queues thinking they were sufficing.

[michaelklishin]

Re-submitted as #12921, running a few relevant suites locally now.

[michaelklishin]

#12921 CI results are much better.

I am now running a few suites related to queues/streams but not mentioned above locally just in case:

# from deps/rabbit
gmake ct-per_vhost_queue_limit
gmake ct-queue_master_location
gmake ct-rabbit_stream_queue
gmake ct-amqqueue_backward_compatibility
gmake ct-unit_priority_queue
gmake ct-classic_queue_prop

[mkuratczyk]

Yup, the CI is green and I don't see any issues. Deduplication plugin seems to work with these changes, at least for some basic functionality.

[noxdafox]

The plugin will be built against the new implementation. I am unsure we can easily guarantee backwards compatibility as the signature change is affecting the return value and not the callback parameters but I'll see what I can do.

Are test suites documented anywhere or is there a simple way to discover them? It was a bit hard to figure out what to run. Running the whole test suite is usually prone to fail early with unrelated issues. Hence, it's a bit hard to figure out if the submitted code works or not.

[mkuratczyk]

I don't think there's anything documented or discoverable. There has been a lot of effort lately to speed up the tests and fix the flaky tests so things should not just fail for unrelated reasons (it still happens but it's rare). For us, the easiest way is to push a branch and let the CI run all the tests but this doesn't currently work for external contributions unfortunately.

[michaelklishin]

@noxdafox see CONTRIBUTING.md and ls -l deps/rabbit/test/*.erl or ls -l deps/**/test/*.erl more generally.

[michaelklishin]

#12930

[michaelklishin]

There were some Actions build environment changes that we need to address first before we can backport this potentially risky change to v4.0.x.

[michaelklishin]

#12921 was merged for 4.1.0.