Repository: valkey-io/valkey
File: src/cluster_legacy.c (C)

Code before:
}

/* Clear the slot bit and return the old value. */
int clusterNodeClearSlotBit(clusterNode *n, int slot) {
    int old = bitmapTestBit(n->slots,slot);
    if (old) {
        bitmapClearBit(n->slots,slot);
        n->numslots--;
    }
    return old;
}

/* Return the slot bit from the cluster node structure. */
int clusterNodeCoversSlot(clusterNode *n, int slot) {
    return bitmapTestBit(n->slots,slot);
}

/* Add the specified slot to the list of slots that node 'n' will
 * serve. Return C_OK if the operation ended with success.
 * If the slot is already assigned to another instance this is considered
 * an error and C_ERR is returned. */
int clusterAddSlot(clusterNode *n, int slot) {
    if (server.cluster->slots[slot]) return C_ERR;
    clusterNodeSetSlotBit(n,slot);
    server.cluster->slots[slot] = n;
    clearCachedClusterSlotsResponse();
    bitmapClearBit(server.cluster->owner_not_claiming_slot, slot);
    return C_OK;
}

/* Delete the specified slot marking it as unassigned.
 * Returns C_OK if the slot was assigned, otherwise if the slot was
 * already unassigned C_ERR is returned. */
int clusterDelSlot(int slot) {
    clusterNode *n = server.cluster->slots[slot];

    if (!n) return C_ERR;

    /* Cleanup the channels in master/replica as part of slot deletion. */
    removeChannelsInSlot(slot);
    /* Clear the slot bit. */
    serverAssert(clusterNodeClearSlotBit(n,slot) == 1);
    server.cluster->slots[slot] = NULL;
    /* Make owner_not_claiming_slot flag consistent with slot ownership information. */
    bitmapClearBit(server.cluster->owner_not_claiming_slot, slot);
    clearCachedClusterSlotsResponse();
    return C_OK;
}

/* Delete all the slots associated with the specified node.
 * The number of deleted slots is returned. */

Review comment (performance):
rather than spreading `clearCachedClusterSlotsResponse`, I wonder if it suffices to clear the cache on any config save operation, i.e. in `clusterSaveConfigOrDie`? I can't think of a case where we would not save nodes.conf on topology changes. 

Code after:
int clusterNodeCoversSlot(clusterNode *n, int slot) {
    return bitmapTestBit(n->slots,slot);
}

/* Add the specified slot to the list of slots that node 'n' will
 * serve. Return C_OK if the operation ended with success.
 * If the slot is already assigned to another instance this is considered
 * an error and C_ERR is returned. */
int clusterAddSlot(clusterNode *n, int slot) {
    if (server.cluster->slots[slot]) return C_ERR;
    clusterNodeSetSlotBit(n,slot);
    server.cluster->slots[slot] = n;
    bitmapClearBit(server.cluster->owner_not_claiming_slot, slot);
    return C_OK;
}

/* Delete the specified slot marking it as unassigned.
 * Returns C_OK if the slot was assigned, otherwise if the slot was
 * already unassigned C_ERR is returned. */
int clusterDelSlot(int slot) {
    clusterNode *n = server.cluster->slots[slot];

    if (!n) return C_ERR;

    /* Cleanup the channels in master/replica as part of slot deletion. */
    removeChannelsInSlot(slot);
    /* Clear the slot bit. */
    serverAssert(clusterNodeClearSlotBit(n,slot) == 1);
    server.cluster->slots[slot] = NULL;
    /* Make owner_not_claiming_slot flag consistent with slot ownership information. */
    bitmapClearBit(server.cluster->owner_not_claiming_slot, slot);
    return C_OK;
}

/* Delete all the slots associated with the specified node.
 * The number of deleted slots is returned. */
int clusterDelNodeSlots(clusterNode *node) {
    int deleted = 0, j;

    for (j = 0; j < CLUSTER_SLOTS; j++) {
        if (clusterNodeCoversSlot(node, j)) {
            clusterDelSlot(j);
            deleted++;
        }
    }
    return deleted;
}

/* Clear the migrating / importing state for all the slots.
 * This is useful at initialization and when turning a master into slave. */
void clusterCloseAllSlots(void) {