Should functions be synchronized among cluster nodes?

[CharlesChen888]

This issue was first raised in Redis community redis/redis#11780, it is a major miss from Redis 7.0, but haven't got a solution. I believe we still need to discuss this issue in this new project, perhaps as a part of new cluster architecture.

The problem:

Let's say:

• a slot, which includes key1, is migrated from node A to node B, operated by client1
• in node A a function myfunc is already loaded, but in node B it is not

Now client2, that used to handle key1 in node A, sends FCALL myfunc 1 key1 to node A, which will be redirected to node B, will then get an unexpected error.
Or if node B also has a function myfunc but it does different things, client2 may trigger unexpected results.

Some thoughts:

1.Should functions be migrated together with slots?
Maybe not. This may overwrite the libraries already loaded in the nodes importing slots. But we may set a version to each library so that we can always choose to keep the latest one.

2.Can functions be broadcasted to all nodes?
May cause confict between nodes. And when adding new nodes we need to have a full synchronization.

[hpatro]

1.Should functions be migrated together with slots?

I don't think slots should be associated with functions. functions should be treated as a resource at node level. On each node addition, all functions existing in the cluster/standalone primary should be applied.

Overall, I think we should treat functions, ACLs and config(s) as resource(s) which should be applied to all the nodes and build it in the cluster revamp which we have been talking for a while.

[zuiderkwast]

IIRC it was decided that functions are similar to configuration and that it's the admin's responsibility to define functions on all nodes. I don't know if it's a good decision.

The official admin tool is valkey-cli, so when managing a cluster (creating a cluster, adding nodes, etc.) perhaps valkey-cli should try to synchronous functions across nodes? That would at least match the current design decisions.

It's the same problem with ACL and other config in a cluster: They are expected to be the same throughout the cluster but the cluster doesn't guarantee this.

I'm not sure I believe in a complete rewrite of the cluster bus. Usually such projects are never finished. Can we try gossipping this information instead?

[madolson]

IIRC it was decided that functions are similar to configuration and that it's the admin's responsibility to define functions on all nodes. I don't know if it's a good decision.

Zhao can keep me honest, but this is the only time there was a real 3-2 decision amongst the old maintainers. (It was a more broad discussion on functions, but basically they said this is what Redis wanted).

I generally want to implement a new system that is distributed configuration, that supports functions, ACLS, configs and allows modules to also store distributed configuration. This can be built ontop of the existing clusterbus extensions, but will be in its best form when built with the cluster v2. (I see Viktor is dubious, but Ping and I are highly motivated to make this happen now)

[zuiderkwast]

I'm motivated, but I also realize we have a lot of things going on, and I want us to make releases more often. Can we make it a focus area for 9.0? (That means start prototyping now.)

[madolson]

I'm motivated, but I also realize we have a lot of things going on, and I want us to make releases more often. Can we make it a focus area for 9.0? (That means start prototyping now.)

My vision, is that we finish the needed refactoring work to make cluster a module for valkey 8. Then it becomes easy to iterate and prototype in a separate repo the new clustering work. Then someone can "opt-in" to the beta of it while Valkey 8 is out and about, and we can finalize it for 9.

[zuiderkwast]

I think we shall start using some of the reserved bits in the clusterMsg to indicate support for new cluster bus messages. From there, we can add new message types and turn the cluster bus more into a proper raft cluster, step by step.

Easier to test, easier to get things right, if we do things in smaller steps.

[PingXie]

Timeline aside, this is actually a problem that can ONLY be properly solved by cluster V2 because it gives you a strongly consistent (meta)data service that manages cluster-wide metadata like cluster topology, node configs, ACLs, and Functions, etc

Cluster V1 being eventually consistent (which is not 100% true by the way) and the coupling of data serving logic and topology management logic on the same node are the source of these oddities, IMO. I also think implementing a true raft cluster inside of the Valkey server is no less work than cluster V2 itself. The coupling shouldn't be there in the first place.

I am in favor of moving the entire cluster out of the engine and into its own module and this needs to be the precursor to the real cluster V2 work. That said, I don't have a clear idea on the complexity involved so I don't know if it can happen in Valkey 8, yet. Also agreed that building cluster V2 into the engine is a non-starter.

[zuiderkwast]

It's very good that we're having this discussion. We need to have all the information and aspects on the table to be able to make informed decisions.

Timeline aside

Beautiful, but I don't think we shall ignore time.

Just adding module APIs normally involves a lot of work just in terms of API design, testing and documentation. This makes adding APIs much more work than adding internal functions.

Which module API functions do we need to add and what will these APIs look like? Will the cluster module keep all the cluster state in its own memory, intercept commands and sometimes return redirects for them?

cluster V2 because it gives you a strongly consistent (meta)data service that manages cluster-wide metadata like cluster topology, node configs, ACLs, and Functions, etc

So we need to have module APIs for all of those, including some event mechanism so if a user or any other module changes any of these, so the cluster module can get these changes. (Not a very small and easy job.)

The coupling shouldn't be there in the first place.
Also agreed that building cluster V2 into the engine is a non-starter.

Who agreed on this? If we're using a raft library, we avoid that coupling. A node can still be a cluster node without serving any data. Agreed that moving things to a module is a guarantee when it comes to separation of concerns, but it is also much more work.

[daniel-house]

Can anyone suggest a use case where we would actually prefer to have different implementations of the same function on different nodes? If not, then we have the answer to the question raised by this issue.

[zuiderkwast]

@daniel-house You're right, it's better that they are synchronized. The open question is how.

Can you suggest how to implement it within the current cluster solution that makes sure they are synchronized on all nodes?

[daniel-house]

OK, we agree that they should be synchronized.

I can't say I understand the current cluster solution, but I can see that we are having problems synchronizing hostnames (#304), so I have serious doubts that we can synchronize functions at this time. It seems to me that a good solution would also solve the hostnames issue and the problems raised in #114 . If anyone can point me to useful reading about the current cluster solution, please do.

I would appreciate any pointers to more information about cluster V2. I get the general impression that it will use Raft leader election, and would be perfect for this, hostnames, and the ultimate goals of #114 . If so, I would suggest punting all of these issues until after cluster V2 is implemented and assigning a very high priority to cluster V2.

[zuiderkwast]

If anyone can point me to useful reading about the current cluster solution, please do.

• The cluster spec
• The cluster manual
• The source code