diff --git a/redis.conf b/redis.conf index ee603b118..97f077b0d 100644 --- a/redis.conf +++ b/redis.conf @@ -1743,6 +1743,11 @@ aof-timestamp-enabled no # # cluster-announce-hostname "" +# Clusters can configure an optional nodename to be used in addition to the node ID for +# debugging and admin information. This name is broadcasted between nodes, so will be used +# in addition to the node ID when reporting cross node events such as node failures. +# cluster-announce-human-nodename "" + # Clusters can advertise how clients should connect to them using either their IP address, # a user defined hostname, or by declaring they have no endpoint. Which endpoint is # shown as the preferred endpoint is set by using the cluster-preferred-endpoint-type diff --git a/src/cluster.c b/src/cluster.c index a4fa29d2b..7389909ac 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -89,6 +89,9 @@ void clusterRemoveNodeFromShard(clusterNode *node); int auxShardIdSetter(clusterNode *n, void *value, int length); sds auxShardIdGetter(clusterNode *n, sds s); int auxShardIdPresent(clusterNode *n); +int auxHumanNodenameSetter(clusterNode *n, void *value, int length); +sds auxHumanNodenameGetter(clusterNode *n, sds s); +int auxHumanNodenamePresent(clusterNode *n); static void clusterBuildMessageHdr(clusterMsg *hdr, int type, size_t msglen); /* Links to the next and previous entries for keys in the same slot are stored @@ -171,8 +174,8 @@ typedef struct { /* Assign index to each aux field */ typedef enum { - af_start, - af_shard_id = af_start, + af_shard_id, + af_human_nodename, af_count, } auxFieldIndex; @@ -182,14 +185,15 @@ typedef enum { * 2. aux name can contain characters that pass the isValidAuxChar check only */ auxFieldHandler auxFieldHandlers[] = { {"shard-id", auxShardIdSetter, auxShardIdGetter, auxShardIdPresent}, + {"nodename", auxHumanNodenameSetter, auxHumanNodenameGetter, auxHumanNodenamePresent}, }; int isValidAuxChar(int c) { - return isalnum(c) || (strchr("!#$%&()*+-.:;<>?@[]^_{|}~", c) != NULL); + return isalnum(c) || (strchr("!#$%&()*+:;<>?@[]^{|}~", c) == NULL); } -int isValidAuxString(sds s) { - for (unsigned i = 0; i < sdslen(s); i++) { +int isValidAuxString(char *s, unsigned int length) { + for (unsigned i = 0; i < length; i++) { if (!isValidAuxChar(s[i])) return 0; } return 1; @@ -219,6 +223,30 @@ int auxShardIdPresent(clusterNode *n) { return strlen(n->shard_id); } +int auxHumanNodenameSetter(clusterNode *n, void *value, int length) { + if (n && !strncmp(value, n->human_nodename, length)) { + return C_OK; + } else if (!n && (length == 0)) { + return C_OK; + } + if (n) { + n->human_nodename = sdscpylen(n->human_nodename, value, length); + } else if (sdslen(n->human_nodename) != 0) { + sdsclear(n->human_nodename); + } else { + return C_ERR; + } + return C_OK; +} + +sds auxHumanNodenameGetter(clusterNode *n, sds s) { + return sdscatprintf(s, "%.40s", n->human_nodename); +} + +int auxHumanNodenamePresent(clusterNode *n) { + return strlen(n->human_nodename); +} + /* clusterLink send queue blocks */ typedef struct { size_t totlen; /* Total length of this block including the message */ @@ -328,7 +356,7 @@ int clusterLoadConfig(char *filename) { clusterAddNode(n); } /* Format for the node address and auxiliary argument information: - * ip:port[@cport][,hostname[,aux=val]*] */ + * ip:port[@cport][,hostname][,aux=val]*] */ aux_argv = sdssplitlen(argv[1], sdslen(argv[1]), ",", 1, &aux_argc); if (aux_argv == NULL) { @@ -362,7 +390,7 @@ int clusterLoadConfig(char *filename) { /* Validate that both aux and value contain valid characters only */ for (unsigned j = 0; j < 2; j++) { - if (!isValidAuxString(field_argv[j])) { + if (!isValidAuxString(field_argv[j],sdslen(field_argv[j]))){ /* Invalid aux field format */ sdsfreesplitres(field_argv, field_argc); sdsfreesplitres(argv,argc); @@ -396,7 +424,6 @@ int clusterLoadConfig(char *filename) { sdsfreesplitres(field_argv, field_argc); } - /* Address and port */ if ((p = strrchr(aux_argv[0],':')) == NULL) { sdsfreesplitres(aux_argv, aux_argc); @@ -811,6 +838,22 @@ static void updateAnnouncedHostname(clusterNode *node, char *new) { clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG); } +static void updateAnnouncedHumanNodename(clusterNode *node, char *new) { + if (new && !strcmp(new, node->human_nodename)) { + return; + } else if (!new && (sdslen(node->human_nodename) == 0)) { + return; + } + + if (new) { + node->human_nodename = sdscpy(node->human_nodename, new); + } else if (sdslen(node->human_nodename) != 0) { + sdsclear(node->human_nodename); + } + clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG); +} + + static void updateShardId(clusterNode *node, const char *shard_id) { if (memcmp(node->shard_id, shard_id, CLUSTER_NAMELEN) != 0) { clusterRemoveNodeFromShard(node); @@ -836,6 +879,11 @@ void clusterUpdateMyselfHostname(void) { updateAnnouncedHostname(myself, server.cluster_announce_hostname); } +void clusterUpdateMyselfHumanNodename(void) { + if (!myself) return; + updateAnnouncedHumanNodename(myself, server.cluster_announce_human_nodename); +} + void clusterInit(void) { int saveconf = 0; @@ -919,6 +967,7 @@ void clusterInit(void) { clusterUpdateMyselfFlags(); clusterUpdateMyselfIp(); clusterUpdateMyselfHostname(); + clusterUpdateMyselfHumanNodename(); } void clusterInitListeners(void) { @@ -1256,6 +1305,7 @@ clusterNode *createClusterNode(char *nodename, int flags) { node->inbound_link = NULL; memset(node->ip,0,sizeof(node->ip)); node->hostname = sdsempty(); + node->human_nodename = sdsempty(); node->port = 0; node->cport = 0; node->pport = 0; @@ -1422,6 +1472,7 @@ void freeClusterNode(clusterNode *n) { serverAssert(dictDelete(server.cluster->nodes,nodename) == DICT_OK); sdsfree(nodename); sdsfree(n->hostname); + sdsfree(n->human_nodename); /* Release links and associated data structures. */ if (n->link) freeClusterLink(n->link); @@ -1694,9 +1745,9 @@ void clusterHandleConfigEpochCollision(clusterNode *sender) { myself->configEpoch = server.cluster->currentEpoch; clusterSaveConfigOrDie(1); serverLog(LL_VERBOSE, - "WARNING: configEpoch collision with node %.40s." + "WARNING: configEpoch collision with node %.40s (%s)." " configEpoch set to %llu", - sender->name, + sender->name,sender->human_nodename, (unsigned long long) myself->configEpoch); } @@ -1812,7 +1863,7 @@ void markNodeAsFailingIfNeeded(clusterNode *node) { if (failures < needed_quorum) return; /* No weak agreement from masters. */ serverLog(LL_NOTICE, - "Marking node %.40s as failing (quorum reached).", node->name); + "Marking node %.40s (%s) as failing (quorum reached).", node->name, node->human_nodename); /* Mark the node as failing. */ node->flags &= ~CLUSTER_NODE_PFAIL; @@ -1840,8 +1891,8 @@ void clearNodeFailureIfNeeded(clusterNode *node) { * node again. */ if (nodeIsSlave(node) || node->numslots == 0) { serverLog(LL_NOTICE, - "Clear FAIL state for node %.40s: %s is reachable again.", - node->name, + "Clear FAIL state for node %.40s (%s):%s is reachable again.", + node->name,node->human_nodename, nodeIsSlave(node) ? "replica" : "master without slots"); node->flags &= ~CLUSTER_NODE_FAIL; clusterDoBeforeSleep(CLUSTER_TODO_UPDATE_STATE|CLUSTER_TODO_SAVE_CONFIG); @@ -1856,8 +1907,8 @@ void clearNodeFailureIfNeeded(clusterNode *node) { (server.cluster_node_timeout * CLUSTER_FAIL_UNDO_TIME_MULT)) { serverLog(LL_NOTICE, - "Clear FAIL state for node %.40s: is reachable again and nobody is serving its slots after some time.", - node->name); + "Clear FAIL state for node %.40s (%s): is reachable again and nobody is serving its slots after some time.", + node->name, node->human_nodename); node->flags &= ~CLUSTER_NODE_FAIL; clusterDoBeforeSleep(CLUSTER_TODO_UPDATE_STATE|CLUSTER_TODO_SAVE_CONFIG); } @@ -1977,15 +2028,15 @@ void clusterProcessGossipSection(clusterMsg *hdr, clusterLink *link) { if (flags & (CLUSTER_NODE_FAIL|CLUSTER_NODE_PFAIL)) { if (clusterNodeAddFailureReport(node,sender)) { serverLog(LL_VERBOSE, - "Node %.40s reported node %.40s as not reachable.", - sender->name, node->name); + "Node %.40s (%s) reported node %.40s (%s) as not reachable.", + sender->name, sender->human_nodename, node->name, node->human_nodename); } markNodeAsFailingIfNeeded(node); } else { if (clusterNodeDelFailureReport(node,sender)) { serverLog(LL_VERBOSE, - "Node %.40s reported node %.40s is back online.", - sender->name, node->name); + "Node %.40s (%s) reported node %.40s (%s) is back online.", + sender->name, sender->human_nodename, node->name, node->human_nodename); } } } @@ -2121,8 +2172,8 @@ int nodeUpdateAddressIfNeeded(clusterNode *node, clusterLink *link, node->cport = cport; if (node->link) freeClusterLink(node->link); node->flags &= ~CLUSTER_NODE_NOADDR; - serverLog(LL_NOTICE,"Address updated for node %.40s, now %s:%d", - node->name, node->ip, node->port); + serverLog(LL_NOTICE,"Address updated for node %.40s (%s), now %s:%d", + node->name, node->human_nodename, node->ip, node->port); /* Check if this is our master and we have to change the * replication target as well. */ @@ -2251,7 +2302,7 @@ void clusterUpdateSlotsConfigWith(clusterNode *sender, uint64_t senderConfigEpoc sender_slots == migrated_our_slots)) { serverLog(LL_NOTICE, "Configuration change detected. Reconfiguring myself " - "as a replica of %.40s", sender->name); + "as a replica of %.40s (%s)", sender->name, sender->human_nodename); clusterSetMaster(sender); clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG| CLUSTER_TODO_UPDATE_STATE| @@ -2267,8 +2318,8 @@ void clusterUpdateSlotsConfigWith(clusterNode *sender, uint64_t senderConfigEpoc * into a replica if its last slot is removed. If no other node takes * over the slot, there is nothing else to trigger replica migration. */ serverLog(LL_NOTICE, - "I'm a sub-replica! Reconfiguring myself as a replica of grandmaster %.40s", - myself->slaveof->slaveof->name); + "I'm a sub-replica! Reconfiguring myself as a replica of grandmaster %.40s (%s)", + myself->slaveof->slaveof->name, myself->slaveof->slaveof->human_nodename); clusterSetMaster(myself->slaveof->slaveof); clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG| CLUSTER_TODO_UPDATE_STATE| @@ -2328,6 +2379,13 @@ uint32_t getHostnamePingExtSize(void) { return getAlignedPingExtSize(sdslen(myself->hostname) + 1); } +uint32_t getHumanNodenamePingExtSize(void) { + if (sdslen(myself->human_nodename) == 0) { + return 0; + } + return getAlignedPingExtSize(sdslen(myself->human_nodename) + 1); +} + uint32_t getShardIdPingExtSize(void) { return getAlignedPingExtSize(sizeof(clusterMsgPingExtShardId)); } @@ -2376,6 +2434,20 @@ uint32_t writePingExt(clusterMsg *hdr, int gossipcount) { extensions++; } + if (sdslen(myself->human_nodename) != 0) { + if (cursor != NULL) { + /* Populate human_nodename */ + clusterMsgPingExtHumanNodename *ext = preparePingExt(cursor, CLUSTERMSG_EXT_TYPE_HUMAN_NODENAME, getHumanNodenamePingExtSize()); + memcpy(ext->human_nodename, myself->human_nodename, sdslen(myself->human_nodename)); + + /* Move the write cursor */ + cursor = nextPingExt(cursor); + } + + totlen += getHumanNodenamePingExtSize(); + extensions++; + } + /* Gossip forgotten nodes */ if (dictSize(server.cluster->nodes_black_list) > 0) { dictIterator *di = dictGetIterator(server.cluster->nodes_black_list); @@ -2424,6 +2496,7 @@ uint32_t writePingExt(clusterMsg *hdr, int gossipcount) { void clusterProcessPingExtensions(clusterMsg *hdr, clusterLink *link) { clusterNode *sender = link->node ? link->node : clusterLookupNode(hdr->sender, CLUSTER_NAMELEN); char *ext_hostname = NULL; + char *ext_humannodename = NULL; char *ext_shardid = NULL; uint16_t extensions = ntohs(hdr->extensions); /* Loop through all the extensions and process them */ @@ -2433,6 +2506,9 @@ void clusterProcessPingExtensions(clusterMsg *hdr, clusterLink *link) { if (type == CLUSTERMSG_EXT_TYPE_HOSTNAME) { clusterMsgPingExtHostname *hostname_ext = (clusterMsgPingExtHostname *) &(ext->ext[0].hostname); ext_hostname = hostname_ext->hostname; + } else if (type == CLUSTERMSG_EXT_TYPE_HUMAN_NODENAME) { + clusterMsgPingExtHumanNodename *humannodename_ext = (clusterMsgPingExtHumanNodename *) &(ext->ext[0].human_nodename); + ext_humannodename = humannodename_ext->human_nodename; } else if (type == CLUSTERMSG_EXT_TYPE_FORGOTTEN_NODE) { clusterMsgPingExtForgottenNode *forgotten_node_ext = &(ext->ext[0].forgotten_node); clusterNode *n = clusterLookupNode(forgotten_node_ext->name, CLUSTER_NAMELEN); @@ -2461,6 +2537,7 @@ void clusterProcessPingExtensions(clusterMsg *hdr, clusterLink *link) { * they don't have an announced hostname. Otherwise, we'll * set it now. */ updateAnnouncedHostname(sender, ext_hostname); + updateAnnouncedHumanNodename(sender, ext_humannodename); updateShardId(sender, ext_shardid); } @@ -2692,8 +2769,8 @@ int clusterProcessPacket(clusterLink *link) { * IP/port of the node with the new one. */ if (sender) { serverLog(LL_VERBOSE, - "Handshake: we already know node %.40s, " - "updating the address if needed.", sender->name); + "Handshake: we already know node %.40s (%s), " + "updating the address if needed.", sender->name, sender->human_nodename); if (nodeUpdateAddressIfNeeded(sender,link,hdr)) { clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG| @@ -2904,8 +2981,8 @@ int clusterProcessPacket(clusterLink *link) { !(failing->flags & (CLUSTER_NODE_FAIL|CLUSTER_NODE_MYSELF))) { serverLog(LL_NOTICE, - "FAIL message received from %.40s about %.40s", - hdr->sender, hdr->data.fail.about.nodename); + "FAIL message received from %.40s (%s) about %.40s (%s)", + hdr->sender, sender->human_nodename, hdr->data.fail.about.nodename, failing->human_nodename); failing->flags |= CLUSTER_NODE_FAIL; failing->fail_time = now; failing->flags &= ~CLUSTER_NODE_PFAIL; @@ -2969,8 +3046,8 @@ int clusterProcessPacket(clusterLink *link) { pauseActions(PAUSE_DURING_FAILOVER, now + (CLUSTER_MF_TIMEOUT * CLUSTER_MF_PAUSE_MULT), PAUSE_ACTIONS_CLIENT_WRITE_SET); - serverLog(LL_NOTICE,"Manual failover requested by replica %.40s.", - sender->name); + serverLog(LL_NOTICE,"Manual failover requested by replica %.40s (%s).", + sender->name, sender->human_nodename); /* We need to send a ping message to the replica, as it would carry * `server.cluster->mf_master_offset`, which means the master paused clients * at offset `server.cluster->mf_master_offset`, so that the replica would @@ -3375,7 +3452,6 @@ void clusterSendPing(clusterLink *link, int type) { estlen = sizeof(clusterMsg) - sizeof(union clusterMsgData); estlen += (sizeof(clusterMsgDataGossip)*(wanted + pfail_wanted)); estlen += writePingExt(NULL, 0); - /* Note: clusterBuildMessageHdr() expects the buffer to be always at least * sizeof(clusterMsg) or more. */ if (estlen < (int)sizeof(clusterMsg)) estlen = sizeof(clusterMsg); @@ -3703,8 +3779,8 @@ void clusterSendFailoverAuthIfNeeded(clusterNode *node, clusterMsg *request) { * request, if the request epoch was greater. */ if (requestCurrentEpoch < server.cluster->currentEpoch) { serverLog(LL_WARNING, - "Failover auth denied to %.40s: reqEpoch (%llu) < curEpoch(%llu)", - node->name, + "Failover auth denied to %.40s (%s): reqEpoch (%llu) < curEpoch(%llu)", + node->name, node->human_nodename, (unsigned long long) requestCurrentEpoch, (unsigned long long) server.cluster->currentEpoch); return; @@ -3713,8 +3789,8 @@ void clusterSendFailoverAuthIfNeeded(clusterNode *node, clusterMsg *request) { /* I already voted for this epoch? Return ASAP. */ if (server.cluster->lastVoteEpoch == server.cluster->currentEpoch) { serverLog(LL_WARNING, - "Failover auth denied to %.40s: already voted for epoch %llu", - node->name, + "Failover auth denied to %.40s (%s): already voted for epoch %llu", + node->name, node->human_nodename, (unsigned long long) server.cluster->currentEpoch); return; } @@ -3727,16 +3803,16 @@ void clusterSendFailoverAuthIfNeeded(clusterNode *node, clusterMsg *request) { { if (nodeIsMaster(node)) { serverLog(LL_WARNING, - "Failover auth denied to %.40s: it is a master node", - node->name); + "Failover auth denied to %.40s (%s): it is a master node", + node->name, node->human_nodename); } else if (master == NULL) { serverLog(LL_WARNING, - "Failover auth denied to %.40s: I don't know its master", - node->name); + "Failover auth denied to %.40s (%s): I don't know its master", + node->name, node->human_nodename); } else if (!nodeFailed(master)) { serverLog(LL_WARNING, - "Failover auth denied to %.40s: its master is up", - node->name); + "Failover auth denied to %.40s (%s): its master is up", + node->name, node->human_nodename); } return; } @@ -3747,9 +3823,9 @@ void clusterSendFailoverAuthIfNeeded(clusterNode *node, clusterMsg *request) { if (mstime() - node->slaveof->voted_time < server.cluster_node_timeout * 2) { serverLog(LL_WARNING, - "Failover auth denied to %.40s: " + "Failover auth denied to %.40s %s: " "can't vote about this master before %lld milliseconds", - node->name, + node->name, node->human_nodename, (long long) ((server.cluster_node_timeout*2)- (mstime() - node->slaveof->voted_time))); return; @@ -3769,9 +3845,9 @@ void clusterSendFailoverAuthIfNeeded(clusterNode *node, clusterMsg *request) { * is served by a master with a greater configEpoch than the one claimed * by the slave requesting our vote. Refuse to vote for this slave. */ serverLog(LL_WARNING, - "Failover auth denied to %.40s: " + "Failover auth denied to %.40s (%s): " "slot %d epoch (%llu) > reqEpoch (%llu)", - node->name, j, + node->name, node->human_nodename, j, (unsigned long long) server.cluster->slots[j]->configEpoch, (unsigned long long) requestConfigEpoch); return; @@ -3782,8 +3858,8 @@ void clusterSendFailoverAuthIfNeeded(clusterNode *node, clusterMsg *request) { node->slaveof->voted_time = mstime(); clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG|CLUSTER_TODO_FSYNC_CONFIG); clusterSendFailoverAuth(node); - serverLog(LL_NOTICE, "Failover auth granted to %.40s for epoch %llu", - node->name, (unsigned long long) server.cluster->currentEpoch); + serverLog(LL_NOTICE, "Failover auth granted to %.40s (%s) for epoch %llu", + node->name, node->human_nodename, (unsigned long long) server.cluster->currentEpoch); } /* This function returns the "rank" of this instance, a slave, in the context @@ -5007,24 +5083,20 @@ sds clusterGenNodeDescription(client *c, clusterNode *node, int use_pport) { /* Node coordinates */ ci = sdscatlen(sdsempty(),node->name,CLUSTER_NAMELEN); - /* Node's ip/port and optional announced hostname */ + ci = sdscatfmt(ci," %s:%i@%i", + node->ip, + port, + node->cport); if (sdslen(node->hostname) != 0) { - ci = sdscatprintf(ci," %s:%i@%i,%s", - node->ip, - port, - node->cport, - node->hostname); - } else { - ci = sdscatprintf(ci," %s:%i@%i,", - node->ip, - port, - node->cport); + ci = sdscatfmt(ci,",%s", node->hostname); + } + if (sdslen(node->hostname) == 0) { + ci = sdscatfmt(ci,",", 1); } - /* Don't expose aux fields to any clients yet but do allow them * to be persisted to nodes.conf */ if (c == NULL) { - for (int i = af_start; i < af_count; i++) { + for (int i = af_count-1; i >=0; i--) { if (auxFieldHandlers[i].isPresent(node)) { ci = sdscatprintf(ci, ",%s=", auxFieldHandlers[i].field); ci = auxFieldHandlers[i].getter(node, ci); @@ -5926,7 +5998,7 @@ NULL { serverLog(LL_NOTICE, "Configuration change detected. Reconfiguring myself " - "as a replica of %.40s", n->name); + "as a replica of %.40s (%s)", n->name, n->human_nodename); clusterSetMaster(n); clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG | CLUSTER_TODO_UPDATE_STATE | diff --git a/src/cluster.h b/src/cluster.h index eb56f581f..8173c97ef 100644 --- a/src/cluster.h +++ b/src/cluster.h @@ -141,6 +141,7 @@ typedef struct clusterNode { long long repl_offset; /* Last known repl offset for this node. */ char ip[NET_IP_STR_LEN]; /* Latest known IP address of this node */ sds hostname; /* The known hostname for this node */ + sds human_nodename; /* The known human readable nodename for this node */ int port; /* Latest known clients port (TLS or plain). */ int pport; /* Latest known clients plaintext port. Only used if the main clients port is for TLS. */ @@ -260,6 +261,7 @@ typedef struct { * consistent manner. */ typedef enum { CLUSTERMSG_EXT_TYPE_HOSTNAME, + CLUSTERMSG_EXT_TYPE_HUMAN_NODENAME, CLUSTERMSG_EXT_TYPE_FORGOTTEN_NODE, CLUSTERMSG_EXT_TYPE_SHARDID, } clusterMsgPingtypes; @@ -271,6 +273,10 @@ typedef struct { char hostname[1]; /* The announced hostname, ends with \0. */ } clusterMsgPingExtHostname; +typedef struct { + char human_nodename[1]; /* The announced nodename, ends with \0. */ +} clusterMsgPingExtHumanNodename; + typedef struct { char name[CLUSTER_NAMELEN]; /* Node name. */ uint64_t ttl; /* Remaining time to blacklist the node, in seconds. */ @@ -288,6 +294,7 @@ typedef struct { uint16_t unused; /* 16 bits of padding to make this structure 8 byte aligned. */ union { clusterMsgPingExtHostname hostname; + clusterMsgPingExtHumanNodename human_nodename; clusterMsgPingExtForgottenNode forgotten_node; clusterMsgPingExtShardId shard_id; } ext[]; /* Actual extension information, formatted so that the data is 8 @@ -425,5 +432,6 @@ void clusterUpdateMyselfAnnouncedPorts(void); sds clusterGenNodesDescription(client *c, int filter, int use_pport); sds genClusterInfoString(void); void freeClusterLink(clusterLink *link); - +void clusterUpdateMyselfHumanNodename(void); +int isValidAuxString(char *s, unsigned int length); #endif /* __CLUSTER_H */ diff --git a/src/config.c b/src/config.c index e506060a9..aee8a629f 100644 --- a/src/config.c +++ b/src/config.c @@ -38,6 +38,7 @@ #include #include #include +#include /*----------------------------------------------------------------------------- * Config file name-value maps. @@ -2378,6 +2379,14 @@ static int isValidShutdownOnSigFlags(int val, const char **err) { return 1; } +static int isValidAnnouncedNodename(char *val,const char **err) { + if (!(isValidAuxString(val,sdslen(val)))) { + *err = "Announced human node name contained invalid character"; + return 0; + } + return 1; +} + static int isValidAnnouncedHostname(char *val, const char **err) { if (strlen(val) >= NET_HOST_STR_LEN) { *err = "Hostnames must be less than " @@ -2629,6 +2638,12 @@ int updateClusterHostname(const char **err) { return 1; } +int updateClusterHumanNodename(const char **err) { + UNUSED(err); + clusterUpdateMyselfHumanNodename(); + return 1; +} + static int applyTlsCfg(const char **err) { UNUSED(err); @@ -3085,6 +3100,7 @@ standardConfig static_configs[] = { createStringConfig("cluster-announce-ip", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_ip, NULL, NULL, updateClusterIp), createStringConfig("cluster-config-file", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.cluster_configfile, "nodes.conf", NULL, NULL), createStringConfig("cluster-announce-hostname", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_hostname, NULL, isValidAnnouncedHostname, updateClusterHostname), + createStringConfig("cluster-announce-human-nodename", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_human_nodename, NULL, isValidAnnouncedNodename, updateClusterHumanNodename), createStringConfig("syslog-ident", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.syslog_ident, "redis", NULL, NULL), createStringConfig("dbfilename", NULL, MODIFIABLE_CONFIG | PROTECTED_CONFIG, ALLOW_EMPTY_STRING, server.rdb_filename, "dump.rdb", isValidDBfilename, NULL), createStringConfig("appendfilename", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.aof_filename, "appendonly.aof", isValidAOFfilename, NULL), diff --git a/src/server.h b/src/server.h index 755a79dae..7b7a32ac9 100644 --- a/src/server.h +++ b/src/server.h @@ -1979,6 +1979,7 @@ struct redisServer { if the master is in failure state. */ char *cluster_announce_ip; /* IP address to announce on cluster bus. */ char *cluster_announce_hostname; /* hostname to announce on cluster bus. */ + char *cluster_announce_human_nodename; /* Human readable node name assigned to a node. */ int cluster_preferred_endpoint_type; /* Use the announced hostname when available. */ int cluster_announce_port; /* base port to announce on cluster bus. */ int cluster_announce_tls_port; /* TLS port to announce on cluster bus. */ diff --git a/tests/support/cluster_util.tcl b/tests/support/cluster_util.tcl index d80dcf062..13a6b05d6 100644 --- a/tests/support/cluster_util.tcl +++ b/tests/support/cluster_util.tcl @@ -150,3 +150,28 @@ proc get_cluster_nodes id { } return $nodes } + +# Returns 1 if no node knows node_id, 0 if any node knows it. +proc node_is_forgotten {node_id} { + for {set j 0} {$j < [llength $::servers]} {incr j} { + set cluster_nodes [R $j CLUSTER NODES] + if { [string match "*$node_id*" $cluster_nodes] } { + return 0 + } + } + return 1 +} + +# Isolate a node from the cluster and give it a new nodeid +proc isolate_node {id} { + set node_id [R $id CLUSTER MYID] + R $id CLUSTER RESET HARD + # Here we additionally test that CLUSTER FORGET propagates to all nodes. + set other_id [expr $id == 0 ? 1 : 0] + R $other_id CLUSTER FORGET $node_id + wait_for_condition 50 100 { + [node_is_forgotten $node_id] + } else { + fail "CLUSTER FORGET was not propagated to all nodes" + } +} diff --git a/tests/test_helper.tcl b/tests/test_helper.tcl index 6ec2ae1fc..aa33a8990 100644 --- a/tests/test_helper.tcl +++ b/tests/test_helper.tcl @@ -98,6 +98,7 @@ set ::all_tests { unit/cluster/cli unit/cluster/scripting unit/cluster/hostnames + unit/cluster/human-announced-nodename unit/cluster/multi-slot-operations unit/cluster/slot-ownership unit/cluster/links diff --git a/tests/unit/cluster/hostnames.tcl b/tests/unit/cluster/hostnames.tcl index 031310172..42024587e 100644 --- a/tests/unit/cluster/hostnames.tcl +++ b/tests/unit/cluster/hostnames.tcl @@ -1,28 +1,3 @@ -# Returns 1 if no node knows node_id, 0 if any node knows it. -proc node_is_forgotten {node_id} { - for {set j 0} {$j < [llength $::servers]} {incr j} { - set cluster_nodes [R $j CLUSTER NODES] - if { [string match "*$node_id*" $cluster_nodes] } { - return 0 - } - } - return 1 -} - -# Isolate a node from the cluster and give it a new nodeid -proc isolate_node {id} { - set node_id [R $id CLUSTER MYID] - R $id CLUSTER RESET HARD - # Here we additionally test that CLUSTER FORGET propagates to all nodes. - set other_id [expr $id == 0 ? 1 : 0] - R $other_id CLUSTER FORGET $node_id - wait_for_condition 50 100 { - [node_is_forgotten $node_id] - } else { - fail "CLUSTER FORGET was not propagated to all nodes" - } -} - # Check if cluster's view of hostnames is consistent proc are_hostnames_propagated {match_string} { for {set j 0} {$j < [llength $::servers]} {incr j} { diff --git a/tests/unit/cluster/human-announced-nodename.tcl b/tests/unit/cluster/human-announced-nodename.tcl new file mode 100644 index 000000000..5db598238 --- /dev/null +++ b/tests/unit/cluster/human-announced-nodename.tcl @@ -0,0 +1,30 @@ +# Check if cluster's view of human announced nodename is reported in logs +start_cluster 3 0 {tags {external:skip cluster}} { + test "Set cluster human announced nodename and let it propagate" { + for {set j 0} {$j < [llength $::servers]} {incr j} { + R $j config set cluster-announce-human-nodename "nodename-$j" + } + + # We wait for everyone to agree on the epoch bump, which means everyone + # has exchanged messages so they know about the nodenames. + R 0 CLUSTER BUMPEPOCH + wait_for_condition 1000 50 { + [CI 0 cluster_current_epoch] == [CI 1 cluster_current_epoch] && + [CI 0 cluster_current_epoch] == [CI 2 cluster_current_epoch] + } else { + fail "Cluster did not converge" + } + } + + test "Human nodenames are visible in log messages" { + # Pause instance 0, so everyone thinks it is dead + pause_process [srv 0 pid] + + # We're going to use a message we will know will be sent, node unreachable, + # since it includes the other node gossiping. + wait_for_log_messages -1 {"*Node * (nodename-2) reported node * (nodename-0) as not reachable*"} 0 10 500 + wait_for_log_messages -2 {"*Node * (nodename-1) reported node * (nodename-0) as not reachable*"} 0 10 500 + + resume_process [srv 0 pid] + } +}