From 6536b7cd68d3ba2c4f84a113dd6da42bed8293e0 Mon Sep 17 00:00:00 2001 From: Vytenis Darulis Date: Mon, 9 Nov 2020 12:58:38 -0500 Subject: [PATCH] [etcd] Set reasonable cluster connection/sync settings by default (#2860) --- src/cluster/client/etcd/client.go | 6 +++-- src/cluster/client/etcd/config_test.go | 6 ++--- src/cluster/client/etcd/options.go | 28 +++++++++++++++---- src/cluster/client/etcd/options_test.go | 36 ++++++++++++++++++++----- src/cluster/client/etcd/types.go | 5 +++- 5 files changed, 63 insertions(+), 18 deletions(-) diff --git a/src/cluster/client/etcd/client.go b/src/cluster/client/etcd/client.go index c3218c2edf..88f85e8dee 100644 --- a/src/cluster/client/etcd/client.go +++ b/src/cluster/client/etcd/client.go @@ -39,8 +39,8 @@ import ( "github.com/m3db/m3/src/x/instrument" "github.com/m3db/m3/src/x/retry" - "go.etcd.io/etcd/clientv3" "github.com/uber-go/tally" + "go.etcd.io/etcd/clientv3" "go.uber.org/zap" ) @@ -282,9 +282,10 @@ func newClient(cluster Cluster) (*clientv3.Client, error) { return nil, err } cfg := clientv3.Config{ + AutoSyncInterval: cluster.AutoSyncInterval(), + DialTimeout: cluster.DialTimeout(), Endpoints: cluster.Endpoints(), TLS: tls, - AutoSyncInterval: cluster.AutoSyncInterval(), } if opts := cluster.KeepAliveOptions(); opts.KeepAliveEnabled() { @@ -296,6 +297,7 @@ func newClient(cluster Cluster) (*clientv3.Client, error) { } cfg.DialKeepAliveTime = keepAlivePeriod cfg.DialKeepAliveTimeout = opts.KeepAliveTimeout() + cfg.PermitWithoutStream = true } return clientv3.New(cfg) diff --git a/src/cluster/client/etcd/config_test.go b/src/cluster/client/etcd/config_test.go index 62c6af66eb..f2c5bb2f5a 100644 --- a/src/cluster/client/etcd/config_test.go +++ b/src/cluster/client/etcd/config_test.go @@ -138,9 +138,9 @@ m3sd: require.True(t, exists) keepAliveOpts = cluster2.KeepAliveOptions() require.Equal(t, true, keepAliveOpts.KeepAliveEnabled()) - require.Equal(t, 5*time.Minute, keepAliveOpts.KeepAlivePeriod()) - require.Equal(t, 5*time.Minute, keepAliveOpts.KeepAlivePeriodMaxJitter()) - require.Equal(t, 20*time.Second, keepAliveOpts.KeepAliveTimeout()) + require.Equal(t, 20*time.Second, keepAliveOpts.KeepAlivePeriod()) + require.Equal(t, 10*time.Second, keepAliveOpts.KeepAlivePeriodMaxJitter()) + require.Equal(t, 10*time.Second, keepAliveOpts.KeepAliveTimeout()) t.Run("TestOptionsNewDirectoryMode", func(t *testing.T) { opts := cfg.NewOptions() diff --git a/src/cluster/client/etcd/options.go b/src/cluster/client/etcd/options.go index e6e3c719de..566d240e10 100644 --- a/src/cluster/client/etcd/options.go +++ b/src/cluster/client/etcd/options.go @@ -36,10 +36,13 @@ import ( ) const ( + defaultAutoSyncInterval = 1 * time.Minute + defaultDialTimeout = 15 * time.Second + defaultKeepAliveEnabled = true - defaultKeepAlivePeriod = 5 * time.Minute - defaultKeepAlivePeriodMaxJitter = 5 * time.Minute - defaultKeepAliveTimeout = 20 * time.Second + defaultKeepAlivePeriod = 20 * time.Second + defaultKeepAlivePeriodMaxJitter = 10 * time.Second + defaultKeepAliveTimeout = 10 * time.Second defaultRetryInitialBackoff = 2 * time.Second defaultRetryBackoffFactor = 2.0 @@ -316,8 +319,10 @@ func (o options) NewDirectoryMode() os.FileMode { // NewCluster creates a Cluster. func NewCluster() Cluster { return cluster{ - keepAliveOpts: NewKeepAliveOptions(), - tlsOpts: NewTLSOptions(), + autoSyncInterval: defaultAutoSyncInterval, + dialTimeout: defaultDialTimeout, + keepAliveOpts: NewKeepAliveOptions(), + tlsOpts: NewTLSOptions(), } } @@ -327,6 +332,7 @@ type cluster struct { keepAliveOpts KeepAliveOptions tlsOpts TLSOptions autoSyncInterval time.Duration + dialTimeout time.Duration } func (c cluster) Zone() string { @@ -373,3 +379,15 @@ func (c cluster) SetAutoSyncInterval(autoSyncInterval time.Duration) Cluster { c.autoSyncInterval = autoSyncInterval return c } + +//nolint:gocritic +func (c cluster) DialTimeout() time.Duration { + return c.dialTimeout +} + +//nolint:gocritic +func (c cluster) SetDialTimeout(dialTimeout time.Duration) Cluster { + c.dialTimeout = dialTimeout + + return c +} diff --git a/src/cluster/client/etcd/options_test.go b/src/cluster/client/etcd/options_test.go index aee7366fab..befd638960 100644 --- a/src/cluster/client/etcd/options_test.go +++ b/src/cluster/client/etcd/options_test.go @@ -32,16 +32,22 @@ import ( ) func TestKeepAliveOptions(t *testing.T) { - opts := NewKeepAliveOptions(). + opts := NewKeepAliveOptions() + require.Equal(t, defaultKeepAliveEnabled, opts.KeepAliveEnabled()) + require.Equal(t, defaultKeepAlivePeriod, opts.KeepAlivePeriod()) + require.Equal(t, defaultKeepAlivePeriodMaxJitter, opts.KeepAlivePeriodMaxJitter()) + require.Equal(t, defaultKeepAliveTimeout, opts.KeepAliveTimeout()) + + opts = NewKeepAliveOptions(). SetKeepAliveEnabled(true). - SetKeepAlivePeriod(10 * time.Second). - SetKeepAlivePeriodMaxJitter(5 * time.Second). - SetKeepAliveTimeout(time.Second) + SetKeepAlivePeriod(1234 * time.Second). + SetKeepAlivePeriodMaxJitter(5000 * time.Second). + SetKeepAliveTimeout(time.Hour) require.Equal(t, true, opts.KeepAliveEnabled()) - require.Equal(t, 10*time.Second, opts.KeepAlivePeriod()) - require.Equal(t, 5*time.Second, opts.KeepAlivePeriodMaxJitter()) - require.Equal(t, time.Second, opts.KeepAliveTimeout()) + require.Equal(t, 1234*time.Second, opts.KeepAlivePeriod()) + require.Equal(t, 5000*time.Second, opts.KeepAlivePeriodMaxJitter()) + require.Equal(t, time.Hour, opts.KeepAliveTimeout()) } func TestCluster(t *testing.T) { @@ -63,6 +69,22 @@ func TestCluster(t *testing.T) { assert.Equal(t, "z", c.Zone()) assert.Equal(t, []string{"e1"}, c.Endpoints()) assert.Equal(t, aOpts, c.TLSOptions()) + assert.Equal(t, defaultAutoSyncInterval, c.AutoSyncInterval()) + assert.Equal(t, defaultDialTimeout, c.DialTimeout()) + + c = c.SetAutoSyncInterval(123 * time.Minute) + assert.Equal(t, "z", c.Zone()) + assert.Equal(t, []string{"e1"}, c.Endpoints()) + assert.Equal(t, aOpts, c.TLSOptions()) + assert.Equal(t, 123*time.Minute, c.AutoSyncInterval()) + assert.Equal(t, defaultDialTimeout, c.DialTimeout()) + + c = c.SetDialTimeout(42 * time.Hour) + assert.Equal(t, "z", c.Zone()) + assert.Equal(t, []string{"e1"}, c.Endpoints()) + assert.Equal(t, aOpts, c.TLSOptions()) + assert.Equal(t, 123*time.Minute, c.AutoSyncInterval()) + assert.Equal(t, 42*time.Hour, c.DialTimeout()) } func TestTLSOptions(t *testing.T) { diff --git a/src/cluster/client/etcd/types.go b/src/cluster/client/etcd/types.go index ffdc284bac..9c052d1e4f 100644 --- a/src/cluster/client/etcd/types.go +++ b/src/cluster/client/etcd/types.go @@ -129,6 +129,9 @@ type Cluster interface { TLSOptions() TLSOptions SetTLSOptions(TLSOptions) Cluster - SetAutoSyncInterval(value time.Duration) Cluster AutoSyncInterval() time.Duration + SetAutoSyncInterval(value time.Duration) Cluster + + DialTimeout() time.Duration + SetDialTimeout(value time.Duration) Cluster }