The reply from HELLO

[zuiderkwast]

> hello 3
1# "server" => "redis"
2# "version" => "7.2.4"
(...)

I think we should change it anyway, immediately in our first compatibility version. It is very low risk that any client depends on this value, since it's static.

For a better decision, what do other forks and redis-compatible services (AWS, Alibaba, ...) return for this field?

[hwware]

For this kind of similar commands, I prefer to keep current field and add more fields to compatible with Redis 7.2.4.

[zuiderkwast]

Problem: When we change it, we make it incompatible with our own previous version.

In some cases, we can have both names overlapping. That includes filenames (one filename is a symlink to the other) and INFO fields. For those, we can introduce our new fields. Then at a later major version, we can remove the old names. It's not possible with HELLO server name.

[hwware]

Problem: When we change it, we make it incompatible with our own previous version.

In some cases, we can have both names overlapping. That includes filenames (one filename is a symlink to the other) and INFO fields. For those, we can introduce our new fields. Then at a later major version, we can remove the old names. It's not possible with HELLO server name.

yes, agree. Later major version, remove the old name.

[zuiderkwast]

Good that we agree about the other issues.

Note: For this issue (HELLO) it is not possible to have an overlapping period of two names.

[madolson]

My concern is that redis might update their clients to look for this field, and error out if they see it. I suppose that is a risk for any user facing change we make. Maybe we should add a config for the compatibility layer?

[zuiderkwast]

Good point @madolson. That's why I'm asking what does ElastiCache return for this field? "redis"?

[madolson]

We just return redis today. I think all the major forks do too.

[zuiderkwast]

OK, very good plan. Let's keep redis for now. Soon enough, let's add a compatibility switch with default ON. (Next minor milestone)

[romange]

I can confirm that there are client libraries/frameworks that expect to see the redis version.

[romange]

One of such frameworks is sidekiq.

[stockholmux]

My concern is that redis might update their clients to look for this field, and error out if they see it. I suppose that is a risk for any user facing change we make. Maybe we should add a config for the compatibility layer?

Would it be possible to make the value user configurable in valkey.conf?

compat-hello-server "valkey" yields

> hello 3
1# "server" => "valkey"
...

compat-hello-server "redis" yields

> hello 3
1# "server" => "redis"
...

compat-hello-server "foobar" yields

> hello 3
1# "server" => "foobar"
...

In later versions a user can make it say whatever they need for compatibility. Same could be done for version numbers, but that's more risky.

[madolson]

Not sure why we need so much tuning, I think just a single flag which indicates what we are doing should be sufficient? I'm thinking just a flag like. redis-compatibility-type [strict, loose, none]. Which does:

1. strict: We look like Redis, nothing else.
2. loose: We look like Redis, but show some extra fields.
3. None: We show zero redis anywhere.

[stockholmux]

So, say there is a change in client libs and a new version of redis which makes whatever strict implementation no longer work. User configurable bits means something like this could just be tweak to your own config file, not valkey revv'ing a release.

I can also think of scenarios where a client lib could look for one thing but maybe a user could sniff somewhere else to know it's running valkey, having tuning here would help.

[parthpatel]

So, say there is a change in client libs and a new version of redis which makes whatever strict implementation no longer work. User configurable bits means something like this could just be tweak to your own config file, not valkey revv'ing a release.

Let's not worry about future hypothetical. For the next version, top level configuration should be more than enough. It is simple and less clutter for users in config file.

I can also think of scenarios where a client lib could look for one thing but maybe a user could sniff somewhere else to know it's running valkey, having tuning here would help.

It is not a problem if a user can see valkey under the hood. We are not trying to masquerade as Redis, only providing support to run existing software without much heavy lifting for users.

[zuiderkwast]

True, this compatibility layer can't be a stealth mode. I don't think any client would try some command that returns an error just to check that error message contains "Redis". That would mean we can never clean up some error messages.

[bradh]

Maybe like browser User Agent: valkey, like redis.

(inspired by like Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36 from https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent)

[madolson]

So, say there is a change in client libs and a new version of redis which makes whatever strict implementation no longer work. User configurable bits means something like this could just be tweak to your own config file, not valkey revv'ing a release.

This would also be a breaking change for clients. I agree about not worrying too much about that, since we can only stay truly compatible with 7.2.

[zuiderkwast]

Keeping "server" => "redis" forever implies that we're also keeping "version" => "7.2.4" forever, right?

We can add fields though? That can't break anyone, right? Let's consider it. It's useful to get this from HELLO so the client can infer what features are available, such as client-side tracking or future features like async blocking commands, even if those are opt-in.

[madolson]

My preference is still let's not touch hello, and consider a larger change for 8.0. I think we can consider changing the server to valkey in the future.

[zuiderkwast]

Yes, I'm only talking about 8.0 here. No change for 7.2.

No need to make a decision about it now, but it can be good to keep in mind if we want to align terminology with INFO fields for example.

[zuiderkwast]

For Valkey 8, I suggest we change it to

"server" => "valkey"
"version" => "8.0.0"

In addition to this, we add a redis-compatibility config #274, disabled by default but users can enable it if they have problems. When enabled, it will instead pretend to be Redis 7.2.4:

"server" => "redis"
"version" => "7.2.4"