Reconnect when a fork is detected

[PatrickTulskie]

Reasoning

Today, when a Dalli connection is active and a forked child process attempts to use it, the connection is closed and an exception is raised. When this happens in a forking app server environment, it can cause the spawned process to fall into a crash back loop, fail to start, etc. In the event the process fully loads, the first request it serves up that hits the cache will crash as well.

Historically, people were encouraged to manually close connections when forking to avoid this. This is not always feasible without monkey patching and hacks though, like in the case where Dalli is used by Rails in the ActiveSupport::Cache::MemCacheStore

Change Details

This change aims to make Dalli play nicer in forking environments and handle the reconnect itself. This mirrors other libraries that do this automatically, like https://github.com/redis/redis-rb

Dalli was already checking for a fork on most calls, so this should not introduce any overhead in most cases.

Additional Notes

I renamed the close_on_fork method to reconnect_on_fork but I'd also be open to adding close_on_fork back and adding a deprecation notice. I added a deprecation warning to the previous close_on_fork method. Reason being, the interface for the ConnectionManager seems to be fully public so I guess there could be other things out there that are depending on the existence of that method or overriding it or something. Please advise how you would like to handle that.

[nickamorim]

I renamed the close_on_fork method to reconnect_on_fork but I'd also be open to adding close_on_fork back and adding a deprecation notice

This seems like the right thing to do

[PatrickTulskie]

I renamed the close_on_fork method to reconnect_on_fork but I'd also be open to adding close_on_fork back and adding a deprecation notice

This seems like the right thing to do

Sounds good. I'll do that and update tests.

[PatrickTulskie]

@nickamorim I updated the PR with some additional unit tests, re-added the close_on_fork method and added a deprecation warning to it in case someone is calling it explicitly.

I think the unit tests in this case might be overkill since they are well covered with the integration tests, but I was doing this with TDD and didn't think they hurt either.

Also, if it looks good and you want I can squash the commits to make it a cleaner merge. Lemme know if you want me to make any other changes!

[petergoldstein]

Can you please rebase against main?

[PatrickTulskie]

Can you please rebase against main?

Rebased and also gave my commits a squash. Holler if you need anything else!

[petergoldstein]

@PatrickTulskie would you mind fixing the Rubocop lints? Thanks.

[PatrickTulskie]

@petergoldstein Looks like I fixed the linting issues, but there's one set of tests in the matrix that failed. I don't think that's related to my changes though, maybe just something flakey?

[petergoldstein]

Thanks!