Limit retry count on connection transientFailure

[chulup]

Question Checklist

• Updated grpc-swift to the latest version
• I read the Contribution Guidelines
• I read README
• I searched for existing GitHub issues (especially, there are many resolved issues related to SSL)

Question Subject

I use gRPC to connect my Swift daemon and client on macOS through Unix Domain Sockets.

Is there a way to limit retry count? There is ConnectionBackoff structure that has maximumBackoff property but it does not stop reconnections ever, just limits maximum time between attempts

Question Description

Expected behavior: set maximum retry count, receive up to maxRetry state changes to transientFailure and next state change is to shutdown

Observed behaviour: with any ConnectionBackoff parameters state changes connecting -> transientFailure -> connecting indefinitely and does not stop. I even tried to set maximumBackoff to 0.01 but it only changes timeout between reconnections.

There is open issue for NIO apple/swift-nio#599 regarding calling setsockopt on UDS and warnings coming out of it, but I don't think it matters here.

[chulup]

Another question that vaguely connected to the previous: Is there a way to close channel from inside the connectivityStateDidChange callback? I am getting EDEADLK from inside the library. It seems like it tries to take the lock recursively if I had to guess.

[Lukasa]

I think @glbrntt is the better question to answer the connection backoff issue, but it's absolutely right that the current implementation incorrectly calls out to the ConnectivityStateDelegate with the ConnectivityState lock held. That's really unwise.

It puts us in a bind though. Right now there are probably a bunch of connectivity state delegates that don't expect to be concurrently called across threads. Removing the lock from that call-out risks exposing them to thread safety issues. I see two options:

1. Accept this as a requirement on ConnectivityStateDelegates: they must be thread safe. We can try to communicate this as a requirement as best we can, and hope updaters notice.
2. Notify ConnectivityStateDelegates on a specific DispatchQueue. This can be potentially provided as an argument to ConnectivityStateMonitor, and be defaulted to a private serial queue if none is provided. This gives the ConnectivityStateDelegate thread-safety guarantees and strict ordering of events, at the cost of potentially notifying it a bit more slowly.

@glbrntt got a preference here?

[glbrntt]

Is there a way to limit retry count? There is ConnectionBackoff structure that has maximumBackoff property but it does not stop reconnections ever, just limits maximum time between attempts

Not at the moment. Adding a maximum retry count is something we can do fairly easily and that I'm happy to add.

I think @glbrntt is the better question to answer the connection backoff issue, but it's absolutely right that the current implementation incorrectly calls out to the ConnectivityStateDelegate with the ConnectivityState lock held. That's really unwise.

Ouch, that is really bad. I think we should be notifying the delegate on a DispatchQueue. The delegate may be called from the event loop which is totally unclear to the user; the dispatch queue would avoid the loop potentially being blocked as well.