search

coders51 / rabbitmq-stream-js-client

rabbitmq-stream-js-client
MIT License
34 stars 5 forks source link

Check if cached connection can support more pub/subs before reusing #194

Closed moughxyz closed 5 months ago

moughxyz commented 5 months ago

Hello,

A single connection can only support 255 publishers or consumers, given that the publisherId is an 8 bit integer.

https://github.com/rabbitmq/rabbitmq-server/blob/main/deps/rabbitmq_stream/docs/PROTOCOL.adoc#declarepublisher

Other libraries like Go and Java create a new connection when this limit is reached: https://groups.google.com/g/rabbitmq-users/c/MQ-Bub3q13c/m/9-3cKfpbAAAJ

The issue with the current implementation is that the cached connection is always used when creating publishers, and this means that on the 256th publisher, it will crash because the publisherId of 256 won't fit into the 8 bit slot.

In this PR, we check to see if a connection can first support an additional publisher. If not, we create a new connection.

This would be a first-stab implementation. Please let me know any shortcomings and would be happy to fix.

tarzacodes commented 5 months ago

I've added some comments - if they don't make sense, let me know. Also, we had some work done on the client, if you'd like some assistance in rebasing, I'm available: let's get in touch. Thanks for your suggestion and the work you've put in implementing that! This is something we really ought to add into the client :fire:

moughxyz commented 5 months ago

Hey @tarzacodes, apologies, I have since reevaluated our usage of RabbitMQ and may go in a different direction. So I'm afraid I can no longer give this PR the care it deserves. I will close it in the meantime and perhaps it can serve as a reference to anyone else encountering this issue.