search

salemove / freddy

Messaging api supporting request-response and acknowledgements.
MIT License
9 stars 0 forks source link

Channel and thread safety #84

Open pgouv opened 1 year ago

pgouv commented 1 year ago

I was looking at SendAndWaitResponseProducer. It seems that it is designed to be thread safe. However since bunny/rabbitmq channel is shared is it really safe? According to bunny documentation channels must not be shared. Is it a special case?

indrekj commented 1 year ago

Hey. It's been a while since I last touched this part of the codebase. IIRC we were thinking about thread safety. We've been running this for years (hundreds of messages per second per instance) without any thread safety problems. I'm relatively sure that regular YARV ruby is fine. The other ruby variants may not though.

pgouv commented 1 year ago

Do you run this in a multi-threaded server such as Puma? Servers like passenger shouldn't have a problem though. A possible solution could be to use ConnectionPool gem to create a pool of channels for publishing. This gem is used by Rails too. However there could be cases where channel is closed by the server and you wont have much control but this could happen to current implementation too. Probably it might be an edge case.

indrekj commented 1 year ago

Yes, we're using puma with multiple threads.

I think before starting to rewrite this, we should first try to produce the issue. I don't want to start changing anything if there's no issue.

indrekj commented 1 year ago

Btw, if you're just starting then our experience says that using regular HTTP is better than deliver_with_response for RPC. We still use deliver and tap_into, but try to avoid deliver_with_response. Mainly because HTTP provides more control and is more standard for RPC use cases.

pgouv commented 1 year ago

Yes, we're using puma with multiple threads.

I think before starting to rewrite this, we should first try to produce the issue. I don't want to start changing anything if there's no issue.

Probably a stress test would help. I may try to test it.