Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the wrong woff when execute WAIT / WAITAOF in script #776

Merged
merged 5 commits into from
Jul 22, 2024

Conversation

enjoy-binbin
Copy link
Member

@enjoy-binbin enjoy-binbin commented Jul 12, 2024

When executing the script, the client passed in is a fake
client, and its woff is always 0.

This results in woff always being 0 when executing wait/waitaof
in the script, and the command returns a wrong number.

@enjoy-binbin
Copy link
Member Author

@zuiderkwast this is the reason why #766 failed. The other test enable the appendonly, and the c->woff is always 0, so 0 >= 0 is always true

When executing the script, the client passed in is a fake
client, and its woff is always 0.

Signed-off-by: Binbin <[email protected]>
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.24%. Comparing base (a4ee8da) to head (746ede2).

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #776      +/-   ##
============================================
+ Coverage     70.04%   70.24%   +0.20%     
============================================
  Files           112      112              
  Lines         60602    60608       +6     
============================================
+ Hits          42447    42574     +127     
+ Misses        18155    18034     -121     
Files Coverage Δ
src/replication.c 87.28% <100.00%> (+0.09%) ⬆️

... and 17 files with indirect coverage changes

Signed-off-by: Binbin <[email protected]>
Copy link
Member

@PingXie PingXie left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

Awesome Binbin!

tests/unit/wait.tcl Outdated Show resolved Hide resolved
Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

I thought with this fix, we don't need to change 0 to * (#775), but that's OK too.

@enjoy-binbin
Copy link
Member Author

I thought with this fix, we don't need to change 0 to * (#775), but that's OK too.

we still need to change 0 to *, if the appendonly is yes, the test will still fail. that is because when we are executing the script, neither fsynced_off nor woff are updated, so we will still get the old (prev) values ​​in waitaof.

@enjoy-binbin enjoy-binbin requested a review from madolson July 15, 2024 07:02
@zuiderkwast zuiderkwast merged commit 14e09e9 into valkey-io:unstable Jul 22, 2024
19 checks passed
@enjoy-binbin enjoy-binbin deleted the fix_client_woff branch July 22, 2024 08:36
@enjoy-binbin enjoy-binbin added the release-notes This issue should get a line item in the release notes label Jul 22, 2024
hwware pushed a commit to hwware/valkey that referenced this pull request Jul 25, 2024
When executing the script, the client passed in is a fake
client, and its woff is always 0.

This results in woff always being 0 when executing wait/waitaof
in the script, and the command returns a wrong number.

---------

Signed-off-by: Binbin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes This issue should get a line item in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants