client: Tweak the keepalive interval and timeout

[cdecker]

As reported by the Breez team the cause for the transport error when
attempting a trampoline_pay appears to be that even short
interruptions in connectivity kill the TCP connections. The long 90s
timeout is causing gl-client to patiently sit there for the 90s,
ignoring all errors in the hope of getting the TCP connection
back. When it then attempts to re-establish the connection, any
pending call gets the dreaded transport error.

Reducing this to 5 seconds timeout and a 1s interval should minimize
the chances of issuing a command during the broken phase.

Suggested-by: Roei Erez <@roeierez>
Suggested-by: Jesse De Wit <@JssDWt>
Closes #428

[roeierez]

I think it makes sense to set the keep alive timeout not more than the interval otherwise we may end up sending a second keepalive packet before the first one is ack'd.
Any reason you see not to set it for 1 second?

[cdecker]

It's a balance of eager timeout vs keeping connections more stable. The timeout is counted for each packet, while the interval tells us how to send the ping, the timeout tells us how long to wait for pings. Here we pipeline up to 5 pings before we require the first pong to be back. So technically, if we send 2 pings, the first one gets replied to, the second one doesn't, then at second 6 we reconnect. If we set the interval and timeout to the same, we'd wait for 10 seconds, because we send only every 5s, and wait for a reply for 5s.

Pipelining pings can help reduce the time to recovery, without increasing the sensitivity to latency. Timeout on the other hand controls latency sensitivity, and as mentioned, I'm hoping that for interruptions <5s the TCP connection can just resume, delivering a late pong but keeping it inside of the timeout. Overall I think this will minimize the number of failed grpc calls.

[cdecker]

I guess long story short, it's a parameter that we should likely tweak as we learn more about the network constraints we work with.

[roeierez]

If we set the interval and timeout to the same, we'd wait for 10 seconds, because we send only every 5s, and wait for a reply for 5s.

I actually commented about making the timeout the same as the interval (both 1 seconds) which means sending every 1 seconds and waiting up to 1 second for ack, lowering the max time to detect network change to 1 second overall and lowering even more the chances to get transport errors (if the call falls into the timeout time frame).
I guess the main question is do we foresee that on a stable connection we will have more than 1 second latency for replying a single tcp packet?
I agree that this fix by itself is a significant improvement.

[cdecker]

The best measure we currently have is the RTT that we get from the signer, which admittedly is a rather large request and response so bandwidth also influences this. We see RTTs up to 1.5s relatively regularly, those would be the affected nodes getting disconnected with timeout=1s, if ping messages (unloaded lines) also have a similar RTT.

[roeierez]

In my tests tracing the keep alive packets I see response in milliseconds but perhaps my bandwidth is better than the average and we should prepare for higher latency. I am ok to start with 5 seconds and gather some feedback from the field.