From 32275fcced7293528dd6cf94c1bc7d801f088c92 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Tue, 6 Aug 2019 15:14:56 -0700 Subject: [PATCH 01/10] 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 | 6 ----- 10 files changed, 104 insertions(+), 61 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/clientconn.go b/clientconn.go index 43ef2548132..0096794d088 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, diff --git a/xds/internal/balancer/lrs/lrs.go b/xds/internal/balancer/lrs/lrs.go index 4b704615557..604d8c3864a 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 4c783c1b223..34a3389ff21 100644 --- a/xds/internal/balancer/xds_client.go +++ b/xds/internal/balancer/xds_client.go @@ -48,12 +48,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. From f8b400300faf61f0877bc5e6ab22b99ddf013d8d Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Tue, 6 Aug 2019 15:21:37 -0700 Subject: [PATCH 02/10] 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 fe12cb60fe8..95c38ab8768 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 0f7db76c42559d8dea658ba1fe82cb477c7aefcc Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 30 Aug 2019 01:56:10 +0000 Subject: [PATCH 03/10] 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 4849c51ea2a..af58980a51f 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 00000000000..d59b3e60802 --- /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 58f8356b86c..ee6ed454d0d 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 162f9e52d42..5143fa207ee 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 95c38ab8768..5fc0ee3da53 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 151cd392466..a52ea08e470 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 0115f0029001f819c802594965ba25c082f04f9c Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 30 Aug 2019 02:00:00 +0000 Subject: [PATCH 04/10] 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 ee6ed454d0d..6402fb3b511 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 5143fa207ee..d4222773391 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 dea6444ad6d3d77bddf1d9648a11d93a940b9a90 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 30 Aug 2019 02:13:50 +0000 Subject: [PATCH 05/10] 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 af58980a51f..01758ecfc5e 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 d59b3e60802..3bb63f2ed14 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 a52ea08e470..cea099de650 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, From f5998d198ae439c35e38a6cbe7be024e1ee4c6d9 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Mon, 30 Sep 2019 22:35:33 +0000 Subject: [PATCH 06/10] Missed during merge conflict resolution. --- xds/internal/balancer/xds_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xds/internal/balancer/xds_client.go b/xds/internal/balancer/xds_client.go index 34a3389ff21..b4bc6fbcee6 100644 --- a/xds/internal/balancer/xds_client.go +++ b/xds/internal/balancer/xds_client.go @@ -260,7 +260,7 @@ func newXDSClient(balancerName string, enableCDS bool, opts balancer.BuildOption newADS: newADS, loseContact: loseContact, cleanup: exitCleanup, - backoff: defaultBackoffConfig, + backoff: backoff.DefaultExponential, loadStore: loadStore, } From 3fce38e2dd9cdf6b62f7a559a03a61b625dd68fd Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Wed, 2 Oct 2019 17:36:05 +0000 Subject: [PATCH 07/10] Review comments. --- backoff.go | 8 ++++---- backoff/backoff.go | 9 +++++---- clientconn_test.go | 24 ++++++++++++------------ dialoptions.go | 24 +++++++++++++++--------- resolver/dns/dns_resolver.go | 10 +++++----- 5 files changed, 41 insertions(+), 34 deletions(-) diff --git a/backoff.go b/backoff.go index 01758ecfc5e..ff7c3ee6f48 100644 --- a/backoff.go +++ b/backoff.go @@ -24,20 +24,20 @@ package grpc import ( "time" - grpcbackoff "google.golang.org/grpc/backoff" + "google.golang.org/grpc/backoff" ) // DefaultBackoffConfig uses values specified for backoff in // https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. // -// Deprecated: use ConnectParams instead. +// Deprecated: use ConnectParams instead. Will be supported throughout 1.x. var DefaultBackoffConfig = BackoffConfig{ MaxDelay: 120 * time.Second, } // BackoffConfig defines the parameters for the default gRPC backoff strategy. // -// Deprecated: use ConnectParams instead. +// Deprecated: use ConnectParams instead. Will be supported throughout 1.x. type BackoffConfig struct { // MaxDelay is the upper bound of backoff delay. MaxDelay time.Duration @@ -51,7 +51,7 @@ type BackoffConfig struct { // This API is EXPERIMENTAL. type ConnectParams struct { // Backoff specifies the configuration options for connection backoff. - Backoff grpcbackoff.Config + Backoff backoff.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 index 3bb63f2ed14..edcc5a4856c 100644 --- a/backoff/backoff.go +++ b/backoff/backoff.go @@ -16,14 +16,15 @@ * */ -// Package backoff provides configuration options for connection backoff. +// Package backoff provides configuration options for backoff. +// +// More details can be found at: +// https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. 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. +// Config defines the configuration options for backoff. // // This API is EXPERIMENTAL. type Config struct { diff --git a/clientconn_test.go b/clientconn_test.go index 6402fb3b511..421aa499d91 100644 --- a/clientconn_test.go +++ b/clientconn_test.go @@ -30,10 +30,10 @@ import ( "time" "golang.org/x/net/http2" - grpcbackoff "google.golang.org/grpc/backoff" + "google.golang.org/grpc/backoff" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials" - "google.golang.org/grpc/internal/backoff" + internalbackoff "google.golang.org/grpc/internal/backoff" "google.golang.org/grpc/internal/transport" "google.golang.org/grpc/keepalive" "google.golang.org/grpc/naming" @@ -656,22 +656,22 @@ func (s) TestCredentialsMisuse(t *testing.T) { } func (s) TestWithBackoffConfigDefault(t *testing.T) { - testBackoffConfigSet(t, backoff.DefaultExponential) + testBackoffConfigSet(t, internalbackoff.DefaultExponential) } func (s) TestWithBackoffConfig(t *testing.T) { b := BackoffConfig{MaxDelay: DefaultBackoffConfig.MaxDelay / 2} - bc := grpcbackoff.DefaultConfig + bc := backoff.DefaultConfig bc.MaxDelay = b.MaxDelay - wantBackoff := backoff.Exponential{Config: bc} + wantBackoff := internalbackoff.Exponential{Config: bc} testBackoffConfigSet(t, wantBackoff, WithBackoffConfig(b)) } func (s) TestWithBackoffMaxDelay(t *testing.T) { md := DefaultBackoffConfig.MaxDelay / 2 - bc := grpcbackoff.DefaultConfig + bc := backoff.DefaultConfig bc.MaxDelay = md - wantBackoff := backoff.Exponential{Config: bc} + wantBackoff := internalbackoff.Exponential{Config: bc} testBackoffConfigSet(t, wantBackoff, WithBackoffMaxDelay(md)) } @@ -679,16 +679,16 @@ func (s) TestWithConnectParams(t *testing.T) { bd := 2 * time.Second mltpr := 2.0 jitter := 0.0 - bc := grpcbackoff.Config{BaseDelay: bd, Multiplier: mltpr, Jitter: jitter} + bc := backoff.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{Config: bc} + // internalbackoff.Exponential as well. + wantBackoff := internalbackoff.Exponential{Config: bc} testBackoffConfigSet(t, wantBackoff, WithConnectParams(crt)) } -func testBackoffConfigSet(t *testing.T, wantBackoff backoff.Exponential, opts ...DialOption) { +func testBackoffConfigSet(t *testing.T, wantBackoff internalbackoff.Exponential, opts ...DialOption) { opts = append(opts, WithInsecure()) conn, err := Dial("passthrough:///foo:80", opts...) if err != nil { @@ -700,7 +700,7 @@ func testBackoffConfigSet(t *testing.T, wantBackoff backoff.Exponential, opts .. t.Fatalf("backoff config not set") } - gotBackoff, ok := conn.dopts.bs.(backoff.Exponential) + gotBackoff, ok := conn.dopts.bs.(internalbackoff.Exponential) if !ok { t.Fatalf("unexpected type of backoff config: %#v", conn.dopts.bs) } diff --git a/dialoptions.go b/dialoptions.go index d4222773391..77acecc4d45 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -24,12 +24,12 @@ import ( "net" "time" - grpcbackoff "google.golang.org/grpc/backoff" + "google.golang.org/grpc/backoff" "google.golang.org/grpc/balancer" "google.golang.org/grpc/credentials" "google.golang.org/grpc/grpclog" "google.golang.org/grpc/internal" - "google.golang.org/grpc/internal/backoff" + internalbackoff "google.golang.org/grpc/internal/backoff" "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/transport" "google.golang.org/grpc/keepalive" @@ -48,7 +48,7 @@ type dialOptions struct { cp Compressor dc Decompressor - bs backoff.Strategy + bs internalbackoff.Strategy block bool insecure bool timeout time.Duration @@ -248,11 +248,17 @@ func WithServiceConfig(c <-chan ServiceConfig) DialOption { } // WithConnectParams configures the dialer to use the provided ConnectParams. + +// The backoff configuration specified as part of the ConnectParams overrides +// all defaults specified in +// https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. Consider +// using the backoff.DefaultConfig as a base, in cases where you want to +// override only a subset of the backoff configuration. // // This API is EXPERIMENTAL. func WithConnectParams(p ConnectParams) DialOption { return newFuncDialOption(func(o *dialOptions) { - o.bs = backoff.Exponential{Config: p.Backoff} + o.bs = internalbackoff.Exponential{Config: p.Backoff} o.minConnectTimeout = func() time.Duration { return p.MinConnectTimeout } @@ -262,7 +268,7 @@ func WithConnectParams(p ConnectParams) DialOption { // WithBackoffMaxDelay configures the dialer to use the provided maximum delay // when backing off after failed connection attempts. // -//Deprecated: use WithConnectParams instead. +// Deprecated: use WithConnectParams instead. Will be supported throughout 1.x. func WithBackoffMaxDelay(md time.Duration) DialOption { return WithBackoffConfig(BackoffConfig{MaxDelay: md}) } @@ -270,18 +276,18 @@ func WithBackoffMaxDelay(md time.Duration) DialOption { // WithBackoffConfig configures the dialer to use the provided backoff // parameters after connection failures. // -//Deprecated: use WithConnectParams instead. +//Deprecated: use WithConnectParams instead. Will be supported throughout 1.x. func WithBackoffConfig(b BackoffConfig) DialOption { - bc := grpcbackoff.DefaultConfig + bc := backoff.DefaultConfig bc.MaxDelay = b.MaxDelay - return withBackoff(backoff.Exponential{Config: bc}) + return withBackoff(internalbackoff.Exponential{Config: bc}) } // withBackoff sets the backoff strategy used for connectRetryNum after a failed // connection attempt. // // This can be exported if arbitrary backoff strategies are allowed by gRPC. -func withBackoff(bs backoff.Strategy) DialOption { +func withBackoff(bs internalbackoff.Strategy) DialOption { return newFuncDialOption(func(o *dialOptions) { o.bs = bs }) diff --git a/resolver/dns/dns_resolver.go b/resolver/dns/dns_resolver.go index cea099de650..abc0f92ca57 100644 --- a/resolver/dns/dns_resolver.go +++ b/resolver/dns/dns_resolver.go @@ -32,9 +32,9 @@ import ( "sync" "time" - grpcbackoff "google.golang.org/grpc/backoff" + "google.golang.org/grpc/backoff" "google.golang.org/grpc/grpclog" - "google.golang.org/grpc/internal/backoff" + internalbackoff "google.golang.org/grpc/internal/backoff" "google.golang.org/grpc/internal/grpcrand" "google.golang.org/grpc/resolver" ) @@ -127,11 +127,11 @@ func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts // DNS address (non-IP). ctx, cancel := context.WithCancel(context.Background()) - bc := grpcbackoff.DefaultConfig + bc := backoff.DefaultConfig bc.MaxDelay = b.minFreq d := &dnsResolver{ freq: b.minFreq, - backoff: backoff.Exponential{Config: bc}, + backoff: internalbackoff.Exponential{Config: bc}, host: host, port: port, ctx: ctx, @@ -203,7 +203,7 @@ func (i *ipResolver) watcher() { // dnsResolver watches for the name resolution update for a non-IP target. type dnsResolver struct { freq time.Duration - backoff backoff.Exponential + backoff internalbackoff.Exponential retryCount int host string port string From e1a1470a357112f9a559fc4caa89fc1f64a16aa7 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Wed, 2 Oct 2019 17:37:15 +0000 Subject: [PATCH 08/10] Make vet happy. --- dialoptions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialoptions.go b/dialoptions.go index 77acecc4d45..b23c98d6ed8 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -248,7 +248,7 @@ func WithServiceConfig(c <-chan ServiceConfig) DialOption { } // WithConnectParams configures the dialer to use the provided ConnectParams. - +// // The backoff configuration specified as part of the ConnectParams overrides // all defaults specified in // https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. Consider From ebebd7da0fe5a75bada5421fad2c433d2cba9c01 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Thu, 3 Oct 2019 17:18:00 +0000 Subject: [PATCH 09/10] Change a weird character masquerading as a space. --- dialoptions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialoptions.go b/dialoptions.go index b23c98d6ed8..b7524f826fd 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -276,7 +276,7 @@ func WithBackoffMaxDelay(md time.Duration) DialOption { // WithBackoffConfig configures the dialer to use the provided backoff // parameters after connection failures. // -//Deprecated: use WithConnectParams instead. Will be supported throughout 1.x. +// Deprecated: use WithConnectParams instead. Will be supported throughout 1.x. func WithBackoffConfig(b BackoffConfig) DialOption { bc := backoff.DefaultConfig bc.MaxDelay = b.MaxDelay From cc7872c09b1db556197392c5f83c1d55be174f3c Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Thu, 3 Oct 2019 22:55:53 +0000 Subject: [PATCH 10/10] Review comments. --- backoff/backoff.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/backoff/backoff.go b/backoff/backoff.go index edcc5a4856c..0787d0b50ce 100644 --- a/backoff/backoff.go +++ b/backoff/backoff.go @@ -20,13 +20,13 @@ // // More details can be found at: // https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md. +// +// All APIs in this package are experimental. package backoff import "time" // Config defines the configuration options for backoff. -// -// This API is EXPERIMENTAL. type Config struct { // BaseDelay is the amount of time to backoff after the first failure. BaseDelay time.Duration @@ -44,8 +44,6 @@ type Config struct { // // 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,