From 6738ad5d1b0876bcc17dfd6dfdd4e94de74e91a5 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Tue, 6 Aug 2019 15:14:56 -0700 Subject: [PATCH] Implement missing pieces for connection backoff. Spec can be found here: https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md Summary of changes: * Added a new type (marked experimental), ConnectParams, which contains the knobs defined in the spec (except for minConnectTimeout). * Added a new API (marked experimental), WithConnectParams() to return a DialOption to dial with the provided parameters. * Added new fields to the implementation of the exponential backoff in internal/backoff which mirror the ones in ConnectParams. * Marked existing APIs WithBackoffMaxDelay() and WithBackoffConfig() as deprecated. * Added a default exponential backoff implementation, for easy use of internal callers. --- backoff.go | 22 ++++++++++++++++++++++ balancer/grpclb/grpclb.go | 16 ++-------------- balancer/xds/lrs/lrs.go | 4 +--- balancer/xds/xds_client.go | 8 +------- clientconn.go | 4 +--- clientconn_test.go | 34 ++++++++++++++++++++++------------ dialoptions.go | 27 ++++++++++++++++++++++----- health/client.go | 26 +++++++++++++------------- internal/backoff/backoff.go | 22 ++++++++++++++++++---- resolver/dns/dns_resolver.go | 4 +++- 10 files changed, 105 insertions(+), 62 deletions(-) diff --git a/backoff.go b/backoff.go index 97c6e2568f4..4849c51ea2a 100644 --- a/backoff.go +++ b/backoff.go @@ -27,12 +27,34 @@ import ( // DefaultBackoffConfig uses values specified for backoff in // https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. +// +// Deprecated: use ConnectParams instead. var DefaultBackoffConfig = BackoffConfig{ MaxDelay: 120 * time.Second, } // BackoffConfig defines the parameters for the default gRPC backoff strategy. +// +// Deprecated: use ConnectParams instead. type BackoffConfig struct { // MaxDelay is the upper bound of backoff delay. MaxDelay time.Duration } + +// ConnectParams defines the parameters for connecting and retrying. Users +// are encouraged to use this instead of BackoffConfig. +// https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. +// +// This API is EXPERIMENTAL. +type ConnectParams struct { + // BackoffBaseDelay is the amount of time to backoff after the first + // connection failure. + BackoffBaseDelay time.Duration + // BackoffMultiplier is the factor with which to multiply backoffs after a + // failed retry. Should ideally be greater than 1. + BackoffMultiplier float64 + // BackoffJitter is the factor with which backoffs are randomized. + BackoffJitter float64 + // BackoffMaxDelay is the upper bound of backoff delay. + BackoffMaxDelay time.Duration +} diff --git a/balancer/grpclb/grpclb.go b/balancer/grpclb/grpclb.go index d881a9211b3..058f206cf7b 100644 --- a/balancer/grpclb/grpclb.go +++ b/balancer/grpclb/grpclb.go @@ -49,19 +49,7 @@ const ( grpclbName = "grpclb" ) -var ( - // defaultBackoffConfig configures the backoff strategy that's used when the - // init handshake in the RPC is unsuccessful. It's not for the clientconn - // reconnect backoff. - // - // It has the same value as the default grpc.DefaultBackoffConfig. - // - // TODO: make backoff configurable. - defaultBackoffConfig = backoff.Exponential{ - MaxDelay: 120 * time.Second, - } - errServerTerminatedConnection = errors.New("grpclb: failed to recv server list: server terminated connection") -) +var errServerTerminatedConnection = errors.New("grpclb: failed to recv server list: server terminated connection") func convertDuration(d *durationpb.Duration) time.Duration { if d == nil { @@ -155,7 +143,7 @@ func (b *lbBuilder) Build(cc balancer.ClientConn, opt balancer.BuildOptions) bal scStates: make(map[balancer.SubConn]connectivity.State), picker: &errPicker{err: balancer.ErrNoSubConnAvailable}, clientStats: newRPCStats(), - backoff: defaultBackoffConfig, // TODO: make backoff configurable. + backoff: backoff.DefaultExponential, // TODO: make backoff configurable. } var err error diff --git a/balancer/xds/lrs/lrs.go b/balancer/xds/lrs/lrs.go index 5b0ab562c65..8a7ecc93f3c 100644 --- a/balancer/xds/lrs/lrs.go +++ b/balancer/xds/lrs/lrs.go @@ -161,9 +161,7 @@ func NewStore(serviceName string) Store { }, }, }, - backoff: backoff.Exponential{ - MaxDelay: 120 * time.Second, - }, + backoff: backoff.DefaultExponential, lastReported: time.Now(), } } diff --git a/balancer/xds/xds_client.go b/balancer/xds/xds_client.go index d057f1ece88..163fb1469b7 100644 --- a/balancer/xds/xds_client.go +++ b/balancer/xds/xds_client.go @@ -46,12 +46,6 @@ const ( endpointRequired = "endpoints_required" ) -var ( - defaultBackoffConfig = backoff.Exponential{ - MaxDelay: 120 * time.Second, - } -) - // client is responsible for connecting to the specified traffic director, passing the received // ADS response from the traffic director, and sending notification when communication with the // traffic director is lost. @@ -287,7 +281,7 @@ func newXDSClient(balancerName string, enableCDS bool, opts balancer.BuildOption newADS: newADS, loseContact: loseContact, cleanup: exitCleanup, - backoff: defaultBackoffConfig, + backoff: backoff.DefaultExponential, loadStore: loadStore, } diff --git a/clientconn.go b/clientconn.go index a7643df7d29..08e736ac45f 100644 --- a/clientconn.go +++ b/clientconn.go @@ -235,9 +235,7 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn * } } if cc.dopts.bs == nil { - cc.dopts.bs = backoff.Exponential{ - MaxDelay: DefaultBackoffConfig.MaxDelay, - } + cc.dopts.bs = backoff.DefaultExponential } if cc.dopts.resolverBuilder == nil { // Only try to parse target when resolver builder is not already set. diff --git a/clientconn_test.go b/clientconn_test.go index 13727ee8a3a..58f8356b86c 100644 --- a/clientconn_test.go +++ b/clientconn_test.go @@ -655,22 +655,35 @@ func (s) TestCredentialsMisuse(t *testing.T) { } func (s) TestWithBackoffConfigDefault(t *testing.T) { - testBackoffConfigSet(t, &DefaultBackoffConfig) + testBackoffConfigSet(t, backoff.DefaultExponential) } func (s) TestWithBackoffConfig(t *testing.T) { b := BackoffConfig{MaxDelay: DefaultBackoffConfig.MaxDelay / 2} - expected := b - testBackoffConfigSet(t, &expected, WithBackoffConfig(b)) + wantBackoff := backoff.DefaultExponential + wantBackoff.MaxDelay = b.MaxDelay + testBackoffConfigSet(t, wantBackoff, WithBackoffConfig(b)) } func (s) TestWithBackoffMaxDelay(t *testing.T) { md := DefaultBackoffConfig.MaxDelay / 2 - expected := BackoffConfig{MaxDelay: md} - testBackoffConfigSet(t, &expected, WithBackoffMaxDelay(md)) + wantBackoff := backoff.DefaultExponential + wantBackoff.MaxDelay = md + testBackoffConfigSet(t, wantBackoff, WithBackoffMaxDelay(md)) } -func testBackoffConfigSet(t *testing.T, expected *BackoffConfig, opts ...DialOption) { +func (s) TestWithConnectParams(t *testing.T) { + bd := 2 * time.Second + mltpr := 2.0 + jitter := 0.0 + crt := ConnectParams{BackoffBaseDelay: bd, BackoffMultiplier: mltpr, BackoffJitter: jitter} + // MaxDelay is not set in the ConnectParams. So it should not be set on + // backoff.Exponential as well. + wantBackoff := backoff.Exponential{BaseDelay: bd, Multiplier: mltpr, Jitter: jitter} + testBackoffConfigSet(t, wantBackoff, WithConnectParams(crt)) +} + +func testBackoffConfigSet(t *testing.T, wantBackoff backoff.Exponential, opts ...DialOption) { opts = append(opts, WithInsecure()) conn, err := Dial("passthrough:///foo:80", opts...) if err != nil { @@ -682,16 +695,13 @@ func testBackoffConfigSet(t *testing.T, expected *BackoffConfig, opts ...DialOpt t.Fatalf("backoff config not set") } - actual, ok := conn.dopts.bs.(backoff.Exponential) + gotBackoff, ok := conn.dopts.bs.(backoff.Exponential) if !ok { t.Fatalf("unexpected type of backoff config: %#v", conn.dopts.bs) } - expectedValue := backoff.Exponential{ - MaxDelay: expected.MaxDelay, - } - if actual != expectedValue { - t.Fatalf("unexpected backoff config on connection: %v, want %v", actual, expected) + if gotBackoff != wantBackoff { + t.Fatalf("unexpected backoff config on connection: %v, want %v", gotBackoff, wantBackoff) } } diff --git a/dialoptions.go b/dialoptions.go index e8f34d0d6ea..162f9e52d42 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -246,8 +246,26 @@ func WithServiceConfig(c <-chan ServiceConfig) DialOption { }) } +// WithConnectParams configures the dialer to use the provided backoff +// parameters for the algorithm defined in +// https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. +// This will override all the default values with the ones provided here. So, +// use with caution. +// +// This API is EXPERIMENTAL. +func WithConnectParams(p ConnectParams) DialOption { + return withBackoff(backoff.Exponential{ + BaseDelay: p.BackoffBaseDelay, + Multiplier: p.BackoffMultiplier, + Jitter: p.BackoffJitter, + MaxDelay: p.BackoffMaxDelay, + }) +} + // WithBackoffMaxDelay configures the dialer to use the provided maximum delay // when backing off after failed connection attempts. +// +//Deprecated: use WithConnectParams instead. func WithBackoffMaxDelay(md time.Duration) DialOption { return WithBackoffConfig(BackoffConfig{MaxDelay: md}) } @@ -255,12 +273,11 @@ func WithBackoffMaxDelay(md time.Duration) DialOption { // WithBackoffConfig configures the dialer to use the provided backoff // parameters after connection failures. // -// Use WithBackoffMaxDelay until more parameters on BackoffConfig are opened up -// for use. +//Deprecated: use WithConnectParams instead. func WithBackoffConfig(b BackoffConfig) DialOption { - return withBackoff(backoff.Exponential{ - MaxDelay: b.MaxDelay, - }) + bs := backoff.DefaultExponential + bs.MaxDelay = b.MaxDelay + return withBackoff(bs) } // withBackoff sets the backoff strategy used for connectRetryNum after a failed diff --git a/health/client.go b/health/client.go index b43746e616c..02bb8a3f9cd 100644 --- a/health/client.go +++ b/health/client.go @@ -33,20 +33,20 @@ import ( "google.golang.org/grpc/status" ) -const maxDelay = 120 * time.Second - -var backoffStrategy = backoff.Exponential{MaxDelay: maxDelay} -var backoffFunc = func(ctx context.Context, retries int) bool { - d := backoffStrategy.Backoff(retries) - timer := time.NewTimer(d) - select { - case <-timer.C: - return true - case <-ctx.Done(): - timer.Stop() - return false +var ( + backoffStrategy = backoff.DefaultExponential + backoffFunc = func(ctx context.Context, retries int) bool { + d := backoffStrategy.Backoff(retries) + timer := time.NewTimer(d) + select { + case <-timer.C: + return true + case <-ctx.Done(): + timer.Stop() + return false + } } -} +) func init() { internal.HealthCheckFunc = clientHealthCheck diff --git a/internal/backoff/backoff.go b/internal/backoff/backoff.go index 1bd0cce5abd..fe12cb60fe8 100644 --- a/internal/backoff/backoff.go +++ b/internal/backoff/backoff.go @@ -37,6 +37,8 @@ type Strategy interface { Backoff(retries int) time.Duration } +// These constants correspond to the ones defined in +// https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. const ( // baseDelay is the amount of time to wait before retrying after the first // failure. @@ -45,11 +47,23 @@ const ( factor = 1.6 // jitter provides a range to randomize backoff delays. jitter = 0.2 + // maxDelay is the upper bound of backoff delay. + maxDelay = 120 * time.Second ) +var DefaultExponential = Exponential{BaseDelay: baseDelay, Multiplier: factor, Jitter: jitter, MaxDelay: maxDelay} + // Exponential implements exponential backoff algorithm as defined in // https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. type Exponential struct { + // BaseDelay is the amount of time to backoff after the first connection + // failure. + BaseDelay time.Duration + // Multiplier is the factor with which to multiply backoffs after a failed + // retry. + Multiplier float64 + // Jitter is the factor with which backoffs are randomized. + Jitter float64 // MaxDelay is the upper bound of backoff delay. MaxDelay time.Duration } @@ -58,11 +72,11 @@ type Exponential struct { // number of retries. func (bc Exponential) Backoff(retries int) time.Duration { if retries == 0 { - return baseDelay + return bc.BaseDelay } - backoff, max := float64(baseDelay), float64(bc.MaxDelay) + backoff, max := float64(bc.BaseDelay), float64(bc.MaxDelay) for backoff < max && retries > 0 { - backoff *= factor + backoff *= bc.Multiplier retries-- } if backoff > max { @@ -70,7 +84,7 @@ func (bc Exponential) Backoff(retries int) time.Duration { } // Randomize backoff delays so that if a cluster of requests start at // the same time, they won't operate in lockstep. - backoff *= 1 + jitter*(grpcrand.Float64()*2-1) + backoff *= 1 + bc.Jitter*(grpcrand.Float64()*2-1) if backoff < 0 { return 0 } diff --git a/resolver/dns/dns_resolver.go b/resolver/dns/dns_resolver.go index 297492e87af..151cd392466 100644 --- a/resolver/dns/dns_resolver.go +++ b/resolver/dns/dns_resolver.go @@ -126,9 +126,11 @@ func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts // DNS address (non-IP). ctx, cancel := context.WithCancel(context.Background()) + bs := backoff.DefaultExponential + bs.MaxDelay = b.minFreq d := &dnsResolver{ freq: b.minFreq, - backoff: backoff.Exponential{MaxDelay: b.minFreq}, + backoff: bs, host: host, port: port, ctx: ctx,