The reply from HELLO about valkey HOT 21 CLOSED

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.

from valkey.

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

from valkey.

One of such frameworks is sidekiq.

from valkey.

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?

from valkey.

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.

from valkey.

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

from valkey.

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.

from valkey.

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

from valkey.

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.

from valkey.

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.

from valkey.

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.

from valkey.

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

from valkey.

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

from valkey.

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.

from valkey.

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.

from valkey.

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.

from valkey.

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)

from valkey.

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.

from valkey.

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.

from valkey.

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.

from valkey.

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"

from valkey.