Initial cleanup for cluster refactoring (#460)

Cleaned up the minor cluster refactoring notes that were intended to be
follow ups that never happened. Basically:
1. Minor style nitpicks
2. Generalized clusterNodeIsMyself so that it wasn't implementation
dependent.
3. Removed getMyClusterId, and just make it an explicit call to myself's
name, which seems more straightforward and removes unnecessary
abstraction.
4. Remove clusterNodeGetSlaveof infavor of clusterNodeGetMaster. We
already do a check if it's a replica, and if it wasn't working it would
have been crashing.

Signed-off-by: Madelyn Olson <[email protected]>

src/cluster.c

@@ -789,8 +789,8 @@ void clusterCommandMyId(client *c) {
     }
 }
 
-char* getMyClusterId(void) {
-    return clusterNodeGetName(getMyClusterNode());
+int clusterNodeIsMyself(clusterNode *n) {
+    return n == getMyClusterNode();
 }
 
 void clusterCommandMyShardId(client *c) {
@@ -1193,7 +1193,7 @@ clusterNode *getNodeByQuery(client *c, struct serverCommand *cmd, robj **argv, i
     if (((c->flags & CLIENT_READONLY) || pubsubshard_included) &&
         !is_write_command &&
         clusterNodeIsSlave(myself) &&
-        clusterNodeGetSlaveof(myself) == n)
+        clusterNodeGetMaster(myself) == n)
     {
         return myself;
     }
@@ -1286,7 +1286,7 @@ int clusterRedirectBlockedClientIfNeeded(client *c) {
              * replica can handle, allow it. */
             if ((c->flags & CLIENT_READONLY) &&
                 !(c->lastcmd->flags & CMD_WRITE) &&
-                clusterNodeIsSlave(myself) && clusterNodeGetSlaveof(myself) == node)
+                clusterNodeIsSlave(myself) && clusterNodeGetMaster(myself) == node)
             {
                 node = myself;
             }

src/cluster.h

@@ -60,33 +60,29 @@ int handleDebugClusterCommand(client *c);
 const char **clusterDebugCommandExtendedHelp(void);
 /* handle implementation specific cluster commands. Return 1 if handled, 0 otherwise. */
 int clusterCommandSpecial(client *c);
-const char** clusterCommandExtendedHelp(void);
+const char **clusterCommandExtendedHelp(void);
 
 int clusterAllowFailoverCmd(client *c);
 void clusterPromoteSelfToMaster(void);
 int clusterManualFailoverTimeLimit(void);
 
-void clusterCommandSlots(client * c);
+void clusterCommandSlots(client *c);
 void clusterCommandMyId(client *c);
 void clusterCommandMyShardId(client *c);
 void clusterCommandShards(client *c);
 sds clusterGenNodeDescription(client *c, clusterNode *node, int tls_primary);
 
 int clusterNodeCoversSlot(clusterNode *n, int slot);
 int getNodeDefaultClientPort(clusterNode *n);
-int clusterNodeIsMyself(clusterNode *n);
 clusterNode *getMyClusterNode(void);
-char *getMyClusterId(void);
 int getClusterSize(void);
 int getMyShardSlotCount(void);
 int handleDebugClusterCommand(client *c);
-int clusterNodePending(clusterNode  *node);
+int clusterNodePending(clusterNode *node);
 int clusterNodeIsMaster(clusterNode *n);
 char **getClusterNodesList(size_t *numnodes);
-int clusterNodeIsMaster(clusterNode *n);
 char *clusterNodeIp(clusterNode *node);
 int clusterNodeIsSlave(clusterNode *node);
-clusterNode *clusterNodeGetSlaveof(clusterNode *node);
 clusterNode *clusterNodeGetMaster(clusterNode *node);
 char *clusterNodeGetName(clusterNode *node);
 int clusterNodeTimedOut(clusterNode *node);
@@ -106,6 +102,7 @@ clusterNode *clusterLookupNode(const char *name, int length);
 void clusterReplicateOpenSlots(void);
 
 /* functions with shared implementations */
+int clusterNodeIsMyself(clusterNode *n);
 clusterNode *getNodeByQuery(client *c, struct serverCommand *cmd, robj **argv, int argc, int *hashslot, int *ask);
 int clusterRedirectBlockedClientIfNeeded(client *c);
 void clusterRedirectClient(client *c, clusterNode *n, int hashslot, int error_code);

src/cluster_legacy.c

@@ -6090,10 +6090,6 @@ unsigned int countChannelsInSlot(unsigned int hashslot) {
     return kvstoreDictSize(server.pubsubshard_channels, hashslot);
 }
 
-int clusterNodeIsMyself(clusterNode *n) {
-    return n == server.cluster->myself;
-}
-
 clusterNode *getMyClusterNode(void) {
     return server.cluster->myself;
 }
@@ -6175,7 +6171,7 @@ int handleDebugClusterCommand(client *c) {
     return 1;
 }
 
-int clusterNodePending(clusterNode  *node) {
+int clusterNodePending(clusterNode *node) {
     return node->flags & (CLUSTER_NODE_NOADDR|CLUSTER_NODE_HANDSHAKE);
 }
 
@@ -6187,10 +6183,6 @@ int clusterNodeIsSlave(clusterNode *node) {
     return node->flags & CLUSTER_NODE_SLAVE;
 }
 
-clusterNode *clusterNodeGetSlaveof(clusterNode *node) {
-    return node->slaveof;
-}
-
 clusterNode *clusterNodeGetMaster(clusterNode *node) {
     while (node->slaveof != NULL) node = node->slaveof;
     return node;

src/module.c

@@ -9017,7 +9017,7 @@ void VM_FreeClusterNodesList(char **ids) {
  * is disabled. */
 const char *VM_GetMyClusterID(void) {
     if (!server.cluster_enabled) return NULL;
-    return getMyClusterId();
+    return clusterNodeGetName(getMyClusterNode());
 }
 
 /* Return the number of nodes in the cluster, regardless of their state
@@ -9064,8 +9064,8 @@ int VM_GetClusterNodeInfo(ValkeyModuleCtx *ctx, const char *id, char *ip, char *
         /* If the information is not available, the function will set the
          * field to zero bytes, so that when the field can't be populated the
          * function kinda remains predictable. */
-        if (clusterNodeIsSlave(node) && clusterNodeGetSlaveof(node))
-            memcpy(master_id, clusterNodeGetName(clusterNodeGetSlaveof(node)) ,VALKEYMODULE_NODE_ID_LEN);
+        if (clusterNodeIsSlave(node) && clusterNodeGetMaster(node))
+            memcpy(master_id, clusterNodeGetName(clusterNodeGetMaster(node)) ,VALKEYMODULE_NODE_ID_LEN);
         else
             memset(master_id,0,VALKEYMODULE_NODE_ID_LEN);
     }