Skip to content
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

Add direct consumer support #172

Merged
merged 8 commits into from
Oct 5, 2020
Merged

Conversation

haljin
Copy link
Contributor

@haljin haljin commented Oct 3, 2020

Since #169 seems dead and @sircinek and I no longer work together, I decided to pick this up and finish it. This adds support for any custom consumer type as specified by :amqp_gen_consumer and adds AMQP.DirectConsumer as an Elixir reimplementation of :amqp_direct_consumer.

The reason for this change is that when we are using this library we are opening a channel per consumer. This starts an :amqp_selective_consumer for each of those. That consumer is a separate process leading to a structure that looks something like this: (:amqp_channel) -- (:amqp_selective_consumer) -- (OurConsumer). In rare cases when OurConsumer crashes, :amqp_selective_consumer remains. That means the broker still sees the channel as open and continues routing messages into it. Those messages are then being lost as OurConsumer does not exist anymore to handle those, neither can :amqp_selective_consumer reject them.

@ono ono mentioned this pull request Oct 3, 2020
Copy link
Collaborator

@ono ono left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 👏 👏 and thank you for explaining!

I left bunch of comments but please don't be overwhelmed. Most of them are trivial and I believe they don't require big changes 🙏

lib/amqp/channel.ex Outdated Show resolved Hide resolved
lib/amqp/channel.ex Outdated Show resolved Hide resolved
lib/amqp/channel.ex Outdated Show resolved Hide resolved
lib/amqp/channel.ex Outdated Show resolved Hide resolved
lib/amqp/channel/direct_consumer.ex Outdated Show resolved Hide resolved
test/direct_consumer_test.exs Show resolved Hide resolved
test/direct_consumer_test.exs Outdated Show resolved Hide resolved
lib/amqp/channel/direct_consumer.ex Outdated Show resolved Hide resolved
lib/amqp/basic.ex Show resolved Hide resolved
lib/amqp/basic.ex Outdated Show resolved Hide resolved
@haljin haljin force-pushed the add-direct-consumer-support branch from e9fac7c to 509b520 Compare October 3, 2020 18:37
Copy link
Collaborator

@ono ono left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating PRs! All looks good apart from a comment on channel_test.exs.

test/channel_test.exs Outdated Show resolved Hide resolved
@ono ono merged commit e63d923 into pma:master Oct 5, 2020
@ono
Copy link
Collaborator

ono commented Oct 5, 2020

Published the changes with v1.6.0 🎉 Thank you @sircinek and @haljin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants