redis / go-redis

Redis Go client
https://redis.uptrace.dev
BSD 2-Clause "Simplified" License
19.81k stars 2.34k forks source link

Can not use a specified DB with NewFailoverClusterClient. suggested possible fix #2824

Open Ankit-Kulkarni opened 8 months ago

Ankit-Kulkarni commented 8 months ago

If you create a redis client using sentinel failover options but you have to use a different db than 0 and route your read only queries to slaves , different db number is not honoured.

Library version I used: v8.11.5

Expected Behavior

We would expect the NewFailoverClusterClient to honor the DB mentioned in FailoverOptions.

Currently I am using redis.FailoverOptions to create a *redis.ClusterClient using NewFailoverClusterClient. I wanted to route read only queries to slaves and hence using NewFailoverClusterClient instead of NewFailoverClient. My failover configuration has a DB say 5 but when i use *redis.ClusterClient it does all operations on db 0 only. Reference code below

var rdClient *redis.ClusterClient
func initRedis() (*redis.ClusterClient) {
    redisSentinels := strings.Split(config["redisSentinels"], ",")
    for i := range redisSentinels {
        redisSentinels[i] = strings.TrimSpace(redisSentinels[i])
    }

    clientConfig := &redis.FailoverOptions{
        MasterName:    config["redisMasterName"],
        SentinelAddrs: redisSentinels,
        RouteRandomly: true,
        PoolSize:      5,
        DB:            5,
        ....
        .....
    }

    return redis.NewFailoverClusterClient(clientConfig)

}
rdclient = initRedis()

Above rdsClient when doing set or get will work only on db:0 and not db:5

Possible Solution

I think the issue is because the ClusterOptions struct doesn't support DB(may be intentional but i couldn't figure out why). I tried quick local modification to library for below structs and functions and it seemed to work. But not sure if this change misses/breaks anything else (specifically if this was unintentional)

// file --> cluster.go
type ClusterOptions struct {
         ...
        // DB of the new client
    DB int
         ...
}

func (opt *ClusterOptions) clientOptions() *Options {
    const disableIdleCheck = -1

    return &Options{
        DB:        opt.DB,
                ....
                ....
    }
}

// file --> sentinel.go 
func (opt *FailoverOptions) clusterOptions() *ClusterOptions {
    return &ClusterOptions{
                ....
        DB:       opt.DB,
                ....
                ....
    }
}
Ankit-Kulkarni commented 7 months ago

Any updates ?

@SoulPancake @ofekshenawa can anyone of you or other contributors can lookinto this ?

rosaekapratama commented 4 months ago

Any updates regarding this? I got same problem here, need to connect to outside of DB 0 of sentinel

@Ankit-Kulkarni do you change the file urself in local for temp fix?

Ankit-Kulkarni commented 4 months ago

@rosaekapratama yes the above code patch worked for me . i have changed it to local and using it since then.

davama commented 1 month ago

Good day,

Thank you for posting this @Ankit-Kulkarni . Was stumbling on this myself on an application I use. Hoping to hear some updates from the devs

Best

SoulPancake commented 1 month ago

Thanks folks and sorry to miss this

Hi @monkey92t Do you see any problems with this approach https://github.com/redis/go-redis/issues/2824#issue-2031030711