Automatic Fork Safety #814
Conversation
Forking servers are a popular deployment method in Ruby. One big footgun is that uppon fork, all file descriptors are inherited, so if you create some FDs before fork and hold on them you may end up with the same connection being used across many process creating havoc. As such it's a good idea for libraries that do keep persistent connections do check the PID before re-using them. The alternative is for libraries to provide some kind of callback that the application has to call before fork, but that's easilly missed by users and the PID check is very cheap.
|
@byroot Thanks. I think the code looks good and it makes sense, my main concern would be around performance since it adds those regular PID lookups to pretty much everything. I was trying to give myself broader context and saw your comments about performance concerns also here: https://bugs.ruby-lang.org/issues/17795. Does that remain a concern for something like this? Should we consider, for instance, wrapping this in an option so you could choose whether or not to impart this cost? (presumably somewhat similar to how we wrap the threadsafe check and logic). Does that make sense? Orthogonally, does this mean shopify is utilizing excon in production? Not that it makes a big difference one way or the other, but it's always interesting (and rewarding) to see where it has been adopted. |
Right. So at that time my concern was that I'd see The fork callback is still worth using IMO, but it's not always convenient. Here you'd need to keep weak references on the connections objects, which would likely be more expensive than a PID check. Ultimately I think the PID check cost here is totally dwarfed by the time even the smallest HTTP request would take, so I'm not too concerned.
Not directly, it's pulled by one of our other dependencies (https://rubygems.org/gems/avro_turf). We kinda use all the http clients in existence because of various hard dependencies like this one |
|
ah, too bad. I suspect something like "not directly" is a pretty common reason folks are using it. Similarly, having LOTS of different http clients getting looped in sounds all too familiar. |
|
thanks for the merge! |
|
Also, sorry for referencing the wrong person/account instead of the one this started with, just realized when I saw multiple avatars. |
|
Of course, thanks for the fix. Do you need a release with the change? |
No worries, makes little difference to me.
If it's not too much bother for you that's be great. But if you have other changes planned, it can wait. |
|
Sounds good. I'm about to wrap up for the day and want to look at one other bug a little more. One way or the other I'll try to get a release either early tomorrow or early next week. |
|
Released in v0.99.0. |
Forking servers are a popular deployment method in Ruby. One big footgun is that uppon fork, all file descriptors are inherited, so if you create some FDs before fork and hold on them you may end up with the same connection being used across many process creating havoc.
As such it's a good idea for libraries that do keep persistent connections do check the PID before re-using them.
The alternative is for libraries to provide some kind of callback that the application has to call before fork, but that's easilly missed by users and the PID check is very cheap.