Skip to content

Commit

Permalink
fix: recycle connections in some Redis Cluster scenarios
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pfreixes committed Oct 2, 2021
1 parent 5072031 commit 13f4f2d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 6 deletions.
24 changes: 20 additions & 4 deletions error.go
Expand Up @@ -65,7 +65,7 @@ func isRedisError(err error) bool {
return ok
}

func isBadConn(err error, allowTimeout bool) bool {
func isBadConn(err error, allowTimeout bool, addr string) bool {
switch err {
case nil:
return false
Expand All @@ -74,9 +74,16 @@ func isBadConn(err error, allowTimeout bool) bool {
}

if isRedisError(err) {
// Close connections in read only state in case domain addr is used
// and domain resolves to a different Redis Server. See #790.
return isReadOnlyError(err)
if isReadOnlyError(err) {
// Close connections in read only state in case domain addr is used
// and domain resolves to a different Redis Server. See #790.
return true
} else if isMovedSameConnAddr(err, addr) {
// Close connections when we are asked to move to the same addr
// of the connection. Force a DNS resolution when all connections
// of the pool are recycled
return true
}
}

if allowTimeout {
Expand Down Expand Up @@ -119,6 +126,15 @@ func isReadOnlyError(err error) bool {
return strings.HasPrefix(err.Error(), "READONLY ")
}

func isMovedSameConnAddr(err error, addr string) bool {
redisError := err.Error()
if !strings.HasPrefix(redisError, "MOVED ") {
return false
}
return strings.HasSuffix(redisError, addr)
}


//------------------------------------------------------------------------------

type timeoutError interface {
Expand Down
2 changes: 1 addition & 1 deletion pubsub.go
Expand Up @@ -141,7 +141,7 @@ func (c *PubSub) releaseConn(ctx context.Context, cn *pool.Conn, err error, allo
if c.cn != cn {
return
}
if isBadConn(err, allowTimeout) {
if isBadConn(err, allowTimeout, c.opt.Addr) {
c.reconnect(ctx, err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion redis.go
Expand Up @@ -261,7 +261,7 @@ func (c *baseClient) releaseConn(ctx context.Context, cn *pool.Conn, err error)
c.opt.Limiter.ReportResult(err)
}

if isBadConn(err, false) {
if isBadConn(err, false, c.opt.Addr) {
c.connPool.Remove(ctx, cn, err)
} else {
c.connPool.Put(ctx, cn)
Expand Down

0 comments on commit 13f4f2d

Please sign in to comment.