From 75c57d53ea82674b16782a5e937e467bbdca985f Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 25 Jun 2014 15:19:35 +0200 Subject: [PATCH] CLUSTER SLOTS: don't output failing slaves. While we have to output failing masters in order to provide an accurate map (that may be the one of a Redis Cluster in down state because not all slots are served by a working master), to provide slaves in FAIL state is not a good idea since those are not necesarely needed, and the client will likely incur into a latency penalty trying to connect with a slave which is down. Note that this means that CLUSTER SLOTS does not provide a *complete* map of slaves, however this would not be of any help since slaves may be added later, and a client that needs to scale reads and requires to stay updated with the list of slaves, need to do a refresh of the map from time to time, anyway. --- src/cluster.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index 264ef4f6..b735b0e6 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -3458,8 +3458,8 @@ void clusterReplyMultiBulkSlots(redisClient *c) { * ... continued until done */ - int repliedRangeCount = 0; - void *slotreplylen = addDeferredMultiBulkLength(c); + int num_masters = 0; + void *slot_replylen = addDeferredMultiBulkLength(c); dictEntry *de; dictIterator *di = dictGetSafeIterator(server.cluster->nodes); @@ -3472,16 +3472,16 @@ void clusterReplyMultiBulkSlots(redisClient *c) { if (!nodeIsMaster(node) || node->numslots == 0) continue; for (j = 0; j < REDIS_CLUSTER_SLOTS; j++) { - int bit; + int bit, i; if ((bit = clusterNodeGetSlotBit(node,j)) != 0) { if (start == -1) start = j; } if (start != -1 && (!bit || j == REDIS_CLUSTER_SLOTS-1)) { - if (bit && j == REDIS_CLUSTER_SLOTS-1) j++; + int nested_elements = 3; /* slots (2) + master addr (1). */ + void *nested_replylen = addDeferredMultiBulkLength(c); - /* nested size: slots (2) + master (1) + slaves */ - addReplyMultiBulkLen(c, 2 + 1 + node->numslaves); + if (bit && j == REDIS_CLUSTER_SLOTS-1) j++; /* If slot exists in output map, add to it's list. * else, create a new output map for this slot */ @@ -3500,20 +3500,22 @@ void clusterReplyMultiBulkSlots(redisClient *c) { addReplyLongLong(c, node->port); /* Remaining nodes in reply are replicas for slot range */ - int i; for (i = 0; i < node->numslaves; i++) { /* This loop is copy/pasted from clusterGenNodeDescription() * with modifications for per-slot node aggregation */ + if (nodeFailed(node->slaves[i])) continue; addReplyMultiBulkLen(c, 2); addReplyBulkCString(c, node->slaves[i]->ip); addReplyLongLong(c, node->slaves[i]->port); + nested_elements++; } - repliedRangeCount++; + setDeferredMultiBulkLength(c, nested_replylen, nested_elements); + num_masters++; } } } dictReleaseIterator(di); - setDeferredMultiBulkLength(c, slotreplylen, repliedRangeCount); + setDeferredMultiBulkLength(c, slot_replylen, num_masters); } void clusterCommand(redisClient *c) {