Cluster V2 modularity options

[madolson]

One of the first steps towards implementing cluster v2, is refactoring the code so that we can use either clustering implementation. There was some initial work, but I view it as incomplete. So I want to lay forward the current challenges and my proposed solutions.

Inconsistent abstraction

Some previous refactoring was done to better isolate the clustering code by defining a set of APIs for manipulating the cluster state. However, the abstraction is surface level as cluster_legacy.c still imports server.h and sets a bunch of external state. It feels like the goal was to just move everything that was "inconvenient" for cluster v2 to cluster_legacy. Some examples of APIs which feel like they're floating on both side of the abstraction:

1. CLUSTER SHARDS is still implemented in cluster_legacy.c
2. delKeysInSlots: This function originally was claimed to be only relevant to the current cluster, but I don't see why.

So, I will take a fresh pass and take the following approach for the files:

1. cluster_legacy.(c|h): These files include all of the code for maintaining the clusterbus and the internal topology it maintains. It will maintain the administration commands for manipulating that topology.
2. cluster.(c|h): This file will be responsible for serving all user cluster requests (CLUSTER SHARDS, CLUSTER SLOTS, CLUSTER INFO, ...) using the abstractions provided.

For cluster administration commands. I don't think we really understand the abstraction yet. For example, should we continue to support CLUSTER REPLICATE in cluster v2? It could simply forward request to the raft group. It seems weird to force it to be driven entirely externally. So for administration commands, I'll leave everything where it is until we have a lower level understanding.

Limited runtime modularity

The current implementation assumed a static compile time flag to switch from one implementation to the other. I don't like this, I think both of the systems should be compiled in by default and you can switch between them at startup. To solve this, I would like to implement 3 "types" that define the interface for cluster:

typedef struct clusterType {
    void (*init)(void); /* A function to be to executed at startup. */
    sds (*info)(void); /* A function to be executed whenever info is called. */ 
    .... /* Other APIs that define 
}

typedef struct clusterTopologyType {
    int (*size)(void); /* Returns the size of the topology */
    clusterNode *(*getMyclusterNode)(void);
    clusterNode *(*lookupNode)(const char *name, int length);
    ....
} clusterTopologyApi;

typedef struct clusterNodeApi {
    char *(*clusterNodeIp)(clusterNode *node); /* Returns the IP for the node. */
    ....
} clusterNodeApi;

Everything other than administration commands should fit into this mold. We'll have helper methods just like the connection abstraction, such as clusterLookupNode which will call the underlying implementation for whichever type is loaded.

This will also allow one other neat thing, which is we will be able to easily have this call an external module, so we can prototype with code and load it in pretty easily. I would like us to explore implementing clusterv2 with rust, so this will get us a better time implementing that.

[madolson]

@PingXie Thoughts ^

[daniel-house]

Here are the cluster subcommands from commands.def

These are the subcommands that are flagged as CMD_ADMIN:

addslots 
addslotsrange 
bumpepoch 
count-failure-reports 
delslots 
delslotsrange 
failover 
flushslots 
forget 
meet 
replicas 
replicate 
reset 
saveconfig 
set-config-epoch 
setslot 
slaves 

These are the rest of the subcommands:

countkeysinslot
getkeysinslot
help 
info
keyslot
links
myid
myshardid
nodes
shards 
slots 

[zuiderkwast]

cluster_legacy.(c|h): These files include all of the code for maintaining the clusterbus and the internal topology it maintains. It will maintain the administration commands for manipulating that topology.

Half of the clusterNode struct is client facing information, like a node's name, its slots, IP, port, TLS port, it's replicas, etc. This information is often accessed from cluster.c for various client-facing commands.

The other half of the same struct is related to the cluster bus. Is it worth splitting the struct?

[madolson]

The other half of the same struct is related to the cluster bus. Is it worth splitting the struct?

The current abstraction is that the clusterbus owns the underlying struct, and can define whatever implementation it wants for modifying the data. It just needs to provide a coherent external API for cluster.c. I'm not exactly sure how we would split it.