diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c index ef5822016..678bd3d82 100644 --- a/src/cluster_legacy.c +++ b/src/cluster_legacy.c @@ -2,8 +2,13 @@ * Copyright (c) 2009-Present, Redis Ltd. * All rights reserved. * + * Copyright (c) 2024-present, Valkey contributors. + * All rights reserved. + * * Licensed under your choice of the Redis Source Available License 2.0 * (RSALv2) or the Server Side Public License v1 (SSPLv1). + * + * Portions of this file are available under BSD3 terms; see REDISCONTRIBUTIONS for more information. */ /* @@ -2579,9 +2584,6 @@ uint32_t writePingExt(clusterMsg *hdr, int gossipcount) { extensions++; if (hdr != NULL) { - if (extensions != 0) { - hdr->mflags[0] |= CLUSTERMSG_FLAG0_EXT_DATA; - } hdr->extensions = htons(extensions); } @@ -2771,6 +2773,9 @@ int clusterProcessPacket(clusterLink *link) { } sender = getNodeFromLinkAndMsg(link, hdr); + if (sender && (hdr->mflags[0] & CLUSTERMSG_FLAG0_EXT_DATA)) { + sender->flags |= CLUSTER_NODE_EXTENSIONS_SUPPORTED; + } /* Update the last time we saw any data from this node. We * use this in order to avoid detecting a timeout from a node that @@ -3536,6 +3541,8 @@ static void clusterBuildMessageHdr(clusterMsg *hdr, int type, size_t msglen) { /* Set the message flags. */ if (clusterNodeIsMaster(myself) && server.cluster->mf_end) hdr->mflags[0] |= CLUSTERMSG_FLAG0_PAUSED; + hdr->mflags[0] |= CLUSTERMSG_FLAG0_EXT_DATA; /* Always make other nodes know that + * this node supports extension data. */ hdr->totlen = htonl(msglen); } @@ -3614,7 +3621,9 @@ void clusterSendPing(clusterLink *link, int type) { * to put inside the packet. */ estlen = sizeof(clusterMsg) - sizeof(union clusterMsgData); estlen += (sizeof(clusterMsgDataGossip)*(wanted + pfail_wanted)); - estlen += writePingExt(NULL, 0); + if (link->node && nodeSupportsExtensions(link->node)) { + estlen += writePingExt(NULL, 0); + } /* Note: clusterBuildMessageHdr() expects the buffer to be always at least * sizeof(clusterMsg) or more. */ if (estlen < (int)sizeof(clusterMsg)) estlen = sizeof(clusterMsg); @@ -3684,7 +3693,9 @@ void clusterSendPing(clusterLink *link, int type) { /* Compute the actual total length and send! */ uint32_t totlen = 0; - totlen += writePingExt(hdr, gossipcount); + if (link->node && nodeSupportsExtensions(link->node)) { + totlen += writePingExt(hdr, gossipcount); + } totlen += sizeof(clusterMsg)-sizeof(union clusterMsgData); totlen += (sizeof(clusterMsgDataGossip)*gossipcount); serverAssert(gossipcount < USHRT_MAX); diff --git a/src/cluster_legacy.h b/src/cluster_legacy.h index a857184ab..c0144ad9c 100644 --- a/src/cluster_legacy.h +++ b/src/cluster_legacy.h @@ -1,3 +1,16 @@ +/* + * Copyright (c) 2009-Present, Redis Ltd. + * All rights reserved. + * + * Copyright (c) 2024-present, Valkey contributors. + * All rights reserved. + * + * Licensed under your choice of the Redis Source Available License 2.0 + * (RSALv2) or the Server Side Public License v1 (SSPLv1). + * + * Portions of this file are available under BSD3 terms; see REDISCONTRIBUTIONS for more information. + */ + #ifndef CLUSTER_LEGACY_H #define CLUSTER_LEGACY_H @@ -51,6 +64,7 @@ typedef struct clusterLink { #define CLUSTER_NODE_MEET 128 /* Send a MEET message to this node */ #define CLUSTER_NODE_MIGRATE_TO 256 /* Master eligible for replica migration. */ #define CLUSTER_NODE_NOFAILOVER 512 /* Slave will not try to failover. */ +#define CLUSTER_NODE_EXTENSIONS_SUPPORTED 1024 /* This node supports extensions. */ #define CLUSTER_NODE_NULL_NAME "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000" #define nodeIsSlave(n) ((n)->flags & CLUSTER_NODE_SLAVE) @@ -59,6 +73,7 @@ typedef struct clusterLink { #define nodeTimedOut(n) ((n)->flags & CLUSTER_NODE_PFAIL) #define nodeFailed(n) ((n)->flags & CLUSTER_NODE_FAIL) #define nodeCantFailover(n) ((n)->flags & CLUSTER_NODE_NOFAILOVER) +#define nodeSupportsExtensions(n) ((n)->flags & CLUSTER_NODE_EXTENSIONS_SUPPORTED) /* This structure represent elements of node->fail_reports. */ typedef struct clusterNodeFailReport { diff --git a/tests/unit/cluster/hostnames.tcl b/tests/unit/cluster/hostnames.tcl index f31824062..1b4bd9dc7 100644 --- a/tests/unit/cluster/hostnames.tcl +++ b/tests/unit/cluster/hostnames.tcl @@ -1,3 +1,16 @@ +# +# Copyright (c) 2009-Present, Redis Ltd. +# All rights reserved. +# +# Copyright (c) 2024-present, Valkey contributors. +# All rights reserved. +# +# Licensed under your choice of the Redis Source Available License 2.0 +# (RSALv2) or the Server Side Public License v1 (SSPLv1). +# +# Portions of this file are available under BSD3 terms; see REDISCONTRIBUTIONS for more information. +# + proc get_slot_field {slot_output shard_id node_id attrib_id} { return [lindex [lindex [lindex $slot_output $shard_id] $node_id] $attrib_id] } @@ -116,10 +129,11 @@ test "Verify the nodes configured with prefer hostname only show hostname for ne # Have everyone forget node 6 and isolate it from the cluster. isolate_node 6 - # Set hostnames for the masters, now that the node is isolated - R 0 config set cluster-announce-hostname "shard-1.com" - R 1 config set cluster-announce-hostname "shard-2.com" - R 2 config set cluster-announce-hostname "shard-3.com" + set primaries 3 + for {set j 0} {$j < $primaries} {incr j} { + # Set hostnames for the masters, now that the node is isolated + R $j config set cluster-announce-hostname "shard-$j.com" + } # Prevent Node 0 and Node 6 from properly meeting, # they'll hang in the handshake phase. This allows us to @@ -149,9 +163,17 @@ test "Verify the nodes configured with prefer hostname only show hostname for ne } else { fail "Node did not learn about the 2 shards it can talk to" } - set slot_result [R 6 CLUSTER SLOTS] - assert_equal [lindex [get_slot_field $slot_result 0 2 3] 1] "shard-2.com" - assert_equal [lindex [get_slot_field $slot_result 1 2 3] 1] "shard-3.com" + wait_for_condition 50 100 { + [lindex [get_slot_field [R 6 CLUSTER SLOTS] 0 2 3] 1] eq "shard-1.com" + } else { + fail "hostname for shard-1 didn't reach node 6" + } + + wait_for_condition 50 100 { + [lindex [get_slot_field [R 6 CLUSTER SLOTS] 1 2 3] 1] eq "shard-2.com" + } else { + fail "hostname for shard-2 didn't reach node 6" + } # Also make sure we know about the isolated master, we # just can't reach it. @@ -170,10 +192,14 @@ test "Verify the nodes configured with prefer hostname only show hostname for ne } else { fail "Node did not learn about the 2 shards it can talk to" } - set slot_result [R 6 CLUSTER SLOTS] - assert_equal [lindex [get_slot_field $slot_result 0 2 3] 1] "shard-1.com" - assert_equal [lindex [get_slot_field $slot_result 1 2 3] 1] "shard-2.com" - assert_equal [lindex [get_slot_field $slot_result 2 2 3] 1] "shard-3.com" + + for {set j 0} {$j < $primaries} {incr j} { + wait_for_condition 50 100 { + [lindex [get_slot_field [R 6 CLUSTER SLOTS] $j 2 3] 1] eq "shard-$j.com" + } else { + fail "hostname information for shard-$j didn't reach node 6" + } + } } test "Test restart will keep hostname information" {