Merge pull request #2171 from go-redis/fix/cleanup-pool-options

fix: remove conn reaper from the pool and uptrace option names

CHANGELOG.md

@@ -1,31 +1,52 @@
 # [9.0.0-beta.1](https://github.com/go-redis/redis/compare/v8.11.5...v9.0.0-beta.1) (2022-06-04)
 
-
 ### Bug Fixes
 
-* **#1943:** xInfoConsumer.Idle should be time.Duration instead of int64 ([#2052](https://github.com/go-redis/redis/issues/2052)) ([997ab5e](https://github.com/go-redis/redis/commit/997ab5e7e3ddf53837917013a4babbded73e944f)), closes [#1943](https://github.com/go-redis/redis/issues/1943)
-* add XInfoConsumers test ([6f1a1ac](https://github.com/go-redis/redis/commit/6f1a1ac284ea3f683eeb3b06a59969e8424b6376))
-* fix tests ([3a722be](https://github.com/go-redis/redis/commit/3a722be81180e4d2a9cf0a29dc9a1ee1421f5859))
-* remove test(XInfoConsumer.idle), not a stable return value when tested. ([f5fbb36](https://github.com/go-redis/redis/commit/f5fbb367e7d9dfd7f391fc535a7387002232fa8a))
-* update ChannelWithSubscriptions to accept options ([c98c5f0](https://github.com/go-redis/redis/commit/c98c5f0eebf8d254307183c2ce702a48256b718d))
-* update COMMAND parser for Redis 7 ([b0bb514](https://github.com/go-redis/redis/commit/b0bb514059249e01ed7328c9094e5b8a439dfb12))
-* use redis over ssh channel([#2057](https://github.com/go-redis/redis/issues/2057)) ([#2060](https://github.com/go-redis/redis/issues/2060)) ([3961b95](https://github.com/go-redis/redis/commit/3961b9577f622a3079fe74f8fc8da12ba67a77ff))
-
+- **#1943:** xInfoConsumer.Idle should be time.Duration instead of int64
+  ([#2052](https://github.com/go-redis/redis/issues/2052))
+  ([997ab5e](https://github.com/go-redis/redis/commit/997ab5e7e3ddf53837917013a4babbded73e944f)),
+  closes [#1943](https://github.com/go-redis/redis/issues/1943)
+- add XInfoConsumers test
+  ([6f1a1ac](https://github.com/go-redis/redis/commit/6f1a1ac284ea3f683eeb3b06a59969e8424b6376))
+- fix tests
+  ([3a722be](https://github.com/go-redis/redis/commit/3a722be81180e4d2a9cf0a29dc9a1ee1421f5859))
+- remove test(XInfoConsumer.idle), not a stable return value when tested.
+  ([f5fbb36](https://github.com/go-redis/redis/commit/f5fbb367e7d9dfd7f391fc535a7387002232fa8a))
+- update ChannelWithSubscriptions to accept options
+  ([c98c5f0](https://github.com/go-redis/redis/commit/c98c5f0eebf8d254307183c2ce702a48256b718d))
+- update COMMAND parser for Redis 7
+  ([b0bb514](https://github.com/go-redis/redis/commit/b0bb514059249e01ed7328c9094e5b8a439dfb12))
+- use redis over ssh channel([#2057](https://github.com/go-redis/redis/issues/2057))
+  ([#2060](https://github.com/go-redis/redis/issues/2060))
+  ([3961b95](https://github.com/go-redis/redis/commit/3961b9577f622a3079fe74f8fc8da12ba67a77ff))
 
 ### Features
 
-* add ClientUnpause ([91171f5](https://github.com/go-redis/redis/commit/91171f5e19a261dc4cfbf8706626d461b6ba03e4))
-* add NewXPendingResult for unit testing XPending ([#2066](https://github.com/go-redis/redis/issues/2066)) ([b7fd09e](https://github.com/go-redis/redis/commit/b7fd09e59479bc6ed5b3b13c4645a3620fd448a3))
-* add WriteArg and Scan net.IP([#2062](https://github.com/go-redis/redis/issues/2062)) ([7d5167e](https://github.com/go-redis/redis/commit/7d5167e8624ac1515e146ed183becb97dadb3d1a))
-* **pool:** add check for badConnection ([a8a7665](https://github.com/go-redis/redis/commit/a8a7665ddf8cc657c5226b1826a8ee83dab4b8c1)), closes [#2053](https://github.com/go-redis/redis/issues/2053)
-* provide a username and password callback method, so that the plaintext username and password will not be stored in the memory, and the username and password will only be generated once when the CredentialsProvider is called. After the method is executed, the username and password strings on the stack will be released. ([#2097](https://github.com/go-redis/redis/issues/2097)) ([56a3dbc](https://github.com/go-redis/redis/commit/56a3dbc7b656525eb88e0735e239d56e04a23bee))
-* upgrade to Redis 7 ([d09c27e](https://github.com/go-redis/redis/commit/d09c27e6046129fd27b1d275e5a13a477bd7f778))
-
-
+- add ClientUnpause
+  ([91171f5](https://github.com/go-redis/redis/commit/91171f5e19a261dc4cfbf8706626d461b6ba03e4))
+- add NewXPendingResult for unit testing XPending
+  ([#2066](https://github.com/go-redis/redis/issues/2066))
+  ([b7fd09e](https://github.com/go-redis/redis/commit/b7fd09e59479bc6ed5b3b13c4645a3620fd448a3))
+- add WriteArg and Scan net.IP([#2062](https://github.com/go-redis/redis/issues/2062))
+  ([7d5167e](https://github.com/go-redis/redis/commit/7d5167e8624ac1515e146ed183becb97dadb3d1a))
+- **pool:** add check for badConnection
+  ([a8a7665](https://github.com/go-redis/redis/commit/a8a7665ddf8cc657c5226b1826a8ee83dab4b8c1)),
+  closes [#2053](https://github.com/go-redis/redis/issues/2053)
+- provide a username and password callback method, so that the plaintext username and password will
+  not be stored in the memory, and the username and password will only be generated once when the
+  CredentialsProvider is called. After the method is executed, the username and password strings on
+  the stack will be released. ([#2097](https://github.com/go-redis/redis/issues/2097))
+  ([56a3dbc](https://github.com/go-redis/redis/commit/56a3dbc7b656525eb88e0735e239d56e04a23bee))
+- upgrade to Redis 7
+  ([d09c27e](https://github.com/go-redis/redis/commit/d09c27e6046129fd27b1d275e5a13a477bd7f778))
 
 ## v9 UNRELEASED
 
 - Added support for [RESP3](https://github.com/antirez/RESP3/blob/master/spec.md) protocol.
 - Removed `Pipeline.Close` since there is no real need to explicitly manage pipeline resources.
   `Pipeline.Discard` is still available if you want to reset commands for some reason.
 - Replaced `*redis.Z` with `redis.Z` since it is small enough to be passed as value.
+- Renamed `MaxConnAge` to `ConnMaxLifetime`.
+- Renamed `IdleTimeout` to `ConnMaxIdleTime`.
+- Removed connection reaper in favor of `MaxIdleConns`.
+- Removed `WithContext`.

cluster.go

@@ -72,12 +72,12 @@ type ClusterOptions struct {
 	PoolFIFO bool
 
 	// PoolSize applies per cluster node and not for the whole cluster.
-	PoolSize           int
-	MinIdleConns       int
-	MaxConnAge         time.Duration
-	PoolTimeout        time.Duration
-	IdleTimeout        time.Duration
-	IdleCheckFrequency time.Duration
+	PoolSize        int
+	PoolTimeout     time.Duration
+	MinIdleConns    int
+	MaxIdleConns    int
+	ConnMaxIdleTime time.Duration
+	ConnMaxLifetime time.Duration
 
 	TLSConfig *tls.Config
 }
@@ -132,8 +132,6 @@ func (opt *ClusterOptions) init() {
 }
 
 func (opt *ClusterOptions) clientOptions() *Options {
-	const disableIdleCheck = -1
-
 	return &Options{
 		Dialer:    opt.Dialer,
 		OnConnect: opt.OnConnect,
@@ -149,13 +147,13 @@ func (opt *ClusterOptions) clientOptions() *Options {
 		ReadTimeout:  opt.ReadTimeout,
 		WriteTimeout: opt.WriteTimeout,
 
-		PoolFIFO:           opt.PoolFIFO,
-		PoolSize:           opt.PoolSize,
-		MinIdleConns:       opt.MinIdleConns,
-		MaxConnAge:         opt.MaxConnAge,
-		PoolTimeout:        opt.PoolTimeout,
-		IdleTimeout:        opt.IdleTimeout,
-		IdleCheckFrequency: disableIdleCheck,
+		PoolFIFO:        opt.PoolFIFO,
+		PoolSize:        opt.PoolSize,
+		PoolTimeout:     opt.PoolTimeout,
+		MinIdleConns:    opt.MinIdleConns,
+		MaxIdleConns:    opt.MaxIdleConns,
+		ConnMaxIdleTime: opt.ConnMaxIdleTime,
+		ConnMaxLifetime: opt.ConnMaxLifetime,
 
 		TLSConfig: opt.TLSConfig,
 		// If ClusterSlots is populated, then we probably have an artificial
@@ -725,10 +723,6 @@ func NewClusterClient(opt *ClusterOptions) *ClusterClient {
 	c.cmdsInfoCache = newCmdsInfoCache(c.cmdsInfo)
 	c.cmdable = c.Process
 
-	if opt.IdleCheckFrequency > 0 {
-		go c.reaper(opt.IdleCheckFrequency)
-	}
-
 	return c
 }
 
@@ -1049,26 +1043,6 @@ func (c *ClusterClient) loadState(ctx context.Context) (*clusterState, error) {
 	return nil, firstErr
 }
 
-// reaper closes idle connections to the cluster.
-func (c *ClusterClient) reaper(idleCheckFrequency time.Duration) {
-	ticker := time.NewTicker(idleCheckFrequency)
-	defer ticker.Stop()
-
-	for range ticker.C {
-		nodes, err := c.nodes.All()
-		if err != nil {
-			break
-		}
-
-		for _, node := range nodes {
-			_, err := node.Client.connPool.(*pool.ConnPool).ReapStaleConns()
-			if err != nil {
-				internal.Logger.Printf(context.TODO(), "ReapStaleConns failed: %s", err)
-			}
-		}
-	}
-}
-
 func (c *ClusterClient) Pipeline() Pipeliner {
 	pipe := Pipeline{
 		exec: c.processPipeline,

internal/pool/bench_test.go

@@ -30,11 +30,10 @@ func BenchmarkPoolGetPut(b *testing.B) {
 	for _, bm := range benchmarks {
 		b.Run(bm.String(), func(b *testing.B) {
 			connPool := pool.NewConnPool(&pool.Options{
-				Dialer:             dummyDialer,
-				PoolSize:           bm.poolSize,
-				PoolTimeout:        time.Second,
-				IdleTimeout:        time.Hour,
-				IdleCheckFrequency: time.Hour,
+				Dialer:          dummyDialer,
+				PoolSize:        bm.poolSize,
+				PoolTimeout:     time.Second,
+				ConnMaxIdleTime: time.Hour,
 			})
 
 			b.ResetTimer()
@@ -74,11 +73,10 @@ func BenchmarkPoolGetRemove(b *testing.B) {
 	for _, bm := range benchmarks {
 		b.Run(bm.String(), func(b *testing.B) {
 			connPool := pool.NewConnPool(&pool.Options{
-				Dialer:             dummyDialer,
-				PoolSize:           bm.poolSize,
-				PoolTimeout:        time.Second,
-				IdleTimeout:        time.Hour,
-				IdleCheckFrequency: time.Hour,
+				Dialer:          dummyDialer,
+				PoolSize:        bm.poolSize,
+				PoolTimeout:     time.Second,
+				ConnMaxIdleTime: time.Hour,
 			})
 
 			b.ResetTimer()

internal/pool/conn_check.go

@@ -27,7 +27,8 @@ func connCheck(conn net.Conn) error {
 	}
 
 	var sysErr error
-	err = rawConn.Read(func(fd uintptr) bool {
+
+	if err := rawConn.Read(func(fd uintptr) bool {
 		var buf [1]byte
 		n, err := syscall.Read(int(fd), buf[:])
 		switch {
@@ -41,8 +42,7 @@ func connCheck(conn net.Conn) error {
 			sysErr = err
 		}
 		return true
-	})
-	if err != nil {
+	}); err != nil {
 		return err
 	}