Unverified Commit e74a1f3b authored by Madelyn Olson's avatar Madelyn Olson Committed by GitHub
Browse files

Optimize the performance of cluster slots for non-continuous slots (#11745)

This change improves the performance of cluster slots by removing the deferring lengths that are used. Deferring lengths are used in two contexts, the first is for determining the number of replicas that serve a slot (Added in 6.2 as part of a different performance improvement) and the second is for determining the extra networking options for each node (Added in 7.0). For continuous slots, (e.g. 0-8196) this improvement is very negligible, however it becomes more significant when slots are not continuous (e.g. 0 2 4 6 etc) which can happen in production for various users.

The `cluster slots` command is deprecated in favor of `cluster shards`, but since most clients don't support the new command yet I think it's important to not degrade performance here.

Benchmarking shows about 2x improvement, however I wasn't able to get a coherent TPS number since the benchmark process was being saturated long before Redis was, so had to run with multiple benchmarks and merge results. If needed I can add this to our memtier framework. Instead the next section shows the number of usec per call from the benchmark results, which shows significant improvement as well as having a more coherent response in the CoB.

| | New Code | Old Code | % Improvements
|----|----|----- |-----
| Uniform slots| usec_per_call=10.46 | usec_per_call=11.03 | 5.7%
| Worst case (Only even slots)| usec_per_call=963.80 | usec_per_call=2950.99 | 307%

This change also removes some extra white space that I added a when making a code change for adding hostnames.
parent 6444214c
...@@ -5347,27 +5347,42 @@ void addNodeToNodeReply(client *c, clusterNode *node) { ...@@ -5347,27 +5347,42 @@ void addNodeToNodeReply(client *c, clusterNode *node) {
addReplyBulkCBuffer(c, node->name, CLUSTER_NAMELEN); addReplyBulkCBuffer(c, node->name, CLUSTER_NAMELEN);
/* Add the additional endpoint information, this is all the known networking information /* Add the additional endpoint information, this is all the known networking information
* that is not the preferred endpoint. */ * that is not the preferred endpoint. Note the logic is evaluated twice so we can
void *deflen = addReplyDeferredLen(c); * correctly report the number of additional network arguments without using a deferred
* map, an assertion is made at the end to check we set the right length. */
int length = 0; int length = 0;
if (server.cluster_preferred_endpoint_type != CLUSTER_ENDPOINT_TYPE_IP) {
length++;
}
if (server.cluster_preferred_endpoint_type != CLUSTER_ENDPOINT_TYPE_HOSTNAME
&& sdslen(node->hostname) != 0)
{
length++;
}
addReplyMapLen(c, length);
if (server.cluster_preferred_endpoint_type != CLUSTER_ENDPOINT_TYPE_IP) { if (server.cluster_preferred_endpoint_type != CLUSTER_ENDPOINT_TYPE_IP) {
addReplyBulkCString(c, "ip"); addReplyBulkCString(c, "ip");
addReplyBulkCString(c, node->ip); addReplyBulkCString(c, node->ip);
length++; length--;
} }
if (server.cluster_preferred_endpoint_type != CLUSTER_ENDPOINT_TYPE_HOSTNAME if (server.cluster_preferred_endpoint_type != CLUSTER_ENDPOINT_TYPE_HOSTNAME
&& sdslen(node->hostname) != 0) && sdslen(node->hostname) != 0)
{ {
addReplyBulkCString(c, "hostname"); addReplyBulkCString(c, "hostname");
addReplyBulkCBuffer(c, node->hostname, sdslen(node->hostname)); addReplyBulkCBuffer(c, node->hostname, sdslen(node->hostname));
length++; length--;
} }
setDeferredMapLen(c, deflen, length); serverAssert(length == 0);
} }
void addNodeReplyForClusterSlot(client *c, clusterNode *node, int start_slot, int end_slot) { void addNodeReplyForClusterSlot(client *c, clusterNode *node, int start_slot, int end_slot) {
int i, nested_elements = 3; /* slots (2) + master addr (1) */ int i, nested_elements = 3; /* slots (2) + master addr (1) */
void *nested_replylen = addReplyDeferredLen(c); for (i = 0; i < node->numslaves; i++) {
if (!isReplicaAvailable(node->slaves[i])) continue;
nested_elements++;
}
addReplyArrayLen(c, nested_elements);
addReplyLongLong(c, start_slot); addReplyLongLong(c, start_slot);
addReplyLongLong(c, end_slot); addReplyLongLong(c, end_slot);
addNodeToNodeReply(c, node); addNodeToNodeReply(c, node);
...@@ -5378,9 +5393,9 @@ void addNodeReplyForClusterSlot(client *c, clusterNode *node, int start_slot, in ...@@ -5378,9 +5393,9 @@ void addNodeReplyForClusterSlot(client *c, clusterNode *node, int start_slot, in
* with modifications for per-slot node aggregation. */ * with modifications for per-slot node aggregation. */
if (!isReplicaAvailable(node->slaves[i])) continue; if (!isReplicaAvailable(node->slaves[i])) continue;
addNodeToNodeReply(c, node->slaves[i]); addNodeToNodeReply(c, node->slaves[i]);
nested_elements++; nested_elements--;
} }
setDeferredArrayLen(c, nested_replylen, nested_elements); serverAssert(nested_elements == 3); /* Original 3 elements */
} }
/* Add detailed information of a node to the output buffer of the given client. */ /* Add detailed information of a node to the output buffer of the given client. */
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment