FIX: Only attempt to disconnect idle client instances

[davidtaylorhq]

Waiting to acquire the mutex on the ::Redis objects is likely to cause a deadlock in multi-threaded environments. For example:

• Thread 1 and thread 2 have in-progress redis requests during a failover
• Thread 1 is the first to raise an exception, triggers RailsFailover::Redis::Handler#disconnect_clients, acquiring the RailsFailover::Redis::Handler mutex
• Thread 1 loops through all ::Redis instances, acquire their locks, and disconnect them
• When it reaches the ::Redis instance used by Thread 2, Thread 1 waits for the mutex to become available

---

• Meanwhile, Thread 2 raises an exception and triggers RailsFailover::Redis::Handler#disconnect_clients
• Thread 2 waits for the RailsFailover::Redis::Handler mutex to become available

This situation is never resolved, and the threads remained deadlocked.

This commit will skip disconnecting ::Redis clients which are currently locked by another thread. If the lock is held by another thread, it is fair to assume that a request is in process, it will fail, and the client will automatically disconnect very soon.

[eviltrout]

Nice one.

[tgxworld]

@davidtaylorhq ahh this only happens in Sidekiq right? When I added the feature I was considering Unicorn workers only which only have one instance of Redis each.

[davidtaylorhq]

Yep, this is only causing problems in sidekiq 👍

[davidtaylorhq]

I realised that this assertion I made

If the lock is held by another thread, it is fair to assume that a request is in process, it will fail, and the client will automatically disconnect very soon.

Is not always correct. When we are failing over due to a connection error, it holds. But if we are failing over manually, or failing back to the master, we cannot rely on a connection error to resolve things.

Going to close this PR and come back with another solution.