Add 'extended-redis-compatibility' config#306
Merged
zuiderkwast merged 7 commits intoApr 18, 2024
Conversation
enjoy-binbin
approved these changes
Apr 15, 2024
Add the config, including docs as comments in valkey.conf. Define SERVER_TITLE "Valkey" in version.h. Make the errors -LOADING, -BUSY and -MISCONF depend on the config. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
… config Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
This commit includes a test that will remind us to delete the temporary config when the time comes. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
161b6d0 to
82b95c8
Compare
Explain the plans for removal in future versions. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Add the config, including docs as comments in valkey.conf. Define SERVER_TITLE "Valkey" in version.h. Make the errors -LOADING, -BUSY and -MISCONF depend on the config. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
82b95c8 to
8c26e98
Compare
PatrickJS
pushed a commit
to PatrickJS/placeholderkv
that referenced
this pull request
Apr 24, 2024
New config 'extended-redis-compatibility' (yes/no) default no
* When yes:
* Use "Redis" in the following error replies:
- `-LOADING Redis is loading the dataset in memory`
- `-BUSY Redis is busy`...
- `-MISCONF Redis is configured to`...
* Use `=== REDIS BUG REPORT` in the crash log delimiters (START and
END).
* The HELLO command returns `"server" => "redis"` and `"version" =>
"7.2.4"` (our Redis OSS compatibility version).
* The INFO field for mode is called `"redis_mode"`.
* When no:
* Use "Valkey" instead of "Redis" in the mentioned errors and crash log
delimiters.
* The HELLO command returns `"server" => "valkey"` and the Valkey
version for `"version"`.
* The INFO field for mode is called `"server_mode"`.
* Documentation added in valkey.conf:
> Valkey is largely compatible with Redis OSS, apart from a few cases
where
> Redis OSS compatibility mode makes Valkey pretend to be Redis. Enable
this
> only if you have problems with tools or clients. This is a temporary
> configuration added in Valkey 8.0 and is scheduled to have no effect
in Valkey
> 9.0 and be completely removed in Valkey 10.0.
* A test case for the config is added. It is designed to fail if the
config is not deprecated (has no effect) in Valkey 9 and deleted in
Valkey 10.
* Other test cases are adjusted to work regardless of this config.
Fixes valkey-io#274
Fixes valkey-io#61
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
7 tasks
6 tasks
Comment on lines
5608
to
5610
| "redis_git_sha1:%s\r\n", serverGitSHA1(), | ||
| "redis_git_dirty:%i\r\n", strtol(serverGitDirty(),NULL,10) > 0, | ||
| "redis_build_id:%s\r\n", serverBuildIdString(), |
Member
There was a problem hiding this comment.
Do you remember why we didn't deal with these fields (redis_git_sha1/redis_git_dirty/redis_build_id)? I thought we did.
Contributor
Author
There was a problem hiding this comment.
You mean why we didn't rename them to valkey or server?
I think some of maintainers were worried that it would break some tools or clients. I'm not sure. I want to get rid of these names some time. Maybe in 10.0 we can do it?
Member
There was a problem hiding this comment.
Yes, why wasn't server_git_sha1 added in the same way when server.extended_redis_compat was used?
Contributor
Author
There was a problem hiding this comment.
I tried to change it before 9.0 here:
Member
There was a problem hiding this comment.
Ok, i see #1736 is changing without server.extended_redis_compat, i think it would be great if it they are changeing together. I feel like now we don't have chance to get rid of it.
"%s_mode:", (server.extended_redis_compat ? "redis" : "server"),
"%s\r\n", mode,
Contributor
Author
There was a problem hiding this comment.
I'm OK with it. I don't know if others will agree... @valkey-io/core-team WDYT?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New config 'extended-redis-compatibility' (yes/no) default no
When yes:
-LOADING Redis is loading the dataset in memory-BUSY Redis is busy...-MISCONF Redis is configured to...=== REDIS BUG REPORTin the crash log delimiters (START and END)."server" => "redis"and"version" => "7.2.4"(our Redis OSS compatibility version)."redis_mode".When no:
"server" => "valkey"and the Valkey version for"version"."server_mode".Documentation added in valkey.conf:
A test case for the config is added. It is designed to fail if the config is not deprecated (has no effect) in Valkey 9 and deleted in Valkey 10.
Other test cases are adjusted to work regardless of this config.
Fixes #274
Fixes #61