From 978ee05506a062906b5fb9d3962cfe0a43ec8dea Mon Sep 17 00:00:00 2001 From: Binbin Date: Sun, 8 Jun 2025 22:18:11 +0800 Subject: [PATCH] Fix cluster myself CLUSTER SLOTS/NODES wrong port after updating port/tls-port When modifying port or tls-port through config set, we need to call clusterUpdateMyselfAnnouncedPorts to update myself's port, otherwise CLUSTER SLOTS/NODES will be old information from myself's perspective. In addition, in some places, such as clusterUpdateMyselfAnnouncedPorts and clusterUpdateMyselfIp, beforeSleep save is added so that the new ip info can be updated to nodes.conf. Remove clearCachedClusterSlotsResponse in updateClusterAnnouncedPort since now we add beforeSleep save in clusterUpdateMyselfAnnouncedPorts, and it will call clearCachedClusterSlotsResponse. Signed-off-by: Binbin --- src/cluster_legacy.c | 2 ++ src/config.c | 3 +- tests/unit/cluster/announced-endpoints.tcl | 39 ++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c index 5e107a17e3..d7eea8f569 100644 --- a/src/cluster_legacy.c +++ b/src/cluster_legacy.c @@ -1020,6 +1020,7 @@ void clusterUpdateMyselfFlags(void) { void clusterUpdateMyselfAnnouncedPorts(void) { if (!myself) return; deriveAnnouncedPorts(&myself->tcp_port, &myself->tls_port, &myself->cport); + clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG); } /* We want to take myself->ip in sync with the cluster-announce-ip option. @@ -1050,6 +1051,7 @@ void clusterUpdateMyselfIp(void) { } else { myself->ip[0] = '\0'; /* Force autodetection. */ } + clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG); } } diff --git a/src/config.c b/src/config.c index 99eb86ab28..c9a678d420 100644 --- a/src/config.c +++ b/src/config.c @@ -2473,6 +2473,7 @@ static int updatePort(const char **err) { listener->bindaddr_count = server.bindaddr_count; listener->port = server.port; listener->ct = connectionByType(CONN_TYPE_SOCKET); + clusterUpdateMyselfAnnouncedPorts(); if (changeListener(listener) == C_ERR) { *err = "Unable to listen on this port. Check server logs."; return 0; @@ -2660,7 +2661,6 @@ int updateClusterFlags(const char **err) { static int updateClusterAnnouncedPort(const char **err) { UNUSED(err); clusterUpdateMyselfAnnouncedPorts(); - clearCachedClusterSlotsResponse(); return 1; } @@ -2719,6 +2719,7 @@ static int applyTLSPort(const char **err) { listener->bindaddr_count = server.bindaddr_count; listener->port = server.tls_port; listener->ct = connectionByType(CONN_TYPE_TLS); + clusterUpdateMyselfAnnouncedPorts(); if (changeListener(listener) == C_ERR) { *err = "Unable to listen on this port. Check server logs."; return 0; diff --git a/tests/unit/cluster/announced-endpoints.tcl b/tests/unit/cluster/announced-endpoints.tcl index b44a2a0e95..47ca8d2e41 100644 --- a/tests/unit/cluster/announced-endpoints.tcl +++ b/tests/unit/cluster/announced-endpoints.tcl @@ -53,4 +53,43 @@ start_cluster 2 2 {tags {external:skip cluster}} { R 0 config set cluster-announce-bus-port 0 assert_match "*@$base_bus_port *" [R 0 CLUSTER NODES] } + + test "Test change port and tls-port on runtime" { + if {$::tls} { + set baseport [lindex [R 0 config get tls-port] 1] + } else { + set baseport [lindex [R 0 config get port] 1] + } + set count [expr [llength $::servers] + 1] + set used_port [find_available_port $baseport $count] + + # We execute CLUSTER SLOTS command to trigger the `debugServerAssertWithInfo` in `clusterCommandSlots` function, ensuring + # that the cached response is invalidated upon updating any of port or tls-port. + R 0 CLUSTER SLOTS + R 1 CLUSTER SLOTS + + # Set port or tls-port to ensure changes are consistent across the cluster. + if {$::tls} { + R 0 config set tls-port $used_port + } else { + R 0 config set port $used_port + } + # Make sure changes in myself node's view are consistent. + assert_match "*:$used_port@*" [R 0 CLUSTER NODES] + assert_match "*$used_port*" [R 0 CLUSTER SLOTS] + # Make sure changes in other node's view are consistent. + wait_for_condition 50 100 { + [string match "*:$used_port@*" [R 1 CLUSTER NODES]] && + [string match "*$used_port*" [R 1 CLUSTER SLOTS]] + } else { + fail "Node port was not propagated via gossip" + } + + # Restore the original configuration item value. + if {$::tls} { + R 0 config set tls-port $baseport + } else { + R 0 config set port $baseport + } + } }