From b5784feabd7fc63c3fc66b44b5a7d844803a797b Mon Sep 17 00:00:00 2001 From: Binbin Date: Tue, 19 Jul 2022 11:35:13 +0800 Subject: [PATCH] Fix timing issue in cluster test (#11008) A timing issue like this was reported in freebsd daily CI: ``` *** [err]: Sanity test push cmd after resharding in tests/unit/cluster/cli.tcl Expected 'CLUSTERDOWN The cluster is down' to match '*MOVED*' ``` We additionally wait for each node to reach a consensus on the cluster state in wait_for_condition to avoid the cluster down error. The fix just like #10495, quoting madolson's comment: Cluster check just verifies the the config state is self-consistent, waiting for cluster_state to be okay is an independent check that all the nodes actually believe each other are healthy. At the same time i noticed that unit/moduleapi/cluster.tcl has an exact same test, may have the same problem, also modified it. (cherry picked from commit 5ce64ab01011781c426074f3fa7398260cce94cc) --- tests/unit/cluster.tcl | 14 ++++++++++---- tests/unit/moduleapi/cluster.tcl | 9 ++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/unit/cluster.tcl b/tests/unit/cluster.tcl index 7d64ccf4b..69ae96f6d 100644 --- a/tests/unit/cluster.tcl +++ b/tests/unit/cluster.tcl @@ -78,10 +78,16 @@ start_multiple_servers 3 [list overrides $base_conf] { } test "Wait for cluster to be stable" { - wait_for_condition 1000 50 { - [catch {exec src/redis-cli --cluster \ - check 127.0.0.1:[srv 0 port] \ - }] == 0 + # Cluster check just verifies the the config state is self-consistent, + # waiting for cluster_state to be okay is an independent check that all the + # nodes actually believe each other are healthy, prevent cluster down error. + wait_for_condition 1000 50 { + [catch {exec src/redis-cli --cluster check 127.0.0.1:[srv 0 port]}] == 0 && + [catch {exec src/redis-cli --cluster check 127.0.0.1:[srv -1 port]}] == 0 && + [catch {exec src/redis-cli --cluster check 127.0.0.1:[srv -2 port]}] == 0 && + [CI 0 cluster_state] eq {ok} && + [CI 1 cluster_state] eq {ok} && + [CI 2 cluster_state] eq {ok} } else { fail "Cluster doesn't stabilize" } diff --git a/tests/unit/moduleapi/cluster.tcl b/tests/unit/moduleapi/cluster.tcl index f4ebaab1b..bbd25628a 100644 --- a/tests/unit/moduleapi/cluster.tcl +++ b/tests/unit/moduleapi/cluster.tcl @@ -101,9 +101,12 @@ start_server [list overrides $base_conf] { test "Wait for cluster to be stable" { wait_for_condition 1000 50 { - [catch {exec src/redis-cli --cluster \ - check 127.0.0.1:[srv 0 port] \ - }] == 0 + [catch {exec src/redis-cli --cluster check 127.0.0.1:[srv 0 port]}] == 0 && + [catch {exec src/redis-cli --cluster check 127.0.0.1:[srv -1 port]}] == 0 && + [catch {exec src/redis-cli --cluster check 127.0.0.1:[srv -2 port]}] == 0 && + [CI 0 cluster_state] eq {ok} && + [CI 1 cluster_state] eq {ok} && + [CI 2 cluster_state] eq {ok} } else { fail "Cluster doesn't stabilize" }