diff --git a/backoff.go b/backoff.go index 97c6e2568f4..ff7c3ee6f48 100644 --- a/backoff.go +++ b/backoff.go @@ -23,16 +23,36 @@ package grpc import ( "time" + + "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. 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. Will be supported throughout 1.x. 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 the BackoffConfig 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 { + // Backoff specifies the configuration options for connection backoff. + 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 new file mode 100644 index 00000000000..0787d0b50ce --- /dev/null +++ b/backoff/backoff.go @@ -0,0 +1,52 @@ +/* + * + * 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 backoff. +// +// 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. +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 +// 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. +var DefaultConfig = Config{ + BaseDelay: 1.0 * time.Second, + Multiplier: 1.6, + Jitter: 0.2, + MaxDelay: 120 * time.Second, +} 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..421aa499d91 100644 --- a/clientconn_test.go +++ b/clientconn_test.go @@ -30,9 +30,10 @@ import ( "time" "golang.org/x/net/http2" + "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" @@ -655,22 +656,39 @@ func (s) TestCredentialsMisuse(t *testing.T) { } func (s) TestWithBackoffConfigDefault(t *testing.T) { - testBackoffConfigSet(t, &DefaultBackoffConfig) + testBackoffConfigSet(t, internalbackoff.DefaultExponential) } func (s) TestWithBackoffConfig(t *testing.T) { b := BackoffConfig{MaxDelay: DefaultBackoffConfig.MaxDelay / 2} - expected := b - testBackoffConfigSet(t, &expected, WithBackoffConfig(b)) + bc := backoff.DefaultConfig + bc.MaxDelay = b.MaxDelay + wantBackoff := internalbackoff.Exponential{Config: bc} + testBackoffConfigSet(t, wantBackoff, WithBackoffConfig(b)) } func (s) TestWithBackoffMaxDelay(t *testing.T) { md := DefaultBackoffConfig.MaxDelay / 2 - expected := BackoffConfig{MaxDelay: md} - testBackoffConfigSet(t, &expected, WithBackoffMaxDelay(md)) + bc := backoff.DefaultConfig + bc.MaxDelay = md + wantBackoff := internalbackoff.Exponential{Config: bc} + 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 + 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 + // internalbackoff.Exponential as well. + wantBackoff := internalbackoff.Exponential{Config: bc} + testBackoffConfigSet(t, wantBackoff, WithConnectParams(crt)) +} + +func testBackoffConfigSet(t *testing.T, wantBackoff internalbackoff.Exponential, opts ...DialOption) { opts = append(opts, WithInsecure()) conn, err := Dial("passthrough:///foo:80", opts...) if err != nil { @@ -682,16 +700,27 @@ 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.(internalbackoff.Exponential) if !ok { t.Fatalf("unexpected type of backoff config: %#v", conn.dopts.bs) } - expectedValue := backoff.Exponential{ - MaxDelay: expected.MaxDelay, + if gotBackoff != wantBackoff { + t.Fatalf("unexpected backoff config on connection: %v, want %v", gotBackoff, wantBackoff) } - if actual != expectedValue { - t.Fatalf("unexpected backoff config on connection: %v, want %v", actual, expected) +} + +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) } } diff --git a/dialoptions.go b/dialoptions.go index e8f34d0d6ea..b7524f826fd 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -24,11 +24,12 @@ import ( "net" "time" + "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" @@ -47,7 +48,7 @@ type dialOptions struct { cp Compressor dc Decompressor - bs backoff.Strategy + bs internalbackoff.Strategy block bool insecure bool timeout time.Duration @@ -246,8 +247,28 @@ 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 = internalbackoff.Exponential{Config: p.Backoff} + o.minConnectTimeout = func() time.Duration { + return p.MinConnectTimeout + } + }) +} + // WithBackoffMaxDelay configures the dialer to use the provided maximum delay // when backing off after failed connection attempts. +// +// Deprecated: use WithConnectParams instead. Will be supported throughout 1.x. func WithBackoffMaxDelay(md time.Duration) DialOption { return WithBackoffConfig(BackoffConfig{MaxDelay: md}) } @@ -255,19 +276,18 @@ 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. Will be supported throughout 1.x. func WithBackoffConfig(b BackoffConfig) DialOption { - return withBackoff(backoff.Exponential{ - MaxDelay: b.MaxDelay, - }) + bc := backoff.DefaultConfig + bc.MaxDelay = b.MaxDelay + 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/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..5fc0ee3da53 100644 --- a/internal/backoff/backoff.go +++ b/internal/backoff/backoff.go @@ -25,44 +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 } -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 -) +// 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{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 { - // 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 baseDelay + return bc.Config.BaseDelay } - backoff, max := float64(baseDelay), float64(bc.MaxDelay) + backoff, max := float64(bc.Config.BaseDelay), float64(bc.Config.MaxDelay) for backoff < max && retries > 0 { - backoff *= factor + backoff *= bc.Config.Multiplier retries-- } if backoff > max { @@ -70,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 + 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 297492e87af..abc0f92ca57 100644 --- a/resolver/dns/dns_resolver.go +++ b/resolver/dns/dns_resolver.go @@ -32,8 +32,9 @@ import ( "sync" "time" + "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" ) @@ -126,9 +127,11 @@ func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts // DNS address (non-IP). ctx, cancel := context.WithCancel(context.Background()) + bc := backoff.DefaultConfig + bc.MaxDelay = b.minFreq d := &dnsResolver{ freq: b.minFreq, - backoff: backoff.Exponential{MaxDelay: b.minFreq}, + backoff: internalbackoff.Exponential{Config: bc}, host: host, port: port, ctx: ctx, @@ -200,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 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..b4bc6fbcee6 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. @@ -266,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, }