gRPC settings improvement

rafael commented 6 years ago

Description

I've started to do some research into gRPC settings to see if there are some knobs that we can tweak in order to:

Improve vitess behavior during failures.
General performance tunning.

The following are some ideas that I think we could consider.

KeepAlive

This seems like a low hanging fruit that we should just do in both server and clients. During network partitions unhealthy connections will be closed. More importantly, this will close all pending rpc requests that are in flight, which effectively puts an upper timeout bound in all gRPC calls.
Turning keep-alives on with a low setting (5s? 1 minute?) would prevent the system from blocking forever in this case we saw before: https://github.com/vitessio/vitess/pull/3898.

HTTP2 Window Size

We should look into setting WithInitialConnWindowSize and WithInitialWindowSize for gRPC client connections. This controls two things with one setting. Http2 window size and dynamic window sizing (when not set, gRPC will try to dynamically set the window size by calculating bandwidth-delay product sending ping packages to the server).
Tidb and cockroachdb both set this to a default value (2mb in cockroachdb). It seems sensible to allow vitess users to control this setting.

Client Connection Timeouts

At the moment we are not setting gRPC connection timeout. We should allow users to set this value.

Compression

Set up compression on the rpc layer to reduce the amount of data sent through the wire. We could look into using gzip or snappy in gRPC.

Circuit Breakers

We should look into protecting gRPC calls with circuit breakers. That way, we could fail fast when requests are failing due to downstream component failures (e.g fail requests fast in the gate if it can't connect to a tablet).

derekperkins commented 6 years ago

@rafael Have you done any testing with https://github.com/golang/protobuf/releases/tag/v1.1.0 yet?

The serialization logic (for both Marshal and Unmarshal) has been optimized. Testing inside Google demonstrate that the new implementation is about 1.3x to 2.1x faster.

rafael commented 6 years ago

Not yet! That's something that we should add to this list for sure.

derekperkins commented 6 years ago

I also seem to remember that @tirsen or someone was experimenting with snappy compression a while back.

rafael commented 6 years ago

Oh didn't know @tirsen already worked in compression. Here it is:

https://github.com/vitessio/vitess/blob/master/go/vt/grpcclient/snappy.go

@tirsen did you rollout this change to prod in your setup?

derekperkins commented 6 years ago

Regarding client timeouts, using the gRPC client should already support client-side timeouts, but I'm not sure how that would work with the MySQL protocol. @bbeaudreault and @hmcgonig are using server-side gRPC timeouts, I think mostly for load balancing. https://vitess.slack.com/archives/C0PQY0PTK/p1520879177000696

rafael commented 6 years ago

Right now, when using MySQL protocol there are no timeouts in the gRPC call. I'm planning on working on that as part of this: https://github.com/vitessio/vitess/issues/3718#issuecomment-388473606.

The timeout I'm proposing here is connection timeout, not general grpc call timeout that you can do in the client.

derekperkins commented 6 years ago

cc @dweitzman

rafael commented 6 years ago

@zmagg also found today that KeepAlive can be set already in the server side, so we will only need to do the counter part in the client side.

There are many knobs in https://github.com/grpc/grpc-go/blob/master/keepalive/keepalive.go#L42, I think we should allow to set all of them.

vitessio / vitess