From b73abf69d8749ca6d5eed17620a33ee7949b2375 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Wed, 14 Aug 2019 13:41:54 -0400 Subject: [PATCH] rpc: use gRPC enforced minimum keepalive timeout Before this commit we'd experience the following annoying log message from gRPC every time we create a new connection telling us that our setting is being ignored. ``` Adjusting keepalive ping interval to minimum period of 10s ``` Release note: None --- pkg/rpc/context_test.go | 2 +- pkg/rpc/keepalive.go | 17 ++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/rpc/context_test.go b/pkg/rpc/context_test.go index 50a0b5fab8b8..bdb43f7456c1 100644 --- a/pkg/rpc/context_test.go +++ b/pkg/rpc/context_test.go @@ -980,7 +980,7 @@ type grpcKeepaliveTestCase struct { func grpcRunKeepaliveTestCase(testCtx context.Context, c grpcKeepaliveTestCase) error { var cKeepalive keepalive.ClientParameters if c.cKeepalive { - cKeepalive = clientTestingKeepalive + cKeepalive = clientKeepalive } var sKeepalive keepalive.ServerParameters if c.sKeepalive { diff --git a/pkg/rpc/keepalive.go b/pkg/rpc/keepalive.go index e14664aee212..cafd4376241e 100644 --- a/pkg/rpc/keepalive.go +++ b/pkg/rpc/keepalive.go @@ -17,16 +17,22 @@ import ( "google.golang.org/grpc/keepalive" ) +// 10 seconds is the minimum keepalive interval permitted by gRPC. +// Setting it to a value lower than this will lead to gRPC adjusting to this +// value and annoyingly logging "Adjusting keepalive ping interval to minimum +// period of 10s". See grpc/grpc-go#2642. +const minimumClientKeepaliveInterval = 10 * time.Second + // 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: base.NetworkTimeout, + 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: base.NetworkTimeout, + Timeout: minimumClientKeepaliveInterval, // Do the pings even when there are no ongoing RPCs. PermitWithoutStream: true, } @@ -48,13 +54,6 @@ var serverEnforcement = keepalive.EnforcementPolicy{ PermitWithoutStream: true, } -// These aggressively low keepalive timeouts ensure that tests which use -// them don't take too long. -var clientTestingKeepalive = keepalive.ClientParameters{ - Time: 200 * time.Millisecond, - Timeout: 300 * time.Millisecond, - PermitWithoutStream: true, -} var serverTestingKeepalive = keepalive.ServerParameters{ Time: 200 * time.Millisecond, Timeout: 300 * time.Millisecond,