9.5.0 breaks app with request for setinfo on older redis

[jimsnab]

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

1. Make a hello world app using v9.5.0 redis client, and connect to a recent but not totally current redis such as 7.1.0
2. Run it

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

[ofekshenawa]

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:)

[jimsnab]

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?

[chrisv]

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.

[chayim]

@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.

[softwarespot]

What is the minimum version of Redis this pkg supports?

[clemensg]

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?

[softwarespot]

@clemensg I set DisableIndentity to true, it worked as expected

[softwarespot]

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.  

[adomaskizogian]

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.

[philippgille]

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.

Source: https://github.com/redis/redis-doc/blob/ccd3a13c6bcc7bdf9389daad9244007639feff35/commands/client-setinfo.md?plain=1#L3-L4

[reenjii]

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.

[chayim]

#2915

[reenjii]

Thanks, I still think my fix is relevant because the options is missing for FailoverClient.

[chayim]

@reenjii Awesome. @ofekshenawa please look as you go through these, of course.

[reenjii]

I fixed the typo too but you can cherry pick this one elsewhere if this is easier!
3009c8f

[softwarespot]

Thanks @reenjii