Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Ankit-Kulkarni opened this issue Dec 7, 2023 · 6 comments

Comments

@Ankit-Kulkarni
Copy link

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 Ankit-Kulkarni changed the title can not use a specified DB with NewFailoverClusterClient can not use a specified DB with NewFailoverClusterClient. suggested possible fix Dec 7, 2023
@Ankit-Kulkarni Ankit-Kulkarni changed the title can not use a specified DB with NewFailoverClusterClient. suggested possible fix Can not use a specified DB with NewFailoverClusterClient. suggested possible fix Dec 7, 2023
@Ankit-Kulkarni
Copy link
Author

Any updates ?

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

@rosaekapratama
Copy link

rosaekapratama commented Apr 20, 2024

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
Copy link
Author

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

@davama
Copy link

davama commented Jul 8, 2024

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
Copy link
Contributor

Thanks folks and sorry to miss this

Hi @monkey92t
Do you see any problems with this approach
#2824 (comment)

@Ankit-Kulkarni
Copy link
Author

Thanks @SoulPancake
@monkey92t could you please take a look on the comment once #2824 comment and see if this is ok . I will create a PR for the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants