search

valkey-io / valkey

A flexible distributed key-value datastore that is optimized for caching and other realtime workloads.
https://valkey.io
Other
17.72k stars 670 forks source link

[NEW] Redis-compatibility mode #274

Closed zuiderkwast closed 7 months ago

zuiderkwast commented 7 months ago

The problem/use-case that the feature addresses

We want to change the word "Redis" in error replies, log entries, HELLO reply, etc. but there is risk of breaking the Redis compatibility if we change them.

This is what we're talking about:

Description of the feature

Introduce a TEMPORARY "compatibility switch", so if a user has some scripts or clients hard-coded to look for "Redis", the user can enable the redis-compatibility mode.

I say temporary, because the purpose is to let users fix their applications.

:question: What's the time plan? I suggest we add the switch in Valkey 8. In Valkey 9 we remove it, but we make the config have no effect, so we don't crash if the user has the config. In Valkey 10 we remove the config.

:question: What shall be controlled by it? I suggest the things listed above. For all other error replies and log messages we can just make a breaking change. Other suggestions?

:question: Shall it be a compile time switch (ifdef) or runtime (config). I suggest a config. Not all users compile from source.

Alternatives you've considered

We could just do breaking changes. (High risk of problems for users?)

We could just keep Redis in these messages forever. (Safe, but not nice.)

Additional information

It has been discussed but no decision was made yet. We need to make decisions about all " :question: " above.

hwware commented 7 months ago

Do we need consider TCL test case redis-compatibility in the thread? Or I need open a new one for test case?

zuiderkwast commented 7 months ago

@hwware i don't understand. Why do we need to change test cases? Just to make CI pass?

hwware commented 7 months ago

@hwware i don't understand. Why do we need to change test cases? Just to make CI pass?

Sorry confused you. In test case outout, there are some redis keywords there, such as runtest-cluster and runtest-sentienl, when test cases begin to run, it prints out "Starting redis #7 at port 30007" or similar message, and for some test cases, there are some “redis” keywords there.

zuiderkwast commented 7 months ago

I don't think changing tests is a breaking change. We can just change them imo.

zuiderkwast commented 7 months ago

@valkey-io/core-team VOTE: Do you agree to the description above? :+1: = yes, :-1: = no.

hwware commented 7 months ago

I partially agree the plan:

Agree: add the switch in Valkey 8 Need to discuss: after Valkey 8, how to deal with this switch and next step plan

madolson commented 7 months ago

The only strong opinion I have from the open questions is we should make it a runtime flag. A three year transition sounds good to me.

soloestoy commented 7 months ago

TBH, I'm not fond of the compatibility config, but adding the adjective "temporary" somehow makes it more acceptable to me.

In my view, the ideal approach would be to complete all replacements and removals of the word 'redis' in version 8.0. However, considering the compatibility risks associated with subsequent version upgrades, it's also a reasonable choice to introduce a compatibility switch in 8.0 and then remove it in 9.0.

The catch is that we must actually be able to remove it in 9.0. I have seen many cases where designs have deviated from their initial objectives and ended up being maintained indefinitely. Often, removing a feature is far more challenging than adding one. If we can be certain that it will be removed in 9.0, I would vote in favor.

Apart from this config, I have some other concerns, such as how we should handle the replacement of 'master' with 'primary'. It's not a good idea adding another new config : )

enjoy-binbin commented 7 months ago

Personally, i don't like ths switches thing, once it is added we have to live with it for a while (years). and then i worry we'll never get out of it. But since we do need to leave some time for people to adapt, i agree with it.

On the other hands, we now have the first fully compatible 7.2 RC and there seems to be enough time for those who need to switch? It's fine for user scripts since they need to do the real "switch", i'm just worried about client libs.

zuiderkwast commented 7 months ago

OK, so let's make another decision about when we'll delete this compat config. I think we have different opinions about it. I think it is totally fine to delete things, if we communicate it in advance. Other projects do that. It was just never done in Redis so there is no way of working for this.

For Erlang (programming language) there are some docs and guidelines for deprecations and removals we can use as inspiration, including already removed features: https://www.erlang.org/doc/general_info/deprecations and https://www.erlang.org/doc/system_principles/misc

We can discuss this topic in general (when we deprecate something, we will delete it in X major versions) or case-by-case. Either way, I think we shall make a statement in the release notes and in the docs what the plans are, for each thing we deprecate.

It is also too early to know how relevant redis-compatility will be in the future. Maybe it will be irrelevant, or maybe not.

Hypothetical: If we re-implement future redis features, we can even say we're compatible with those future redis versions and update our "redis version" to that. (It's unlikely but not impossible.)

PingXie commented 7 months ago

❓ What's the time plan? I suggest we add the switch in Valkey 8. In Valkey 9 we remove it, but we make the config have no effect, so we don't crash if the user has the config. In Valkey 10 we remove the config.

The catch is that we must actually be able to remove it in 9.0. I have seen many cases where designs have deviated from their initial objectives and ended up being maintained indefinitely. Often, removing a feature is far more challenging than adding one. If we can be certain that it will be removed in 9.0, I would vote in favor.

i'm just worried about client libs.

I would say this is a late-binding decision and we should not set a timeline at this moment. The key factor is indeed the client. Until we have a robust native Valkey client ecosystem, breaking these existing applications would risk losing on the adoption. So there is a chance that this compat layer might stay with us for quite a while. That said, I think we have an opportunity to create an active client ecosystem of our own through Glide.

❓ What shall be controlled by it? I suggest the things listed above. For all other error replies and log messages we can just make a breaking change. Other suggestions?

IMO, any customer facing artifacts, including both error strings and log messages, also the command output like that of INFO.

If we squint a bit harder at the "user", there are two different personas when we are talking about "compat": there is the application developers, who interface with Valkey through (mostly non-admin) commands only, and then there is the admins, who own the operations hence need to use the admin commands and deal with log messages among other things. For self managed users, the two roles can be played by the same team and that is why both are important.

❓ Shall it be a compile time switch (ifdef) or runtime (config). I suggest a config. Not all users compile from source.

My vote would be on "run-time". Agreed that not all users (very few IMO) build from source. A run-time config is also a lot easier to deal with for image builders.

If we re-implement future redis features, we can even say we're compatible with those future redis versions and update our "redis version" to that. (It's unlikely but not impossible.

I don't think being compatible with future Redis versions (than 7.2) is a goal. We don't need to intentionally break away from future Redis versions but realistically speaking, 7.2 is the first and last version of Valkey we can say fully compatible with OSS Redis 7.2. The more important message though is "backward compat", i.e., there is a seamless upgrade path from Redis 7.2.4 (and below) and any Valkey version to any future Valkey versions and that is it. We shouldn't promise any other migration path from Valkey to Redis or vice versa.

We can discuss this topic in general (when we deprecate something, we will delete it in X major versions) or case-by-case. Either way, I think we shall make a statement in the release notes and in the docs what the plans are, for each thing we deprecate.

I understand this is just a follow up discussion on the tactics so it won't affect the decision here. But just wanted to add that, without a tight coupling of client and server, deprecation is almost impossible. Case in point: CLUSTER SLOTS.

zuiderkwast commented 7 months ago

@PingXie I think your most important point here is that you want this to cover all error replies and fields, not just a few? I can live with that. What do you others think about that?

without a tight coupling of client and server, deprecation is almost impossible. Case in point: CLUSTER SLOTS.

CLUSTER SLOTS was deprecated immediately when CLUSTER SHARDS was introduced. Give it 2-3 major version and it might be possible to remove it, when all versions without CLUSTER SHARDS are eol.

We can decide that the plan is to remove it in Valkey 9 (or 10, or 11) and then change this decision later, in another major decision, if it turns out to be necessary. (It's a possibility, not a plan.)

Are you saying we should not communicate anything about when we plan to remove this knob?

PingXie commented 7 months ago

@PingXie I think your most important point here is that you want this to cover all error replies and fields, not just a few? I can live with that. What do you others think about that?

Yes and my concerns are more on removal and rename. Adding a few fields, new error strings, or log messages is less of a concern to me and I am fine discussing this new "information" on a case by case basis, e.g., our discussion around the server_name, and valkey_version in INFO, and the new AUX RDB field.

Are you saying we should not communicate anything about when we plan to remove this knob?

Declaring "deprecation" is totally fine with me and we can set a timeline (including right away). "Removing" is the part that I have concern with. We did the exact same thing with CLUSTER SLOTS - I don't remember we ever set a timeline to remove it in our past conversation, even though we all agree it should be "deprecated".

yangbodong22011 commented 7 months ago

In my opinion, if it must be changed to avoid legal risks, introducing a temporary compatibility mark does not make much sense, because it will eventually be removed. (I think there were obvious difficulties in pushing those common client adaptations of valkey that have been acquired by Redis during the time the temporary flag existed).

If we do not have to change these fields, I recommend not to make any changes to the interface return values (except for logging and testing).

zuiderkwast commented 7 months ago

@yangbodong22011 This is not for any legal risk. When it is part of a protocol or API, it's legal to use trademarks. We just think it would be nice to remove it.

yangbodong22011 commented 7 months ago

@yangbodong22011 This is not for any legal risk. When it is part of a protocol or API, it's legal to use trademarks. We just think it would be nice to remove it.

What I'm worried about here is whether the client compatibility feature can be completed on time, but if we Fork common clients, I can guarantee that it will be completed :)

zuiderkwast commented 7 months ago

We can fork some, but also remember that most of the clients are not official Redis clients. I think we can convince many of those client authors to support Valkey and Redis in parallel.

It seems Redis now removed all clients and modules from their website, except their own ones. They obviously don't want a community client and module authors. I think this will just make those even more motivated to transform/rebrand their clients and modules to Valkey.

yangbodong22011 commented 7 months ago

We can fork some, but also remember that most of the clients are not official Redis clients. I think we can convince many of those client authors to support Valkey and Redis in parallel.

Replied here (because I think client issues should be discussed in that issue)

daniel-house commented 7 months ago

We can deprecate anything at any time because it is just a comment or warning. Until we set a hard date on when it will be removed (breaking old systems), it has little meaning and will be ignored by nearly everyone. Even after a removal date is published, far too many will ignore it until two weeks before the removal date (I have seen this happen in real life with mission critical software). Then, when the removal actually happens, a huge percentage of businesses will simply stop upgrading to new versions of Valkey until forced to by a declaration that we will no longer issue security fixes for the version just before the feature was deleted (I have been paid to help upgrade code to jump 2 major releases of Oracle).

I like Madelyn's suggestion of "A three year transition". Perhaps we should publish a commitment to always giving any hard date for removal at least three years in advance.