Skip to content

Commit

Permalink
Rollback changes made as part of grpc#2735.
Browse files Browse the repository at this point in the history
I need to roll these back because I had sent out changes from my master
branch, and this is causing a lot of pain right now for my work on other
branches. I will create a branch for this issue and will send out these
changes in a fresh PR.
  • Loading branch information
easwars committed Aug 6, 2019
1 parent a1307ea commit b084e74
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 227 deletions.
23 changes: 0 additions & 23 deletions backoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,35 +27,12 @@ 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
// MinConnectTimeout is the minimum amount of time we are willing to give a
// connection to complete.
MinConnectTimeout time.Duration
}
8 changes: 5 additions & 3 deletions balancer/grpclb/grpclb.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,16 @@ const (
)

var (
// defaultBackoffStrategy configures the backoff strategy that's used when the
// 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.
defaultBackoffStrategy = backoff.NewExponentialBuilder().Build()
defaultBackoffConfig = backoff.Exponential{
MaxDelay: 120 * time.Second,
}
errServerTerminatedConnection = errors.New("grpclb: failed to recv server list: server terminated connection")
)

Expand Down Expand Up @@ -153,7 +155,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: defaultBackoffStrategy, // TODO: make backoff configurable.
backoff: defaultBackoffConfig, // TODO: make backoff configurable.
}

var err error
Expand Down
8 changes: 6 additions & 2 deletions balancer/xds/xds_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ const (
endpointRequired = "endpoints_required"
)

var defaultBackoffStrategy = backoff.NewExponentialBuilder().Build()
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
Expand Down Expand Up @@ -251,7 +255,7 @@ func newXDSClient(balancerName string, serviceName string, enableCDS bool, opts
newADS: newADS,
loseContact: loseContact,
cleanup: exitCleanup,
backoff: defaultBackoffStrategy,
backoff: defaultBackoffConfig,
}

c.ctx, c.cancel = context.WithCancel(context.Background())
Expand Down
8 changes: 6 additions & 2 deletions clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ import (
)

const (
// minimum time to give a connection to complete
minConnectTimeout = 20 * time.Second
// must match grpclbName in grpclb/grpclb.go
grpclbName = "grpclb"
)
Expand Down Expand Up @@ -233,7 +235,9 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *
}
}
if cc.dopts.bs == nil {
cc.dopts.bs = backoff.NewExponentialBuilder().Build()
cc.dopts.bs = backoff.Exponential{
MaxDelay: DefaultBackoffConfig.MaxDelay,
}
}
if cc.dopts.resolverBuilder == nil {
// Only try to parse target when resolver builder is not already set.
Expand Down Expand Up @@ -1003,7 +1007,7 @@ func (ac *addrConn) resetTransport() {
addrs := ac.addrs
backoffFor := ac.dopts.bs.Backoff(ac.backoffIdx)
// This will be the duration that dial gets to finish.
dialDuration := ac.dopts.mct
dialDuration := minConnectTimeout
if ac.dopts.minConnectTimeout != nil {
dialDuration = ac.dopts.minConnectTimeout()
}
Expand Down
34 changes: 14 additions & 20 deletions clientconn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,31 +655,22 @@ func (s) TestCredentialsMisuse(t *testing.T) {
}

func (s) TestWithBackoffConfigDefault(t *testing.T) {
testBackoffStrategySet(t, backoff.NewExponentialBuilder().Build())
testBackoffConfigSet(t, &DefaultBackoffConfig)
}

func (s) TestWithBackoffConfig(t *testing.T) {
md := DefaultBackoffConfig.MaxDelay / 2
expected := backoff.NewExponentialBuilder().MaxDelay(md).Build()
testBackoffStrategySet(t, expected, WithBackoffConfig(BackoffConfig{MaxDelay: md}))
b := BackoffConfig{MaxDelay: DefaultBackoffConfig.MaxDelay / 2}
expected := b
testBackoffConfigSet(t, &expected, WithBackoffConfig(b))
}

func (s) TestWithBackoffMaxDelay(t *testing.T) {
md := DefaultBackoffConfig.MaxDelay / 2
expected := backoff.NewExponentialBuilder().MaxDelay(md).Build()
testBackoffStrategySet(t, expected, WithBackoffMaxDelay(md))
}

func (s) TestWithConnectParams(t *testing.T) {
bd := 2 * time.Second
mltpr := 2.0
jitter := 0.0
crt := ConnectParams{BackoffBaseDelay: bd, BackoffMultiplier: mltpr, BackoffJitter: jitter}
expected := backoff.NewExponentialBuilder().BaseDelay(bd).Multiplier(mltpr).Jitter(jitter).MaxDelay(time.Duration(0)).Build()
testBackoffStrategySet(t, expected, WithConnectParams(crt))
expected := BackoffConfig{MaxDelay: md}
testBackoffConfigSet(t, &expected, WithBackoffMaxDelay(md))
}

func testBackoffStrategySet(t *testing.T, expected backoff.Strategy, opts ...DialOption) {
func testBackoffConfigSet(t *testing.T, expected *BackoffConfig, opts ...DialOption) {
opts = append(opts, WithInsecure())
conn, err := Dial("passthrough:///foo:80", opts...)
if err != nil {
Expand All @@ -688,16 +679,19 @@ func testBackoffStrategySet(t *testing.T, expected backoff.Strategy, opts ...Dia
defer conn.Close()

if conn.dopts.bs == nil {
t.Fatalf("backoff strategy not set")
t.Fatalf("backoff config not set")
}

actual, ok := conn.dopts.bs.(backoff.Exponential)
if !ok {
t.Fatalf("unexpected type of backoff strategy: %#v", conn.dopts.bs)
t.Fatalf("unexpected type of backoff config : %#v", conn.dopts.bs)
}

if actual != expected.(backoff.Exponential) {
t.Errorf("unexpected backoff strategy on connection: %v, want %v", actual, expected)
expectedValue := backoff.Exponential{
MaxDelay: expected.MaxDelay,
}
if actual != expectedValue {
t.Fatalf("unexpected backoff config on connection: %v, want %v", actual, expected)
}
}

Expand Down
32 changes: 5 additions & 27 deletions dialoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@ import (
"google.golang.org/grpc/stats"
)

const (
// Minimum time to give a connection to complete.
minConnectTimeout = 20 * time.Second
)

// dialOptions configure a Dial call. dialOptions are set by the DialOption
// values passed to Dial.
type dialOptions struct {
Expand Down Expand Up @@ -73,7 +68,6 @@ type dialOptions struct {
minConnectTimeout func() time.Duration
defaultServiceConfig *ServiceConfig // defaultServiceConfig is parsed from defaultServiceConfigRawJSON.
defaultServiceConfigRawJSON *string
mct time.Duration
}

// DialOption configures how we set up the connection.
Expand Down Expand Up @@ -252,36 +246,21 @@ 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 {
b := backoff.NewExponentialBuilder()
b.BaseDelay(p.BackoffBaseDelay).Multiplier(p.BackoffMultiplier).Jitter(p.BackoffJitter).MaxDelay(p.BackoffMaxDelay)
return newFuncDialOption(func(o *dialOptions) {
o.bs = b.Build()
o.mct = p.MinConnectTimeout
})
}

// 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})
}

// WithBackoffConfig configures the dialer to use the provided backoff
// parameters after connection failures.
//
// Deprecated: use WithConnectParams instead.
// Use WithBackoffMaxDelay until more parameters on BackoffConfig are opened up
// for use.
func WithBackoffConfig(b BackoffConfig) DialOption {
return withBackoff(backoff.NewExponentialBuilder().MaxDelay(b.MaxDelay).Build())
return withBackoff(backoff.Exponential{
MaxDelay: b.MaxDelay,
})
}

// withBackoff sets the backoff strategy used for connectRetryNum after a failed
Expand Down Expand Up @@ -560,7 +539,6 @@ func defaultDialOptions() dialOptions {
WriteBufferSize: defaultWriteBufSize,
ReadBufferSize: defaultReadBufSize,
},
mct: minConnectTimeout,
}
}

Expand Down
4 changes: 3 additions & 1 deletion health/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ import (
"google.golang.org/grpc/status"
)

var backoffStrategy = backoff.NewExponentialBuilder().Build()
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)
Expand Down

0 comments on commit b084e74

Please sign in to comment.