search

mperham / connection_pool

Generic connection pooling for Ruby
MIT License
1.63k stars 143 forks source link

Ability to optionally drop all connections after fork #177

Closed shayonj closed 1 year ago

shayonj commented 1 year ago

There was a recent feature to automatically drop all connections after fork. This is quite nice and makes sense.

However, for some rails app that usually don't follow the fork model (like w/ unicorn/puma), and additionally have some logic to fork processes to perform internal business logic that doesn't rely or use ConnectionPool, the application can observe Redis connection issues or resets. These forks can happen during application run time. Like ours.

In such a case, it'd be nice to not automatically drop all the connections, since the underlying process isn't working with Redis/ConnectionPool, and as a sideeffect the pool in the primary process is impacted.

This PR proposes a new attribute auto_reload_after_fork as a config option. By default it is true. However, application users can turn it to false and not opt in for the feature to auto drop connections after fork.

This could be quite useful for us

Closes: https://github.com/mperham/connection_pool/issues/175

mperham commented 1 year ago

Seems fine to me; I can see how this would be useful for some pools.

shayonj commented 1 year ago

Thanks for the fast review and release 🙌🏾

KJTsanaktsidis commented 1 year ago

as a sideeffect the pool in the primary process is impacted.

This sounded strange to me when I first read it, but it happened to us too and I figured out why - it happens if the pool is managing TLS connections to something (Redis in our case, but anything using Ruby's openssl would work the same)

Calling OpenSSL::SSL::SSLSocket#close calls SSL_shutdown on the openssl context, which results in sending a TLS "close notify" packet down the wire, which will prompt the remote side to send a TCP FIN and close the connection; that then is what affects the parent process (it sees the connection as closed underneath it from the remote side).

In our main unicorn boot process, we actually close all the connections before forking, so this isn't an issue there, but we discovered this issue because there was some unexpected fork happening inside our Rails controllers!