Replace Dict with Rax for Cluster Nodes

Replace Dict with Rax for Cluster Nodes and construct primaries list on the go, instead of maintaining shards/masters list. Signed-off-by: Ram Prasad Voleti <[email protected]>
valkey-io · Jul 5, 2024 · 45b7926 · 45b7926
1 parent 622a1bf
commit 45b7926
Show file tree

Hide file tree

Showing 8 changed files with 211 additions and 266 deletions.
diff --git a/src/cluster.c b/src/cluster.c
@@ -849,10 +849,10 @@ void clusterCommand(client *c) {
     } else if (!strcasecmp(c->argv[1]->ptr, "slots") && c->argc == 2) {
         /* CLUSTER SLOTS */
         clusterCommandSlots(c);
-    } else if (!strcasecmp(c->argv[1]->ptr,"shards") && c->argc == 2) {
+    } else if (!strcasecmp(c->argv[1]->ptr, "shards") && c->argc == 2) {
         /* CLUSTER SHARDS */
         clusterCommandShards(c);
-    } else if (!strcasecmp(c->argv[1]->ptr,"info") && c->argc == 2) {
+    } else if (!strcasecmp(c->argv[1]->ptr, "info") && c->argc == 2) {
         /* CLUSTER INFO */
 
         sds info = genClusterInfoString();

diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c
diff --git a/src/cluster_legacy.h b/src/cluster_legacy.h
@@ -320,10 +320,8 @@ struct clusterState {
     uint64_t currentEpoch;
     int state;              /* CLUSTER_OK, CLUSTER_FAIL, ... */
     int size;               /* Num of primary nodes with at least one slot */
-    dict *nodes;            /* Hash table of name -> clusterNode structures */
+    rax *nodes;             /* Table mapping of name -> clusterNode structures */
     dict *nodes_black_list; /* Nodes we don't re-add for a few seconds. */
-    clusterNode **masters; /* pointers to master nodes */
-    int nummasters;        /* Number of master nodes */
     clusterNode *migrating_slots_to[CLUSTER_SLOTS];
     clusterNode *importing_slots_from[CLUSTER_SLOTS];
     clusterNode *slots[CLUSTER_SLOTS];

diff --git a/src/commands.def b/src/commands.def
@@ -896,7 +896,9 @@ struct COMMAND_ARG CLUSTER_SETSLOT_Args[] = {
 
 #ifndef SKIP_CMD_TIPS_TABLE
 /* CLUSTER SHARDS tips */
-#define CLUSTER_SHARDS_Tips NULL
+const char *CLUSTER_SHARDS_Tips[] = {
+"nondeterministic_output",
+};
 #endif
 
 #ifndef SKIP_CMD_KEY_SPECS_TABLE
@@ -1027,7 +1029,7 @@ struct COMMAND_STRUCT CLUSTER_Subcommands[] = {
 {MAKE_CMD("saveconfig","Forces a node to save the cluster configuration to disk.","O(1)","3.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTER_SAVECONFIG_History,0,CLUSTER_SAVECONFIG_Tips,0,clusterCommand,2,CMD_NO_ASYNC_LOADING|CMD_ADMIN|CMD_STALE,0,CLUSTER_SAVECONFIG_Keyspecs,0,NULL,0)},
 {MAKE_CMD("set-config-epoch","Sets the configuration epoch for a new node.","O(1)","3.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTER_SET_CONFIG_EPOCH_History,0,CLUSTER_SET_CONFIG_EPOCH_Tips,0,clusterCommand,3,CMD_NO_ASYNC_LOADING|CMD_ADMIN|CMD_STALE,0,CLUSTER_SET_CONFIG_EPOCH_Keyspecs,0,NULL,1),.args=CLUSTER_SET_CONFIG_EPOCH_Args},
 {MAKE_CMD("setslot","Binds a hash slot to a node.","O(1)","3.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTER_SETSLOT_History,1,CLUSTER_SETSLOT_Tips,0,clusterCommand,-4,CMD_NO_ASYNC_LOADING|CMD_ADMIN|CMD_STALE|CMD_MAY_REPLICATE,0,CLUSTER_SETSLOT_Keyspecs,0,NULL,3),.args=CLUSTER_SETSLOT_Args},
-{MAKE_CMD("shards","Returns the mapping of cluster slots to shards.","O(N) where N is the total number of cluster nodes","7.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTER_SHARDS_History,0,CLUSTER_SHARDS_Tips,0,clusterCommand,2,CMD_LOADING|CMD_STALE,0,CLUSTER_SHARDS_Keyspecs,0,NULL,0)},
+{MAKE_CMD("shards","Returns the mapping of cluster slots to shards.","O(N) where N is the total number of cluster nodes","7.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTER_SHARDS_History,0,CLUSTER_SHARDS_Tips,1,clusterCommand,2,CMD_LOADING|CMD_STALE,0,CLUSTER_SHARDS_Keyspecs,0,NULL,0)},
 {MAKE_CMD("slaves","Lists the replica nodes of a primary node.","O(N) where N is the number of replicas.","3.0.0",CMD_DOC_DEPRECATED,"`CLUSTER REPLICAS`","5.0.0","cluster",COMMAND_GROUP_CLUSTER,CLUSTER_SLAVES_History,0,CLUSTER_SLAVES_Tips,1,clusterCommand,3,CMD_ADMIN|CMD_STALE,0,CLUSTER_SLAVES_Keyspecs,0,NULL,1),.args=CLUSTER_SLAVES_Args},
 {MAKE_CMD("slot-stats","Return an array of slot usage statistics for slots assigned to the current node.","O(N) where N is the total number of slots based on arguments. O(N*log(N)) with ORDERBY subcommand.","8.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTER_SLOT_STATS_History,0,CLUSTER_SLOT_STATS_Tips,2,clusterSlotStatsCommand,-4,CMD_STALE|CMD_LOADING,0,CLUSTER_SLOT_STATS_Keyspecs,0,NULL,1),.args=CLUSTER_SLOT_STATS_Args},
 {MAKE_CMD("slots","Returns the mapping of cluster slots to nodes.","O(N) where N is the total number of Cluster nodes","3.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTER_SLOTS_History,2,CLUSTER_SLOTS_Tips,1,clusterCommand,2,CMD_LOADING|CMD_STALE,0,CLUSTER_SLOTS_Keyspecs,0,NULL,0)},

diff --git a/src/commands/cluster-shards.json b/src/commands/cluster-shards.json
@@ -11,6 +11,9 @@
             "LOADING",
             "STALE"
         ],
+        "command_tips": [
+            "NONDETERMINISTIC_OUTPUT"
+        ],
         "reply_schema": {
             "description": "A nested list of a map of hash ranges and shard nodes describing individual shards.",
             "type": "array",

diff --git a/src/rax.h b/src/rax.h
@@ -95,6 +95,7 @@
  */
 
 #define RAX_NODE_MAX_SIZE ((1 << 29) - 1)
+#define RAX_OK 1
 typedef struct raxNode {
     uint32_t iskey : 1;   /* Does this node contain a key? */
     uint32_t isnull : 1;  /* Associated value is NULL (don't store it). */

diff --git a/tests/cluster/tests/28-cluster-shards.tcl b/tests/cluster/tests/28-cluster-shards.tcl
@@ -285,24 +285,3 @@ test "CLUSTER MYSHARDID reports same shard id after cluster restart" {
         assert_equal [dict get $node_ids $i] [R $i cluster myshardid]
     }
 }
-
-test "Deterministic order of CLUSTER SHARDS response" {
-    set node_ids {}
-    for {set j 0} {$j < 8} {incr j} {
-        set shards_cfg [R $j CLUSTER SHARDS]
-        set i 0
-        foreach shard_cfg $shards_cfg {
-            set nodes [dict get $shard_cfg nodes]
-            foreach node $nodes {
-                if {$j == 0} {
-                    # Save the node ids from the first node response
-                    dict set node_ids $i [dict get $node id]
-                } else {
-                    # Verify the order of the node ids is the same as the first node response
-                    assert_equal [dict get $node id] [dict get $node_ids $i]
-                }
-                incr i
-            }
-        }
-    }
-}
diff --git a/tests/unit/cluster/cluster-shards.tcl b/tests/unit/cluster/cluster-shards.tcl
@@ -0,0 +1,22 @@
+start_cluster 4 4 {tags {external:skip cluster}} {
+    test "Deterministic order of CLUSTER SHARDS response" {
+        set node_ids {}
+        for {set j 0} {$j < 8} {incr j} {
+            set shards_cfg [R $j CLUSTER SHARDS]
+            set i 0
+            foreach shard_cfg $shards_cfg {
+                set nodes [dict get $shard_cfg nodes]
+                foreach node $nodes {
+                    if {$j == 0} {
+                        # Save the node ids from the first node response
+                        dict set node_ids $i [dict get $node id]
+                    } else {
+                        # Verify the order of the node ids is the same as the first node response
+                        assert_equal [dict get $node id] [dict get $node_ids $i]
+                    }
+                    incr i
+                }
+            }
+        }
+    }
+}