Allow overriding of the default keepalive server-side enforcement policy min time#4130
Conversation
the server doesn't preemptively close connections due to our client keepalive being more aggressive than the allowed server policy. Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
go/vt/servenv/grpc_server.go
Outdated
|
|
||
| // EnforcementPolicy MinTime that sets the keepalive enforcement policy on the server. | ||
| // This is the minimum amount of time a client should wait before sending a keepalive ping. | ||
| GRPCEnforcementPolicyMinTime = flag.Duration("grpc_enforcement_min_time", 5*time.Minute, "grpc server minimum keepalive time") |
There was a problem hiding this comment.
Actually I clicked approve a bit too soon :)
IMO we should be clearer that this has to do with keepalives, so I would change the flag name to grpc_keepalive_enforcement_policy_min_time and the variable name to the corresponding camelCase.
go/vt/servenv/grpc_server.go
Outdated
|
|
||
| // EnforcementPolicy MinTime that sets the keepalive enforcement policy on the server. | ||
| // This is the minimum amount of time a client should wait before sending a keepalive ping. | ||
| GRPCEnforcementPolicyMinTime = flag.Duration("grpc_enforcement_min_time", 5*time.Minute, "grpc server minimum keepalive time") |
There was a problem hiding this comment.
I would call this grpc_server_... to be consistent with all the other flags.
Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
Signed-off-by: Maggie Zhou <mzhou@slack-corp.com>
sougou
left a comment
sougou
left a comment
There was a problem hiding this comment.
Do you also need to set the PermitWithoutStream flag? If so, you may have to spin up a new PR. I'll merge this.
|
@sougou I don't believe we need |
4 participants
Allow setting the keepalive server-side enforcement policy min time, so that the server doesn't preemptively close connections due to setting our client keepalive to a more aggressive value than the allowed server policy.
Signed-off-by: Maggie Zhou mzhou@slack-corp.com