Deflake test: fix race causing missing manual failover timeout log

[ShubhamTaple]

Problem

#4310 The test Replica can update the config epoch when trigger the failover - manual intermittently failed with the error:
log message of '"Manual failover timed out"' not found

Cause

A race condition between re-enabling cluster gossip and issuing CLUSTER FAILOVER FORCE.

After removing the packet filter, the test waited for the cluster to become fail before sending the failover command. During that wait, gossip messages from other primaries could update the replica's currentEpoch, making its first failover attempt succeed immediately, therefore the expected timeout log was not being generated.

Fix

Fix the manual failover case by:

1. Waiting for replica cluster links to reconnect after removing the packet filter.
2. Retrying the manual failover until both primaries log the denial request, then confirming the timeout log.
3. Fixing the replica epoch-update guard to check the replica (node 3) instead of a primary.

[coderabbitai]

📝 Walkthrough

Walkthrough

The cluster failover test now waits for replica links to reconnect, retries manual failover until both primaries log rejected authorization requests, and checks the replica config epoch on node 3.

Changes

Cluster failover test validation

Layer / File(s)	Summary
Failover synchronization and assertions tests/unit/cluster/failover2.tcl	The test waits for both replica links to reconnect. Manual failover retries until both primaries log rejected authorization requests, then checks the manual timeout. The config-epoch assertion targets node 3.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• valkey-io/valkey#3889 — Both changes update failover timing and assertions in failover2.tcl.
• valkey-io/valkey#4310 — This change retries until rejected authorization logs appear before checking the manual-failover timeout.

Possibly related PRs

• valkey-io/valkey#4181 — Both PRs adjust failover test wait and retry logic.

Suggested reviewers: enjoy-binbin

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the race-condition fix for the missing manual failover timeout log.
Description check	✅ Passed	The description explains the race condition, its cause, and the test changes that resolve it.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[valkey-review-bot]

The config-epoch half of the reordering is sound: requestConfigEpoch in the vote request is the replica's (stale) view of R0's configEpoch, and the primaries deny on slots[j]->configEpoch > requestConfigEpoch at src/cluster_legacy.c:5441, which is exactly the state line 168 asserts. So the first forced attempt is deterministically denied. My concern is whether the vote request is actually delivered now that it is sent microseconds after the cluster-bus links were being torn down — details inline.

Separately, and pre-existing rather than introduced here: line 203 reads cluster_get_node_by_id 1 $R0_nodeid while the failure message says "The replica does not update the config epoch". cluster_get_node_by_id takes the instance index first (tests/support/cluster_util.tcl:314) and line 168 correctly uses 3 for the replica, so as written that guard inspects primary R1's view, always passes immediately, and does not gate the second CLUSTER FAILOVER FORCE on the replica having received the UPDATE. Since this PR is deflaking this proc, changing it to 3 looks worthwhile.

[valkey-review-bot]

This runs a few hundred microseconds after CLOSE-CLUSTER-LINK-ON-PACKET-DROP 0, so the replica's cluster-bus links are very likely still down: with the drop-all filter active every packet reaching R3 caused freeClusterLink() on the link it arrived on (src/cluster_legacy.c:3893-3898), and outbound links are only re-created from clusterCron -> clusterNodeCronHandleReconnect — i.e. on the 100ms cron, further throttled to cluster-node-timeout/2/10 (750ms at the 15s default) while there is no inbound link (src/cluster_legacy.c:6164-6173).

If node->link is NULL for R1/R2 when the election fires, clusterRequestFailoverAuth() -> clusterBroadcastMessage() -> clusterSendMessage() returns silently (src/cluster_legacy.c:4764), yet failover_auth_sent is set anyway (src/cluster_legacy.c:5880) and nothing is retried until auth_age > auth_retry_time, which is 2 * max(2 * cluster-node-timeout, 2000) = 60s here (src/cluster_legacy.c:5710-5712, 5761). Manual failover timed out. still gets logged at mf_end, so the wait below passes, but the primaries never see the request: the waits at lines 196-199 for Failover auth denied ... reqConfigEpoch / has old slots configuration then hang, and the UPDATE that is the only thing teaching the replica the new config epoch (src/cluster_legacy.c:5454-5461) is never sent. The old ordering was immune to this because the cluster_state == fail wait gave the links at least cluster-node-timeout to come back before the election started.

One way to keep the request observable and re-armed:

Suggested change

	R 3 cluster failover force
	# The links were just torn down by CLOSE-CLUSTER-LINK-ON-PACKET-DROP, and a
	# vote request broadcast over a NULL link is silently dropped and not retried
	# for auth_retry_time, so keep re-arming the manual failover until both
	# primaries have actually seen (and denied) the request.
	wait_for_condition 50 100 {
	[catch {R 3 cluster failover force}] == 0 &&
	[count_log_message -1 "reqConfigEpoch"] > 0 &&
	[count_log_message -2 "reqConfigEpoch"] > 0
	} else {
	fail "The primaries did not receive the failover auth request"
	}

[ShubhamTaple]

Addressed