Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: support Redis 7+ Cluster changes #46

Open
mna opened this issue Oct 29, 2022 · 0 comments
Open

Proposal: support Redis 7+ Cluster changes #46

mna opened this issue Oct 29, 2022 · 0 comments

Comments

@mna
Copy link
Owner

mna commented Oct 29, 2022

Description

Starting with Redis 7, CLUSTER SLOTS is considered deprecated (but is still supported and works as before), and CLUSTER SHARDS is the new recommended way for cluster clients to discover the cluster topology.

This new command also supports a more flexible way for Redis admins to configure how clients are expected to connect to the cluster nodes, via the new cluster-preferred-endpoint-type configuration option. See the command's documentation for details.

For backwards compatibility (that is, redisc compatibility with older Redis versions), redisc should still support running CLUSTER SLOTS. The proposed approach is as follows:

  • Add Cluster.RedisMajor and Cluster.RedisMinor fields (ints) to explicitly state the version of Redis that will be used for the cluster. The minor version can be set to <0 if unknown/unspecified/irrelevant. If set to major version 7+, the cluster will use CLUSTER SHARDS, otherwise CLUSTER SLOTS. This should of course be set before using the cluster. Specifying the version allows similar upgrades/evolution in the future.
  • If the version fields are not set (both are <=0), if the WaitForCluster proposal is implemented, automatically read the version from Redis using INFO server. Otherwise, or if the Redis version has not been loaded yet, if Refresh gets called, do the same thing (read the version using INFO server).
  • Otherwise, if the version is not set explicity and neither WaitForCluster nor Refresh gets called, keep using CLUSTER SLOTS (assume Redis < 7).

The rationale for the auto-detection is that it may not be convenient to explicitly set the Redis version in code when the application e.g. supports different Redis versions or uses different ones in different environments. Doing so during WaitForCluster makes sense because the application is explicitly saying that it is a good place to block for a bit (although it does consume part of the deadline allocated to wait for the cluster, but the implementation should always check for the cluster's stability at least once before returning).

The same can be said when Refresh is called, as it is highly recommended to call it before using the cluster. Finally, if none of those are called and the new fields are not set, then the fallback is to work as before (Redis < 7). The application should change its code to either call those explicitly, or set the new fields.

As this new API consists of new fields and new internal implementation details, it could be released as part of a minor version.

Use Case

This is required to support the new Redis 7+ feature of cluster-preferred-endpoint-type configuration option, and to prepare in case CLUSTER SLOTS support gets removed in a future Redis version.

Implementation Concerns

What if in auto-detection mode INFO server fails? I think that in this case any other command would likely fail too (e.g. network error), so return the error from WaitForCluster or Refresh, prompting the application to handle that failure. If it keeps running despite the error, then it will behave as for Redis < 7.

Is the minor version really relevant in the new API? I'd like to say no and we can do without it, but Redis has a history of releasing important changes in minor versions. Even recently, for example, ACL SETUSER has seen important additions in 6.2 (https://redis.io/commands/acl-setuser/).


(note that this proposal is not a promise of implementation, nor is it a promise of merging a PR that would implement this feature - it is first and foremost for me to give it some thought and as a reminder when I have some time to work on the package, and published for visibility)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant