[Flaky Tests] Deflake many-slot-migration under valgrind#3497
Closed
sarthakaggarwal97 wants to merge 1 commit into
Closed
[Flaky Tests] Deflake many-slot-migration under valgrind#3497sarthakaggarwal97 wants to merge 1 commit into
sarthakaggarwal97 wants to merge 1 commit into
Conversation
Signed-off-by: Sarthak Aggarwal <25262500+sarthakaggarwal97@users.noreply.github.com>
16211bb to
27383f7
Compare
Member
|
What do you think would be better in this case: scale timeout if under valgrind #3462 or reduce amount of migrations like here? |
Contributor
Author
|
Oh I missed that change somehow. We need a better way to not dedup effort (maybe creating issues for tests will help). |
Contributor
Author
|
I will actually mark this PR for draft. I will talk to @roshkhatri as well offline. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #3497 +/- ##
============================================
+ Coverage 76.39% 76.41% +0.01%
============================================
Files 157 159 +2
Lines 79345 79809 +464
============================================
+ Hits 60617 60984 +367
- Misses 18728 18825 +97 🚀 New features to boost your workflow:
|
Contributor
Author
|
Decided to port this fix in #3462 |
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
2 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.
tests/unit/cluster/many-slot-migration.tclhas been timing out repeatedly in the dedicated valgrind Daily jobs.The failure is driven by the amount of work rather than a functional regression: the test loads 40,000 keys and leaves 1,000 slots half-migrated before running cluster fix, which is significantly slower under valgrind.
This change keeps the default coverage unchanged for normal runs, but scales the workload down only when
::valgrindis enabled. Under valgrind, the test now uses 10,000 keys and 250 migrated slots, which still exercises the same cluster repair path while avoiding the repeated CI timeout.Passing Valgrind Run: https://github.com/sarthakaggarwal97/valkey/actions/runs/24360238670/job/71137719287
Failing CI: https://github.com/valkey-io/valkey/actions/runs/24374139344/job/71183925533#step:8:4797