mperham / connection_pool

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

Automatically drop all connections after fork #166

Closed casperisfine closed 2 years ago

casperisfine commented 2 years ago

Fix: https://github.com/mperham/connection_pool/issues/165

Using connections inherited from a parent process can have very nasty consequences and it's pretty much never desired.

This patch use the Process._fork API added in Ruby 3.1 to automatically detect when a fork is happening, and when it does, drop all existing connections.

Notes

Performance

ObjectSpace.each_object is very slow for large applications:

[1] pry(main)> Benchmark.realtime { ObjectSpace.each_object(ConnectionPool) { |c| c }.size }
=> 0.06875399989075959
[2] pry(main)> Rails.application.eager_load!
=> nil
[3] pry(main)> Benchmark.realtime { ObjectSpace.each_object(ConnectionPool) { |c| c }.size }
=> 0.14990400010719895
[4] pry(main)> Benchmark.realtime { fork { exit! }}
=> 0.004583500092849135

So I'd really recommend to keep track of instance with ObjectSpace::WeakMap. I'll push a commit in another branch and let you decide.

Connection closing

Since ConnectionPool doesn't know how to close a connection, this feature simply drop the references, and assume the Ruby GC will do the right thing.

casperisfine commented 2 years ago

Here's what keeping track of instances in a WeakMap would involve: https://github.com/casperisfine/connection_pool/commit/3ff19dd80d12cf2c6656caf4900d802f57b70768.

casperisfine commented 2 years ago

Thanks! ❤️

hugobarthelemy commented 1 year ago

A new release is planned to integrate this PR?

mperham commented 1 year ago

Eventually. Do you need it soon for some reason?

hugobarthelemy commented 1 year ago

Periodically, nearly restart or scaling event, we have more connections open than the theoretical count. We don't investigate a lot because we have needed to upscale Redis plan for other reasons and pics it's never more than 2x higher than the theoretical count. But we would appreciate updating our gem with this feature only to see if it resolves our problem.

mperham commented 1 year ago

You could run main, right?

hugobarthelemy commented 1 year ago

We could. It's not very clean, but we could.

mperham commented 1 year ago

I could use the feedback, let me know if it helps. Not sure what you mean by clean, it’s one line in your Gemfile.

oskarpearson commented 1 year ago

I could use the feedback, let me know if it helps.

We've been running 428c06f34209ee7a99f1ace5b96e567841c00d1c for a couple months now without any issues on a system that's probably processed 40 million jobs over the time.

mperham commented 1 year ago

2.4.0 is out with this new feature.