Increase timeouts in faster-failover test for slow CI runners#3463
Merged
zuiderkwast merged 1 commit intoApr 9, 2026
Merged
Conversation
… runners The test 'The best replica can initiate an election immediately in an automatic failover' has been flaky on sanitizer and slow CI runners. Changes: - Increase FAIL detection wait from 10s to 50s (wait_for_condition 1000 50) since FAIL propagation takes longer on slow runners. - Double psync_max_retries from 1200 to 2400 (normal) and 6000 to 12000 (valgrind) to give more time for partial resync log messages. Fixes flaky test introduced by commit 6822a67. Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
8ce488e to
9d76223
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #3463 +/- ##
============================================
- Coverage 76.52% 76.51% -0.01%
============================================
Files 157 157
Lines 79035 79035
============================================
- Hits 60478 60475 -3
- Misses 18557 18560 +3 🚀 New features to boost your workflow:
|
enjoy-binbin
approved these changes
Apr 9, 2026
sarthakaggarwal97
approved these changes
Apr 9, 2026
dvkashapov
reviewed
Apr 9, 2026
dvkashapov
left a comment
Member
There was a problem hiding this comment.
I suggest we modify timeouts similar to this PR #3462 where we multiply timeouts if we're under sanitizer or valgrind, WDYT?
zuiderkwast
approved these changes
Apr 9, 2026
Contributor
@dvkashapov Should we scale also server configs such as cluster-node-timeout and failover timeouts? |
Member
Yes, that would make sense for me, what's your opinion on that? |
Contributor
It can be complex to implement it. :) |
Member
Author
|
This seems like a good idea to scale the timeouts according to the environments. I can see if I can implement it |
sarthakaggarwal97
pushed a commit
to sarthakaggarwal97/valkey
that referenced
this pull request
Apr 16, 2026
…-io#3463) Two changes to tests/unit/cluster/faster-failover.tcl: 1. FAIL detection timeout: `wait_for_condition 1000 10` → `1000 50` (10s → 50s) 2. psync_max_retries: 1200 → 2400 normal (120s → 240s), 6000 → 12000 valgrind (600s → 1200s) The test `The best replica can initiate an election immediately in an automatic failover` in `tests/unit/cluster/faster-failover.tcl` has been flaky since it was introduced on March 27, 2026 by valkey-io#2227. **Frequency:** 8 out of 15 days (Mar 27 – Apr 8), across valgrind, sanitizer, and slow CI runners. **Common errors:** - `log message of "Successful partial resynchronization with primary" not found` (timeout waiting for psync) - `expected pattern found in srv -N log file: *best ranked replica*` (timeout waiting for FAIL propagation) The test spins up a 12-node cluster (5 primaries + 7 replicas), pauses nodes, and waits for FAIL detection to propagate across all nodes before failover + partial resync. A previous fix attempt valkey-io#3424 increased the psync timeout from 50s to 120s (600s valgrind), which reduced frequency but did not eliminate it. Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
madolson
pushed a commit
that referenced
this pull request
Apr 27, 2026
Two changes to tests/unit/cluster/faster-failover.tcl: 1. FAIL detection timeout: `wait_for_condition 1000 10` → `1000 50` (10s → 50s) 2. psync_max_retries: 1200 → 2400 normal (120s → 240s), 6000 → 12000 valgrind (600s → 1200s) The test `The best replica can initiate an election immediately in an automatic failover` in `tests/unit/cluster/faster-failover.tcl` has been flaky since it was introduced on March 27, 2026 by #2227. **Frequency:** 8 out of 15 days (Mar 27 – Apr 8), across valgrind, sanitizer, and slow CI runners. **Common errors:** - `log message of "Successful partial resynchronization with primary" not found` (timeout waiting for psync) - `expected pattern found in srv -N log file: *best ranked replica*` (timeout waiting for FAIL propagation) The test spins up a 12-node cluster (5 primaries + 7 replicas), pauses nodes, and waits for FAIL detection to propagate across all nodes before failover + partial resync. A previous fix attempt #3424 increased the psync timeout from 50s to 120s (600s valgrind), which reduced frequency but did not eliminate it. Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The test
The best replica can initiate an election immediately in an automatic failoverintests/unit/cluster/faster-failover.tclhas been flaky since it was introduced on March 27, 2026 by #2227.Frequency: 8 out of 15 days (Mar 27 – Apr 8), across valgrind, sanitizer, and slow CI runners.
Common errors:
log message of "Successful partial resynchronization with primary" not found(timeout waiting for psync)expected pattern found in srv -N log file: *best ranked replica*(timeout waiting for FAIL propagation)Example failing runs:
A previous fix attempt (#3424) increased the psync timeout from 50s to 120s (600s valgrind), which reduced frequency but did not eliminate it.
Root Cause
The test spins up a 12-node cluster (5 primaries + 7 replicas), pauses nodes, and waits for FAIL detection to propagate across all nodes before failover + partial resync. The original timeouts were too tight for slow CI environments:
wait_for_condition 1000 10= 10s max. Withcluster-node-timeout 5000and 12 nodes exchanging gossip, slow runners need more time.Fix
Two changes to
tests/unit/cluster/faster-failover.tcl:wait_for_condition 1000 10→1000 50(10s → 50s)1200→2400normal (120s → 240s),6000→12000valgrind (600s → 1200s)Testing
Ran
unit/cluster/faster-failover100 loops on valgrind, sanitizer, and ubuntu runners — all passed:--loops 100 --single unit/cluster/faster-failoveronsanitizer-address (clang/gcc),sanitizer-undefined (clang/gcc),sanitizer-force-defrag,ubuntu-jemalloc,ubuntu-arm