Merge pull request #116 from go-redis/fix/reduce-options

Reduce number of various options structs. Slightly reword comments.

cluster.go

@@ -267,20 +267,28 @@ func (c *ClusterClient) reaper() {
 
 //------------------------------------------------------------------------------
 
+// ClusterOptions are used to configure a cluster client and should be
+// passed to NewClusterClient.
 type ClusterOptions struct {
-	// A seed-list of host:port addresses of known cluster nodes
+	// A seed list of host:port addresses of cluster nodes.
 	Addrs []string
 
-	// An optional password
+	// The maximum number of MOVED/ASK redirects to follow before
-	Password string
+	// giving up.
-
+	// Default is 16
-	// The maximum number of MOVED/ASK redirects to follow, before
-	// giving up. Default: 16
 	MaxRedirects int
 
-	// Following options are copied from `redis.Options`.
+	// Following options are copied from Options struct.
+
+	Password string
+
+	DialTimeout  time.Duration
+	ReadTimeout  time.Duration
+	WriteTimeout time.Duration
+
 	PoolSize    int
-	DialTimeout, ReadTimeout, WriteTimeout, PoolTimeout, IdleTimeout time.Duration
+	PoolTimeout time.Duration
+	IdleTimeout time.Duration
 }
 
 func (opt *ClusterOptions) getMaxRedirects() int {

conn.go

@@ -21,9 +21,10 @@ type conn struct {
 	WriteTimeout time.Duration
 }
 
-func newConnDialer(opt *options) func() (*conn, error) {
+func newConnDialer(opt *Options) func() (*conn, error) {
+	dialer := opt.getDialer()
 	return func() (*conn, error) {
-		netcn, err := opt.Dialer()
+		netcn, err := dialer()
 		if err != nil {
 			return nil, err
 		}
@@ -36,7 +37,7 @@ func newConnDialer(opt *options) func() (*conn, error) {
 	}
 }
 
-func (cn *conn) init(opt *options) error {
+func (cn *conn) init(opt *Options) error {
 	if opt.Password == "" && opt.DB == 0 {
 		return nil
 	}

pool.go

@@ -110,17 +110,11 @@ func (l *connList) Close() (retErr error) {
 	return retErr
 }
 
-type connPoolOptions struct {
-	Dialer             func() (*conn, error)
-	PoolSize           int
-	PoolTimeout        time.Duration
-	IdleTimeout        time.Duration
-	IdleCheckFrequency time.Duration
-}
-
 type connPool struct {
+	dialer func() (*conn, error)
+
 	rl        *ratelimit.RateLimiter
-	opt       *connPoolOptions
+	opt       *Options
 	conns     *connList
 	freeConns chan *conn
 
@@ -129,14 +123,16 @@ type connPool struct {
 	lastDialErr error
 }
 
-func newConnPool(opt *connPoolOptions) *connPool {
+func newConnPool(opt *Options) *connPool {
 	p := &connPool{
-		rl:        ratelimit.New(2*opt.PoolSize, time.Second),
+		dialer: newConnDialer(opt),
+
+		rl:        ratelimit.New(2*opt.getPoolSize(), time.Second),
 		opt:       opt,
-		conns:     newConnList(opt.PoolSize),
+		conns:     newConnList(opt.getPoolSize()),
-		freeConns: make(chan *conn, opt.PoolSize),
+		freeConns: make(chan *conn, opt.getPoolSize()),
 	}
-	if p.opt.IdleTimeout > 0 && p.opt.IdleCheckFrequency > 0 {
+	if p.opt.getIdleTimeout() > 0 {
 		go p.reaper()
 	}
 	return p
@@ -147,7 +143,7 @@ func (p *connPool) closed() bool {
 }
 
 func (p *connPool) isIdle(cn *conn) bool {
-	return p.opt.IdleTimeout > 0 && time.Since(cn.usedAt) > p.opt.IdleTimeout
+	return p.opt.getIdleTimeout() > 0 && time.Since(cn.usedAt) > p.opt.getIdleTimeout()
 }
 
 // First returns first non-idle connection from the pool or nil if
@@ -170,7 +166,7 @@ func (p *connPool) First() *conn {
 
 // wait waits for free non-idle connection. It returns nil on timeout.
 func (p *connPool) wait() *conn {
-	deadline := time.After(p.opt.PoolTimeout)
+	deadline := time.After(p.opt.getPoolTimeout())
 	for {
 		select {
 		case cn := <-p.freeConns:
@@ -196,7 +192,7 @@ func (p *connPool) new() (*conn, error) {
 		return nil, err
 	}
 
-	cn, err := p.opt.Dialer()
+	cn, err := p.dialer()
 	if err != nil {
 		p.lastDialErr = err
 		return nil, err
@@ -241,7 +237,7 @@ func (p *connPool) Put(cn *conn) error {
 		log.Printf("redis: connection has unread data: %q", b)
 		return p.Remove(cn)
 	}
-	if p.opt.IdleTimeout > 0 {
+	if p.opt.getIdleTimeout() > 0 {
 		cn.usedAt = time.Now()
 	}
 	p.freeConns <- cn
@@ -295,7 +291,7 @@ func (p *connPool) Close() (retErr error) {
 }
 
 func (p *connPool) reaper() {
-	ticker := time.NewTicker(p.opt.IdleCheckFrequency)
+	ticker := time.NewTicker(time.Minute)
 	defer ticker.Stop()
 
 	for _ = range ticker.C {

redis.go

@@ -9,7 +9,7 @@ import (
 
 type baseClient struct {
 	connPool pool
-	opt      *options
+	opt      *Options
 }
 
 func (c *baseClient) String() string {
@@ -87,10 +87,10 @@ func (c *baseClient) Close() error {
 //------------------------------------------------------------------------------
 
 type Options struct {
-	// The network type, either "tcp" or "unix".
+	// The network type, either tcp or unix.
-	// Default: "tcp"
+	// Default is tcp.
 	Network string
-	// The network address.
+	// host:port address.
 	Addr string
 
 	// Dialer creates new network connection and has priority over
@@ -98,14 +98,17 @@ type Options struct {
 	Dialer func() (net.Conn, error)
 
 	// An optional password. Must match the password specified in the
-	// `requirepass` server configuration option.
+	// requirepass server configuration option.
 	Password string
-	// Select a database.
+	// A database to be selected after connecting to server.
-	// Default: 0
 	DB int64
 
+	// The maximum number of retries before giving up.
+	// Default is to not retry failed commands.
+	MaxRetries int
+
 	// Sets the deadline for establishing new connections. If reached,
-	// deal attepts will fail with a timeout.
+	// dial will fail with a timeout.
 	DialTimeout time.Duration
 	// Sets the deadline for socket reads. If reached, commands will
 	// fail with a timeout instead of blocking.
@@ -115,28 +118,16 @@ type Options struct {
 	WriteTimeout time.Duration
 
 	// The maximum number of socket connections.
-	// Default: 10
+	// Default is 10 connections.
 	PoolSize int
-	// PoolTimeout specifies amount of time client waits for a free
+	// Specifies amount of time client waits for connection if all
-	// connection in the pool. Default timeout is 1s.
+	// connections are busy before returning an error.
+	// Default is 5 seconds.
 	PoolTimeout time.Duration
-	// Evict connections from the pool after they have been idle for longer
+	// Specifies amount of time after which client closes idle
-	// than specified in this option.
+	// connections. Should be less than server's timeout.
-	// Default: 0 = no eviction
+	// Default is to not close idle connections.
 	IdleTimeout time.Duration
-
-	// MaxRetries specifies maximum number of times client will retry
-	// failed command. Default is to not retry failed command.
-	MaxRetries int
-}
-
-func (opt *Options) getDialer() func() (net.Conn, error) {
-	if opt.Dialer == nil {
-		return func() (net.Conn, error) {
-			return net.DialTimeout(opt.getNetwork(), opt.Addr, opt.getDialTimeout())
-		}
-	}
-	return opt.Dialer
 }
 
 func (opt *Options) getNetwork() string {
@@ -146,6 +137,15 @@ func (opt *Options) getNetwork() string {
 	return opt.Network
 }
 
+func (opt *Options) getDialer() func() (net.Conn, error) {
+	if opt.Dialer == nil {
+		opt.Dialer = func() (net.Conn, error) {
+			return net.DialTimeout(opt.getNetwork(), opt.Addr, opt.getDialTimeout())
+		}
+	}
+	return opt.Dialer
+}
+
 func (opt *Options) getPoolSize() int {
 	if opt.PoolSize == 0 {
 		return 10
@@ -167,49 +167,8 @@ func (opt *Options) getPoolTimeout() time.Duration {
 	return opt.PoolTimeout
 }
 
-func (opt *Options) options() *options {
+func (opt *Options) getIdleTimeout() time.Duration {
-	return &options{
+	return opt.IdleTimeout
-		Addr:        opt.Addr,
-		Dialer:      opt.getDialer(),
-		PoolSize:    opt.getPoolSize(),
-		PoolTimeout: opt.getPoolTimeout(),
-		IdleTimeout: opt.IdleTimeout,
-
-		DB:       opt.DB,
-		Password: opt.Password,
-
-		DialTimeout:  opt.getDialTimeout(),
-		ReadTimeout:  opt.ReadTimeout,
-		WriteTimeout: opt.WriteTimeout,
-
-		MaxRetries: opt.MaxRetries,
-	}
-}
-
-type options struct {
-	Addr        string
-	Dialer      func() (net.Conn, error)
-	PoolSize    int
-	PoolTimeout time.Duration
-	IdleTimeout time.Duration
-
-	Password string
-	DB       int64
-
-	DialTimeout  time.Duration
-	ReadTimeout  time.Duration
-	WriteTimeout time.Duration
-
-	MaxRetries int
-}
-
-func (opt *options) connPoolOptions() *connPoolOptions {
-	return &connPoolOptions{
-		Dialer:      newConnDialer(opt),
-		PoolSize:    opt.PoolSize,
-		PoolTimeout: opt.PoolTimeout,
-		IdleTimeout: opt.IdleTimeout,
-	}
 }
 
 //------------------------------------------------------------------------------
@@ -219,7 +178,7 @@ type Client struct {
 	commandable
 }
 
-func newClient(opt *options, pool pool) *Client {
+func newClient(opt *Options, pool pool) *Client {
 	base := &baseClient{opt: opt, connPool: pool}
 	return &Client{
 		baseClient:  base,
@@ -227,8 +186,7 @@ func newClient(opt *options, pool pool) *Client {
 	}
 }
 
-func NewClient(clOpt *Options) *Client {
+func NewClient(opt *Options) *Client {
-	opt := clOpt.options()
+	pool := newConnPool(opt)
-	pool := newConnPool(opt.connPoolOptions())
 	return newClient(opt, pool)
 }

sentinel.go

@@ -11,75 +11,41 @@ import (
 
 //------------------------------------------------------------------------------
 
+// FailoverOptions are used to configure a failover client and should
+// be passed to NewFailoverClient.
 type FailoverOptions struct {
 	// The master name.
 	MasterName string
-	// Seed addresses of sentinel nodes.
+	// A seed list of host:port addresses of sentinel nodes.
 	SentinelAddrs []string
 
-	// An optional password. Must match the password specified in the
+	// Following options are copied from Options struct.
-	// `requirepass` server configuration option.
+
 	Password string
-	// Select a database.
-	// Default: 0
 	DB       int64
 
-	// Sets the deadline for establishing new connections. If reached,
-	// deal attepts will fail with a timeout.
 	DialTimeout  time.Duration
-	// Sets the deadline for socket reads. If reached, commands will
-	// fail with a timeout instead of blocking.
 	ReadTimeout  time.Duration
-	// Sets the deadline for socket writes. If reached, commands will
-	// fail with a timeout instead of blocking.
 	WriteTimeout time.Duration
 
-	// The maximum number of socket connections.
-	// Default: 10
 	PoolSize    int
-	// If all socket connections is the pool are busy, the pool will wait
-	// this amount of time for a conection to become available, before
-	// returning an error.
-	// Default: 5s
 	PoolTimeout time.Duration
-	// Evict connections from the pool after they have been idle for longer
-	// than specified in this option.
-	// Default: 0 = no eviction
 	IdleTimeout time.Duration
 }
 
-func (opt *FailoverOptions) getPoolSize() int {
+func (opt *FailoverOptions) options() *Options {
-	if opt.PoolSize == 0 {
+	return &Options{
-		return 10
+		Addr: "FailoverClient",
-	}
-	return opt.PoolSize
-}
 
-func (opt *FailoverOptions) getPoolTimeout() time.Duration {
-	if opt.PoolTimeout == 0 {
-		return 5 * time.Second
-	}
-	return opt.PoolTimeout
-}
-
-func (opt *FailoverOptions) getDialTimeout() time.Duration {
-	if opt.DialTimeout == 0 {
-		return 5 * time.Second
-	}
-	return opt.DialTimeout
-}
-
-func (opt *FailoverOptions) options() *options {
-	return &options{
 		DB:       opt.DB,
 		Password: opt.Password,
 
-		DialTimeout:  opt.getDialTimeout(),
+		DialTimeout:  opt.DialTimeout,
 		ReadTimeout:  opt.ReadTimeout,
 		WriteTimeout: opt.WriteTimeout,
 
-		PoolSize:    opt.getPoolSize(),
+		PoolSize:    opt.PoolSize,
-		PoolTimeout: opt.getPoolTimeout(),
+		PoolTimeout: opt.PoolTimeout,
 		IdleTimeout: opt.IdleTimeout,
 	}
 }
@@ -104,11 +70,10 @@ type sentinelClient struct {
 	*baseClient
 }
 
-func newSentinel(clOpt *Options) *sentinelClient {
+func newSentinel(opt *Options) *sentinelClient {
-	opt := clOpt.options()
 	base := &baseClient{
 		opt:      opt,
-		connPool: newConnPool(opt.connPoolOptions()),
+		connPool: newConnPool(opt),
 	}
 	return &sentinelClient{
 		baseClient:  base,
@@ -141,7 +106,7 @@ type sentinelFailover struct {
 	masterName    string
 	sentinelAddrs []string
 
-	opt *options
+	opt *Options
 
 	pool     pool
 	poolOnce sync.Once
@@ -161,7 +126,7 @@ func (d *sentinelFailover) dial() (net.Conn, error) {
 func (d *sentinelFailover) Pool() pool {
 	d.poolOnce.Do(func() {
 		d.opt.Dialer = d.dial
-		d.pool = newConnPool(d.opt.connPoolOptions())
+		d.pool = newConnPool(d.opt)
 	})
 	return d.pool
 }