-
Notifications
You must be signed in to change notification settings - Fork 3.9k
By @noxdafox: Revival of #9620 Pass the message to rabbit_backing_queue:discard callback #13374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The previous behaviour was passing solely the message ID making queue implementations such as, for example, the priority one hard to fulfil. Signed-off-by: Matteo Cafasso <[email protected]> (cherry picked from commit 1f7a27c)
Signed-off-by: Matteo Cafasso <[email protected]> (cherry picked from commit facddb3)
@@ -105,7 +105,7 @@ | |||
|
|||
%% Called to inform the BQ about messages which have reached the | |||
%% queue, but are not going to be further passed to BQ. | |||
-callback discard(rabbit_types:msg_id(), pid(), state()) -> state(). | |||
-callback discard(rabbit_types:basic_message(), pid(), state()) -> state(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be mc:state()
instead of rabbit_types:basic_message()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@noxdafox can you please see if you can refactor this to mc:state/0
and make it work for the deduplication plugin? rabbit_types:basic_message/0
is AMQP 0-9-1-specific so we'd prefer to use the more generic mc
module (which is AMQP 1.0-inspired but works for all protocols).
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do. Will submit another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in #13382
This is #12743 #9620 by @noxdafox rebased and re-submitted by me.