From ab8d9e8c97f5cab7335f3a38754f830941c526e1 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Tue, 6 Aug 2019 15:14:56 -0700 Subject: [PATCH 1/5] 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 ++------------ 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 +++- xds/internal/balancer/lrs/lrs.go | 4 +--- xds/internal/balancer/xds_client.go | 8 +------ 10 files changed, 105 insertions(+), 62 deletions(-) diff --git a/backoff.go b/backoff.go index 97c6e2568f45..4849c51ea2ae 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 d881a9211b38..058f206cf7bf 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/clientconn.go b/clientconn.go index a7643df7d297..08e736ac45f4 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 13727ee8a3a6..58f8356b86c0 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 e8f34d0d6eaa..162f9e52d424 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 b43746e616cf..02bb8a3f9cd3 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 1bd0cce5abdf..fe12cb60fe8e 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 297492e87af4..151cd3924664 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, diff --git a/xds/internal/balancer/lrs/lrs.go b/xds/internal/balancer/lrs/lrs.go index 4b7046155578..604d8c3864a6 100644 --- a/xds/internal/balancer/lrs/lrs.go +++ b/xds/internal/balancer/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/xds/internal/balancer/xds_client.go b/xds/internal/balancer/xds_client.go index 14912dd9b374..5db46b009dcd 100644 --- a/xds/internal/balancer/xds_client.go +++ b/xds/internal/balancer/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, } From 1c683d2776a3be4d35365ebb9122b0f97f36c30a Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Tue, 6 Aug 2019 15:21:37 -0700 Subject: [PATCH 2/5] Add comment for exported var DefaultExponential. --- internal/backoff/backoff.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/backoff/backoff.go b/internal/backoff/backoff.go index fe12cb60fe8e..95c38ab8768d 100644 --- a/internal/backoff/backoff.go +++ b/internal/backoff/backoff.go @@ -51,6 +51,9 @@ const ( maxDelay = 120 * time.Second ) +// DefaultExponential is an exponential backoff implementation using the +// default values for all the configurable knobs defined in +// https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. var DefaultExponential = Exponential{BaseDelay: baseDelay, Multiplier: factor, Jitter: jitter, MaxDelay: maxDelay} // Exponential implements exponential backoff algorithm as defined in From 000fdc7a85a5ebf2bf9b35df91543ce7bea9b11d Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 30 Aug 2019 01:56:10 +0000 Subject: [PATCH 3/5] Handle review comments. Added a new backoff package which defines the backoff configuration options, and is used by both the grpc package and the internal/backoff package. This allows us to have all backoff related options in a separate package. --- backoff.go | 22 +++++++-------- backoff/backoff.go | 53 ++++++++++++++++++++++++++++++++++++ clientconn_test.go | 31 +++++++++++++++++---- dialoptions.go | 23 +++++++--------- internal/backoff/backoff.go | 38 ++++++-------------------- resolver/dns/dns_resolver.go | 7 +++-- 6 files changed, 110 insertions(+), 64 deletions(-) create mode 100644 backoff/backoff.go diff --git a/backoff.go b/backoff.go index 4849c51ea2ae..af58980a51f3 100644 --- a/backoff.go +++ b/backoff.go @@ -23,6 +23,8 @@ package grpc import ( "time" + + grpcbackoff "google.golang.org/grpc/backoff" ) // DefaultBackoffConfig uses values specified for backoff in @@ -41,20 +43,16 @@ type BackoffConfig struct { MaxDelay time.Duration } -// ConnectParams defines the parameters for connecting and retrying. Users -// are encouraged to use this instead of BackoffConfig. +// ConnectParams defines the parameters for connecting and retrying. Users are +// encouraged to use this instead of BackoffConfig the type defined above. See +// here for more details: // 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 + // Backoff specifies the configuration options for connection backoff. + Backoff grpcbackoff.Config + // MinConnectTimeout is the minimum amount of time we are willing to give a + // connection to complete. + MinConnectTimeout time.Duration } diff --git a/backoff/backoff.go b/backoff/backoff.go new file mode 100644 index 000000000000..d59b3e60802f --- /dev/null +++ b/backoff/backoff.go @@ -0,0 +1,53 @@ +/* + * + * Copyright 2019 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +// Package backoff provides configuration options for connection backoff. +package backoff + +import "time" + +// Config defines the configuration options for connection backoff. More +// details can be found at +// https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. +// +// This API is EXPERIMENTAL. +type Config struct { + // BaseDelay is the amount of time to backoff after the first failure. + BaseDelay time.Duration + // Multiplier is the factor with which to multiply backoffs after a + // failed retry. Should ideally be greater than 1. + Multiplier float64 + // Jitter is the factor with which backoffs are randomized. + Jitter float64 + // MaxDelay is the upper bound of backoff delay. + MaxDelay time.Duration +} + +// DefaultConfig is a backoff configuration with the default values specfied +// in https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. +// +// This should be useful for callers who want to configure backoff with +// non-default values only for a subset of the options. +// +// This API is EXPERIMENTAL. +var DefaultConfig = Config{ + BaseDelay: 1.0 * time.Second, + Multiplier: 1.6, + Jitter: 0.2, + MaxDelay: 120 * time.Second, +} diff --git a/clientconn_test.go b/clientconn_test.go index 58f8356b86c0..ee6ed454d0d4 100644 --- a/clientconn_test.go +++ b/clientconn_test.go @@ -30,6 +30,7 @@ import ( "time" "golang.org/x/net/http2" + grpcbackoff "google.golang.org/grpc/backoff" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials" "google.golang.org/grpc/internal/backoff" @@ -660,15 +661,17 @@ func (s) TestWithBackoffConfigDefault(t *testing.T) { func (s) TestWithBackoffConfig(t *testing.T) { b := BackoffConfig{MaxDelay: DefaultBackoffConfig.MaxDelay / 2} - wantBackoff := backoff.DefaultExponential - wantBackoff.MaxDelay = b.MaxDelay + bc := grpcbackoff.DefaultConfig + bc.MaxDelay = b.MaxDelay + wantBackoff := backoff.Exponential{bc} testBackoffConfigSet(t, wantBackoff, WithBackoffConfig(b)) } func (s) TestWithBackoffMaxDelay(t *testing.T) { md := DefaultBackoffConfig.MaxDelay / 2 - wantBackoff := backoff.DefaultExponential - wantBackoff.MaxDelay = md + bc := grpcbackoff.DefaultConfig + bc.MaxDelay = md + wantBackoff := backoff.Exponential{bc} testBackoffConfigSet(t, wantBackoff, WithBackoffMaxDelay(md)) } @@ -676,10 +679,12 @@ func (s) TestWithConnectParams(t *testing.T) { bd := 2 * time.Second mltpr := 2.0 jitter := 0.0 - crt := ConnectParams{BackoffBaseDelay: bd, BackoffMultiplier: mltpr, BackoffJitter: jitter} + bc := grpcbackoff.Config{BaseDelay: bd, Multiplier: mltpr, Jitter: jitter} + + crt := ConnectParams{Backoff: bc} // 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} + wantBackoff := backoff.Exponential{bc} testBackoffConfigSet(t, wantBackoff, WithConnectParams(crt)) } @@ -705,6 +710,20 @@ func testBackoffConfigSet(t *testing.T, wantBackoff backoff.Exponential, opts .. } } +func (s) TestConnectParamsWithMinConnectTimeout(t *testing.T) { + // Default value specified for minConnectTimeout in the spec is 20 seconds. + mct := 1 * time.Minute + conn, err := Dial("passthrough:///foo:80", WithInsecure(), WithConnectParams(ConnectParams{MinConnectTimeout: mct})) + if err != nil { + t.Fatalf("unexpected error dialing connection: %v", err) + } + defer conn.Close() + + if got := conn.dopts.minConnectTimeout(); got != mct { + t.Errorf("unexpect minConnectTimeout on the connection: %v, want %v", got, mct) + } +} + // emptyBalancer returns an empty set of servers. type emptyBalancer struct { ch chan []Address diff --git a/dialoptions.go b/dialoptions.go index 162f9e52d424..5143fa207eef 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -24,6 +24,7 @@ import ( "net" "time" + grpcbackoff "google.golang.org/grpc/backoff" "google.golang.org/grpc/balancer" "google.golang.org/grpc/credentials" "google.golang.org/grpc/grpclog" @@ -246,19 +247,15 @@ 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. +// WithConnectParams configures the dialer to use the provided ConnectParams. // // 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, + return newFuncDialOption(func(o *dialOptions) { + o.bs = backoff.Exponential{p.Backoff} + o.minConnectTimeout = func() time.Duration { + return p.MinConnectTimeout + } }) } @@ -275,9 +272,9 @@ func WithBackoffMaxDelay(md time.Duration) DialOption { // //Deprecated: use WithConnectParams instead. func WithBackoffConfig(b BackoffConfig) DialOption { - bs := backoff.DefaultExponential - bs.MaxDelay = b.MaxDelay - return withBackoff(bs) + bc := grpcbackoff.DefaultConfig + bc.MaxDelay = b.MaxDelay + return withBackoff(backoff.Exponential{bc}) } // withBackoff sets the backoff strategy used for connectRetryNum after a failed diff --git a/internal/backoff/backoff.go b/internal/backoff/backoff.go index 95c38ab8768d..5fc0ee3da53b 100644 --- a/internal/backoff/backoff.go +++ b/internal/backoff/backoff.go @@ -25,61 +25,39 @@ package backoff import ( "time" + grpcbackoff "google.golang.org/grpc/backoff" "google.golang.org/grpc/internal/grpcrand" ) // Strategy defines the methodology for backing off after a grpc connection // failure. -// type Strategy interface { // Backoff returns the amount of time to wait before the next retry given // the number of consecutive failures. 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. - baseDelay = 1.0 * time.Second - // factor is applied to the backoff after each retry. - 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 -) - // DefaultExponential is an exponential backoff implementation using the // default values for all the configurable knobs defined in // https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. -var DefaultExponential = Exponential{BaseDelay: baseDelay, Multiplier: factor, Jitter: jitter, MaxDelay: maxDelay} +var DefaultExponential = Exponential{Config: grpcbackoff.DefaultConfig} // 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 + // Config contains all options to configure the backoff algorithm. + Config grpcbackoff.Config } // Backoff returns the amount of time to wait before the next retry given the // number of retries. func (bc Exponential) Backoff(retries int) time.Duration { if retries == 0 { - return bc.BaseDelay + return bc.Config.BaseDelay } - backoff, max := float64(bc.BaseDelay), float64(bc.MaxDelay) + backoff, max := float64(bc.Config.BaseDelay), float64(bc.Config.MaxDelay) for backoff < max && retries > 0 { - backoff *= bc.Multiplier + backoff *= bc.Config.Multiplier retries-- } if backoff > max { @@ -87,7 +65,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 + bc.Jitter*(grpcrand.Float64()*2-1) + backoff *= 1 + bc.Config.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 151cd3924664..a52ea08e470a 100644 --- a/resolver/dns/dns_resolver.go +++ b/resolver/dns/dns_resolver.go @@ -32,6 +32,7 @@ import ( "sync" "time" + grpcbackoff "google.golang.org/grpc/backoff" "google.golang.org/grpc/grpclog" "google.golang.org/grpc/internal/backoff" "google.golang.org/grpc/internal/grpcrand" @@ -126,11 +127,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 + bc := grpcbackoff.DefaultConfig + bc.MaxDelay = b.minFreq d := &dnsResolver{ freq: b.minFreq, - backoff: bs, + backoff: backoff.Exponential{bc}, host: host, port: port, ctx: ctx, From 097dd8e2d8776393d6f6e52c4565d6c674d078a0 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 30 Aug 2019 02:00:00 +0000 Subject: [PATCH 4/5] Make vet happy. --- clientconn_test.go | 6 +++--- dialoptions.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clientconn_test.go b/clientconn_test.go index ee6ed454d0d4..6402fb3b5117 100644 --- a/clientconn_test.go +++ b/clientconn_test.go @@ -663,7 +663,7 @@ func (s) TestWithBackoffConfig(t *testing.T) { b := BackoffConfig{MaxDelay: DefaultBackoffConfig.MaxDelay / 2} bc := grpcbackoff.DefaultConfig bc.MaxDelay = b.MaxDelay - wantBackoff := backoff.Exponential{bc} + wantBackoff := backoff.Exponential{Config: bc} testBackoffConfigSet(t, wantBackoff, WithBackoffConfig(b)) } @@ -671,7 +671,7 @@ func (s) TestWithBackoffMaxDelay(t *testing.T) { md := DefaultBackoffConfig.MaxDelay / 2 bc := grpcbackoff.DefaultConfig bc.MaxDelay = md - wantBackoff := backoff.Exponential{bc} + wantBackoff := backoff.Exponential{Config: bc} testBackoffConfigSet(t, wantBackoff, WithBackoffMaxDelay(md)) } @@ -684,7 +684,7 @@ func (s) TestWithConnectParams(t *testing.T) { crt := ConnectParams{Backoff: bc} // MaxDelay is not set in the ConnectParams. So it should not be set on // backoff.Exponential as well. - wantBackoff := backoff.Exponential{bc} + wantBackoff := backoff.Exponential{Config: bc} testBackoffConfigSet(t, wantBackoff, WithConnectParams(crt)) } diff --git a/dialoptions.go b/dialoptions.go index 5143fa207eef..d4222773391c 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -252,7 +252,7 @@ func WithServiceConfig(c <-chan ServiceConfig) DialOption { // This API is EXPERIMENTAL. func WithConnectParams(p ConnectParams) DialOption { return newFuncDialOption(func(o *dialOptions) { - o.bs = backoff.Exponential{p.Backoff} + o.bs = backoff.Exponential{Config: p.Backoff} o.minConnectTimeout = func() time.Duration { return p.MinConnectTimeout } @@ -274,7 +274,7 @@ func WithBackoffMaxDelay(md time.Duration) DialOption { func WithBackoffConfig(b BackoffConfig) DialOption { bc := grpcbackoff.DefaultConfig bc.MaxDelay = b.MaxDelay - return withBackoff(backoff.Exponential{bc}) + return withBackoff(backoff.Exponential{Config: bc}) } // withBackoff sets the backoff strategy used for connectRetryNum after a failed From 4d26f5e207b8d7d039bda1f77770f1e7b4e6945d Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 30 Aug 2019 02:13:50 +0000 Subject: [PATCH 5/5] vets and comments --- backoff.go | 2 +- backoff/backoff.go | 2 +- resolver/dns/dns_resolver.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backoff.go b/backoff.go index af58980a51f3..01758ecfc5e5 100644 --- a/backoff.go +++ b/backoff.go @@ -44,7 +44,7 @@ type BackoffConfig struct { } // ConnectParams defines the parameters for connecting and retrying. Users are -// encouraged to use this instead of BackoffConfig the type defined above. See +// encouraged to use this instead of the BackoffConfig type defined above. See // here for more details: // https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. // diff --git a/backoff/backoff.go b/backoff/backoff.go index d59b3e60802f..3bb63f2ed14a 100644 --- a/backoff/backoff.go +++ b/backoff/backoff.go @@ -39,7 +39,7 @@ type Config struct { } // DefaultConfig is a backoff configuration with the default values specfied -// in https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. +// at https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. // // This should be useful for callers who want to configure backoff with // non-default values only for a subset of the options. diff --git a/resolver/dns/dns_resolver.go b/resolver/dns/dns_resolver.go index a52ea08e470a..cea099de6500 100644 --- a/resolver/dns/dns_resolver.go +++ b/resolver/dns/dns_resolver.go @@ -131,7 +131,7 @@ func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts bc.MaxDelay = b.minFreq d := &dnsResolver{ freq: b.minFreq, - backoff: backoff.Exponential{bc}, + backoff: backoff.Exponential{Config: bc}, host: host, port: port, ctx: ctx,