Reduce execution time for replica migration tests by sushilpaneru1 · Pull Request #4006 · valkey-io/valkey

sushilpaneru1 · 2026-06-18T20:59:03Z

Description

The test "Migrated replica reports zero repl offset and rank, and fails to win election - shutdown" takes upto 25s. This PR reduces it to ~4s while maintaining the same coverage.

This might end up closing #3992 as well

From https://github.com/valkey-io/valkey/actions/runs/27586402593/job/81557596685

[ok]: Migrated replica reports zero repl offset and rank, and fails to win election - shutdown (26545 ms)

After the fix

[ok]: Migrated replica reports zero repl offset and rank, and fails to win election - shutdown (4847 ms)

Update - Applied similar fix to other tests as well resulting

Test	Before	After
Migrated replica - shutdown	~25s	~4s
Migrated replica - sigstop	~4s	~4s
Non-empty replica - shutdown	~12.5s	~1.5s
Non-empty replica - sigstop	~3.5s	~2.5s
Sub-replica - shutdown	~13.5s	~2s
Sub-replica - sigstop	~3.5s	~2s

Fix

Replace ~10k individual INCR round-trips with single SET commands using large values.
Set shutdown-timeout 0 before shutting down primary 0.

coderabbitai · 2026-06-18T20:59:19Z

📝 Walkthrough

Walkthrough

Three cluster replica-migration test procedures (test_migrated_replica, test_nonempty_replica, test_sub_replica) are updated to seed data using fixed-size string repeat values (1 KB and 10 KB) instead of looped incr counters, validate post-write get results immediately, compare post-failover consistency checks against the captured string values, and add shutdown-timeout 0 to cluster startup configurations.

Changes

Replica migration test updates

Layer / File(s)	Summary
test_migrated_replica updates `tests/unit/cluster/replica-migration.tcl`	Direct writes of 1 KB and 10 KB strings to primary nodes replace looped increments; immediate `get` assertions validate seeded values. Post-failover consistency check on nodes 3/4/7 now compares reads against `$small_value`/`$large_value`. Shutdown-timeout 0 added to startup config for shutdown and sigstop variants.
test_nonempty_replica updates `tests/unit/cluster/replica-migration.tcl`	Direct writes of 1 KB and 10 KB strings to primary nodes replace looped increments; immediate `get` assertions validate seeded values. Consistency check on nodes 4/7 now compares reads against `$small_value`/`$large_value`. Shutdown-timeout 0 added to startup config for shutdown and sigstop variants.
test_sub_replica updates `tests/unit/cluster/replica-migration.tcl`	Direct writes of 1 KB and 10 KB strings to primary nodes replace looped increments; immediate `get` assertions validate seeded values. Consistency check on nodes 3/4/7 now compares reads against `$small_value`/`$large_value`. Shutdown-timeout 0 added to startup config for shutdown and sigstop variants.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

zuiderkwast

🚥 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 'Reduce execution time for replica migration tests' directly reflects the main objective of the changeset, which is to optimize test performance by replacing repetitive operations with efficient SET commands and adjusting shutdown configuration.
Description check	✅ Passed	The PR description clearly explains the performance optimization, showing specific timing improvements and listing the changes made to address test execution time.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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

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

sushilpaneru1 · 2026-06-18T21:01:07Z

Looks @enjoy-binbin is the author of this test. Can you take a look please? Thanks.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/cluster/replica-migration.tcl`:
- Around line 30-34: The test writes a 10KB large value to key_977613 in slot 0
on primary 3, and when slot 0 is migrated to primary 0, this large payload
inflates server 4's replication offset, which weakens the regression check. To
fix this, keep the large padding write to generate the initial large
repl_offset, but then overwrite key_977613 with a small final value before the
slot migration/rebalancing occurs. This ensures the offset padding is
established without the migrated value itself being large. Apply the same fix to
the similar code at lines 110-112 where the large value write pattern is
repeated.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8777d27d-edaf-4ff5-9847-727d4c35521f

📥 Commits

Reviewing files that changed from the base of the PR and between 436dcae and 5273e4e.

📒 Files selected for processing (1)

tests/unit/cluster/replica-migration.tcl

enjoy-binbin · 2026-06-19T04:05:06Z

This might end up closing #3992 as well

I see the test fail in "failover not happen", did you know the reason?

codecov · 2026-06-19T04:22:15Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.77%. Comparing base (436dcae) to head (75857af).

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #4006      +/-   ##
============================================
+ Coverage     76.75%   76.77%   +0.02%     
============================================
  Files           162      162              
  Lines         81017    81017              
============================================
+ Hits          62187    62204      +17     
+ Misses        18830    18813      -17

see 16 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sushilpaneru1 · 2026-06-19T04:34:22Z

I see the test fail in "failover not happen", did you know the reason?

No, my hunch is something to do with long running time. I have done 20+ runs and have not encountered any failure. (finger crossed) this fixes the flakiness as well.

Signed-off-by: Sushil Paneru <sushil.paneru1@gmail.com>

enjoy-binbin

LGTM, thanks.

Replace ~10k individual INCR round-trips with single SET commands using large values. And set shutdown-timeout 0 before shutting down the node. Signed-off-by: Sushil Paneru <sushil.paneru1@gmail.com>

github-actions Bot assigned sushilpaneru1 Jun 18, 2026

coderabbitai Bot reviewed Jun 18, 2026

View reviewed changes

Comment thread tests/unit/cluster/replica-migration.tcl

enjoy-binbin reviewed Jun 19, 2026

View reviewed changes

Comment thread tests/unit/cluster/replica-migration.tcl

enjoy-binbin reviewed Jun 19, 2026

View reviewed changes

Comment thread tests/unit/cluster/replica-migration.tcl Outdated

enjoy-binbin reviewed Jun 19, 2026

View reviewed changes

Comment thread tests/unit/cluster/replica-migration.tcl Outdated

Reduce replica migration test execution time

75857af

Signed-off-by: Sushil Paneru <sushil.paneru1@gmail.com>

sushilpaneru1 force-pushed the simplify_replica_migration branch from 5273e4e to 75857af Compare June 19, 2026 04:43

sushilpaneru1 requested a review from enjoy-binbin June 19, 2026 04:59

sushilpaneru1 changed the title ~~Reduce test time for - Migrated replica reports zero repl offset and rank, and fails to win election~~ Reduce execution time for replica migration tests Jun 19, 2026

enjoy-binbin approved these changes Jun 19, 2026

View reviewed changes

enjoy-binbin merged commit fa62dce into valkey-io:unstable Jun 22, 2026
64 checks passed

sarthakaggarwal97 added this to Valkey 9.1 Jul 2, 2026

github-project-automation Bot moved this to To be backported in Valkey 9.1 Jul 2, 2026

valkeyrie-ops Bot mentioned this pull request Jul 2, 2026

[backport] Backport sweep for 9.1 #3987

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Reduce execution time for replica migration tests#4006

Reduce execution time for replica migration tests#4006
enjoy-binbin merged 1 commit into
valkey-io:unstablefrom
sushilpaneru1:simplify_replica_migration

sushilpaneru1 commented Jun 18, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 18, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Uh oh!

sushilpaneru1 commented Jun 18, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

enjoy-binbin commented Jun 19, 2026

Uh oh!

codecov Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

sushilpaneru1 commented Jun 19, 2026

Uh oh!

enjoy-binbin left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

sushilpaneru1 commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Fix

Uh oh!

coderabbitai Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Uh oh!

sushilpaneru1 commented Jun 18, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

enjoy-binbin commented Jun 19, 2026

Uh oh!

codecov Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sushilpaneru1 commented Jun 19, 2026

Uh oh!

enjoy-binbin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sushilpaneru1 commented Jun 18, 2026 •

edited

Loading

coderabbitai Bot commented Jun 18, 2026 •

edited

Loading

codecov Bot commented Jun 19, 2026 •

edited

Loading