Skip to content

Commit 98bb99d

Browse files
authored
Fix Redis Cluster issue during roll outs of new nodes with same addr (#1914)
* fix: recycle connections in some Redis Cluster scenarios This issue was surfaced in a Cloud Provider solution that used for rolling out new nodes using the same address (hostname) of the nodes that will be replaced in a Redis Cluster, while the former ones once depromoted as Slaves would continue in service during some mintues for redirecting traffic. The solution basically identifies when the connection could be stale since a MOVED response will be returned using the same address (hostname) that is being used by the connection. At that moment we consider the connection as no longer usable forcing to recycle the connection.
1 parent 5072031 commit 98bb99d

File tree

3 files changed

+24
-6
lines changed

3 files changed

+24
-6
lines changed

error.go

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func isRedisError(err error) bool {
6565
return ok
6666
}
6767

68-
func isBadConn(err error, allowTimeout bool) bool {
68+
func isBadConn(err error, allowTimeout bool, addr string) bool {
6969
switch err {
7070
case nil:
7171
return false
@@ -74,9 +74,19 @@ func isBadConn(err error, allowTimeout bool) bool {
7474
}
7575

7676
if isRedisError(err) {
77-
// Close connections in read only state in case domain addr is used
78-
// and domain resolves to a different Redis Server. See #790.
79-
return isReadOnlyError(err)
77+
switch {
78+
case isReadOnlyError(err):
79+
// Close connections in read only state in case domain addr is used
80+
// and domain resolves to a different Redis Server. See #790.
81+
return true
82+
case isMovedSameConnAddr(err, addr):
83+
// Close connections when we are asked to move to the same addr
84+
// of the connection. Force a DNS resolution when all connections
85+
// of the pool are recycled
86+
return true
87+
default:
88+
return false
89+
}
8090
}
8191

8292
if allowTimeout {
@@ -119,6 +129,14 @@ func isReadOnlyError(err error) bool {
119129
return strings.HasPrefix(err.Error(), "READONLY ")
120130
}
121131

132+
func isMovedSameConnAddr(err error, addr string) bool {
133+
redisError := err.Error()
134+
if !strings.HasPrefix(redisError, "MOVED ") {
135+
return false
136+
}
137+
return strings.HasSuffix(redisError, addr)
138+
}
139+
122140
//------------------------------------------------------------------------------
123141

124142
type timeoutError interface {

pubsub.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func (c *PubSub) releaseConn(ctx context.Context, cn *pool.Conn, err error, allo
141141
if c.cn != cn {
142142
return
143143
}
144-
if isBadConn(err, allowTimeout) {
144+
if isBadConn(err, allowTimeout, c.opt.Addr) {
145145
c.reconnect(ctx, err)
146146
}
147147
}

redis.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ func (c *baseClient) releaseConn(ctx context.Context, cn *pool.Conn, err error)
261261
c.opt.Limiter.ReportResult(err)
262262
}
263263

264-
if isBadConn(err, false) {
264+
if isBadConn(err, false, c.opt.Addr) {
265265
c.connPool.Remove(ctx, cn, err)
266266
} else {
267267
c.connPool.Put(ctx, cn)

0 commit comments

Comments
 (0)