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

Code before:
        return node->human_nodename;
    }

    /* Avoid allocating heap memory so that users can call the function with ease.
     * Use a small ring of thread-local buffers here so that multiple function calls
     * in the same logging statement are safe. */
    enum { BUF_COUNT = 8 };
    enum { BUF_SIZE = 64 }; /* Long enough to hold "255.255.255.255:65535" */
    static _Thread_local char buffers[BUF_COUNT][BUF_SIZE];
    static _Thread_local int idx;

    char *buffer = buffers[idx];
    idx = (idx + 1) % BUF_COUNT;

    const int port = server.tls_cluster ? node->tls_port : node->tcp_port;
    /* Be defensive in case IP is empty. */
    if (node->ip[0] == '\0') {
        snprintf(buffer, BUF_SIZE, ":%d", port);
    } else {
        snprintf(buffer, BUF_SIZE, "%s:%d", node->ip, port);
    }
    return buffer;
}

/* Return human assigned node name if the link has the node associated to it
 * or else return "<unknown>". */
static inline char *clusterLinkGetHumanNodeName(clusterLink *link) {
    return link->node ? humanNodename(link->node) : "<unknown>";
}

#define isSlotUnclaimed(slot) \
    (server.cluster->slots[slot] == NULL || bitmapTestBit(server.cluster->owner_not_claiming_slot, slot))

#define RCVBUF_INIT_LEN 1024
#define RCVBUF_MIN_READ_LEN 14
static_assert(offsetof(clusterMsg, type) + sizeof(uint16_t) == RCVBUF_MIN_READ_LEN,
              "Incorrect length to read to identify type");

#define RCVBUF_MAX_PREALLOC (1 << 20) /* 1MB */

/* Cluster nodes hash table, mapping nodes addresses 1.2.3.4:6379 to
 * clusterNode structures. */
dictType clusterNodesDictType = {
    dictSdsHash,       /* hash function */
    NULL,              /* key dup */
    dictSdsKeyCompare, /* key compare */
    dictSdsDestructor, /* key destructor */
    NULL,              /* val destructor */
    NULL               /* allow to expand */
};


Review comment (performance):
I think logic is too complex. Most of the time (always?) we will only need one string. A round-robin cache is complex and it's even not enough in the worst case. In the worst case, some code wants to collect all human nodes names of all the cluster in an array and then do something with them.

Let's use only one static (thread-local) string and document that the returned string is only valid until the next call to this function?

Code after:
 * or else return "<unknown>". */
static inline char *clusterLinkGetNodeName(clusterLink *link) {
    return link->node ? link->node->name : "<unknown>";
}

/* By default, a server doesn't have a human-readable nodename unless explicitly
 * assigned by CONFIG SET cluster-announce-human-nodename command or config file
 * edit, so we simply fall back to using the node's IP and port as the nodename.
 *
 * WARNING: THIS IS ONLY USED FOR LOGGING PURPOSE.
 *
 * Returns either the SDS field or a pointer to a thread-local scratch buffer. */
char *humanNodename(clusterNode *node) {
    if (sdslen(node->human_nodename) > 0) {
        return node->human_nodename;
    }

    /* Avoid allocating heap memory so that users can call the function with ease.
     * Use a small ring of thread-local buffers here so that multiple function calls
     * in the same logging statement are safe. */
    enum { BUF_COUNT = 8 };
    static _Thread_local char buffers[BUF_COUNT][CONN_ADDR_STR_LEN];
    static _Thread_local int idx;

    char *buffer = buffers[idx];
    idx = (idx + 1) % BUF_COUNT;

    const int port = server.tls_cluster ? node->tls_port : node->tcp_port;
    formatAddr(buffer, CONN_ADDR_STR_LEN, node->ip, port);
    return buffer;
}

/* Return human assigned node name if the link has the node associated to it
 * or else return "<unknown>". */
static inline char *clusterLinkGetHumanNodeName(clusterLink *link) {
    return link->node ? humanNodename(link->node) : "<unknown>";
}

#define isSlotUnclaimed(slot) \
    (server.cluster->slots[slot] == NULL || bitmapTestBit(server.cluster->owner_not_claiming_slot, slot))
/* Treating slot bitmaps as 8-byte words to speedup iteration */
#define CLUSTER_SLOT_WORDS (CLUSTER_SLOTS / 64)
#define SLOT_WORD_OFFSET(w) ((w) << 3)

#define RCVBUF_INIT_LEN 1024
#define RCVBUF_MIN_READ_LEN 14
static_assert(offsetof(clusterMsg, type) + sizeof(uint16_t) == RCVBUF_MIN_READ_LEN,
              "Incorrect length to read to identify type");

#define RCVBUF_MAX_PREALLOC (1 << 20) /* 1MB */