kvserver: reconsider gRPC server-side keepalive timeout

[erikgrinaker]

The gRPC server-side keepalive timeout defaults to 2 seconds, via COCKROACH_NETWORK_TIMEOUT. This sets TCP_USER_TIMEOUT on the socket, instructing the OS to close connections if a send remains unacknowledged by the peer for 2 seconds.

We would ideally want this on the gRPC client side instead, since that would more quickly detect network outages and allow the client to fail over to a different node, reducing the duration of unavailability. Unfortunately, gRPC does not allow client-side keepalive timeouts below 10 seconds. Instead, we use application-level client-side RPC heartbeats, but unlike TCP_USER_TIMEOUT these are subject to head-of-line blocking in the RPC stack (including effects like scheduling latency). We therefore have to use a large RPC heartbeat timeout of 6 seconds to tolerate heavy loads in high-latency clusters (see #93397).

Given this, it isn't clear that an aggressive server-side gRPC timeout provides much benefit. It doesn't help in the case of a network outage -- the server is unlikely to care about the connection remaining open, and the client has to wait for its own timeout to fire. It would more quickly detect asymmetric partitions, allowing the dialback mechanism to promote it to a symmetric partition (see #94778), but this is likely rare enough that a higher timeout is ok. Under persistently high latency it might be helpful to eagerly close the connection and let the client fail over to a lower-latency node, but this assumes the lease also moves and that the client won't block and re-dial the node (mostly true in 23.2 with #99191). It can also be detrimental since it can cause additional disruption with transient latency spikes like intermittent packet loss.

We should reconsider the value for the server-side keepalive timeout, subject to availability benchmarks (especially with asymmetric partitions and latency spikes).

See also #93007.

cockroach/pkg/rpc/keepalive.go

Lines 26 to 47 in e97fe41

	// To prevent unidirectional network partitions from keeping an unhealthy
	// connection alive, we use both client-side and server-side keepalive pings.
	var clientKeepalive = keepalive.ClientParameters{
	// Send periodic pings on the connection.
	Time: minimumClientKeepaliveInterval,
	// If the pings don't get a response within the timeout, we might be
	// experiencing a network partition. gRPC will close the transport-level
	// connection and all the pending RPCs (which may not have timeouts) will
	// fail eagerly. gRPC will then reconnect the transport transparently.
	Timeout: minimumClientKeepaliveInterval,
	// Do the pings even when there are no ongoing RPCs.
	PermitWithoutStream: true,
	}
	var serverKeepalive = keepalive.ServerParameters{
	// Send periodic pings on the connection when there is no other traffic.
	Time: base.PingInterval,
	// If the pings don't get a response within the timeout, we might be
	// experiencing a network partition. gRPC will close the transport-level
	// connection and all the pending RPCs (which may not have timeouts) will
	// fail eagerly.
	Timeout: base.NetworkTimeout,
	}

Jira issue: CRDB-30871

[erikgrinaker]

Turns out the timeout is also used for gRPC's own keepalive pings, which unlike TCP_USER_TIMEOUT is subject to scheduling latency and such. This explains why we saw this in cases such as #108157.

https://github.com/grpc/grpc-go/blob/9b9b38127038e347420ef08c42e5d74d181b8749/internal/transport/http2_server.go#L1175-L1204

[erikgrinaker]

We increased the server-side timeout in #109578 (2 to 4 seconds) and the keepalive ping interval in #109715 (1 to 2 seconds).

However, we see a possible regression in failover/non-system/blackhole-recv on August 29, i.e. an asymmetric partition dropping inbound packets to a node. I think this is expected:

Consider a client C sending a request to the partitioned leaseholder L. C will normally have to wait out the RPC heartbeat timeout (6 seconds) before failing and trying someone else. However, L's server-side timeout would have fired after 3 seconds (1s keepalive interval + 2s server-side timeout), closing the connection and forcing the client elsewhere. It can do this since the outbound TCP RST packets can still make it through (only inbound packets C->L are dropped). Thus, increasing the server-side timeout will increase the time until the TCP RST is sent, and the client will have to fall back to the RPC heartbeat timeout, increasing the overall recovery time.

Considering the timeout is sensitive to node overload and Go runtime scheduling latencies and such, and this kind of asymmetric partition is relatively rare, this seems like a justified tradeoff.