Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Are these WithKeepaliveParams supposed to cause a connection shutdown / affect streams? #3837

Closed
sithembiso opened this issue Aug 24, 2020 · 3 comments

Comments

@sithembiso
Copy link

I just read the README and it is suggesting that keepalive params can cause connections to shutdown if, for example maximum connection age is reached, but I can't figure out why it's happening here:

client.go

...
c.conn, err = grpc.Dial(fmt.Sprintf("%s:%d", c.opts.CentralDir.Address, c.opts.CentralDir.Port),
    grpc.WithTimeout(time.Second*5),
    grpc.WithMaxMsgSize(maxMessageSize),
    grpc.WithInsecure(),
    grpc.WithKeepaliveParams(keepalive.ClientParameters{
        Time:                10 * time.Second,
	Timeout:             time.Second, 
	PermitWithoutStream: true, 
    }),
)
...

No keepalive options are set on the server since my intention is to keep the connections alive until the user decides to disconnect. As per the documentation:

    MaxConnectionAge time.Duration // The current default value is infinity.

So, my server looks like this:

server.go

...
opts := []grpc.ServerOption{
    grpc.MaxSendMsgSize(maxMessageSize),
    grpc.MaxRecvMsgSize(maxMessageSize),
}
grpcServer := grpc.NewServer(opts...)
...

What I have observed is that when this is running and the client is connected, the stream seems to disconnect every ~35 seconds. I have a loop reading from the stream like this:

for {
    msg := &proto.Message{}
    if err := s.stream.RecvMsg(msg); err != nil {
        return err
    }
}

This returns rpc error: code = Unavailable desc = transport is closing, but I can still send messages on sync RPC functions.
This seems odd to me. The only reason I am sending pings every 10 seconds is to keep the stream alive. Am I missing something here?

@easwars
Copy link
Contributor

easwars commented Aug 26, 2020

You would need to setup keepalives on the server as well. Without that, the server will receive too many PINGS and will close the connection because it thinks that the client is misbehaving.

See here for a description of how the server enforcement works.

Also see here for an example which shows both the client and server configurations.

Hope this helps.

@easwars easwars closed this as completed Aug 26, 2020
@dfawley
Copy link
Member

dfawley commented Aug 26, 2020

You would need to setup keepalives on the server as well. Without that, the server will receive too many PINGS and will close the connection because it thinks that the client is misbehaving.

Clients auto-scale when they receive a signal from the server they are pinging too frequently:

From this section of the spec:

When a client receives a GOAWAY with error code ENHANCE_YOUR_CALM and debug data equal to ASCII "too_many_pings", it should log the occurrence at a log level that is enabled by default and double the configure KEEPALIVE_TIME used for new connections on that channel.

So this behavior should be temporary until the client scales back to a rate acceptable for the server.

This returns rpc error: code = Unavailable desc = transport is closing, but I can still send messages on sync RPC functions.

Clients will always reconnect, so future RPCs will work.

@sithembiso
Copy link
Author

Thank you @dfawley and @easwars. It appears I completely missed the server-side enforcement.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants