[etcd] Set reasonable cluster connection/sync settings by default (#2860

)

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)

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()

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
+}

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) {

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
 }