broadway_rabbitmq icon indicating copy to clipboard operation
broadway_rabbitmq copied to clipboard

proposal to allow usage of connection/channel pool

Open fcevado opened this issue 3 years ago • 4 comments

this relates with the discussion on #60 but i'm creating a new issue to address just the points related to handling the resource that the producer uses to connect/communicate with rabbitmq. just stating some things about rabbitmq and how AMQP lib model stuff:

  • to properly consume mesages a producer just needs a amqp channel not a connection.
  • a %AMQP.Channel{} has a pid for the channel that can be monitored.
  • to be aware of disconnections it's just needed to monitor the channel pid.
  • to handle reconnect it would be necessary just to checkout a new channel once the old one died.

With that in mind, what do you think about adding an option pool to the producer config, that option accept a 1-arity function or a mfa. Once a channel is needed we checkout a channel by calling the function passing the producer pid for the pool manager.

fcevado avatar Jun 08 '22 18:06 fcevado

If you can encapsulate all of the operations, mentioned above, through the channel, then I don't see an issue with this approach. Although note that you will also need an option to ask this lib to not start its own pool (or we change the option name to be custom_pool to make it clearer).

josevalim avatar Jun 08 '22 18:06 josevalim

@josevalim I'll work on that, just to define the default behavior. The precedence is in favor of connection option so, if connection is nil and custom_pool is present, the producer try to use the custom_pool.

fcevado avatar Jun 08 '22 19:06 fcevado

@fcevado are AMQP channels linked to the connection they belong to? #lazyweb

In any case, you'll also have to handle retries when checking out a channel out of an existing pool of connections, and the AMQP lib makes it quite error-prone to do operations without crashing since you'll need to catch left and right 😄

whatyouhide avatar Jun 09 '22 06:06 whatyouhide

@whatyouhide they are linked, but the connection trap exit for known reasons. if the reason a channel dies is caused by amqp operations the connection won't die, but if you send a exit signal to the channel pid, it will die. anyway, this wouldn't be responsability of broadway_rabbitmq. I guess the point here is that the broadway producer just needs a channel to do it's job. if that channel dies, it asks for a new one, the pool manager should be able to handle that correctly. I'm not suggesting that brodway_rabbitmq provides a pool manager.

What worries me is that even if the backoff strategy is stop the broadway pipeline won't die although the producer won't try to restart, I'm inclined to think that this is a broadway behavior in general, not related to broadway_rabbitmq. i need to test it with a custom producer to make sure.

fcevado avatar Jun 09 '22 11:06 fcevado

#118 is ready for review. not sure who I should tag for it. cc @josevalim @whatyouhide

fcevado avatar Feb 08 '23 16:02 fcevado

Closed in #118. Thanks @fcevado! 💟

whatyouhide avatar Feb 15 '23 10:02 whatyouhide