Transport closing when client is idle

[dennwc]

Currently, the client opens a connection when the client is created and keeps it open even when idle. The connection can then be terminated if it's idle for too long.

However, there are cases when the gRPC library cannot determine connection state precisely and may fail the first request with transport closing after being idle for long enough.

The solution is to use a grpc.WithKeepaliveParams to keep the connection healthy while idle. We can either hard-code it or allow passing additional DialOptions when creating a client.

[creachadair]

It sounds to me as if there may be a bug in how we are using gRPC. Once we have a gRPC stub open, it should transparently re-connect as needed (even if the underlying connection actually went away due to a timeout) provided the resolver can still find a valid network address for the stub to connect to.

It seems like a good idea to let idle connections close, since otherwise they are tying up a descriptor, but there may be some combination of options we can set to ensure the stub will correctly reconnect on a timeout.

[dennwc]

I was also thinking that we are using it in the wrong way, but I can't find anything relevant in the code.

Also, here is the log (after idling):

2019/06/24 18:23:23 State of bblfish client is READY - Filename App.java
2019/06/24 18:23:47 Babelfish can't parse the file App.java rpc error: code = Unavailable desc = transport is closing
2019/06/24 18:23:47 State of bblfish client is CONNECTING - Filename Cache.java
2019/06/24 18:23:47 State of bblfish client is READY - Filename command/Command.java
2019/06/24 18:23:47 State of bblfish client is READY - Filename command/CommandFactory.java

So gRPC thinks that the channel is alive (as seen by READY status), and tries to send the parse request. Then for some reason, it returns the transport is closing to the client instead of reconnecting the underlying TCP connection. The next parse request is sent during CONNECTING status and eventually succeeds when the connection is re-established. So it may be a bug in gRPC after all.

In any case, allowing to set additional dial options sounds useful, so I'll PR it shortly.

[creachadair]

I wonder if maybe gRPC only reconnects if there is a different address it can use, i.e., another endpoint in a load-balancing channel, rather than the original?

[dennwc]

It would make sense if it tries to connect, fails and then returns an error because there is only one endpoint, but that's not that case, as far as I know.

Related: grpc/grpc-go#2443

[bzz]

Related: grpc/grpc-go#2443

Great find @dennwc !

So does it mean that we should set grpc.KeepaliveEnforcementPolicy and PermitWithoutStream=true on the server as well, to enforce keepalive packets coming?

[dennwc]

@bzz Good point! We probably should.

[bzz]

Most probably should be good to close as soon as #299 and #127 are merged now