Always acquire client lock before coordinator lock to avoid deadlocks

[dpkp]

This is an attempt to address the deadlock issue described in #1461 . The first option discussed, delaying errback processing until after release of the client lock, would require a lot of work. I've instead tried to change the lock acquisition paths so that the coordinator never attempts to acquire the client lock after it holds the coordinator lock. This is done by either releasing the coordinator lock before calling functions that acquire the client lock, or by preemptively acquiring the client lock before acquiring the coordinator lock.

[dpkp]

Any comments on this? If not, I'm going to merge. I have been able to reproduce the deadlock locally on master (though not consistently). With this patch applied I have not seen a deadlock.

[jeffwidman]

Afraid I haven't had much time lately for open source, and so just now looking at it. Everything looks fine to me at a surface level, but this is a somewhat hairy problem and I'm afraid that right now I don't have the time to dig deeply into the design/problems with the alternative.

If you don't hear from @tvoinarovskyi , then just go ahead and merge.

[haosdent]

@dpkp We still encounter the same issue in 1.4.3 Do you need to modify https://github.com/dpkp/kafka-python/blob/1.4.3/kafka/coordinator/base.py#L928 to with self. coordinator._client._lock, self. coordinator._lock:

[dpkp]

Yes, I think you're right!

Edit: looking again at this code, there is no path within the block that leads to an attempt to acquire the client lock afaik. So that is why I did not acquire both locks upfront. So I don't think acquiring the client lock here will fix.

If you are still seeing deadlock issues, can you start a new issue and post debug logs ?