Skip to content

Commit

Permalink
Rename offensive defines and members in API (#140)
Browse files Browse the repository at this point in the history 
- Rename offensive cluster role defines in the API:
  Use `VALKEY_ROLE_PRIMARY` instead of `VALKEY_ROLE_MASTER`.
  Use `VALKEY_ROLE_REPLICA` instead of `VALKEY_ROLE_SLAVE`.
  Use `VALKEY_ROLE_UNKNOWN` instead of `VALKEY_ROLE_NULL`.
- Rename `valkeyClusterNode` member `slaves` to `replicas`.
- Rename offensive variables.

Signed-off-by: Björn Svensson <[email protected]>
  • Loading branch information
@bjosv
bjosv authored Dec 18, 2024
1 parent c370ca9 commit ea72f48
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 83 deletions.
7 changes: 7 additions & 0 deletions docs/migration-guide.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,17 @@ The type `sds` is removed from the public API.
* `ctx_get_by_node` is renamed to `valkeyClusterGetValkeyContext`.
* `actx_get_by_node` is renamed to `valkeyClusterGetValkeyAsyncContext`.

### Renamed API defines

* `REDIS_ROLE_NULL` is renamed to `VALKEY_ROLE_UNKNOWN`.
* `REDIS_ROLE_MASTER` is renamed to `VALKEY_ROLE_PRIMARY`.
* `REDIS_ROLE_SLAVE` is renamed to `VALKEY_ROLE_REPLICA`.

### Removed API functions

* `redisClusterSetMaxRedirect` removed and replaced with `valkeyClusterSetOptionMaxRetry`.
* `redisClusterSetOptionAddNode` removed and replaced with `valkeyClusterSetOptionAddNodes`.
(Note the "s" in the end of the function name.)
* `redisClusterSetOptionConnectBlock` removed since it was deprecated.
* `redisClusterSetOptionConnectNonBlock` removed since it was deprecated.
* `parse_cluster_nodes` removed from API, for internal use only.
Expand Down
10 changes: 5 additions & 5 deletions include/valkey/cluster.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@

#define VALKEYCLUSTER_SLOTS 16384

#define VALKEY_ROLE_NULL 0
#define VALKEY_ROLE_MASTER 1
#define VALKEY_ROLE_SLAVE 2
#define VALKEY_ROLE_UNKNOWN 0
#define VALKEY_ROLE_PRIMARY 1
#define VALKEY_ROLE_REPLICA 2

/* Configuration flags */
#define VALKEYCLUSTER_FLAG_NULL 0x0
Expand Down Expand Up @@ -84,13 +84,13 @@ typedef struct valkeyClusterNode {
valkeyAsyncContext *acon;
int64_t lastConnectionAttempt; /* Timestamp */
struct hilist *slots;
struct hilist *slaves;
struct hilist *replicas;
} valkeyClusterNode;

typedef struct cluster_slot {
uint32_t start;
uint32_t end;
valkeyClusterNode *node; /* master that this slot region belong to */
valkeyClusterNode *node; /* Owner of slot region. */
} cluster_slot;

/* Context for accessing a Valkey Cluster */
Expand Down
109 changes: 54 additions & 55 deletions src/cluster.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ static void freeValkeyClusterNode(valkeyClusterNode *node) {
valkeyAsyncFree(node->acon);
}
listRelease(node->slots);
listRelease(node->slaves);
listRelease(node->replicas);
vk_free(node);
}

Expand All @@ -289,7 +289,7 @@ static cluster_slot *cluster_slot_create(valkeyClusterNode *node) {
slot->node = node;

if (node != NULL) {
assert(node->role == VALKEY_ROLE_MASTER);
assert(node->role == VALKEY_ROLE_PRIMARY);
if (node->slots == NULL) {
node->slots = listCreate();
if (node->slots == NULL) {
Expand All @@ -314,7 +314,7 @@ static int cluster_slot_ref_node(cluster_slot *slot, valkeyClusterNode *node) {
return VALKEY_ERR;
}

if (node->role != VALKEY_ROLE_MASTER) {
if (node->role != VALKEY_ROLE_PRIMARY) {
return VALKEY_ERR;
}

Expand Down Expand Up @@ -424,7 +424,7 @@ static valkeyClusterNode *node_get_with_slots(valkeyClusterContext *cc,
goto oom;
}

if (role == VALKEY_ROLE_MASTER) {
if (role == VALKEY_ROLE_PRIMARY) {
node->slots = listCreate();
if (node->slots == NULL) {
goto oom;
Expand Down Expand Up @@ -519,7 +519,7 @@ static dict *parse_cluster_slots(valkeyClusterContext *cc, valkeyReply *reply) {
valkeyReply *elem_slots_begin, *elem_slots_end;
valkeyReply *elem_nodes;
valkeyReply *elem_ip, *elem_port;
valkeyClusterNode *master = NULL, *slave;
valkeyClusterNode *primary = NULL, *replica;
uint32_t i, idx;

if (reply->type != VALKEY_REPLY_ARRAY) {
Expand Down Expand Up @@ -599,12 +599,12 @@ static dict *parse_cluster_slots(valkeyClusterContext *cc, valkeyReply *reply) {
elem_port->type != VALKEY_REPLY_INTEGER) {
valkeyClusterSetError(cc, VALKEY_ERR_OTHER,
"Command(cluster slots) reply error: "
"master ip or port is not correct.");
"ip or port is not correct.");
goto error;
}

// this is master.
if (idx == 2) {
/* Parse a primary node. */
sds address = sdsnewlen(elem_ip->str, elem_ip->len);
if (address == NULL) {
goto oom;
Expand All @@ -616,11 +616,10 @@ static dict *parse_cluster_slots(valkeyClusterContext *cc, valkeyReply *reply) {

den = dictFind(nodes, address);
sdsfree(address);
// master already exists, break to the next slots region.
if (den != NULL) {

master = dictGetEntryVal(den);
ret = cluster_slot_ref_node(slot, master);
/* Skip parsing this primary node since it's already known. */
primary = dictGetEntryVal(den);
ret = cluster_slot_ref_node(slot, primary);
if (ret != VALKEY_OK) {
goto oom;
}
Expand All @@ -629,50 +628,50 @@ static dict *parse_cluster_slots(valkeyClusterContext *cc, valkeyReply *reply) {
break;
}

master = node_get_with_slots(cc, elem_ip, elem_port,
VALKEY_ROLE_MASTER);
if (master == NULL) {
primary = node_get_with_slots(cc, elem_ip, elem_port,
VALKEY_ROLE_PRIMARY);
if (primary == NULL) {
goto error;
}

sds key = sdsnewlen(master->addr, sdslen(master->addr));
sds key = sdsnewlen(primary->addr, sdslen(primary->addr));
if (key == NULL) {
freeValkeyClusterNode(master);
freeValkeyClusterNode(primary);
goto oom;
}

ret = dictAdd(nodes, key, master);
ret = dictAdd(nodes, key, primary);
if (ret != DICT_OK) {
sdsfree(key);
freeValkeyClusterNode(master);
freeValkeyClusterNode(primary);
goto oom;
}

ret = cluster_slot_ref_node(slot, master);
ret = cluster_slot_ref_node(slot, primary);
if (ret != VALKEY_OK) {
goto oom;
}

slot = NULL;
} else if (cc->flags & VALKEYCLUSTER_FLAG_ADD_SLAVE) {
slave = node_get_with_slots(cc, elem_ip, elem_port,
VALKEY_ROLE_SLAVE);
if (slave == NULL) {
replica = node_get_with_slots(cc, elem_ip, elem_port,
VALKEY_ROLE_REPLICA);
if (replica == NULL) {
goto error;
}

if (master->slaves == NULL) {
master->slaves = listCreate();
if (master->slaves == NULL) {
freeValkeyClusterNode(slave);
if (primary->replicas == NULL) {
primary->replicas = listCreate();
if (primary->replicas == NULL) {
freeValkeyClusterNode(replica);
goto oom;
}

master->slaves->free = listClusterNodeDestructor;
primary->replicas->free = listClusterNodeDestructor;
}

if (listAddNodeTail(master->slaves, slave) == NULL) {
freeValkeyClusterNode(slave);
if (listAddNodeTail(primary->replicas, replica) == NULL) {
freeValkeyClusterNode(replica);
goto oom;
}
}
Expand Down Expand Up @@ -740,9 +739,9 @@ static int store_replica_nodes(dict *nodes, dict *replicas) {
/* Move replica nodes related to this primary. */
dictEntry *der = dictFind(replicas, primary->name);
if (der != NULL) {
assert(primary->slaves == NULL);
assert(primary->replicas == NULL);
/* Move replica list from replicas dict to nodes dict. */
primary->slaves = dictGetEntryVal(der);
primary->replicas = dictGetEntryVal(der);
dictSetHashVal(replicas, der, NULL);
}
}
Expand Down Expand Up @@ -785,29 +784,29 @@ static int parse_cluster_nodes_line(valkeyClusterContext *cc, char *line,

/* Parse flags, a comma separated list of following flags:
* myself, master, slave, fail?, fail, handshake, noaddr, nofailover, noflags. */
uint8_t role = VALKEY_ROLE_NULL;
uint8_t role = VALKEY_ROLE_UNKNOWN;
while (*flags != '\0') {
if ((p = strchr(flags, ',')) != NULL)
*p = '\0';
if (memcmp(flags, "master", 6) == 0) {
role = VALKEY_ROLE_MASTER;
role = VALKEY_ROLE_PRIMARY;
break;
}
if (memcmp(flags, "slave", 5) == 0) {
role = VALKEY_ROLE_SLAVE;
role = VALKEY_ROLE_REPLICA;
break;
}
if (p == NULL) /* No more flags. */
break;
flags = p + 1; /* Start of next flag. */
}
if (role == VALKEY_ROLE_NULL) {
if (role == VALKEY_ROLE_UNKNOWN) {
valkeyClusterSetError(cc, VALKEY_ERR_OTHER, "Unknown role");
return VALKEY_ERR;
}

/* Only parse replicas when requested. */
if (role == VALKEY_ROLE_SLAVE && parsed_primary_id == NULL) {
if (role == VALKEY_ROLE_REPLICA && parsed_primary_id == NULL) {
*parsed_node = NULL;
return VALKEY_OK;
}
Expand Down Expand Up @@ -852,8 +851,8 @@ static int parse_cluster_nodes_line(valkeyClusterContext *cc, char *line,
p++; // Skip separator character.
node->port = vk_atoi(p, strlen(p));

/* No slot parsing needed for replicas, but return master id. */
if (node->role == VALKEY_ROLE_SLAVE) {
/* No slot parsing needed for replicas, but return primary id. */
if (node->role == VALKEY_ROLE_REPLICA) {
*parsed_primary_id = primary_id;
*parsed_node = node;
return VALKEY_OK;
Expand Down Expand Up @@ -941,7 +940,7 @@ static dict *parse_cluster_nodes(valkeyClusterContext *cc, valkeyReply *reply) {
goto error;
if (node == NULL)
continue; /* Line skipped. */
if (node->role == VALKEY_ROLE_MASTER) {
if (node->role == VALKEY_ROLE_PRIMARY) {
sds key = sdsnew(node->addr);
if (key == NULL) {
freeValkeyClusterNode(node);
Expand All @@ -962,7 +961,7 @@ static dict *parse_cluster_nodes(valkeyClusterContext *cc, valkeyReply *reply) {
slot_ranges_found += listLength(node->slots);

} else {
assert(node->role == VALKEY_ROLE_SLAVE);
assert(node->role == VALKEY_ROLE_REPLICA);
if (replicas == NULL) {
if ((replicas = dictCreate(&clusterNodeListDictType, NULL)) == NULL) {
freeValkeyClusterNode(node);
Expand Down Expand Up @@ -1124,19 +1123,19 @@ static int updateNodesAndSlotmap(valkeyClusterContext *cc, dict *nodes) {

dictEntry *de;
while ((de = dictNext(&di))) {
valkeyClusterNode *master = dictGetEntryVal(de);
if (master->role != VALKEY_ROLE_MASTER) {
valkeyClusterNode *node = dictGetEntryVal(de);
if (node->role != VALKEY_ROLE_PRIMARY) {
valkeyClusterSetError(cc, VALKEY_ERR_OTHER,
"Node role must be master");
"Node role must be primary");
goto error;
}

if (master->slots == NULL) {
if (node->slots == NULL) {
continue;
}

listIter li;
listRewind(master->slots, &li);
listRewind(node->slots, &li);

listNode *ln;
while ((ln = listNext(&li))) {
Expand All @@ -1152,7 +1151,7 @@ static int updateNodesAndSlotmap(valkeyClusterContext *cc, dict *nodes) {
"Different node holds same slot");
goto error;
}
table[i] = master;
table[i] = node;
}
}
}
Expand Down Expand Up @@ -1590,20 +1589,20 @@ int valkeyClusterSetOptionTimeout(valkeyClusterContext *cc,
valkeySetTimeout(node->con, tv);
}

if (node->slaves && listLength(node->slaves) > 0) {
valkeyClusterNode *slave;
if (node->replicas && listLength(node->replicas) > 0) {
valkeyClusterNode *replica;
listNode *ln;

listIter li;
listRewind(node->slaves, &li);
listRewind(node->replicas, &li);

while ((ln = listNext(&li)) != NULL) {
slave = listNodeValue(ln);
if (slave->acon) {
valkeyAsyncSetTimeout(slave->acon, tv);
replica = listNodeValue(ln);
if (replica->acon) {
valkeyAsyncSetTimeout(replica->acon, tv);
}
if (slave->con && slave->con->err == 0) {
valkeySetTimeout(slave->con, tv);
if (replica->con && replica->con->err == 0) {
valkeySetTimeout(replica->con, tv);
}
}
}
Expand Down Expand Up @@ -1861,7 +1860,7 @@ static valkeyClusterNode *getNodeFromRedirectReply(valkeyClusterContext *cc,
if (node == NULL) {
goto oom;
}
node->role = VALKEY_ROLE_MASTER;
node->role = VALKEY_ROLE_PRIMARY;
node->addr = part[2];
part[2] = NULL; /* Memory ownership moved */

Expand Down
Loading

0 comments on commit ea72f48

Please sign in to comment.