Closed jimsnab closed 4 months ago
Hey @jimsnab,
The client setinfo command has been available since Redis version 7.2.0. This command is automatically run at the start of each connection. If you wish to prevent this from happening, you can modify your connection options to include DisableIdentity: true
. This will stop the command from executing at the beginning of each connection.
Please let us know if this solution addresses your issue or if you need any further assistance:)
Going to be a bump for many I predict. There's not a lot of code in github setting DisableIdentity, and Elasticache is on 7.1.
It would be better if this breaking fix was handled with more grace. If the LibraryInfo is empty string, why not skip it?
Same problem here. 9.5.0 breaks our existing builds and 9.4.0 does not. Skipping this release for now as clearly this will need to get tested heavily in our EL8/go1.20.10/Redis6.2 Sentinel/HA-setups.
I'm guessing a warning on the release page would be welcomed by many, as I suspect lots of other people are going to suddenly have issues when bumping their go.mod.
Many thanks for all the hard work. Very much appreciated.
@ofekshenawa The README should at least highlight disabling setinfo appropriately, as in other clients. I think it's high time we extend CI across versions too. I'l update the techdebt issues list, at the very least. Can you please address this with a README change at the very least.
What is the minimum version of Redis this pkg supports?
Hey @jimsnab,
The client setinfo command has been available since Redis version 7.2.0. This command is automatically run at the start of each connection. If you wish to prevent this from happening, you can modify your connection options to include
DisableIdentity: true
. This will stop the command from executing at the beginning of each connection.Please let us know if this solution addresses your issue or if you need any further assistance:)
DisableIndentity or DisableIdentity?
@clemensg I set DisableIndentity
to true, it worked as expected
So in my app, I am creating a failover client via the universal client and then using ping, which results in an error, even though I have DisableIndentity
set to true.
I have redacted the values in the options, as this is to give you an idea of how I use it, as well as the error I get back.
opts := &goredis.UniversalOptions{
Addrs: []string{"..."},
DB: "...",
MasterName: "...",
Password: "...",
SentinelPassword: "...",
DisableIndentity: true,
}
rdb := goredis.NewUniversalClient(opts)
if err := rdb.Ping(context.Background()).Err(); err != nil {
return nil, err // Returns - redis: all sentinels specified in config duration are unreachable
}
I also have verbose logging on and see
redis: 2024/02/20 08:49:51 sentinel.go:552: sentinel: GetMasterAddrByName master="..." failed: ERR unknown subcommand 'setinfo'. Try CLIENT HELP.
In my opinion this is a poorly communicated breaking change.
We use elasticache on aws which latest supported version is 7.1. That means every elasticache integration with 9.5.0 will fail. Not mentioning every other non serverless deployment as 7.2.0 was released only ~half a year ago 7.2.0
This should had been mentioned on release notes followed by major version bump or was made opt-in and not enabled by default.
Not mentioned in the comments so far, the official command docs explicitly say that client libraries should ignore failures:
Client libraries are expected to pipeline this command after authentication on all connections and ignore failures since they could be connected to an older version that doesn't support them.
So in my app, I am creating a failover client via the universal client and then using ping, which results in an error, even though I have
DisableIndentity
set to true.I have redacted the values in the options, as this is to give you an idea of how I use it, as well as the error I get back.
opts := &goredis.UniversalOptions{ Addrs: []string{"..."}, DB: "...", MasterName: "...", Password: "...", SentinelPassword: "...", DisableIndentity: true, } rdb := goredis.NewUniversalClient(opts) if err := rdb.Ping(context.Background()).Err(); err != nil { return nil, err // Returns - redis: all sentinels specified in config duration are unreachable }
I also have verbose logging on and see
redis: 2024/02/20 08:49:51 sentinel.go:552: sentinel: GetMasterAddrByName master="..." failed: ERR unknown subcommand 'setinfo'. Try CLIENT HELP.
Hello @softwarespot @ofekshenawa
I stumbled into the same issue when using FailoverClient, setting the DisableIdentity option does not work.
I made some digging and I found out that DisableIdentity parameter is not correctly forwarded to client configuration in sentinelOptions
and clusterOptions
methods. Thus, the DisableIdentity is not applied when spawning a client leading to the setinfo error.
https://github.com/redis/go-redis/blob/v9.5.0/sentinel.go#L155 https://github.com/redis/go-redis/blob/v9.5.0/sentinel.go#L192
This is a bug to me, I'll send a PR to fix.
Thanks, I still think my fix is relevant because the options is missing for FailoverClient.
@reenjii Awesome. @ofekshenawa please look as you go through these, of course.
I fixed the typo too but you can cherry pick this one elsewhere if this is easier! https://github.com/redis/go-redis/pull/2923/commits/3009c8fcb1df4aab7002f7cdd766f7b1b4651c86
Thanks @reenjii
Issue tracker is used for reporting bugs and discussing new features. Please use stackoverflow for supporting issues.
Expected Behavior
golang client connects to redis.
If the server does not support
CLIENT SETINFO
, the golang client shouldn't try to use it, or should at least not error on connection.Current Behavior
Upon the first command:
ERR unknown subcommand 'setinfo'. Try CLIENT HELP.
Possible Solution
opt.DisableIndentity = true
Steps to Reproduce
https://goplay.tools/snippet/rbwI-UrfSkd
Won't run on goplay.tools because there are too many packages to download. Copy the code into a local scratch folder and run it.
Context (Environment)
Apps don't start up, I had to revert to 9.4.0
Detailed Description
This is caused by 9.5.0 change that moves LibraryInfo logic into a pipeline. The prior code did not check for errors. Now it fails the startup pipeline.
Possible Implementation