Skip to content

[flaky-tests] atomically snapshot dual-channel memory stats#3336

Merged
hpatro merged 2 commits into
valkey-io:unstablefrom
sarthakaggarwal97:fix-dualchannel-membuffer-test
Mar 9, 2026
Merged

[flaky-tests] atomically snapshot dual-channel memory stats#3336
hpatro merged 2 commits into
valkey-io:unstablefrom
sarthakaggarwal97:fix-dualchannel-membuffer-test

Conversation

@sarthakaggarwal97

Copy link
Copy Markdown
Contributor

This fixes a flaky dual-channel replication integration test: https://github.com/valkey-io/valkey/actions/runs/22810251608/job/66165776198#step:8:7701

INFO memory field used_memory_overhead and MEMORY STATS field overhead.total can change during dual-channel sync if replica's pending replication buffer is still changing. This is probably more visible in slower environments.

The test now collects INFO and MEMORY STATS in a single MULTI/EXEC on both the primary and replica, so the compared values come from the same snapshot.

Passing here: https://github.com/sarthakaggarwal97/valkey/actions/runs/22864585326/job/66327772967

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@sarthakaggarwal97 sarthakaggarwal97 force-pushed the fix-dualchannel-membuffer-test branch from 766ea11 to 31789c1 Compare March 9, 2026 17:12
@codecov

codecov Bot commented Mar 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.97%. Comparing base (fc217fc) to head (85653b2).
⚠️ Report is 6 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3336      +/-   ##
============================================
- Coverage     75.05%   74.97%   -0.08%     
============================================
  Files           129      129              
  Lines         71553    71632      +79     
============================================
+ Hits          53705    53709       +4     
- Misses        17848    17923      +75     

see 24 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.

@Nikhil-Manglore Nikhil-Manglore left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix, I have noticed this failing for a while.

@roshkhatri roshkhatri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
fixes #3337

@hpatro hpatro merged commit 5deebd0 into valkey-io:unstable Mar 9, 2026
58 checks passed

@enjoy-binbin enjoy-binbin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks.

JimB123 added a commit to JimB123/valkey that referenced this pull request Mar 10, 2026
@sarthakaggarwal97 sarthakaggarwal97 moved this from Done to Todo in Valkey 9.1 Mar 16, 2026
JimB123 pushed a commit that referenced this pull request Mar 19, 2026
This fixes a flaky dual-channel replication integration test:
https://github.com/valkey-io/valkey/actions/runs/22810251608/job/66165776198#step:8:7701

`INFO memory` field `used_memory_overhead` and `MEMORY STATS` field
`overhead.total` can change during dual-channel sync if replica's
pending replication buffer is still changing. This is probably more
visible in slower environments.

The test now collects `INFO` and `MEMORY STATS` in a single `MULTI/EXEC`
on both the primary and replica, so the compared values come from the
same snapshot.

Passing here:
https://github.com/sarthakaggarwal97/valkey/actions/runs/22864585326/job/66327772967

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

5 participants